administrator/blaise
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

refactor(codegen): hoist ARC ownership predicate to blaise.codegen (F2)

Browse source 
ExprOwnsRef (QBE backend) and NativeExprOwnsRef (native backend) were
byte-identical twins: a pure walk over the AST node + its resolved type
deciding whether an r-value leaves an ARC-managed value at refcount +1
that the consuming site must not AddRef again. Two drift-prone copies of
an ARC decision is exactly the failure mode that leaks/UAFs when one side
is updated and the other is not.

Hoist the single source of truth into blaise.codegen as ArcExprOwnsRef
(distinct name so an unqualified call inside a method cannot bind to a
same-named method — the F4 gotcha). Both backends keep their existing
free function as a one-line delegator, so every call site is unchanged.

Cross-backend dedup, no behaviour change: native .s AND QBE IR both
byte-identical on an ARC-heavy program (function-returned class, string
property read via getter, method call returning a class, dynamic-array
return). All four fixpoints and the full suite (QBE-built and native-built
test runners) unchanged.
This commit is contained in:
Graeme Geldenhuys 2026-06-27 11:57:28 +01:00
parent aa52ab6bef
commit 1e0b7fc46a
3 changed files with 81 additions and 112 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

56
compiler/src/main/pascal/blaise.codegen.native.x86_64.pas
View file

