Skip to content

codegen: shadow-frame is sized by HashMap len while slot indices come from a counter — duplicate qualifying Stmt::Let can assign an index past the frame end #6995

Description

@proggeramlug

Summary

collect_pointer_typed_locals assigns shadow-slot indices from a running counter, but the caller sizes the shadow frame from the resulting HashMap length. Those two numbers disagree whenever the same local id qualifies twice, because the map insert overwrites while the counter does not.

crates/perry-codegen/src/collectors/pointer_locals.rs:889:

Stmt::Let { id, ty, .. }
    if is_ptr_typed(ty) && !non_pointer_locals.contains(id) && !flat_row_alias_ids.contains(id) =>
{
    out.insert(*id, *next_slot);   // overwrites on a duplicate id
    *next_slot += 1;               // increments regardless
}

crates/perry-codegen/src/codegen/function.rs:402:

lf.enable_shadow_frame(m.len() as u32);

So with k duplicate qualifying Stmt::Lets for already-seen ids, next_slot == m.len() + k, and the largest index actually stored in the map can be ≥ the frame size.

Consequence

Shadow slots are GC roots. An index at or past the frame's end is an out-of-range slot write, and in a method the reserved receiver slot is a neighbour — so the two plausible outcomes are a dropped root (a live value never traced) or a clobbered receiver. Both are the failure class this campaign has been chasing: silent, and invisible until an evacuating minor actually moves something (#6950, #6977).

Note the frame is only sized from the map, while the index is stored in the map — so this cannot be caught by comparing the map against itself. It needs next_slot vs m.len().

Status of this report — read before acting

The arithmetic above is verified by reading origin/main (aa1c15028). The reachability is NOT. I have not exhibited a TypeScript program that produces two qualifying Stmt::Lets with the same id, and that is the first thing to establish:

  1. Determine whether AST→HIR lowering can emit Stmt::Let twice for one id. Duplicate hoisted var declarations (var x; ... var x;, or var redeclared across branches that hoist to one binding) are the obvious candidate; a var in a loop body that hoists to the function scope is another.
  2. If it can, add an assertion or a debug check that next_slot == out.len() at the end of collect_pointer_typed_locals — that turns this from a silent miscompile into a loud one, independent of whether a reproducer is found today.
  3. If it cannot, the invariant is still worth asserting, since nothing in the collector's signature enforces it and a future lowering change would reintroduce it silently.

Whatever the answer, sizing the frame from next_slot rather than m.len() is the conservative fix and costs at most a few unused slots.

Verification guidance

A reproducer must be checked on the evacuating precise-roots arm, where a dropped root actually bites:

PERRY_GC_HEAP_LIMIT=8 PERRY_GC_INCREMENTAL=0 PERRY_CONSERVATIVE_STACK_SCAN=off

with [gc-copy-minor] copied_objects=N, N > 0. A green run where nothing moved proves nothing — see #6942/#6946/#6950. Note also #6993: the per-PR matrix arms cannot reach this configuration at all, so a defect here would not be caught by PR CI even once a reproducer exists.

Found as a side observation during the #6982 investigation (PR #6994); filed separately rather than folded in.

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