fix(arc): release managed fields of record arg temporaries after the call
Companion to the callee-side ARC pass. When a record-returning function result is consumed inline as a by-value record argument — DoSomething(GetRec()) — the caller allocates an sret buffer for the return, hands it to the callee as the aggregate arg, then drops the buffer at end of statement. With no per-managed-leaf release on that buffer the temp's string, dynarray, class, and interface fields leaked one ref per caller invocation, accumulating quickly in loops. EmitOwnedArgReleases now handles tyRecord: when the argument expression is a record-returning function or method call (the only shape that produces a fresh sret temp), the helper walks the buffer with EmitRecordReleaseFields and emits a release per managed leaf, recursing through nested record fields. Variable references (DoSomething(W)) are explicitly excluded — their storage belongs to the enclosing scope's own ARC pass and must not be touched by the call site. POD record temps reach the helper too, but the field walk finds nothing to release, so the cleanup compiles down to a no-op for them. Covered by IR-level assertions on the temp / variable / nested-record cases, plus an e2e stress loop that runs the inline pattern 1000 times with a two-string nested record.
This commit is contained in:
parent
f5368c0699
commit
4313c6e53a
|
|
@ -3576,6 +3576,22 @@ begin
|
|||
end;
|
||||
end;
|
||||
|
||||
{ True when the expression produces a fresh sret record temporary that
|
||||
nothing else holds — i.e. a record-returning function/method call. A
|
||||
plain variable reference (record-typed TIdentExpr) is NOT a temp: its
|
||||
storage belongs to the enclosing scope and must not be cleaned up here. }
|
||||
function IsRecordSretTempExpr(AExpr: TASTExpr): Boolean;
|
||||
begin
|
||||
Result := False;
|
||||
if AExpr = nil then Exit;
|
||||
if AExpr.ResolvedType = nil then Exit;
|
||||
if AExpr.ResolvedType.Kind <> tyRecord then Exit;
|
||||
if AExpr is TFuncCallExpr then
|
||||
Result := TFuncCallExpr(AExpr).ResolvedDecl <> nil
|
||||
else if AExpr is TMethodCallExpr then
|
||||
Result := not TMethodCallExpr(AExpr).IsConstructorCall;
|
||||
end;
|
||||
|
||||
procedure TCodeGenQBE.EmitOwnedArgReleases(AArgs: TObjectList;
|
||||
AArgTemps: TStringList; AParams: TObjectList);
|
||||
var
|
||||
|
|
@ -3588,6 +3604,18 @@ begin
|
|||
if I >= AArgTemps.Count then Break;
|
||||
if AArgTemps.Strings[I] = '' then Continue;
|
||||
Arg := TASTExpr(AArgs.Items[I]);
|
||||
{ Record-typed sret temporary from an inline call (DoSomething(GetRec())):
|
||||
release each managed leaf of the temp buffer. The buffer itself is
|
||||
stack-allocated and dies with the caller's frame, but its string /
|
||||
dynarray / class / interface fields hold heap refs that need balancing.
|
||||
Reg-returned (POD) records have no managed leaves, so the helper is
|
||||
a safe no-op in that case. }
|
||||
if IsRecordSretTempExpr(Arg) then
|
||||
begin
|
||||
EmitRecordReleaseFields(TRecordTypeDesc(Arg.ResolvedType),
|
||||
AArgTemps.Strings[I]);
|
||||
Continue;
|
||||
end;
|
||||
if not ExprOwnsRef(Arg) then Continue;
|
||||
Par := nil;
|
||||
if (AParams <> nil) and (I < AParams.Count) then
|
||||
|
|
|
|||
|
|
@ -40,6 +40,7 @@ type
|
|||
procedure TestRun_Class_RecordField_NestedClass_FullCleanup;
|
||||
procedure TestRun_Record_InterfaceField_AssignCallAndCopy;
|
||||
procedure TestRun_Record_ByValParam_StringField_HeapARC;
|
||||
procedure TestRun_Record_ByValArg_InlineCallResult_NestedManaged_NoLeak;
|
||||
end;
|
||||
|
||||
implementation
|
||||
|
|
@ -434,6 +435,38 @@ const
|
|||
end.
|
||||
''';
|
||||
|
||||
{ Caller-side cleanup of an inline call result used as a by-value
|
||||
record arg: Consume(MakeOuter()) where TOuter has a string field
|
||||
AND a nested TInner whose own field is a string. Before the fix
|
||||
the temporary's two heap strings leaked once per Driver call; over
|
||||
many iterations the process either OOMs or the allocator's free
|
||||
list ages noticeably. Successful run prints both fields once and
|
||||
exits 0; we drive the pattern in a tight loop so a leak would
|
||||
show up under stress. }
|
||||
SrcRecordByValArg_InlineNestedManaged = '''
|
||||
program P;
|
||||
type
|
||||
TInner = record N: string; end;
|
||||
TOuter = record S: string; Inner: TInner; end;
|
||||
function MakeOuter: TOuter;
|
||||
begin
|
||||
Result.S := 'outer-' + 'heap';
|
||||
Result.Inner.N := 'inner-' + 'heap'
|
||||
end;
|
||||
procedure Consume(R: TOuter);
|
||||
begin
|
||||
WriteLn(R.S, '|', R.Inner.N)
|
||||
end;
|
||||
procedure Driver;
|
||||
begin
|
||||
Consume(MakeOuter())
|
||||
end;
|
||||
var i: Integer;
|
||||
begin
|
||||
for i := 1 to 1000 do Driver()
|
||||
end.
|
||||
''';
|
||||
|
||||
SrcRecordDynArrayReturnByValueNoLeak = '''
|
||||
program P;
|
||||
type
|
||||
|
|
@ -700,6 +733,17 @@ begin
|
|||
'before: heap-allocated' + LE + 'after: heap-allocated' + LE, Output);
|
||||
end;
|
||||
|
||||
procedure TE2ERecordsTests.TestRun_Record_ByValArg_InlineCallResult_NestedManaged_NoLeak;
|
||||
var Output: string; RCode: Integer;
|
||||
begin
|
||||
if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit; end;
|
||||
AssertTrue('compile+run',
|
||||
CompileAndRun(SrcRecordByValArg_InlineNestedManaged, Output, RCode));
|
||||
AssertEquals('exit code 0 over 1000 inline-temp iterations', 0, RCode);
|
||||
AssertTrue('Driver printed nested strings',
|
||||
Pos('outer-heap|inner-heap' + LE, Output) >= 0);
|
||||
end;
|
||||
|
||||
initialization
|
||||
RegisterTest(TE2ERecordsTests);
|
||||
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ unit cp.test.records;
|
|||
interface
|
||||
|
||||
uses
|
||||
blaise.testing,
|
||||
blaise.testing, strutils,
|
||||
uLexer, uParser, uAST, uSymbolTable, uSemantic, uCodeGenQBE;
|
||||
|
||||
type
|
||||
|
|
@ -85,6 +85,9 @@ type
|
|||
procedure TestCodegen_RecordByValParam_StringField_AddRefRelease;
|
||||
procedure TestCodegen_RecordByValParam_DynArrayField_AddRefRelease;
|
||||
procedure TestCodegen_RecordByValParam_ConstParam_NoARC;
|
||||
procedure TestCodegen_RecordByValArg_CallResultTemp_CleansFields;
|
||||
procedure TestCodegen_RecordByValArg_VarRef_DoesNotClean;
|
||||
procedure TestCodegen_RecordByValArg_NestedManagedField_Recurses;
|
||||
end;
|
||||
|
||||
implementation
|
||||
|
|
@ -919,6 +922,93 @@ begin
|
|||
-1, Pos('_StringAddRef', IR));
|
||||
end;
|
||||
|
||||
{ Caller-side: DoSomething(GetRec()) — the sret temporary from GetRec is
|
||||
consumed by DoSomething and not bound to a named variable. The call
|
||||
site must release each managed leaf of the temp buffer after the call,
|
||||
otherwise the temp's heap string leaks every time the caller runs. }
|
||||
procedure TRecordTests.TestCodegen_RecordByValArg_CallResultTemp_CleansFields;
|
||||
var
|
||||
IR, DriverBody: string;
|
||||
StartIdx, EndIdx: Integer;
|
||||
begin
|
||||
IR := GenIR(
|
||||
'''
|
||||
program P;
|
||||
type TR = record S: string; end;
|
||||
function MakeIt: TR; begin Result.S := 'x' end;
|
||||
procedure Consume(R: TR); begin end;
|
||||
procedure Driver; begin Consume(MakeIt()) end;
|
||||
begin Driver() end.
|
||||
''');
|
||||
StartIdx := Pos('$Driver(', IR);
|
||||
AssertTrue('Driver emitted', StartIdx > 0);
|
||||
EndIdx := StartIdx;
|
||||
while (EndIdx <= Length(IR)) and (IR[EndIdx] <> '}') do Inc(EndIdx);
|
||||
DriverBody := Copy(IR, StartIdx, EndIdx - StartIdx);
|
||||
AssertTrue('Driver releases temp''s string field after Consume() call',
|
||||
Pos('_StringRelease', DriverBody) > 0);
|
||||
end;
|
||||
|
||||
{ Caller-side, variable arg: DoSomething(W) where W is a named record
|
||||
variable. The variable's storage belongs to the enclosing scope (and
|
||||
is cleaned up at scope exit), so the call site must NOT emit a
|
||||
per-field Release after the call — doing so would corrupt W. }
|
||||
procedure TRecordTests.TestCodegen_RecordByValArg_VarRef_DoesNotClean;
|
||||
var
|
||||
IR, DriverBody: string;
|
||||
StartIdx, EndIdx: Integer;
|
||||
begin
|
||||
IR := GenIR(
|
||||
'''
|
||||
program P;
|
||||
type TR = record S: string; end;
|
||||
procedure Consume(R: TR); begin end;
|
||||
procedure Driver;
|
||||
var W: TR;
|
||||
begin W.S := 'x'; Consume(W) end;
|
||||
begin Driver() end.
|
||||
''');
|
||||
StartIdx := Pos('$Driver(', IR);
|
||||
AssertTrue('Driver emitted', StartIdx > 0);
|
||||
EndIdx := StartIdx;
|
||||
while (EndIdx <= Length(IR)) and (IR[EndIdx] <> '}') do Inc(EndIdx);
|
||||
DriverBody := Copy(IR, StartIdx, EndIdx - StartIdx);
|
||||
{ Two pre-existing _StringReleases: (1) release-old in W.S := 'x' and
|
||||
(2) W's scope-exit cleanup. My call-site cleanup must NOT add a
|
||||
third for the Consume(W) call site — W is a named variable, not a
|
||||
temp. Three Releases would indicate the call site corrupted W. }
|
||||
AssertEquals('no extra _StringRelease at variable-arg call site',
|
||||
2, CountOccurrences('_StringRelease', DriverBody));
|
||||
end;
|
||||
|
||||
{ Nested-record-with-managed-leaf temporary: the helper recurses through
|
||||
TInner so both the outer string and the inner string get released. }
|
||||
procedure TRecordTests.TestCodegen_RecordByValArg_NestedManagedField_Recurses;
|
||||
var
|
||||
IR, DriverBody: string;
|
||||
StartIdx, EndIdx: Integer;
|
||||
begin
|
||||
IR := GenIR(
|
||||
'''
|
||||
program P;
|
||||
type
|
||||
TInner = record N: string; end;
|
||||
TOuter = record S: string; Inner: TInner; end;
|
||||
function MakeIt: TOuter;
|
||||
begin Result.S := 'a'; Result.Inner.N := 'b' end;
|
||||
procedure Consume(R: TOuter); begin end;
|
||||
procedure Driver; begin Consume(MakeIt()) end;
|
||||
begin Driver() end.
|
||||
''');
|
||||
StartIdx := Pos('$Driver(', IR);
|
||||
AssertTrue('Driver emitted', StartIdx > 0);
|
||||
EndIdx := StartIdx;
|
||||
while (EndIdx <= Length(IR)) and (IR[EndIdx] <> '}') do Inc(EndIdx);
|
||||
DriverBody := Copy(IR, StartIdx, EndIdx - StartIdx);
|
||||
AssertEquals('two _StringRelease — outer.S and inner.N — after Consume call',
|
||||
2, CountOccurrences('_StringRelease', DriverBody));
|
||||
end;
|
||||
|
||||
initialization
|
||||
RegisterTest(TRecordTests);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue