Skip to content

array growth forwarding: build()-and-return leaves a stale head in the binding (minimal N = 17) #7661

Description

@proggeramlug

Split out of #7660, which fixed the consumer half. This is the producer half,
and it is a latent hazard for every future consumer of a raw array head.

The observation

function build(n: number): Node[] {
  const out: Node[] = [];
  for (let i = 0; i < n; i++) out.push(new Node(i, i * 2));
  return out;
}
const keep = build(1000);   // `keep` holds a growth-forwarding STUB, not the live head

keep is demonstrably a stub. Adding a module-scope keep.push(x); keep.pop();
— which forces a write-back of the resolved head into the global — changes
the behaviour of a compiler in which nothing else differs; and #7660's fix,
whose only effect is js_array_refresh_local_head + write-back, would be a
literal no-op if the binding were already live (that helper returns its input
untouched when there is nothing to follow). It is not a no-op: it turns a
SIGBUS into the right answer.

Why that is surprising

expr/array_push.rs already does the write-back. Both reallocating arms of the
inline push emit it:

apush.realloc.15:
  %r106 = call i64 @js_array_push_f64(i64 %r59, double %r55)
  ...
  store ptr addrspace(1) %rs4gc.s4, ptr %r2        ; <- out's slot, updated

So out's own slot is kept live inside build. The stub is being reintroduced
somewhere between that slot and the caller's binding — candidates, unverified:
the alwaysinline + $spec_i32_b specialization pair (the return value may be
derived from the head cached in %r59 rather than a fresh load of %r2), or
the return/assignment lowering. Worth an hour with --trace llvm on the
reproducer above.

Why it matters beyond #7660

Growth forwarding is transparent through the runtime — every entry point
calls clean_arr_ptr — so a stale binding is invisible until emitted code
dereferences the head itself. Perry now has several such consumers (4a's inline
element tier, the masked-window reads, #7612's element-shape clone), and each
one has had to remember js_array_refresh_local_head independently. #7612
forgot, and the result was a bus error that shipped. Fixing the producer would
turn "remember the refresh" from a correctness requirement into an
optimization.

Suggested acceptance

A test that asserts the binding, not just the observable behaviour: compile
the reproducer and assert the value reaching the caller resolves to itself
under clean_arr_ptr (i.e. carries no GC_FLAG_FORWARDED). Behaviour alone
cannot see this — every runtime path resolves the chain and prints the right
answer either way, which is exactly why it went unnoticed.

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