Skip to content

codegen: lower_new_impl standalone-ctor exit returns an instance without js_gc_init_typed_shape_layout (blocks the pointer-masked layout-note elision) #6921

Closed

Description

@proggeramlug

Summary

lower_new_impl has one exit that returns a freshly allocated class instance without emitting js_gc_init_typed_shape_layout. Every other new path emits it. An instance produced through that exit is left at GC_LAYOUT_POINTER_FREE with no TypedLayoutDescriptor, which is the one layout state where layout_note_slot is load-bearing for GC correctness rather than merely a precision hint.

Found while proving the class-field store elision in #6919 (representation-selection Phase 4b.1). It did not cause a bug there — the elision was narrowed instead of being shipped on the unproven premise — but it blocks the larger win, so it is worth closing on its own.

The exit

crates/perry-codegen/src/lower_call/new.rs, the standalone-constructor-symbol branch:

// new.rs:811
if ctx.class_stack.iter().any(|active| active == class_name)
    || ctor_alias_collision
    || force_ctor_call
{
    ...
    if let Some(ctor_ret) = call_local_constructor_symbol(ctx, class, &obj_box, &lowered_args, caps_absent_from_args) {
        ...
        emit_typed_shape_layout_init(ctx, class_name, &obj_handle);   // new.rs:870  ✅
        ...
        return Ok(final_box);                                          // new.rs:899
    }
    if let Some(prev) = &saved_new_target {
        ctx.block().call(DOUBLE, "js_new_target_set", &[(DOUBLE, prev)]);
    }
    return Ok(obj_box);                                                // new.rs:905  ❌ no init
}
...
emit_typed_shape_layout_init(ctx, class_name, &obj_handle);            // new.rs:1661 ✅ inline path tail

The None arm of call_local_constructor_symbol falls through to return Ok(obj_box) and never reaches either emit_typed_shape_layout_init call.

Reachability

call_local_constructor_symbol (new_ctor_args.rs:321) returns None exactly when ctx.methods has no (class.name, "<Class>_constructor") entry:

let ctor_name = ctx.methods.get(&(class.name.clone(), ctor_method_name)).cloned()?;

Two of the three branch conditions pre-check that the symbol exists, so they cannot reach the None arm:

  • force_ctor_call — requires local_constructor_symbol_exists(ctx, class) (new.rs:810)
  • ctor_alias_collision — requires local_constructor_symbol_exists(ctx, class) (new.rs:791)

The third does not:

  • ctx.class_stack.iter().any(|active| active == class_name) — the recursion guard for new Foo() issued while already lowering Foo. No symbol-existence check.

So the exit is reachable for a recursive new Foo() inside Foo's own constructor or a method of Foo, when Foo has no <Foo>_constructor entry in this module's ctx.methods (no own constructor, or a class whose ctor symbol wasn't emitted in this compilation unit).

I did not construct a runnable reproducer — this was identified statically while discharging a soundness obligation, and the correct response there was to stop depending on the premise rather than to rely on the path being rare. A reproducer would be worth writing as part of the fix.

Why it matters

js_object_alloc_class_inline_keys leaves a fresh instance at GC_LAYOUT_POINTER_FREE. Under that state, layout_note_slot (gc/layout.rs) is what promotes the object to GC_LAYOUT_SIDE_MASK and sets the per-slot pointer bit on the first pointer-valued field store:

LAYOUT_SLOT_MASKS.with(|m| {
    let mut masks = m.borrow_mut();
    if pointer {
        if let Some(mask) = masks.get_mut(&parent_user) { mask.set_slot(slot_index); }
        else if state == GC_LAYOUT_POINTER_FREE {
            let mut mask = LayoutSlotMask::Inline(0);
            mask.set_slot(slot_index);
            masks.insert(parent_user, mask);
            set_layout_state(header, GC_LAYOUT_SIDE_MASK);
        } else { set_layout_state(header, GC_LAYOUT_UNKNOWN); }
    }
    ...
});

With a descriptor installed, that bit is already published by init_typed_shape_layout (it inserts pointer_mask straight into LAYOUT_SLOT_MASKS and sets SIDE_MASK), so the note is a pure no-op for pointer-masked slots. Without one, the note is the only writer, and the collector would otherwise scan zero slots on an object that is POINTER_FREE by header but holds live children.

Today nothing is broken: the note is still emitted unconditionally at both class-field store sites, so the mask gets built the slow way. The gap is that the invariant "a user-class instance reaching a class-field store has a typed descriptor, or is UNKNOWN" does not hold, and any optimization that wants to lean on it is blocked.

Proposed fix

Emit emit_typed_shape_layout_init(ctx, class_name, &obj_handle) on the return Ok(obj_box) path too, so every new <user class> under a class_keys_globals entry either installs a descriptor or explicitly downgrades.

This should be safe by construction: init_typed_shape_layout already validates the live field words before promoting (each raw-f64 slot must hold raw-f64 bits, no pointer outside pointer_mask) and calls layout_set_typed_unknown — i.e. GC_LAYOUT_UNKNOWN, the conservative state — when they don't. On this path the constructor did not run, so the fields are TAG_UNDEFINED; for a class with any number field that fails layout_raw_f64_bits and the object lands in UNKNOWN rather than a wrong mask. That is a correct-but-conservative outcome and worth measuring (it trades a POINTER_FREE fast scan for a conservative one on this rare path).

Worth pairing with a codegen assertion or a debug-mode check that no new of a class with a keys global returns without one of the two emitters having run.

Why this is worth closing

It is the prerequisite for the full pointer-masked layout-note elision — the larger half of Phase 4b.1 in #6919, which had to be dropped to the value-only predicate because of this exit. With the invariant total, a class-field store on a Ptr<Shape>-proven receiver could retire js_gc_note_slot_layout for every value, not just provably-non-pointer ones, since layout_note_slot is a no-op for a pointer-masked slot under an intact descriptor and a no-op in UNKNOWN. That covers the Foo-typed / string-typed / number[]-typed field stores that currently still pay the call.

Refs: #6919 (docs/representation-selection-rfc.md §5.7 records the narrowed scope and names this exit as the blocker).

Metadata

Metadata

Assignees

No one assigned

    Labels

    performanceRuntime, compile-time, build-size, or memory performancerustPull requests that update rust code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions