Skip to content

gc: constructor arguments (new C(a, b)) are not precise roots across the instance allocation #6969

Description

@proggeramlug

Found while fixing #6951, which fixed the variadic argument accumulator (console.log, spread/rest lists) and the concat/literal families. Constructor arguments are the same bug class at a different site and are not covered by that fix.

Repro

let sink: unknown[] = [];
function churn(n: number): number {
  let acc = 0;
  for (let i = 0; i < n; i++) {
    sink.push({ i: i, s: "x" + (i & 255), a: [i, i + 1] });
    if (sink.length > 4096) { acc = (acc + sink.length) | 0; sink = []; }
  }
  return acc | 0;
}
function fresh(k: number): string { return "f" + k + "-" + (k * 7); }
class Pair {
  a: unknown;
  b: unknown;
  constructor(a: unknown, b: unknown) { this.a = a; this.b = b; }
  toString(): string { return "<" + this.a + "," + this.b + ">"; }
}
const N = 420000;
// Repeat 6x: the failure is a use-after-free and needs the block recycled.
{ console.log(String(new Pair(fresh(0), churn(N)))); }
{ console.log(String(new Pair(fresh(1), churn(N)))); }
{ console.log(String(new Pair(fresh(2), churn(N)))); }
{ console.log(String(new Pair(fresh(3), churn(N)))); }
{ console.log(String(new Pair(fresh(4), churn(N)))); }
{ console.log(String(new Pair(fresh(5), churn(N)))); }

PERRY_CONSERVATIVE_STACK_SCAN=off PERRY_GC_HEAP_LIMIT=8 ./repro prints wrong output (pre-#6951 it segfaulted; post-#6951 it is a silent DIFF because the surrounding concat is now rooted).

Why

lower_call/new.rs lowers all constructor arguments into lowered_args up front, then allocates the instance and calls the constructor:

%r10 = call double @..._fresh__spec_i32(i32 0)          ; heap string, SSA register
%r13 = call double @..._churn(double %r12)               ; collects -- %r10 dies
%r17 = call i64 @js_object_alloc_class_inline_keys(...)  ; collects again
%r20 = call double @..._Pair_constructor(double %r19, double %r10, double %r13)

%r10 is live across two collection points with no root.

Fix shape

crates/perry-codegen/src/expr/temp_root.rs (added by #6951) has the primitive: lower_exprs_rooted roots each already-evaluated value across the later ones and hands back re-read values plus a guard. The obstacle is purely structural — lower_new is ~1700 lines with ~20 return paths and lowered_args is consumed at a dozen of them, so the temp_root_release has to be placed at each. A statement-boundary truncate (mark at statement entry, truncate at statement exit) would make this and the sibling sites mechanical; that is the design question to settle first.

Related: #6951, #6968, #6970, #6971, #6950.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions