Skip to content

Commit cd0326d

Browse files
committed
fix classic-var autofree cleanup leaking out of try-body / nested block
`x := autofree T.Create` inside a try-body (or nested begin..end) used to fire the auto-Free on exit from that inner scope, leaving x = nil after the try/block although x is still in the routine's var-block. Mark the desugared defer with `var_scope=true` so inner-scope rewrites in try_statement, nested statement_block, and with-body skip it; it is captured at the routine's main begin..end instead, which matches the variable's actual lifetime.
1 parent 01b9691 commit cd0326d

3 files changed

Lines changed: 41 additions & 15 deletions

File tree

compiler/nbas.pas

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ tnothingnodeclass = class of tnothingnode;
4747
Created by parser, rewritten away in statement_block post-pass.
4848
Should never reach typecheck/firstpass - internalerror if it does. }
4949
tdefernode = class(tunarynode)
50+
{ true when the defer is bound to a variable's scope (classic-var
51+
autofree desugar) - inner-block rewrites must skip it, only the
52+
variable's owning scope captures it }
53+
var_scope : boolean;
5054
constructor create(l:tnode);virtual;
5155
function pass_1 : tnode;override;
5256
function pass_typecheck:tnode;override;

compiler/pexpr.pas

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5930,6 +5930,7 @@ (current_structdef.typ=recorddef) then
59305930
free_guarded : tnode;
59315931
chain_block : tblocknode;
59325932
chain_stat : tstatementnode;
5933+
defer_node : tdefernode;
59335934
begin
59345935
// require a simple variable load on the LHS
59355936
if lhs.nodetype <> loadn then
@@ -5961,7 +5962,12 @@ (current_structdef.typ=recorddef) then
59615962
chain_block := internalstatements(chain_stat);
59625963
Include(chain_block.blocknodeflags, bnf_defer_transparent);
59635964
addstatement(chain_stat, cassignmentnode.create(lhs, rhs));
5964-
addstatement(chain_stat, cdefernode.create(free_guarded));
5965+
// mark the defer as bound to the variable's scope -- inner-block
5966+
// rewrites (try-body, nested begin..end, with-body) must skip it;
5967+
// only the routine's main begin..end captures it
5968+
defer_node := cdefernode.create(free_guarded);
5969+
defer_node.var_scope := true;
5970+
addstatement(chain_stat, defer_node);
59655971
result := chain_block;
59665972
end;
59675973

compiler/pstatmnt.pas

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ implementation
6565

6666
function statement : tnode;forward;
6767
function defer_statement : tnode;forward;
68-
procedure rewrite_defers_in_block(var first: tnode);forward;
68+
procedure rewrite_defers_in_block(var first: tnode; is_routine_body: boolean = false);forward;
6969

7070
function branch_type(olddef, branchdef: tdef): tdef; inline;
7171
begin
@@ -3908,8 +3908,12 @@ tdeferinfo = record
39083908

39093909
pdefercollect = ^tdefercollect;
39103910
tdefercollect = record
3911-
items : tfplist;
3912-
counter : longint;
3911+
items : tfplist;
3912+
counter : longint;
3913+
{ capture classic-var autofree defers (bnf_defer_var_scope blocks)
3914+
only when rewriting the routine's main begin..end -- otherwise
3915+
they leak into try-body / nested-block / with-body scopes }
3916+
is_routine_body : boolean;
39133917
end;
39143918

39153919

@@ -3941,13 +3945,18 @@ tdefercollect = record
39413945
asgn : tnode;
39423946
begin
39433947
result:=fen_false;
3948+
ctx:=pdefercollect(arg);
39443949
// inner blocks have their own defer scope - skip, unless flagged
39453950
// as a parser-generated helper (e.g. autofree desugar)
39463951
if (n.nodetype=blockn) and not (bnf_defer_transparent in tblocknode(n).blocknodeflags) then
39473952
exit(fen_norecurse_false);
39483953
if n.nodetype=defern then
39493954
begin
3950-
ctx:=pdefercollect(arg);
3955+
// classic-var autofree's defer is bound to the variable's scope,
3956+
// not whatever block surrounds the assignment - leave it alone
3957+
// for an outer rewrite to capture at the variable's owning scope
3958+
if tdefernode(n).var_scope and not ctx^.is_routine_body then
3959+
exit(fen_false);
39513960
inc(ctx^.counter);
39523961
str(ctx^.counter,flagname);
39533962
flagname:='$defer_flag_'+flagname;
@@ -3973,7 +3982,7 @@ tdefercollect = record
39733982
end;
39743983

39753984

3976-
procedure rewrite_defers_in_block(var first: tnode);
3985+
procedure rewrite_defers_in_block(var first: tnode; is_routine_body: boolean = false);
39773986
var
39783987
ctx : tdefercollect;
39793988
i : longint;
@@ -3987,6 +3996,7 @@ tdefercollect = record
39873996
begin
39883997
ctx.items:=tfplist.create;
39893998
ctx.counter:=0;
3999+
ctx.is_routine_body:=is_routine_body;
39904000
try
39914001
foreachnodestatic(pm_preprocess,first,@defer_collect_callback,@ctx);
39924002
if ctx.items.count=0 then
@@ -4042,26 +4052,32 @@ tdefercollect = record
40424052
first,last : tnode;
40434053
filepos : tfileposinfo;
40444054
blockst : tblocksymtable;
4055+
is_routine_body : boolean;
40454056

40464057
begin
40474058
first:=nil;
40484059
last:=nil;
40494060
filepos:=current_tokenpos;
40504061
consume(starttoken);
40514062

4063+
{ capture whether this is the routine's main begin..end before
4064+
consuming the flag - used by rewrite_defers_in_block to decide
4065+
whether to capture classic-var autofree defers (which belong to
4066+
the variable's scope, i.e. routine, not to inner blocks) }
4067+
is_routine_body := assigned(current_procinfo) and (starttoken=_BEGIN) and
4068+
current_procinfo.parsing_main_block;
4069+
if is_routine_body then
4070+
current_procinfo.parsing_main_block:=false;
4071+
40524072
{ Push a block-scope symtable so that inline vars declared inside
40534073
this begin..end are scoped to the block (Delphi-style).
40544074
Only active when m_inline_var is set; avoids overhead in other modes. }
40554075
blockst:=nil;
4056-
if assigned(current_procinfo) and (m_inline_var in current_settings.modeswitches) then
4076+
if assigned(current_procinfo) and (m_inline_var in current_settings.modeswitches) and
4077+
not is_routine_body then
40574078
begin
4058-
if (starttoken=_BEGIN) and current_procinfo.parsing_main_block then
4059-
current_procinfo.parsing_main_block:=false
4060-
else
4061-
begin
4062-
blockst:=tblocksymtable.create(symtablestack.top);
4063-
symtablestack.push(blockst);
4064-
end;
4079+
blockst:=tblocksymtable.create(symtablestack.top);
4080+
symtablestack.push(blockst);
40654081
end;
40664082

40674083
while not((current_scanner.token=_END) or (current_scanner.token=_FINALIZATION)) do
@@ -4101,7 +4117,7 @@ tdefercollect = record
41014117
Must run while the block-scope symtable is still on the stack so that
41024118
generated flag-vars land in the right scope. No-op if no defer in tree. }
41034119
if assigned(first) then
4104-
rewrite_defers_in_block(first);
4120+
rewrite_defers_in_block(first, is_routine_body);
41054121

41064122
{ Pop the block-scope symtable and keep it on the procdef so nested
41074123
debug scopes can still follow the original parent chain later on. }

0 commit comments

Comments
 (0)