@ -27,7 +27,7 @@ interface
uses
SysUtils, Classes, contnrs, Generics.Collections, uAST, uSymbolTable, uStrCompat,
blaise.codegen.arcshapes, uDebugFacts,
blaise.codegen, blaise.codegen.arcshapes, uDebugFacts,
blaise.codegen.native.backend, blaise.codegen.target;
const
@ -1778,58 +1778,10 @@ begin
end;
function NativeExprOwnsRef(AExpr: TASTExpr): Boolean;
var
FA: TFieldAccessExpr;
MC: TMethodCallExpr;
IE: TIdentExpr;
begin
Result := False;
if AExpr = nil then Exit;
if AExpr.ResolvedType = nil then Exit;
{ Ownership transfer applies to every ARC-managed return value, not just
classes: a function/method returning a String or dynamic array leaves its
Result at refcount +1 (the callee AddRef'd on `Result := x` and did not
release Result at scope exit). The caller's assignment site must therefore
NOT AddRef again — it consumes that transferred reference. Without covering
tyString/tyDynArray here the assignment branches below emit a spurious
_StringAddRef/_DynArrayAddRef on the call result, which is never balanced
and leaks one buffer per call. }
if not (AExpr.ResolvedType.Kind in [tyClass, tyDynArray])
and not AExpr.ResolvedType.IsString() then Exit;
if AExpr is TIdentExpr then
begin
IE := TIdentExpr(AExpr);
if IE.IsImplicitSelfMethod then
Exit(True);
end;
if AExpr is TFieldAccessExpr then
begin
FA := TFieldAccessExpr(AExpr);
if FA.IsConstructorCall then Exit;
if FA.IsMethodCall then begin Result := True; Exit end;
if (FA.PropRead <> nil) and (FA.PropRead.ReadMethod <> '') then
Exit(True);
end;
if AExpr is TMethodCallExpr then
begin
MC := TMethodCallExpr(AExpr);
if not MC.IsConstructorCall then Result := True;
Exit;
end;
if AExpr is TFuncCallExpr then
begin
if (TFuncCallExpr(AExpr).ResolvedDecl <> nil) or
TFuncCallExpr(AExpr).IsIndirectCall then
Result := True;
Exit;
end;
if AExpr is TStringSubscriptExpr then
begin
if (TStringSubscriptExpr(AExpr).StrExpr is TFieldAccessExpr) and
(TFieldAccessExpr(TStringSubscriptExpr(AExpr).StrExpr).PropRead <> nil) and
(TFieldAccessExpr(TStringSubscriptExpr(AExpr).StrExpr).PropRead.ReadMethod <> '') then
Result := True;
end;
{ Delegates to the shared backend-neutral predicate in blaise.codegen
(formerly a byte-identical twin of the QBE backend's ExprOwnsRef). }
Result := ArcExprOwnsRef(AExpr);
end;
{ Emit an immortal class-name string blob and return the label+12 reference

74
compiler/src/main/pascal/blaise.codegen.pas
View file

@ -121,6 +121,18 @@ function RecretEightbyteIsSSE(ARec: TRecordTypeDesc; AStartByte: Integer): Boole
function RecretClassify(ARec: TRecordTypeDesc;
const ATarget: TTargetDesc): TRecReturnClass;
{ ----------------------------------------------------------------------
Shared ARC ownership-transfer predicate.
True when AExpr, used as an r-value, leaves an ARC-managed value at
refcount +1 that the consuming site must NOT AddRef again (it consumes the
transferred reference). Covers function/method calls and method-backed
property reads returning a class, string, or dynamic array. The decision is
a pure walk over the AST node + its resolved type — no backend state — so it
is single-sourced here and both backends call it (formerly the byte-identical
twins ExprOwnsRef / NativeExprOwnsRef). }
function ArcExprOwnsRef(AExpr: TASTExpr): Boolean;
implementation
function RecretManagedClean(ARec: TRecordTypeDesc): Boolean;
@ -283,4 +295,66 @@ begin
end;
end;
function ArcExprOwnsRef(AExpr: TASTExpr): Boolean;
var
FA: TFieldAccessExpr;
MC: TMethodCallExpr;
IE: TIdentExpr;
begin
Result := False;
if AExpr = nil then Exit;
if AExpr.ResolvedType = nil then Exit;
{ Ownership transfer applies to every ARC-managed return value, not just
classes: a function/method returning a String or dynamic array leaves
its Result at refcount +1 (the callee AddRef'd on `Result := x` and did
not release Result at scope exit). The caller's assignment site must
therefore NOT AddRef again — it consumes that transferred reference.
Without covering tyString/tyDynArray here the assignment branches emit a
spurious _StringAddRef/_DynArrayAddRef on the call result, which is never
balanced and leaks one buffer per call. }
if not (AExpr.ResolvedType.Kind in [tyClass, tyDynArray])
and not AExpr.ResolvedType.IsString() then Exit;
if AExpr is TIdentExpr then
begin
IE := TIdentExpr(AExpr);
if IE.IsImplicitSelfMethod then
Exit(True);
end;
{ Constructor calls via TFieldAccessExpr (TFoo.Create) — do NOT own. }
if AExpr is TFieldAccessExpr then
begin
FA := TFieldAccessExpr(AExpr);
if FA.IsConstructorCall then Exit;
if FA.IsMethodCall then begin Result := True; Exit end;
{ Method-backed property read (read GetX): the getter returns +1.
Field-backed reads (read FX) emit a plain load and do NOT own. }
if (FA.PropRead <> nil) and (FA.PropRead.ReadMethod <> '') then
Exit(True);
end;
{ TMethodCallExpr: constructor calls do NOT own; all other method calls DO. }
if AExpr is TMethodCallExpr then
begin
MC := TMethodCallExpr(AExpr);
if not MC.IsConstructorCall then Result := True;
Exit;
end;
if AExpr is TFuncCallExpr then
begin
if (TFuncCallExpr(AExpr).ResolvedDecl <> nil) or
TFuncCallExpr(AExpr).IsIndirectCall then
Result := True;
Exit;
end;
{ Indexed property subscript: L[I] desugars to Subscript(FieldAccess(Items))
where Items has a ReadMethod. The subscript emitter delegates to the
getter — the result inherits the +1. }
if AExpr is TStringSubscriptExpr then
begin
if (TStringSubscriptExpr(AExpr).StrExpr is TFieldAccessExpr) and
(TFieldAccessExpr(TStringSubscriptExpr(AExpr).StrExpr).PropRead <> nil) and
(TFieldAccessExpr(TStringSubscriptExpr(AExpr).StrExpr).PropRead.ReadMethod <> '') then
Result := True;
end;
end;
end.

63
compiler/src/main/pascal/blaise.codegen.qbe.pas
View file

@ -3792,67 +3792,10 @@ end;
Variable reads, field reads, type casts, and lookups do NOT own their result
and always need the assignment-site AddRef. }
function ExprOwnsRef(AExpr: TASTExpr): Boolean;
var
FA: TFieldAccessExpr;
MC: TMethodCallExpr;
IE: TIdentExpr;
begin
Result := False;
if AExpr = nil then Exit;
if AExpr.ResolvedType = nil then Exit;
{ Ownership transfer applies to every ARC-managed return value, not just
classes: a function/method returning a String or dynamic array leaves
its Result at refcount +1 (the callee AddRef'd on `Result := x` and did
not release Result at scope exit). The caller's assignment site must
therefore NOT AddRef again — it consumes that transferred reference.
Without covering tyString/tyDynArray here the assignment branches below
emit a spurious _StringAddRef/_DynArrayAddRef on the call result, which
is never balanced and leaks one buffer per call. }
if not (AExpr.ResolvedType.Kind in [tyClass, tyDynArray])
and not AExpr.ResolvedType.IsString() then Exit;
if AExpr is TIdentExpr then
begin
IE := TIdentExpr(AExpr);
if IE.IsImplicitSelfMethod then
Exit(True);
end;
{ Constructor calls via TFieldAccessExpr (TFoo.Create) — do NOT own }
if AExpr is TFieldAccessExpr then
begin
FA := TFieldAccessExpr(AExpr);
if FA.IsConstructorCall then Exit;
if FA.IsMethodCall then begin Result := True; Exit end;
{ Method-backed property read (read GetX): the getter returns +1.
Field-backed reads (read FX) emit a plain load and do NOT own. }
if (FA.PropRead <> nil) and (FA.PropRead.ReadMethod <> '') then
begin
Exit(True);
end;
end;
{ TMethodCallExpr: constructor calls do NOT own; all other method calls DO }
if AExpr is TMethodCallExpr then
begin
MC := TMethodCallExpr(AExpr);
if not MC.IsConstructorCall then Result := True;
Exit;
end;
if AExpr is TFuncCallExpr then
begin
if (TFuncCallExpr(AExpr).ResolvedDecl <> nil) or
TFuncCallExpr(AExpr).IsIndirectCall then
Result := True;
Exit;
end;
{ Indexed property subscript: L[I] desugars to Subscript(FieldAccess(Items))
where Items has a ReadMethod. EmitStringSubscriptExpr delegates to
EmitExpr(StrExpr) which calls the getter — the result inherits the +1. }
if AExpr is TStringSubscriptExpr then
begin
if (TStringSubscriptExpr(AExpr).StrExpr is TFieldAccessExpr) and
(TFieldAccessExpr(TStringSubscriptExpr(AExpr).StrExpr).PropRead <> nil) and
(TFieldAccessExpr(TStringSubscriptExpr(AExpr).StrExpr).PropRead.ReadMethod <> '') then
Result := True;
end;
{ Delegates to the shared backend-neutral predicate in blaise.codegen
(formerly a byte-identical twin of the native backend's NativeExprOwnsRef). }
Result := ArcExprOwnsRef(AExpr);
end;
{ True when the expression produces a fresh sret record temporary that