Skip to content

GC: a Symbol-typed local gets no shadow slot — Type::Symbol is classified as a non-pointer #7236

Description

@proggeramlug

Summary

collectors/pointer_locals.rs classifies Type::Symbol as a non-pointer, so a Symbol-typed local never gets a shadow-stack slot. alloc_symbol is a gc_malloc of a movable GC-heap object that nothing else refers to, so the local sits in a plain alloca across every collection point in its scope — the #7202 shape, on a type the collector definitely moves.

Found by scripts/gc_root_dominance_check.py --unrooted-allocas --moving-only after #7210's predicate split (#7235) removed the 96 false positives that were burying it. It is the only remaining violation on the corpus, so it is also the single thing standing between gc-root-dominance and promotion to a required context (#7198).

The classification

// crates/perry-codegen/src/collectors/pointer_locals.rs:196
pub(crate) fn is_definitely_non_pointer_type(ty: &Type) -> bool {
    matches!(
        ty,
        Type::Number
            | Type::Int32
            | Type::Boolean
            | Type::Null
            | Type::Void
            | Type::Never
            | Type::Symbol          // <-- this one is not like the others
    ) || matches!(ty, Type::Union(variants) if variants.iter().all(is_definitely_non_pointer_type))
}

Every other member is an immediate. Symbol is not:

// crates/perry-runtime/src/symbol.rs:370
pub(crate) unsafe fn alloc_symbol(description: *mut StringHeader, registered: bool)
    -> *mut SymbolHeader {
    let raw = crate::gc::gc_malloc(
        std::mem::size_of::<SymbolHeader>(),
        crate::gc::GC_TYPE_STRING,
    );

GC_TYPE_STRING is movable, and js_symbol_new returns it POINTER_TAG-boxed. The function's own comment concedes the liveness half: "The description pointer is kept alive through the SYMBOL_REGISTRY (for registered symbols) or not at all (for fresh symbols — in practice they live for the duration of the program, which is fine for test workloads)."

This is the same defect as the Map/Set one documented eight lines above it in the same function, which produced "grown Map must retain its side-allocation owner record" and was closed by treating Type::Generic as a pointer. The doc comment on is_definitely_non_pointer_type also warns that a second copy drifting by one Type variant is "a use-after-move under the evacuating minor (#7019), not a cosmetic inconsistency"collectors/ptr_shape_returns.rs is the second caller and inherits the same wrong answer.

Reproducer, already in the corpus

test-files/test_gap_class_forward_capture_6523.ts — a const s = Symbol("x") in an IIFE that a class's static get ANY() closes over:

; perry_closure_test_gap_class_forward_capture_6523_ts__2
%r14  = alloca double                       ; never an operand of js_shadow_slot_bind
%r16  = call double @js_symbol_new(double %r15)
store double %r16, ptr %r14
…  js_array_alloc, js_array_push_f64, js_closure_alloc,
   js_class_register_capture_values, js_object_alloc,
   js_object_set_field_by_name, js_put_value_set_ic_miss  …
%r205 = load double, ptr %r14               ; ~250 instructions later

__7 is the identical shape in the second IIFE. Both classify MOVING via js_object_set_field_by_name.

Command (needs a --trace llvm corpus from scripts/gc_root_dominance_corpus.sh):

python3 scripts/gc_root_dominance_check.py ir-corpus --unrooted-allocas --moving-only -v

Why it has not crashed yet

The two live sites are inside js_object_set_field_by_name and friends, i.e. runtime-helper allocations, which reach gc_check_trigger's alloc-point arm and take ManualGcScanGuard::force_full_scan — a forced conservative stack scan, which both makes the copying minor ineligible and (accidentally) keeps the symbol alive. The same "closed by accident" argument #7226 recorded for #7213, resting on the same property the moving-GC work keeps eroding. A loop back-edge poll anywhere in that window makes it live.

Fix shape

Move Type::Symbol out of is_definitely_non_pointer_type and into is_ptr_typed. One-sided in the safe direction — the GC decode rejects any slot value that is not a live heap pointer, which is the argument already written down for Type::Generic.

It is not a one-liner to land, which is why it is filed rather than folded into #7235:

  • it grows the shadow frame for every function holding a Symbol local, so it wants a before/after on frame sizes and on the emitted-IR diff;
  • it needs a runtime witness of its own (a Symbol local held across a loop-poll collection and then used as a computed key), red at base under PERRY_GC_MOVING_LOOP_POLLS=1;
  • collectors/ptr_shape_returns.rs consumes the same predicate and must be re-measured with it.

Acceptance

--unrooted-allocas --moving-only reaches 0 on the corpus, and gc-root-dominance becomes promotable to a required context (#7198's follow-up, which is the reason this is worth doing now rather than later).

Refs #7235, #7210, #7198, #7202, #7019, #7213.

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