Skip to content

A keyless shape reads as "no own properties" to a long tail of consumers — the class fix is ShapeObjectKind::Dictionary (#10868 step 2.5) #10942

Description

@proggeramlug

Filed out of PR #10938 (dictionary mode, step 2.5 stage 1). Latent today — that PR's latch is stubbed off and cannot fire on main. This issue exists so nobody enables a latch, here or anywhere else, without knowing what it costs.

The invariant that does not hold

A dictionary-mode receiver publishes a ShapeId that describes no keys (keys = NULL, logical_key_count = 0) while its real ordered key list lives in ObjectMeta::dictionary_keys. object::object_keys_array is branched to answer with the private list, so every consumer that asks that is correct.

Many consumers do not ask that. They read descriptor.keys.is_null() or descriptor.logical_key_count == 0 and conclude "this receiver has no own properties." For a dictionary receiver that is false, and the failure is usually a wrong value rather than a slow path.

The nine, and their verdicts

Six found by auditing descriptor.keys / logical_key_count consumers before the first build:

# site verdict
1 object/field_get_set/ic_miss.rs, the R::ObjectNoKeys arm wrong value — the arm's own comment says "A receiver with no keys array has NO own properties at all" and primes the inherited-read cache on it, so an own property is answered from the prototype chain
2 object/native_call_method.rs:~165, own-field shadowing scan wrong value — key_count == 0 makes "no own key shadows this method" vacuously true, so a vtable method beats an own field (obj.toString = …)
3 object/native_call_method.rs:~2193, the same scan, second spelling wrong value, same reason
4 object/inherited_read_cache.rs, the prototype-hop own-key check wrong value — a dictionary-mode prototype reads as having no own keys, so the walk continues past it and caches a value from farther up
5 object/field_set_by_name/fast_paths.rs unsafe — takes its bound from the descriptor and its keys pointer from object_keys_array; for a dictionary receiver those name different arrays, and a write that lands is out of bounds
6 object/reserved_floor.rs, the floor restamp wrong value — republishes an explicit keys edge the receiver does not have, and would publish live = 0, destroying its real bound

Three more found by running the whole parity file with the latch forced on for every receiver:

# symptom
7 delete holes the value slot and leaves the key in the list: Object.keys still lists it, JSON.stringify prints null, in returns true
8 delete-then-re-add does not move the key to the end (same root as 7)
9 an own property answered from the prototype chain — the read side, i.e. a site beyond the four guarded above

Six-line repro for 7 (against PR #10938's branch, PERRY_OBJECT_DICTIONARY_MIN_KEYS=0):

const o = {}; o.a = 1; o.b = 2; o.c = 3; o.d = 4;
delete o.b;
console.log(JSON.stringify(Object.keys(o)), JSON.stringify(o), "b" in o);
node / latch off : ["a","c","d"]      {"a":1,"c":3,"d":4}           false
latch on         : ["a","b","c","d"]  {"a":1,"b":null,"c":3,"d":4}  true

Two more that are correct by accident rather than by a guard, and would stop being so under a small edit:

  • object/shapes.rs:1024-1050 js_shape_ordinary_inline_slot_for_key (a codegen PIC helper) — its hole_count == 0 && live_inline_slot_count == logical_key_count test passes on 0 == 0, and only then does keys_array_dense_slots(0) return null and yield -1. The guard it wants (keys != 0) is not there.
  • object/delete_rest.rs:887 — the O(1) tombstone lane declines on logical_key_count == 0. Safe (it falls back), but it is a silent cliff.

Nine is unlikely to be all of them. Six came from an audit and three from one corpus run; there is no enumeration anywhere of "consumers that treat the shape's key list as complete", and finding the tenth by running the corpus again is not a method.

The class fix

Add ShapeObjectKind::Dictionary to object/shapes.rs:213. It is already one of the six components of facts_key, so it costs nothing new to carry, and it fixes the class by construction instead of by finding the consumers:

  • every keys.is_null() / logical_key_count == 0 consumer keeps its meaning, because a dictionary receiver no longer looks like an empty one;
  • object::object_is_regular answers false for it, so every object_is_regular-gated fast lane declines on its own instead of needing a hand-written guard at each site;
  • is_dictionary becomes one field of a descriptor the caller has already loaded, instead of a try_read_gc_header plus a ShapeId slab probe. That last point is also most of the measured cost: a dictionary-mode read is 3,292 instructions against 163 for the same spilled read with the latch off (PR feat(runtime): object dictionary mode — a receiver can carry its own keys (#10868 step 2.5 stage 1) #10938), and the gap is the six declined fast paths plus that repeated probe — not the key lookup.

shapes.rs is owned exclusively by step 2.5 (#10868) and is being rewritten there, which is why PR #10938 does not add the variant.

Until then

PR #10938's should_latch_to_dictionary is stubbed and can only answer true when explicitly armed (PERRY_OBJECT_DICTIONARY_MIN_KEYS, PERRY_OBJECT_DICTIONARY_LAYOUT_ID_BUDGET, or test_arm_latch). Nothing in a normal build reaches any of the nine. Do not wire a production trigger before this is closed — including step 2.5's own layout-id-exhaustion trigger, which is a correctness path and therefore fires whether or not anyone armed anything.

Activity

  1. proggeramlug commented on Sep 22, 2026

    @proggeramlug
    ContributorAuthor

    Triage: is this latent today? Answered — the nine sites are, the defect class is not.

    Asked because "latent because the latch is stubbed" only covers dictionary mode; the nine sites key on any keyless shape.

    Source sweep. No production path mints a keyless shape: the only shape_descriptor_ensure(std::ptr::null(), …) callers on upstream/main outside *_tests.rs / gc/tests/ are none. (reserved_floor.rs:122 passes a real keys array.)

    Behavioural sweep, 14 cases against node v26.8.1 on pristine main v0.5.1632 (841b605c9) — every way I could find to reach a keyless shape, each read 300× to prime an IC:

    case result
    A1–A3 {}, Object.create(null), an object with every key deleted ✅ keyless, and genuinely empty — the "no own properties" conclusion is correct, not a bug
    B keyless receiver + an own accessor shadowing a prototype data property ✅ own-getter
    C, D keyless receiver + an own accessor / data property named toString ✅ own-ts, own-ts-data
    E symbol-only own property (never in the keys array) ✅ — and symbols cannot shadow a string key, so the string-key conclusion stays sound
    F1–F3 expando-backed own properties on Error / Map / RegExp ✅ read and enumerated correctly
    H1, H2 drained-to-empty, then prototype shadowing, then re-add ✅
    I arguments ✅
    G own property named like a native method on an exotic cell ❌ wrong value

    So: no reachable combination of "keyless shape" + "has own properties" exists on main today, and the nine sites in this issue stay correctly labelled latent. That is a real answer, not an absence of evidence — B, C and D are exactly the keyless-plus-own-property shape the sites need, and they pass.

    But the defect class is live, via a different trigger, and I have filed it as #10943: an own property shadowing a native method is ignored on Map/Set/RegExp/Date/Array (m.get = () => x; m.get() runs the native method), because object_shape_descriptor returns None for a cell whose +4 word is capacity rather than a ShapeId, and the shadowing check's ? turns "I cannot tell" into "no own field". Seven wrong rows, plain JS, no flags.

    Same defect — a conclusion drawn from absence — with a wider trigger: this issue needs a shape that says no keys, #10943 needs one that cannot be read, and every non-GC_TYPE_OBJECT cell already has that.

    What this changes here: nothing about the fix or the priority of this issue, and the precondition stands unchanged — ShapeObjectKind::Dictionary must land in or before whatever wires a trigger. What it does change is that #10943 should not wait for step 2.5, because it is shipping wrong values now.

    Method note: both perry arms (pristine main, and an unrelated branch off train 253) produced byte-identical output on all 14 rows, which is also the control proving that branch's guards are inert when nothing is latched.

  2. proggeramlug commented on Sep 22, 2026

    @proggeramlug
    ContributorAuthor

    Measured against ShapeObjectKind::Dictionary: eight of nine go away; one defect survives

    Lane 8 landed the kind (b858fe900). I rebased #10938 onto it, deleted every hand-written guard, and re-ran the force-latch parity file. This is the number, not an estimate.

    Result

    arm rows differing from node
    latch OFF 0
    latch ON, zero hand guards, kind only 7 rows — one defect
    latch ON, six hand guards, no kind (plain main) 3 defects

    The 7 rows are tomb (4) and hole (3): delete on a dictionary receiver, one root cause with two symptoms. Everything else that needed a hand guard is gone.

    Most telling: child p=from-proto — an own property answered from the prototype chain, the wrong value I considered the most serious in this issue — passes with no guard at all. object_is_regular answers false for a non-Ordinary kind, so the fast lane declines by construction.

    Which of the nine, and why — from the source, not just the run

    # site gated by the kind?
    1 ic_miss.rs R::ObjectNoKeys primer yes — :1067 object_kind == Ordinary feeds is_regular
    2 native_call_method.rs shadowing scan yes — :143 !object_is_regular → return None
    3 native_call_method.rs 2nd spelling (~:2193) no — gated on GC_TYPE_OBJECT only
    4 inherited_read_cache.rs prototype hop yes — :677, :770
    5 fast_paths.rs store lane (the unsafe one) yes — :87 object_kind != Ordinary → return false
    6 reserved_floor.rs floor restamp no — only unwrap_or(Ordinary) as a default
    7,8 delete leaves the key / re-add ordering no — the surviving defect
    9 own property from the prototype chain yes — same mechanism as 1 and 4

    Plus one upgrade: js_shape_ordinary_inline_slot_for_key (shapes.rs:1027) was correct by accident — it passed its hole_count == 0 && live == logical test on 0 == 0 and only then fell out. It now declines on object_kind != Ordinary, i.e. correct by construction.

    So I deleted the guards for 1, 2, 4, 5 and kept only 3 and 6, which source analysis shows the kind does not reach. Re-measured with those three deleted: still 7 rows, unchanged — they were genuinely unnecessary.

    What this prices

    One honest fact on the shape replaced eight of nine hand-wired guards, including the memory-unsafe one. That is the charter's thesis with a number on it, and it is the argument for putting a fact in the shape rather than teaching each consumer a new spelling.

    What remains

    delete on a dictionary receiver: the key stays in the list and only the value slot is holed, so Object.keys lists it, JSON.stringify prints null and in returns true. One defect, in my lane, on top of the rebase. Six lines reproduce it:

    const o = {}; o.a = 1; o.b = 2; o.c = 3; o.d = 4;
    delete o.b;
    console.log(JSON.stringify(Object.keys(o)), JSON.stringify(o), "b" in o);

    Measurement tree: /root/wt-dict2 on perrymaster (detached at b858fe900 + #10938). The real rebase lands after lane 8's branch does, as agreed.

  3. proggeramlug commented on Sep 22, 2026

    @proggeramlug
    ContributorAuthor

    The surviving delete defect: traced to the branch, one concrete bug found, not yet closed

    Handing off the state precisely rather than guessing further.

    Symptom triple, which is what narrows it

    const o = {}; o.a = 1; o.b = 2; o.c = 3; o.d = 4;
    delete o.b;
    Object.keys(o)        // ["a","b","c","d"]   node: ["a","c","d"]
    JSON.stringify(o)     // {"a":1,"c":3,"d":4}  node: same  <-- MATCHES
    "b" in o              // true                 node: false

    in is true and Object.keys lists b, but JSON.stringify agrees with node. That combination says the key string survived and only the value was cleared: JSON.stringify omits an undefined-valued property, so it matches by coincidence, not because the delete worked.

    Path, from source

    delete_rest.rs:

    • :331 let mut keys = object_keys_array(obj) — for a dictionary receiver this is the private list in ObjectMeta, which is correct;
    • :398 keys_owned is true (the private array is freshly allocated and carries no GC_FLAG_SHAPE_SHARED), so the clone-before-tombstone branch at :412 is skipped;
    • :484 stable_candidate requires object_kind == Ordinary, so a dictionary receiver is not stable — correct, and it means OBJ_FLAG_STABLE_TOMBSTONES is not set and the value is cleared to TAG_UNDEFINED rather than TAG_HOLE;
    • :508 the tombstone lane publishes a successor and, on successor != 0, writes TAG_HOLE into the key slot of keys and clears the value slot.

    The value clearly got cleared, so that block ran. What has not been established is why the key-slot TAG_HOLE is not visible to enumeration afterwards.

    One concrete bug found on the way, worth fixing regardless

    shapes_slot_list.rs:854-855, inside publish_object_shape_delete_transition:

    let keys_ptr = current.keys as usize as *mut super::ArrayHeader;
    let logical_key_count = crate::array::keys_array_len_capped_to_capacity(keys_ptr) as u32;

    Its own comment says "The key count comes from the ARRAY, not the lineage" — but it takes the array from current.keys, the shape's keys word, not from the receiver. For a dictionary receiver that word is NULL, so it publishes logical_key_count = 0 with hole_count = holes + 1: a shape claiming one hole in a list of zero keys. That is incoherent on its face and cannot be what any consumer expects.

    The comment's intent — read the array, stay honest if a caller publishes after a length change — is satisfied by object_keys_array(obj) and not by current.keys. This is the same defect shape as #10939: a keys array taken from the wrong place.

    What is needed next

    An instrumented run of the delete on a dictionary receiver, printing which branch ran and what the array holds at index i afterwards. Reading has taken this as far as it goes; the remaining question — key-slot hole written but not observed — is a two-line eprintln away and I am handing it off rather than starting another build-debug loop at the end of a long session.

    Everything else in #10942 is closed by the kind (see the previous comment: eight of nine). This is the last one, and it is one defect, not two.

  4. proggeramlug commented on Sep 22, 2026

    @proggeramlug
    ContributorAuthor

    The surviving delete defect: closed. The cause is not shapes_slot_list.rs:854, and that line is not a bug at all.

    Picking up the handoff. The trace in the previous comment is good and I verified rather than redid it — but the leading candidate is refuted, and by two independent arguments.

    Two knobs, zero builds, and the candidate is gone

    The first thing the existing binary answers:

    arm Object.keys in
    latch ON ["a","b","c","d"] true
    latch ON + PERRY_DELETE_SHAPE_TRANSITION=0 ["a","b","c","d"] true
    latch ON + PERRY_OBJECT_TOMBSTONES=0 ["a","c","d"] false

    PERRY_DELETE_SHAPE_TRANSITION=0 routes the publish through publish_object_shape_holes instead of publish_object_shape_delete_transition, i.e. around shapes_slot_list.rs:854 entirely, and the symptom is unchanged. The tombstone kill switch fixes it. So the defect is in the tombstone lane, not in the delete-transition publish.

    The construction is the discriminator, and it names the branch

    construction n=2 n=4 n=8 n=15 n=16 n=20
    literal / o.k = v, {k:v}, o["k"] = v ✗ ✗ ✗ ✗ ✓ ✓
    dynamic key (o[computed] = v) ✓ ✓ ✓ ✓ ✓ ✓

    n < 16 fails and n >= 16 passes: that is exactly delete_rest.rs:412's !keys_owned && key_count < 16 — the clone-before-tombstone branch. A dynamically built receiver's private array is not GC_FLAG_SHAPE_SHARED, so it never enters the branch, and delete on a genuinely pre-latched dictionary receiver was working all along.

    The cause, from the instrumented run

    [dbg A] i=1 key_count=4 keys=(…497224, len 4)  dict=false  descr=(keys=…497224, logical=4, kind=Ordinary)
    [dbg B] after set_object_keys_array: local=(…128064)  published=(…128208)  dict=true  descr=(keys=0, logical=0, kind=Dictionary)
    [dbg C] local[1]=TAG_HOLE            published[1]=<the "b" string>
    

    The receiver is ordinary when delete starts. Its keys array is shared, so the delete clones it and publishes the clone to take ownership — and the tail of that publication is where the latch fires. latch_object_to_dictionary takes its own private copy of the key list (…208) and installs that. The array the delete is still holding (…064) is an orphan by the time the tombstone is written, so TAG_HOLE goes into a dead copy while the receiver's live list keeps the key.

    The value slot clears on the receiver either way. That is the whole reason JSON.stringify agreed with node: it omits an undefined-valued property.

    It is a stale pointer across a publish, not a dictionary bug. The latch is one reason the receiver's key list can change under set_object_keys_array; a mint that collects and moves the array is another, and that one is live on main today with the latch stubbed off.

    The fix

    delete_rest.rs, the clone branch: re-derive from the receiver instead of trusting the address handed in, across a handle so a moved receiver is reloaded too, and read ownership off the array the receiver actually carries rather than asserting it.

    let ((), reloaded_obj) = obj_handle
        .across_mut::<ObjectHeader, _>(|| set_object_keys_array(obj, keys_cloned));
    obj = reloaded_obj;
    keys = crate::object::object_keys_array(obj);
    …
    keys_owned = (*reloaded_gc).gc_flags & crate::gc::GC_FLAG_SHAPE_SHARED == 0;

    shapes_slot_list.rs:854 is correct, and the proposed fix would have broken it

    Worth stating plainly, because I filed it myself and it is wrong.

    object_keys_array(obj) returns descriptor.keys whenever that word is nonzero and only otherwise consults the private list. So current.keys and object_keys_array(obj) are the same array for every receiver whose shape publishes keys, and they differ only for a dictionary — where 0 is not an accident but the invariant debug_assert_dictionary_parity enforces (descriptor.keys == 0 && descriptor.logical_key_count == 0).

    Taking the count from object_keys_array(obj) there would publish keys = 0, logical_key_count = 4, which shape_descriptor_ensure_with_holes rejects outright (keys_id == 0 && logical_key_count != 0 → InvalidFacts). The publish would return 0 and silently disable the tombstone lane for every dictionary receiver. The line stays as it is.

    What is left over is a smaller thing, and it is not this: hole_count on a keyless shape. A dictionary receiver now ends up with hole_count = 1 beside logical_key_count = 0, which is coherent if hole_count means "holes in the receiver's key list, wherever that list lives" — but restamp_dictionary_shape hardcodes 0, so a later live-bound change silently drops the count. No wrong value follows (enumeration reads TAG_HOLE out of the array directly); the squeeze threshold stops firing. Follow-up, not a blocker.

    Witness

    object::dictionary_tests::a_delete_that_latches_tombstones_the_key_list_the_receiver_keeps. It builds a sibling first so the transition cache shares the array, asserts every precondition by name (ordinary at the start, key list present, four keys, below the clone branch's bound, GC_FLAG_SHAPE_SHARED set, and — after the delete — that the receiver actually latched), then asserts the deleted key is absent from the receiver's live list.

    Reverting the three lines reddens it:

    the deleted key is still in the receiver's LIVE key list: the tombstone was
    written into the array the delete handed to the publisher, and the latch
    replaced it with a private copy
    

    Numbers

    test_parity_dictionary_mode_order.ts against node, both arms byte-identical, 0 rows differing — latch off and PERRY_OBJECT_DICTIONARY_MIN_KEYS=0. That is the 7 rows (tomb ×4, hole ×3) gone, including tomb keys=["b","c","a"], i.e. symptom 8's delete-then-re-add ordering.

    Suite, --test-threads=1: 4229 passed / 1 failed / 6 ignored. The one failure is not from this change —
    object::inherited_read_cache::tests::a_second_receiver_of_the_same_shape_shares_the_entry fails with delete_rest.rs pristine, fails on lane 8's kind-only tree at b858fe900 with no #10938 at all, and passes at train 253 (0fa391529). It is a regression somewhere in the four commits on feat/canonical-shape-identity; 8d29d790b (prototype divergence mints a deterministic generation) is the obvious first suspect. Raised separately — it is not a dictionary-mode problem and should not ride on this.

    Patch is in /root/wt-dict2 on perrymaster, on top of the rebase, for the real landing after lane 8's branch.

    Eight of nine, and then the ninth was not what it looked like

    Worth recording next to the kind's win. ShapeObjectKind::Dictionary closed eight sites by construction. The ninth was never a dictionary defect: it is a caller keeping a keys pointer across a call that can replace the receiver's key list, and dictionary mode only made it reachable. Three bugs in one night were "a keys array taken from the wrong place" (#10939's four sites, the #10476 kind guard, and this) — see the separate note on making that shape impossible rather than documented.

  5. proggeramlug commented on Sep 22, 2026

    @proggeramlug
    ContributorAuthor

    Correction to my own comment: this is not reachable on main today. The publish path allocates nothing on the JS heap.

    I wrote above that "a mint that collects and moves the array does the same thing on main today", and used it to say the class is live with the latch stubbed off. That is wrong, and I am correcting it before it gets priced into anyone's ordering.

    I took it from restamp_dictionary_shape's own comment — "the mint inserts into a HashMap and can therefore collect and MOVE the receiver" — and repeated it as a reachability claim when it is only a discipline claim. Checking it instead of quoting it:

    1. #[global_allocator] is mimalloc or std::alloc::System (lib.rs:30-60), with no GC hook. A Rust-heap allocation cannot trigger a JS collection.
    2. alloc_shape_id is a pure atomic CAS (shapes.rs:558) — it does not allocate at all.
    3. arena_alloc_gc / js_array_alloc / js_object_alloc / js_string_* appear nowhere in shapes.rs, shapes_store.rs or shapes_slot_list.rs outside #[test] code.
    4. mark_object_dynamic_shape_unknown flips header bits and a side-table entry; clear_array_subclass_named_prefix_token clears one meta word.

    So set_object_keys_array performs no JS-heap allocation, cannot collect, and cannot move the receiver's keys array. On main, with the latch stubbed off, the only publisher that can replace a receiver's key list is the one that does not exist yet.

    What this changes

    The restamp_dictionary_shape comment should probably say "must not assume otherwise" rather than "can"; as written it reads as a statement of fact about the allocator and it is not one.

  6. proggeramlug commented on Sep 22, 2026

    @proggeramlug
    ContributorAuthor

    The restamp_dictionary_shape hole_count = 0 leftover: unreachable, not latent. No fix — a guard instead.

    I flagged this above as "a follow-up, perf not correctness". Checked before building anything, and it is the second thing I flagged today that dissolves on a reachability check.

    restamp_dictionary_shape publishes hole_count = 0 unconditionally. It has exactly two callers:

    • latch_object_to_dictionary — refuses a receiver with hole_count != 0 outright, so 0 is correct, and a_receiver_with_holes_is_refused already pins that.
    • publish_keys, when values_may_have_moved || live_changed —
      • values_may_have_moved is the compacting delete, which squeezed the holes out, so 0 is correct;
      • live_changed is structurally false for every non-birth call. set_object_keys_array passes object_live_slot_count(obj), which is shape_live_inline_slot_count_by_id(object_shape_stamp(obj)) — the descriptor's own current value — so it cannot differ from the descriptor it is compared against. Every caller that supplies an explicit bound instead, via set_object_keys_array_with_live, is an allocator (alloc.rs ×4, arguments.rs, json_construction.rs ×2) working on a fresh object with no holes to lose. The by-name append path does not grow the live inline bound; it spills.

    So there is no reachable restamp that drops a tombstone count, and I am not changing the code. A fix here would be a change with no reachable bug and no buildable witness — the same thing I just argued against for shapes_slot_list.rs:854.

    What I added instead

    a_dictionary_receivers_hole_count_keeps_matching_its_key_list — a guard on the invariant rather than on my reasoning: after a delete, and again after 30 appends that reallocate the private array several times, the shape's hole_count must equal the number of TAG_HOLE slots the receiver's key list physically carries. It asserts the premise too (the delete must actually have taken the tombstone lane), so it cannot pass vacuously.

    Proven able to fail: forcing live_changed = swapped in publish_keys reddens it with left: 0, right: 1, which is precisely the dropped count. Reverted.

    That converts "I read the callers and think it cannot happen" into a checked property. If anyone later gives the live inline bound a way to grow after birth, this is what tells them.

    Updated landing artefacts

    /root/lane16b/landing/0002-witness-dictionary-tests.patch now carries both tests (216 lines) and is verified to apply cleanly to the pre-rebase dictionary_tests.rs. combined.patch is 0001 + 0002. The fix commit is unchanged at ffba86288.

    Suite on the tree, --test-threads=1: 4230 passed / 6 ignored, with the one pre-existing lane-8 failure (a_second_receiver_of_the_same_shape_shares_the_entry) still the only red and still not from this work. All 7 dictionary_tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugConfirmed defect or regression

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions