fix(codegen): native scalar field of a record-call result as a call argument
The native x86-64 backend corrupted other arguments when a SCALAR FIELD of a
record-returning call result was read inline as a call argument — e.g.
Check('vals', 1, MakeR().A); // 'vals' lost on native; QBE was fine
Reading MakeR().A went through EmitExprToEax, which materialises the call's
sret buffer (a stack subq) and never frees it (the frame epilogue
reclaims it for a standalone expression). Inside the argument push/pop
sequence that stray allocation shifted %rsp, so an already-pushed argument
(typically the const-string) was reloaded from the wrong slot and lost —
crashing later in _StringAddRef when the callee retained the garbage pointer.
Fix: in the chained-field-read path, when the base is a record-returning call
and the field is scalar, allocate the sret buffer, read the field out of it,
then FREE the buffer so the expression nets zero stack change and the argument
protocol stays balanced. Record/static-array fields are excluded — they
return an address INTO the buffer, whose lifetime must outlive the read.
Regression test (both backends) in cp.test.e2e.recordret; the inline
HostTarget().OS assertion in cp.test.toolchain (previously worked around) now
exercises the fixed path directly.
FIXPOINT_OK, NATIVE_FIXPOINT_OK, NATIVE_INTERNAL_OK; full suite OK (3363).
This commit is contained in:
parent
c266a8ee74
commit
1753b5d2a8
|
|
@ -6650,6 +6650,29 @@ begin
|
|||
(TFieldAccessExpr(AExpr).Base <> nil) then
|
||||
begin
|
||||
FAE := TFieldAccessExpr(AExpr);
|
||||
{ When the base is a record-returning call, EmitExprToEax materialises an
|
||||
sret buffer with `subq $N,%rsp` and never frees it — fine for a
|
||||
standalone expression (the frame epilogue reclaims it), but inside an
|
||||
argument push/pop sequence that stray allocation shifts %rsp and
|
||||
corrupts the other already-pushed arguments (e.g. a const-string arg
|
||||
reloaded by %rsp offset). Allocate the buffer here, read the scalar
|
||||
field out of it, then FREE it so this expression nets zero stack change.
|
||||
Only safe for scalar fields — a record/static-array field returns an
|
||||
address INTO the buffer, so its lifetime must outlive this read and the
|
||||
buffer cannot be freed here. }
|
||||
if Self.IsNativeRecordCall(FAE.Base) and
|
||||
not (IsJumboSet(FAE.FieldInfo.TypeDesc) or
|
||||
(FAE.FieldInfo.TypeDesc.Kind in [tyRecord, tyStaticArray])) then
|
||||
begin
|
||||
Self.Emit(Format(#9'subq $%d, %%rsp', [Self.RecArgBufBytes(FAE.Base)]));
|
||||
Self.EmitRecordCallSretAt(FAE.Base, '(%rsp)');
|
||||
Self.Emit(#9'movq %rsp, %rcx');
|
||||
if FAE.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'leaq %d(%%rcx), %%rcx', [FAE.FieldInfo.Offset]));
|
||||
Self.EmitLoadVar('(%rcx)', FAE.FieldInfo.TypeDesc);
|
||||
Self.Emit(Format(#9'addq $%d, %%rsp', [Self.RecArgBufBytes(FAE.Base)]));
|
||||
Exit;
|
||||
end;
|
||||
Self.EmitExprToEax(FAE.Base);
|
||||
Self.Emit(#9'movq %rax, %rcx');
|
||||
if FAE.FieldInfo.Offset > 0 then
|
||||
|
|
|
|||
|
|
@ -70,6 +70,12 @@ type
|
|||
so the argument pointer was garbage and the callee read junk (crash).
|
||||
Both backends. }
|
||||
procedure TestRun_RecordMethodResult_AsConstArg;
|
||||
{ Regression: a SCALAR FIELD of a record-returning call result, read inline
|
||||
as an argument alongside a const-string argument. Reading the field via
|
||||
EmitExprToEax left the call's sret buffer allocated on the stack, shifting
|
||||
%rsp so the already-pushed string argument was reloaded from the wrong
|
||||
slot and lost. Both backends. }
|
||||
procedure TestRun_RecordCallFieldArg_WithStringArg;
|
||||
end;
|
||||
|
||||
implementation
|
||||
|
|
@ -512,6 +518,29 @@ begin
|
|||
AssertRunsOnAll(Src, '3' + LE, 0);
|
||||
end;
|
||||
|
||||
procedure TE2ERecordReturnTests.TestRun_RecordCallFieldArg_WithStringArg;
|
||||
const
|
||||
Src = '''
|
||||
program P;
|
||||
type TR = record A, B: Integer; end;
|
||||
function MakeR: TR;
|
||||
begin
|
||||
Result.A := 1;
|
||||
Result.B := 2
|
||||
end;
|
||||
procedure Check(const Msg: string; Expected, Actual: Integer);
|
||||
begin
|
||||
WriteLn(Msg, ' ', Expected, ' ', Actual)
|
||||
end;
|
||||
begin
|
||||
Check('vals', 1, MakeR().A)
|
||||
end.
|
||||
''';
|
||||
begin
|
||||
if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end;
|
||||
AssertRunsOnAll(Src, 'vals 1 1' + LE, 0);
|
||||
end;
|
||||
|
||||
initialization
|
||||
RegisterTest(TE2ERecordReturnTests);
|
||||
|
||||
|
|
|
|||
|
|
@ -114,16 +114,13 @@ begin
|
|||
end;
|
||||
|
||||
procedure TToolchainTests.TestHostTarget_OnThisHost_IsLinux;
|
||||
var
|
||||
HostOS: Integer;
|
||||
begin
|
||||
{ The compiler binary running the suite has no .exe suffix, so HostTarget
|
||||
detects linux. (On a Windows host this test would need adjusting — but
|
||||
the suite is built and run on linux.) Read .OS into a local first: a
|
||||
record-call result read inline as an AssertEquals argument trips a
|
||||
separate native sret bug (see bugs.txt). }
|
||||
HostOS := Ord(HostTarget().OS);
|
||||
AssertEquals('host OS is linux', Ord(osLinux), HostOS);
|
||||
the suite is built and run on linux.) The inline HostTarget().OS read as
|
||||
an AssertEquals argument also exercises the native sret-call-field-arg
|
||||
path fixed alongside this work. }
|
||||
AssertEquals('host OS is linux', Ord(osLinux), Ord(HostTarget().OS));
|
||||
end;
|
||||
|
||||
procedure TToolchainTests.TestHostExeExt_OnThisHost_IsEmpty;
|
||||
|
|
|
|||
Loading…
Reference in a new issue