From facd18bdca772ddc38b163830b0e749df6320ef6 Mon Sep 17 00:00:00 2001 From: Graeme Geldenhuys Date: Tue, 9 Jun 2026 17:25:39 +0100 Subject: [PATCH] fix(native): load Result after epilogue ARC releases, not before MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EmitFunctionDef loaded the function Result into %rax (or %xmm0) at the top of the epilogue, then ran the local/param ARC release pass — which calls _ClassRelease / _StringRelease / _DynArrayRelease. Those calls clobber %rax, so any value-returning function that also releases an ARC-managed parameter or local returned garbage. Most visibly, a method returning Integer but taking an interface (or class) value parameter releases that parameter on exit, so `U.Use(I)` returned 3 instead of 55; a String-returning function taking a String parameter returned a clobbered pointer. Move the Result load to AFTER every ARC release call, immediately before leave/ret, so nothing can clobber it. sret (record-returning) functions are unaffected — they return via the caller's buffer and load no Result. The $main epilogue already sets %eax=0 after its global-release pass, so it was correct. Two e2e regression tests on both backends: TestRun_Native_RetValSurvivesArcRelease (value return past a class-param release) and TestRun_Native_IntfArgToMethod (interface value arg to a method, dispatched in the callee). Full suite OK (2681), FIXPOINT_OK (native-backend-only change). --- .../pascal/blaise.codegen.native.x86_64.pas | 25 ++++--- .../src/test/pascal/cp.test.e2e.native.pas | 70 +++++++++++++++++++ 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/compiler/src/main/pascal/blaise.codegen.native.x86_64.pas b/compiler/src/main/pascal/blaise.codegen.native.x86_64.pas index d7f63b7..abf0a24 100644 --- a/compiler/src/main/pascal/blaise.codegen.native.x86_64.pas +++ b/compiler/src/main/pascal/blaise.codegen.native.x86_64.pas @@ -6802,16 +6802,13 @@ begin if ADecl.Body <> nil then for I := 0 to ADecl.Body.Stmts.Count - 1 do Self.EmitStmt(TASTStmt(ADecl.Body.Stmts.Items[I])); - { Epilogue: Exit lands here; load Result into %rax (int) or %xmm0 (float). - For sret functions the caller's buffer already holds the result — just ret. } + { Epilogue: Exit lands here. The Result is loaded into %rax/%xmm0 only AFTER + the ARC release passes below — those call _ClassRelease/_StringRelease, which + clobber %rax (and may touch the XMM regs), so loading Result first would + return garbage from a value-returning function that also releases ARC + params/locals. For sret functions the caller's buffer already holds the + result, so no load is needed. } Self.Emit(FExitLabel + ':'); - if (ADecl.ResolvedReturnType <> nil) and not FSretFunc then - begin - if IsFloatFamily(ADecl.ResolvedReturnType) then - Self.EmitLoadFloat(Self.VarOperand('Result'), ADecl.ResolvedReturnType) - else - Self.EmitLoadVar(Self.VarOperand('Result'), ADecl.ResolvedReturnType); - end; { Release ARC-managed locals (not params, not Result). Result is returned to the caller who owns it. } if ADecl.Body <> nil then @@ -6892,6 +6889,16 @@ begin Self.Emit(#9'callq _ClassRelease'); end; end; + { Now that every ARC release call is done (none can clobber the result), load + Result into %rax (int) or %xmm0 (float). sret functions return via the + caller's buffer and need no load. } + if (ADecl.ResolvedReturnType <> nil) and not FSretFunc then + begin + if IsFloatFamily(ADecl.ResolvedReturnType) then + Self.EmitLoadFloat(Self.VarOperand('Result'), ADecl.ResolvedReturnType) + else + Self.EmitLoadVar(Self.VarOperand('Result'), ADecl.ResolvedReturnType); + end; Self.Emit(#9'movq %rbp, %rsp'); Self.Emit(#9'popq %rbp'); Self.Emit(#9'ret'); diff --git a/compiler/src/test/pascal/cp.test.e2e.native.pas b/compiler/src/test/pascal/cp.test.e2e.native.pas index f8727ad..40b68a4 100644 --- a/compiler/src/test/pascal/cp.test.e2e.native.pas +++ b/compiler/src/test/pascal/cp.test.e2e.native.pas @@ -181,6 +181,8 @@ type procedure TestRun_Native_ArcInterfaceField_AssignAndDispatch; procedure TestRun_Native_IntfFieldDispatch; procedure TestRun_Native_IntfFieldReadIntoLocal; + procedure TestRun_Native_RetValSurvivesArcRelease; + procedure TestRun_Native_IntfArgToMethod; procedure TestRun_Native_ArcNestedRecordField_FullCleanup; procedure TestRun_Native_ArcStringReturnToField_NoDoubleRetain; procedure TestRun_Native_ArcImplicitSelfStringField_Reassign; @@ -2589,6 +2591,62 @@ const end. '''; + { Value-returning function whose return value must survive the epilogue ARC + release pass. A function returning Integer but taking an ARC (class) value + param releases that param on exit (_ClassRelease clobbers %rax); the Result + must be loaded into %rax AFTER that release, not before. Previously the + native backend loaded Result first and returned garbage (e.g. 3 not 55). } + SrcRetValSurvivesArcRelease = ''' + program P; + type + TGreeter = class + function Greet: Integer; + end; + TUser = class + function Use(O: TGreeter): Integer; + end; + function TGreeter.Greet: Integer; begin Result := 55 end; + function TUser.Use(O: TGreeter): Integer; + begin + Result := O.Greet() + end; + var T: TGreeter; U: TUser; + begin + T := TGreeter.Create(); + U := TUser.Create(); + WriteLn(U.Use(T)) + end. + '''; + + { Interface value argument passed to a method, dispatched inside the callee. + Exercises the fat-pointer arg ABI at a method call site AND the + return-value-survives-release fix (the callee releases the interface param). } + SrcIntfArgToMethod = ''' + program P; + type + IGreeter = interface + function Greet: Integer; + end; + TGreeter = class(TObject, IGreeter) + function Greet: Integer; + end; + TUser = class + function Use(G: IGreeter): Integer; + end; + function TGreeter.Greet: Integer; begin Result := 55 end; + function TUser.Use(G: IGreeter): Integer; + begin + Result := G.Greet() + end; + var T: TGreeter; I: IGreeter; U: TUser; + begin + T := TGreeter.Create(); + I := T; + U := TUser.Create(); + WriteLn(U.Use(I)) + end. + '''; + { Dyn-array field inside a class: the field must be ARC-refcounted on store and released when the holder is destroyed (f74e5cc). Observed by reading an element back after the field assignment — a dropped/garbled buffer would @@ -3165,6 +3223,18 @@ begin AssertRunsOnBoth(SrcIntfFieldReadIntoLocal, '40' + LE, 0); end; +procedure TE2ENativeTests.TestRun_Native_RetValSurvivesArcRelease; +begin + if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end; + AssertRunsOnBoth(SrcRetValSurvivesArcRelease, '55' + LE, 0); +end; + +procedure TE2ENativeTests.TestRun_Native_IntfArgToMethod; +begin + if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end; + AssertRunsOnBoth(SrcIntfArgToMethod, '55' + LE, 0); +end; + procedure TE2ENativeTests.TestRun_Native_ArcNestedRecordField_FullCleanup; begin if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end;