Skip to content

index_get/index_set/property_set: ~20 arms make no rooting decision at all (the population the Layer 1 ledger cannot see) #7640

Description

@proggeramlug

Found while migrating the computed read/write modules onto the Layer 1 rooting API (slice 4 of #7615). Filed rather than fixed — see "Why not in slice 4" at the bottom.

The finding

Migrating the guarded arms of index_set.rs / index_get.rs / property_set.rs
made it visible that a much larger population of arms in the same files makes
no rooting decision at all. They are not ordering mistakes against the raw
API — they never call it. They lower a receiver (and sometimes a key), lower more
user code, and then use the register.

This is a distinction the Layer 1 ledger cannot see, and it is worth stating on
#7615 directly: a module listed in MIGRATED_MODULES is not an audited
module.
The ledger asserts "every rooting decision this module makes goes
through crate::rooting". It says nothing about a window with no decision.

Three of the arms below were adjacent to code slice 4 was already rewriting and
are fixed there (#7637, #7638, #7639). Everything in this issue is what is left.

A. Store side — receiver (and sometimes key) live across the value

expr/index_set.rs shape exposed
bounded-index-pair array store for (let i=0;i<a.length;i++) a[i] = {v:i}; receiver
the #5525 recv_unknown inline dyn-TA store function put(o:any,k:any){ o[k]=mk(); } receiver and key
globalThis[k] = v key (heap string); receiver MEDIUM
width-tracked TA, non-numeric index ta[k] = mk() with k: any receiver and key
TA runtime-key / TA final fallback / Uint8Array runtime-key receiver

The bounded-index one is the sharpest, because it sits immediately above the
generic array-store arm that closed exactly this window in #7341, and because
classify_for_length_hoist's body predicate (expr_preserves_array_length)
accepts Expr::Object / Expr::Array / Expr::ArraySpread values — so the
registered bounded pair and an allocating RHS coexist by design:

const a: unknown[] = new Array(1024);
for (let i = 0; i < a.length; i++) a[i] = { v: i };

Note also that is_width_tracked_typed_array_receiver keys on
receiver_class_name, a declared type. Per CLAUDE.md a declared type is a
dispatch hint, never a layout fact, so those registers can hold any heap object.

B. Read side — receiver live across the KEY (expr/index_get.rs)

index_get.rs guards two arms (the dynamic-string-key read and the last-resort
fallback, both now migrated). Ten others lower the receiver, then lower the
index — arbitrary user code, o[f()] — then use the receiver:

width-tracked TA runtime-key; TA final fallback; Uint8Array runtime-key;
Uint8Array unproven bounds; string receiver s[f()] (a heap string, very
much movable); recv_unknown inline dyn-TA get; is_array_expr && !is_numeric_expr;
numeric_index_needs_runtime_key; and both number-context entry points
(lower_numeric_index_get_for_number_context,
lower_unknown_local_index_get_for_number_context).

Two more are MEDIUM because the key is a SymbolFor (allocates the interned
symbol, a narrower window): the TA symbol-key and array symbol-key arms.

One shared change closes all of these: put [object, index] in a
rooting::with_operands_rooted group, which is what the two already-guarded
arms now are.

C. property_set.rs class-field store — an in-code claim I could not substantiate

property_set.rs says, above the sloppy class-field raw store and again above
the strict one:

the receiver's relocation across an allocating RHS is handled by the same
statepoint re-read that arm relies on.

I could not substantiate it. function/precise_roots.rs's RS4GC retype pass
rewrites a root-alloca load double into load ptr addrspace(1)ptrtoint
bitcast, so the addrspace(1) value is dead at the next instruction and only
the plain double crosses the statepoint — RS4GC does not relocate it. The
setter-dispatch arm has the same shape and the same absence of a guard.

Either the claim is true and should say why in terms of what RS4GC actually
relocates, or it is false and this is the hottest store path in the compiler
running unguarded.
It should not stay a sentence nobody can check.

D. A derived raw handle taken before an allocating unbox_str_handle

unbox_str_handle is not a pure mask: it calls js_get_string_pointer_unified,
which materialises an SSO value into a fresh heap StringHeader — one
allocation per SSO unbox. Several sites compute the receiver's raw untagged
pointer first and call it second, so a pointer no root can name crosses a
potential collection point:

  • index_set.rs — the arr[stringKey] arm and the globalThis arm
  • index_set.rs — inside the guarded dynamic-string-key arm: the temp root
    protects obj_box, not the obj_handle derived from it
  • index_get.rs — the same residual inside both arms that are otherwise guarded

This is the #7280 taxonomy's case (a): a pointer already unboxed to raw i64
cannot be repaired by re-reading a double from a shadow slot, so
crate::rooting structurally cannot express it. Swapping the two lines — unbox
the key first, the receiver second — closes every one at zero runtime cost
, and
is the cheapest item in this issue.

E. Callees with the same window

Called from the audited files with (object, index, value) and lowering all
three, so they can carry the identical shape and were not in slice 4's scope:
ptr_numarray_access::try_lower_num_array_guard_free_{set,get},
try_lower_proven_view_checked_store, lower_typed_array_store,
lower_buffer_store, lower_index_set_fast,
masked_window::lower_masked_window_index_get,
property_get/generic_dispatch.rs::lower_generic_property_get.

Why not in slice 4

Slice 4's rule was: fix a found bug in-slice unless the fix changes observable
behaviour or measured performance
. The three fixed there are arms whose store
already routes through a runtime helper call, so a temp root is noise beside it.
Everything in this issue is the opposite — inline store double fast paths,
guard diamonds, and the bounded-index loop store, which is the hot path. Adding
roots there is a measured change, and slice 4 was explicitly not permitted to
benchmark. Section D is the exception on cost but is a different repair shape
(re-ordering, not rooting), so it belongs with its siblings rather than smuggled
into a rooting refactor.

Suggested order

  1. D — free, mechanical, closes six sites.
  2. The bounded-index array store — deterministic repro above, sits next to an
    already-fixed twin.
  3. C — resolve the statepoint claim before touching anything else on that path.
  4. B — one shared group closes ten arms.
  5. A's typed-array arms, then E.

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