From 06a6bc0d1e72ca45a41ab6b83d9b066e9479b310 Mon Sep 17 00:00:00 2001 From: Graeme Geldenhuys Date: Sat, 20 Jun 2026 03:25:31 +0100 Subject: [PATCH] fix(native): method call on a transient record-returning result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A method call whose receiver is itself a record-returning call — A.Plus(B).Val() — crashed on the native backend. The receiver expression was lowered with EmitExprToEax, leaving the record VALUE (the register-return payload, or the first bytes of the sret buffer) in %rax, which was then used directly as the Self POINTER. Dereferencing that garbage segfaulted. QBE handled the chained form correctly, so the dual-backend e2e harness flagged it as a native/QBE parity divergence. Fix: when a record method's receiver is a record-returning call, materialise the call result into a stack buffer and pass its ADDRESS as Self. The buffer address is carried in callee-saved %rbx so it survives the argument push/pop sequence; both are freed after the call. Applied to both EmitMethodCallExpr paths (<=6 and >6 arg slots) and all four receiver sites in EmitMethodSretCall. For the sret path, a forwarded %rsp-relative destination (the nested chain A.Plus(B).Plus(A), where EmitRecordCallSretAt forwards '(%rsp)') is resolved to an absolute pointer in callee-saved %r14 BEFORE the receiver-materialisation prologue moves %rsp, so the destination no longer drifts. Regression tests in cp.test.e2e.recordret.pas cover the register-return chain, the sret-record chain, the >5-arg overflow path, the nested double chain, and the managed-field (TDecimal-shaped) record — all via AssertRunsOnAll (QBE + native parity). Verified: FIXPOINT_OK, NATIVE_FIXPOINT_OK, NATIVE_INTERNAL_OK; full suite green on both the QBE-built and native-built test runners (3621 tests). --- .../pascal/blaise.codegen.native.x86_64.pas | 132 +++++++++++++++- .../src/test/pascal/cp.test.e2e.recordret.pas | 149 ++++++++++++++++++ 2 files changed, 273 insertions(+), 8 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 90507de..d4ccc4a 100644 --- a/compiler/src/main/pascal/blaise.codegen.native.x86_64.pas +++ b/compiler/src/main/pascal/blaise.codegen.native.x86_64.pas @@ -419,6 +419,8 @@ type procedure EmitRecordCallSretAt(AExpr: TASTExpr; const ADest: string); procedure EmitMethodSretCall(ACall: TMethodCallExpr; const ASretAddr: string; ASretIsIndirect: Boolean); + { Free a record-call-receiver buffer materialised by EmitMethodSretCall. } + procedure EmitMethodSretRecvCleanup(ABytes: Integer); { Emit the base ADDRESS of a NAMED LOCAL record into AReg (e.g. '%rcx', '%rax'). Normally a stack value record, so leaq the slot. The one exception is the sret-function Result: its frame slot holds @@ -7615,6 +7617,7 @@ var HD: TList; HK: TList; HTotal: Integer; + RecvBufBytes: Integer; begin { Interface method dispatch: receiver is an interface fat pointer; route through the itab rather than a static method symbol. } @@ -7791,6 +7794,22 @@ begin if TotalSlots <= 6 then begin + { Record-returning-call receiver (e.g. A.Plus(B).Val()): the receiver is a + transient record value with no home, but a record method needs Self as an + ADDRESS. Materialise the call result into a stack buffer FIRST (before any + args are pushed) and carry its address in callee-saved %rbx, which survives + the arg push/pop sequence. %rbx is saved/restored around the whole call. } + RecvBufBytes := 0; + if (ACall.ObjExpr <> nil) and MD.IsRecordMethod and + Self.IsNativeRecordCall(ACall.ObjExpr) then + begin + RecvBufBytes := Self.RecArgBufBytes(ACall.ObjExpr); + Self.Emit(#9'pushq %rbx'); + Self.Emit(Format(#9'subq $%d, %%rsp', [RecvBufBytes])); + Self.EmitRecordCallSretAt(ACall.ObjExpr, '(%rsp)'); + Self.Emit(#9'movq %rsp, %rbx'); { receiver address -> callee-saved } + end; + Self.BeginCallArgs(MD.Params, ACall.Args); for I := 0 to ACall.Args.Count - 1 do begin @@ -7822,6 +7841,8 @@ begin Self.Emit(Format(#9'movq %s(%%rip), %%r10', [ACall.ObjectName])); end; end + else if (ACall.ObjExpr <> nil) and (RecvBufBytes > 0) then + Self.Emit(#9'movq %rbx, %r10') { record-call receiver address } else if ACall.ObjExpr <> nil then begin Self.EmitExprToEax(ACall.ObjExpr); @@ -7842,9 +7863,29 @@ begin else Self.Emit(#9'callq ' + Sym); Self.EndCallArgs(); + if RecvBufBytes > 0 then + begin + { Free the receiver buffer and restore %rbx (does not touch the return + registers %rax/%rdx/%xmm0/%xmm1). } + Self.Emit(Format(#9'addq $%d, %%rsp', [RecvBufBytes])); + Self.Emit(#9'popq %rbx'); + end; end else begin + { Record-returning-call receiver on the >6-slot path: materialise the result + into a stack buffer and carry its address in callee-saved %rbx (immune to + the hoist/alloc %rsp movement below). See the <=6 path for the rationale. } + RecvBufBytes := 0; + if (ACall.ObjExpr <> nil) and MD.IsRecordMethod and + Self.IsNativeRecordCall(ACall.ObjExpr) then + begin + RecvBufBytes := Self.RecArgBufBytes(ACall.ObjExpr); + Self.Emit(#9'pushq %rbx'); + Self.Emit(Format(#9'subq $%d, %%rsp', [RecvBufBytes])); + Self.EmitRecordCallSretAt(ACall.ObjExpr, '(%rsp)'); + Self.Emit(#9'movq %rsp, %rbx'); + end; OverflowSlots := TotalSlots - 6; CleanUp := ((OverflowSlots * 8 + 15) and (-16)); AllocSz := 6 * 8 + CleanUp; @@ -7895,6 +7936,8 @@ begin Self.Emit(Format(#9'movq %s(%%rip), %%rax', [ACall.ObjectName])); end; end + else if (ACall.ObjExpr <> nil) and (RecvBufBytes > 0) then + Self.Emit(#9'movq %rbx, %rax') { record-call receiver address } else if ACall.ObjExpr <> nil then Self.EmitExprToEax(ACall.ObjExpr) else @@ -7916,6 +7959,11 @@ begin Self.EmitHoistEpilogue(ACall.Args, HD, HK, HTotal, CleanUp, True); HD.Free(); HK.Free(); + if RecvBufBytes > 0 then + begin + Self.Emit(Format(#9'addq $%d, %%rsp', [RecvBufBytes])); + Self.Emit(#9'popq %rbx'); + end; end; end; @@ -13918,6 +13966,21 @@ begin HK.Free(); end; +{ Free a record-call-receiver buffer materialised by EmitMethodSretCall (see + the RecvBufBytes prologue there). No-op when ABytes = 0. Does not touch the + sret destination (written via its own pointer) nor any return register. } +procedure TX86_64Backend.EmitMethodSretRecvCleanup(ABytes: Integer); +begin + if ABytes > 0 then + begin + { Mirror the prologue's pushes: subq buffer, pushq %rbx, pushq %r14 + (in that order) -> free buffer, popq %rbx, popq %r14. } + Self.Emit(Format(#9'addq $%d, %%rsp', [ABytes])); + Self.Emit(#9'popq %rbx'); + Self.Emit(#9'popq %r14'); + end; +end; + { Emit a method call that returns a record via the sret convention. Register layout: %rdi = sret ptr, %rsi = Self, %rdx.. = user args. The destination buffer is at ASretAddr (AT&T operand, already allocated). } @@ -13938,6 +14001,9 @@ var HasFloat: Boolean; ParamType: TTypeDesc; IntIdx, XmmIdx, SlotOff: Integer; + RecvBufBytes: Integer; + LSretAddr: string; + LSretIndirect: Boolean; begin MD := TMethodDecl(ACall.ResolvedMethod); if MD = nil then @@ -13945,6 +14011,45 @@ begin 'native backend: method-sret call has no ResolvedMethod (' + ACall.Name + ')'); Sym := MethodEmitNameNative(MD, MD.OwnerTypeName, ACall.Name); + { Record-returning-call receiver (e.g. A.Plus(B).Scale(2), where the OUTER + method Scale itself returns a record via sret): materialise the inner call + result into a stack buffer up front and carry its address in callee-saved + %rbx, which survives all the arg-evaluation %rsp movement below. Every + ObjExpr-receiver site loads %rbx instead of evaluating the call value as a + pointer; the buffer + %rbx are freed at each exit (EmitMethodSretCleanup). + + Because this prologue moves %rsp, a caller-supplied %rsp-relative ASretAddr + (the recursive EmitRecordCallSretAt('(%rsp)') case in a nested chain like + A.Plus(B).Plus(A)) would drift. Resolve the destination to an ABSOLUTE + pointer FIRST, save it in callee-saved %r14, and re-express the rest of the + routine as an indirect store through %r14. } + RecvBufBytes := 0; + LSretAddr := ASretAddr; + LSretIndirect := ASretIsIndirect; + if (ACall.ObjExpr <> nil) and MD.IsRecordMethod and + Self.IsNativeRecordCall(ACall.ObjExpr) then + begin + RecvBufBytes := Self.RecArgBufBytes(ACall.ObjExpr); + { Resolve the destination to an absolute pointer BEFORE pushing anything — + a %rsp-relative ASretAddr (the recursive EmitRecordCallSretAt('(%rsp)') + case) must be read while %rsp still has its caller value. } + if ASretIsIndirect then + Self.Emit(Format(#9'movq %s, %%rax', [ASretAddr])) + else + Self.Emit(Format(#9'leaq %s, %%rax', [ASretAddr])); + Self.Emit(#9'pushq %r14'); + Self.Emit(#9'movq %rax, %r14'); + { %r14 now HOLDS the destination address. Express it as a non-indirect + operand so consumers do `leaq (%r14), %reg` (i.e. %reg := %r14), not a + double dereference. } + LSretAddr := '(%r14)'; + LSretIndirect := False; + Self.Emit(#9'pushq %rbx'); + Self.Emit(Format(#9'subq $%d, %%rsp', [RecvBufBytes])); + Self.EmitRecordCallSretAt(ACall.ObjExpr, '(%rsp)'); + Self.Emit(#9'movq %rsp, %rbx'); + end; + { Check for register-return: no hidden sret param, Self goes in %rdi, args in %rsi onwards. } RC := rcSret; @@ -13953,10 +14058,10 @@ begin RC := ClassifyRecordReturn(TRecordTypeDesc(MD.ResolvedReturnType)); if RC <> rcSret then begin - if ASretIsIndirect then - Self.Emit(Format(#9'movq %s, %%r10', [ASretAddr])) + if LSretIndirect then + Self.Emit(Format(#9'movq %s, %%r10', [LSretAddr])) else - Self.Emit(Format(#9'leaq %s, %%r10', [ASretAddr])); + Self.Emit(Format(#9'leaq %s, %%r10', [LSretAddr])); Self.Emit(#9'movq %r10, %rdi'); Self.Emit(#9'xorl %esi, %esi'); Self.Emit(Format(#9'movq $%d, %%rdx', @@ -13992,6 +14097,8 @@ begin Self.Emit(Format(#9'movq %s(%%rip), %%rdi', [ACall.ObjectName])); end; end + else if (ACall.ObjExpr <> nil) and (RecvBufBytes > 0) then + Self.Emit(#9'movq %rbx, %rdi') { record-call receiver address } else if ACall.ObjExpr <> nil then begin Self.EmitExprToEax(ACall.ObjExpr); @@ -14010,18 +14117,19 @@ begin else Self.Emit(#9'callq ' + Sym); Self.EndCallArgs(); - Self.EmitRecordRegReturnCapture(ASretAddr, - TRecordTypeDesc(MD.ResolvedReturnType), RC, ASretIsIndirect); + Self.EmitRecordRegReturnCapture(LSretAddr, + TRecordTypeDesc(MD.ResolvedReturnType), RC, LSretIndirect); + Self.EmitMethodSretRecvCleanup(RecvBufBytes); Exit; end; { Save the destination address below the hoist region — no caller-saved register survives argument evaluation, and a %rsp-relative ASretAddr would drift once the region is live. } - if ASretIsIndirect then - Self.Emit(Format(#9'movq %s, %%rax', [ASretAddr])) + if LSretIndirect then + Self.Emit(Format(#9'movq %s, %%rax', [LSretAddr])) else - Self.Emit(Format(#9'leaq %s, %%rax', [ASretAddr])); + Self.Emit(Format(#9'leaq %s, %%rax', [LSretAddr])); Self.Emit(#9'pushq %rax'); if (MD.ResolvedReturnType <> nil) then begin @@ -14117,6 +14225,8 @@ begin else Self.Emit(Format(#9'movq %s(%%rip), %%rax', [ACall.ObjectName])); end + else if (ACall.ObjExpr <> nil) and (RecvBufBytes > 0) then + Self.Emit(#9'movq %rbx, %rax') { record-call receiver address } else if ACall.ObjExpr <> nil then Self.EmitExprToEax(ACall.ObjExpr) else @@ -14168,6 +14278,7 @@ begin HD.Free(); HK.Free(); Self.Emit(#9'addq $8, %rsp'); { reclaim the saved dest slot } + Self.EmitMethodSretRecvCleanup(RecvBufBytes); Exit; end; @@ -14204,6 +14315,8 @@ begin Self.Emit(Format(#9'movq %s(%%rip), %%rsi', [ACall.ObjectName])); end; end + else if (ACall.ObjExpr <> nil) and (RecvBufBytes > 0) then + Self.Emit(#9'movq %rbx, %rsi') { record-call receiver address } else if ACall.ObjExpr <> nil then begin Self.EmitExprToEax(ACall.ObjExpr); @@ -14254,6 +14367,8 @@ begin else Self.Emit(Format(#9'movq %s(%%rip), %%rax', [ACall.ObjectName])); end + else if (ACall.ObjExpr <> nil) and (RecvBufBytes > 0) then + Self.Emit(#9'movq %rbx, %rax') { record-call receiver address } else if ACall.ObjExpr <> nil then Self.EmitExprToEax(ACall.ObjExpr) else @@ -14271,6 +14386,7 @@ begin end; { Reclaim the saved dest slot. } Self.Emit(#9'addq $8, %rsp'); + Self.EmitMethodSretRecvCleanup(RecvBufBytes); end; { Emit a standalone function call that returns an interface via sret. diff --git a/compiler/src/test/pascal/cp.test.e2e.recordret.pas b/compiler/src/test/pascal/cp.test.e2e.recordret.pas index ab4c8f3..eaaa976 100644 --- a/compiler/src/test/pascal/cp.test.e2e.recordret.pas +++ b/compiler/src/test/pascal/cp.test.e2e.recordret.pas @@ -86,6 +86,17 @@ type procedure TestRun_FloatArg_ManagedRecordReturn_Func; procedure TestRun_TwoFloatArgs_ManagedRecordReturn_Func; procedure TestRun_FloatArg_ManagedRecordReturn_Method; + { Regression: a method call whose RECEIVER is itself a record-returning + call — A.Plus(B).Val(). The native backend used the record VALUE (the + reg-return payload, or the sret buffer's first bytes) as the Self POINTER, + dereferencing garbage and crashing. It now materialises the receiver + result into a stack buffer and passes its address as Self. QBE was always + correct; both backends asserted equal. } + procedure TestRun_ChainedRecvRegReturn_ScalarResult; + procedure TestRun_ChainedRecvSretReturn_RecordResult; + procedure TestRun_ChainedRecvManyArgs; + procedure TestRun_ChainedRecvDoubleChain; + procedure TestRun_ChainedRecvManagedRecord_TDecimalLike; end; implementation @@ -611,6 +622,144 @@ begin AssertRunsOnAll(Src, '0.25' + LE, 0); end; +{ ------------------------------------------------------------------ } +{ Chained record-call receiver (A.Plus(B).Method()) } +{ ------------------------------------------------------------------ } + +procedure TE2ERecordReturnTests.TestRun_ChainedRecvRegReturn_ScalarResult; +const + Src = ''' + program P; + type + TR = record + V: Integer; + function Plus(const B: TR): TR; + function Val: Integer; + end; + function TR.Plus(const B: TR): TR; begin Result.V := Self.V + B.V end; + function TR.Val: Integer; begin Result := Self.V end; + var A, B: TR; N: Integer; + begin + A.V := 10; B.V := 5; + N := A.Plus(B).Val(); + WriteLn(N) + end. + '''; +begin + if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end; + AssertRunsOnAll(Src, '15' + LE, 0); +end; + +procedure TE2ERecordReturnTests.TestRun_ChainedRecvSretReturn_RecordResult; +const + { The OUTER method (Scale) itself returns a record via sret, and its receiver + is the transient result of Plus — exercises the sret-call receiver path. } + Src = ''' + program P; + type + TR = record + V: Integer; + function Plus(const B: TR): TR; + function Scale(F: Integer): TR; + function Val: Integer; + end; + function TR.Plus(const B: TR): TR; begin Result.V := Self.V + B.V end; + function TR.Scale(F: Integer): TR; begin Result.V := Self.V * F end; + function TR.Val: Integer; begin Result := Self.V end; + var A, B, C: TR; + begin + A.V := 10; B.V := 5; + C := A.Plus(B).Scale(2); + WriteLn(C.Val()) + end. + '''; +begin + if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end; + AssertRunsOnAll(Src, '30' + LE, 0); +end; + +procedure TE2ERecordReturnTests.TestRun_ChainedRecvManyArgs; +const + { >5 user args forces the overflow (stack-arg) call path with a chained + record-call receiver. } + Src = ''' + program P; + type + TR = record + V: Integer; + function Plus(const B: TR): TR; + function Sum6(A1,A2,A3,A4,A5,A6: Integer): Integer; + end; + function TR.Plus(const B: TR): TR; begin Result.V := Self.V + B.V end; + function TR.Sum6(A1,A2,A3,A4,A5,A6: Integer): Integer; + begin Result := Self.V + A1+A2+A3+A4+A5+A6 end; + var A, B: TR; N: Integer; + begin + A.V := 10; B.V := 5; + N := A.Plus(B).Sum6(1,2,3,4,5,6); + WriteLn(N) + end. + '''; +begin + if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end; + AssertRunsOnAll(Src, '36' + LE, 0); +end; + +procedure TE2ERecordReturnTests.TestRun_ChainedRecvDoubleChain; +const + { Nested chain: the receiver of .Plus(A) is itself A.Plus(B). Catches a + %rsp-drift bug where the forwarded sret destination was resolved after a + push had already moved the stack. } + Src = ''' + program P; + type + TR = record + V: Integer; + function Plus(const B: TR): TR; + function Val: Integer; + end; + function TR.Plus(const B: TR): TR; begin Result.V := Self.V + B.V end; + function TR.Val: Integer; begin Result := Self.V end; + var A, B: TR; N: Integer; + begin + A.V := 10; B.V := 5; + N := A.Plus(B).Plus(A).Val(); + WriteLn(N) + end. + '''; +begin + if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end; + AssertRunsOnAll(Src, '25' + LE, 0); +end; + +procedure TE2ERecordReturnTests.TestRun_ChainedRecvManagedRecord_TDecimalLike; +const + { A managed-field record (dynamic array) returned by value, then chained — + the shape TDecimal uses. Single and double chain both checked. } + Src = ''' + program P; + type + TR = record + V: Integer; + M: array of UInt32; + function Plus(const B: TR): TR; + function Val: Integer; + end; + function TR.Plus(const B: TR): TR; + begin SetLength(Result.M, 1); Result.M[0] := 0; Result.V := Self.V + B.V end; + function TR.Val: Integer; begin Result := Self.V end; + var A, B: TR; + begin + A.V := 10; B.V := 5; + WriteLn(A.Plus(B).Val()); + WriteLn(A.Plus(B).Plus(A).Val()) + end. + '''; +begin + if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end; + AssertRunsOnAll(Src, '15' + LE + '25' + LE, 0); +end; + initialization RegisterTest(TE2ERecordReturnTests);