From 2d8497f2b37ba15bfa701d5d9a6beff9bbdec5b9 Mon Sep 17 00:00:00 2001 From: Graeme Geldenhuys Date: Thu, 18 Jun 2026 08:44:54 +0100 Subject: [PATCH] fix(native): keep %rsp 16-aligned in the record-call argument hoist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DateAddDays(MakeDate(...), 5) segfaulted on native while the non-nested form worked. The args arrived correct — the fault was a stack-alignment bug. The argument hoist for a record-returning-call argument allocated a 16-byte-aligned sret buffer (RecArgBufBytes) and then pushed an 8-byte pointer to it, a 24-byte contribution that left %rsp 8 bytes off 16-alignment. The misalignment was harmless until the callee chain reached an aligned SSE access — here libm's mktime (via DateUtils _TimeJoin) executing `movdqa`, which faults on a misaligned address. EmitArgHoist now reserves a full 16-byte slot for the saved pointer (subq $8 + pushq) so the hoist region stays a multiple of 16. The pad sits above the saved pointer, so reload offsets are unchanged. This is the shared hoist, so it fixes the alignment for every record-call-argument site, not just DateAddDays. The DateAddDays e2e test is re-enabled on both backends; a self-contained regression (record-call result as a record-by-value arg feeding an SSE op) is added to the gaps suite. All three fixpoints green; 3482 tests pass. --- .../pascal/blaise.codegen.native.x86_64.pas | 10 ++++- .../src/test/pascal/cp.test.e2e.dateutils.pas | 12 +++--- compiler/src/test/pascal/cp.test.e2e.gaps.pas | 37 +++++++++++++++++++ 3 files changed, 52 insertions(+), 7 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 880a333..dc7aef3 100644 --- a/compiler/src/main/pascal/blaise.codegen.native.x86_64.pas +++ b/compiler/src/main/pascal/blaise.codegen.native.x86_64.pas @@ -12269,8 +12269,16 @@ begin end else Self.EmitExprToEax(Arg); { buffer at %rsp, pointer in %rax } + { The buffer (RecArgBufBytes, 16-aligned) plus the 8-byte saved pointer + would leave the hoist region 8 bytes off 16-alignment. Reserve a + 16-byte slot for the pointer (push + 8-byte pad) so the total stays a + multiple of 16 — otherwise a later call in this argument list (or the + callee, e.g. a libm routine using `movdqa`) faults on a misaligned %rsp. + The pad sits ABOVE the saved pointer, so the reload offset (which uses + Result - depth) is unchanged. } + Self.Emit(#9'subq $8, %rsp'); Self.Emit(#9'pushq %rax'); - Result := Result + Self.RecArgBufBytes(Arg) + 8; + Result := Result + Self.RecArgBufBytes(Arg) + 16; ADepths.Add(Result); AKinds.Add(akRecCall); Continue; diff --git a/compiler/src/test/pascal/cp.test.e2e.dateutils.pas b/compiler/src/test/pascal/cp.test.e2e.dateutils.pas index b0a185d..1c226cc 100644 --- a/compiler/src/test/pascal/cp.test.e2e.dateutils.pas +++ b/compiler/src/test/pascal/cp.test.e2e.dateutils.pas @@ -1148,12 +1148,12 @@ var Lines: TStringList; begin if not ToolchainAvailable() then begin Fail(''); Exit end; - { QBE-only for now: native segfaults when a record-returning (sret) function - receives a record-by-value argument that is itself a record-returning call - result, e.g. DateAddDays(MakeDate(...), 5). EmitSretCall mishandles the - record-value argument in that nested shape (the non-nested form works). - Tracked in bugs.txt; the rest of this suite runs on both backends. } - AssertTrue('compile+run', CompileAndRunWithRTLQBEOnly(SrcDateAddDays, Output, RCode)); + { Dual-backend again: the native segfault on DateAddDays(MakeDate(...), 5) was + a stack-alignment bug in the record-call-argument hoist (buffer 16 + 8-byte + saved pointer = 24, leaving %rsp 8 off 16-alignment), which only faulted once + the callee chain reached a movdqa (libm mktime). Fixed by padding the hoist + region to a 16-byte multiple. } + AssertTrue('compile+run', CompileAndRunWithRTL(SrcDateAddDays, Output, RCode)); AssertEquals('exit 0', 0, RCode); Lines := TStringList.Create(); try diff --git a/compiler/src/test/pascal/cp.test.e2e.gaps.pas b/compiler/src/test/pascal/cp.test.e2e.gaps.pas index 70fc80e..ff75b0d 100644 --- a/compiler/src/test/pascal/cp.test.e2e.gaps.pas +++ b/compiler/src/test/pascal/cp.test.e2e.gaps.pas @@ -82,6 +82,10 @@ type { Interface-returning call result passed positionally as an arg } procedure TestRun_IntfArg_CallResult_AsParam; + + { Record-returning call result passed as a record-by-value arg (the hoist + must keep %rsp 16-aligned) } + procedure TestRun_RecordCallResult_AsValueArg; end; implementation @@ -947,6 +951,39 @@ begin AssertRunsOnAll(Src, '42' + Chr(10), 0); end; +procedure TE2EGapTests.TestRun_RecordCallResult_AsValueArg; +{ A record-returning function's result passed directly as a record-by-value + argument to another function — Use(Make(10), 16.0). The native arg-hoist + materialises Make's result into a stack buffer and saves a pointer; that + region must stay a multiple of 16 bytes or %rsp drifts off 16-alignment and a + later SSE op (here Sqrt) — or a libm routine using movdqa — faults. This + regressed DateAddDays(MakeDate(...), 5). } +const + Src = + ''' + program P; + type + TR = record + A, B, C: Integer; + end; + function Make(N: Integer): TR; + begin + Result.A := N; Result.B := N + 1; Result.C := N + 2 + end; + function Use(R: TR; D: Double): Double; + begin + Result := Sqrt(D) + R.A + R.B + R.C + end; + var X: Double; + begin + X := Use(Make(10), 16.0); + WriteLn(X) + end. + '''; +begin + AssertRunsOnAll(Src, '37' + Chr(10), 0); +end; + initialization RegisterTest(TE2EGapTests);