← board

Found while writing test_record_variant_member_leaks: the obvious spelling of its assertion helper (Chk(const got: AnsiString) fed a[1].v) leaked, which would have put an unrelated bug inside the bound of a leak test. That test compares inline and says why, as does test_managed_record_gate_leaks.

The boundary, re-measured 2026-09-01

An earlier revision of this ticket had the boundary wrong, and wrong in the direction that costs the most: it named two ingredients (a dyn array, and SetLength churn) that are not ingredients at all, so anyone picking it up would have gone looking inside the resize path. Neither is required. What is required is that a Variant be converted to an AnsiString and the result be a temporary nobody owns.

1000 trips each, -O2 -dPXX_ALLOC_CENSUS, live blocks at exit, no record and no array anywhere in these rows:

v = ('lit' + Chr(..))       Variant vs a COMPUTED string   live=936   LEAK
Take(v), const AnsiString   Variant as a string ARGUMENT   live=921   LEAK
v = s                       Variant vs a string VARIABLE   live=1     clean
v = 'literal'               Variant vs a LITERAL           live=1     clean
s := v                      Variant INTO a string variable live=1     clean
v := 'lit' + Chr(..)        assignment alone               live=1     clean
s = ('lit' + Chr(..))       string vs computed string      live=1     clean

The last two rows are the controls that make this a finding rather than a coincidence: the same computed temporary compared against an AnsiString is released correctly, so the temporary machinery works — it is the Variant boundary that drops it. And s := v is clean, so a Variant→AnsiString conversion is fine when its result lands in a variable that owns it.

One block per conversion. Three conversions per trip over 1000 trips gives ~2800 expected against 921 measured for a single-conversion row; the rows above each do one conversion per trip and land at ~930.

What the old rows actually showed

Re-measured on today's binary, the two shapes the old text called decisive:

variant in a dyn-array record, WITH churn, read via param   live=921   LEAK
the same, NO churn at all                                   live=967   LEAK
the same churn, reading only the AnsiString member          live=7     clean

The no-churn row was recorded as live=1 and is not; that single number is where the false boundary came from. The third row still holds and is the useful part — it says the leak follows the VARIANT read, not the resize.

Not caused by anything landed since

Verified against the pinned stable compiler stable_linux_amd64/default/stable_pinned (Aug 30, predating b2997a31b, f806993c8 and the managed-record gate widening): the plain-local row measures live=921 there too, identical. So this is pre-existing and independent of the record-descriptor work, which is also why no row here involves a record at all.

Where to look

The conversion site that materialises an AnsiString from a Variant for a non-owning destination — an argument slot or a comparison operand — and does not register the temporary for release. s := v taking the owning path is the contrast that should localise it.

Root cause, found 2026-09-01 (frankB)

The predicate asks about the AST SHAPE; the ownership question is about what the LOWERING produced.

ir.inc gives a managed-string argument an owning temp like this (7 sites, the one at ~11862 shown):

argIsManagedTemp :=
  (not isRefArg) and ParamWantsManagedStrTemp(cpi, pathIdx) and
  (ASTKind[ASTLeft[item]] <> AN_IDENT) and
  (ASTKind[ASTLeft[item]] <> AN_FIELD) and
  (ASTKind[ASTLeft[item]] <> AN_INDEX) and
  (ASTKind[ASTLeft[item]] <> AN_DEREF);

When true it allocates a hidden tyAnsiString var, stores the value into it and passes that, so scope exit releases it. The four exclusions say "this argument is a place somebody else already owns, so no temp is needed."

For Take(v) where v: Variant and the parameter is const AnsiString, the argument node IS an AN_IDENT, so the exclusion fires and no owning temp is made. But the value passed is not v: IRLowerCallArg has routed through IRLowerVariantAsScalar (ir.inc ~6215), which ends at

IRLowerVariantAsScalar := IRAppendCall(vuProc, vuArg, -1, Ord(vuRet));

calling VariantToStrPas and returning a BRAND-NEW AnsiString with a refcount nobody holds. The AST still looks like a plain variable. The same applies to a[1].v (AN_INDEX / AN_FIELD), which is why the dyn-array-record spelling leaks identically — and why the old "needs SetLength churn" reading looked plausible.

Confirmed by IR dump (PXXDBG=a.ir:Leaky) — node 13 is the conversion call returning tk=23 (AnsiString), node 14 passes it, and nothing releases it:

11: lea      [sym=v]
12: arg      a=11  tk=22
13: call     a=156 b=12  tk=23     <- fresh AnsiString, unowned
14: arg      a=13  tk=22
15: call     a=248 b=14            <- Take

The temp machinery itself is not broken, which is what makes this narrow. Measured, 1000 trips: a binop string temp as an argument, a CALL-RESULT string temp as an argument, a call-result temp as a comparison operand, and a call-result temp assigned to a variable are all clean (live=1). Only the Variant coercion leaks (921). So every other producer of an unowned AnsiString is registered; this one seam is not.

Fix shape, and the trap in it. The predicate needs to be true when the argument is a Variant being coerced to AnsiString, regardless of AST kind. The trap is that the same predicate-plus-four-exclusions is spelled out at SEVEN sites (~11862, 12107, 12346, 12545, 12666, 13806, 13923) — the comment at 12346 calls itself "THE SEVENTH SITE" — so adding a clause in one place fixes one spelling and leaves six. The right change is one helper taking the arg AST that answers "does this argument need an owning temp", replacing the repeated predicate at all seven, per normalise-dont-special-case.

Resolved 2026-09-01 (frankA) — fixed at the seam, not at the seven sites

Two lowerings, both handing out a fresh managed string nobody owned:

Both are fixed where the value is created, upstream of the seven call-argument sites that ask the argument's AST SHAPE. That predicate is not wrong — AN_IDENT/AN_FIELD/AN_INDEX/AN_DEREF do name storage someone else owns — it is asking about a node the variant lowering already replaced. Ownership of the parked value stays IR_STORE_SYM's call, which already MOVES a fresh call result and RETAINS anything else (IRNodeOwnsFreshCallResult).

The box temp was never the leak, which needed two controls to establish: v = w builds no box temp and was clean; v = s against a NAMED AnsiString local builds one and was clean too. Only a computed operand leaked. Without the second control the natural reading of the comparison row is "temp variants leak", and the fix would have gone in the wrong place.

Baseline built by reverting the hunk and rebuilding to converged (71b70fe885fe vs 9fe88c4bfc09): arg 921 -> 1, cmp-right 936 -> 1, cmp-left 936 -> 1, fn-result 936 -> 1, allocs unchanged on every row. Controls vv/vs/into/ctlstr unmoved at 1.

test/test_variant_string_temp_leaks.pas carries all nine arms and is wired into the i386 and aarch64 blocks. Live before -> after: x86-64 1549 -> 2, aarch64 1549 -> 2, i386 3616 -> 1, arm32 3856 -> 2, riscv32 364 -> 1. The pre-fix binary REJECTS it (live=1549, bound 50) while printing byte-identical output on all five targets — so the differential rows beside it are blind to this class and only the absolute bound sees it.

Not closed by this: the i386/arm32/riscv32 baselines differ from each other and from x86-64, and the allocation COUNT moves on those three (i386 8671 -> 6850) where it is unchanged on x86-64 and aarch64. That says those targets carry additional holes in these same shapes. Nobody owns that residual question yet — it is a separate measurement, not part of this fix, and the test header says so rather than implying five equal numbers.

Log