Repository navigation
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
Activity
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 onupstream/mainoutside*_tests.rs/gc/tests/are none. (reserved_floor.rs:122passes a real keys array.)Behavioural sweep, 14 cases against node v26.8.1 on pristine
mainv0.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-getterC, D keyless receiver + an own accessor / data property named toString✅ own-ts,own-ts-dataE 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), becauseobject_shape_descriptorreturnsNonefor a cell whose+4word iscapacityrather 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_OBJECTcell already has that.What this changes here: nothing about the fix or the priority of this issue, and the precondition stands unchanged —
ShapeObjectKind::Dictionarymust 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.
Measured against
ShapeObjectKind::Dictionary: eight of nine go away; one defect survivesLane 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) andhole(3):deleteon 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_regularanswersfalsefor a non-Ordinarykind, 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.rsR::ObjectNoKeysprimeryes — :1067object_kind == Ordinaryfeedsis_regular2 native_call_method.rsshadowing scanyes — :143!object_is_regular → return None3 native_call_method.rs2nd spelling (~:2193)no — gated on GC_TYPE_OBJECTonly4 inherited_read_cache.rsprototype hopyes — :677,:7705 fast_paths.rsstore lane (the unsafe one)yes — :87object_kind != Ordinary → return false6 reserved_floor.rsfloor restampno — only unwrap_or(Ordinary)as a default7,8 deleteleaves the key / re-add orderingno — 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 itshole_count == 0 && live == logicaltest on0 == 0and only then fell out. It now declines onobject_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
deleteon a dictionary receiver: the key stays in the list and only the value slot is holed, soObject.keyslists it,JSON.stringifyprintsnullandinreturnstrue. 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-dict2on perrymaster (detached atb858fe900+ #10938). The real rebase lands after lane 8's branch does, as agreed.The surviving
deletedefect: traced to the branch, one concrete bug found, not yet closedHanding 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
inis true andObject.keyslistsb, butJSON.stringifyagrees with node. That combination says the key string survived and only the value was cleared:JSON.stringifyomits anundefined-valued property, so it matches by coincidence, not because the delete worked.Path, from source
delete_rest.rs::331let mut keys = object_keys_array(obj)— for a dictionary receiver this is the private list inObjectMeta, which is correct;:398keys_ownedis true (the private array is freshly allocated and carries noGC_FLAG_SHAPE_SHARED), so the clone-before-tombstone branch at:412is skipped;:484stable_candidaterequiresobject_kind == Ordinary, so a dictionary receiver is not stable — correct, and it meansOBJ_FLAG_STABLE_TOMBSTONESis not set and the value is cleared toTAG_UNDEFINEDrather thanTAG_HOLE;:508the tombstone lane publishes a successor and, onsuccessor != 0, writesTAG_HOLEinto the key slot ofkeysand clears the value slot.
The value clearly got cleared, so that block ran. What has not been established is why the key-slot
TAG_HOLEis not visible to enumeration afterwards.One concrete bug found on the way, worth fixing regardless
shapes_slot_list.rs:854-855, insidepublish_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 publisheslogical_key_count = 0withhole_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 bycurrent.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
iafterwards. Reading has taken this as far as it goes; the remaining question — key-slot hole written but not observed — is a two-lineeprintlnaway 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.
The surviving
deletedefect: closed. The cause is notshapes_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.keysinlatch 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=0routes the publish throughpublish_object_shape_holesinstead ofpublish_object_shape_delete_transition, i.e. aroundshapes_slot_list.rs:854entirely, 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 < 16fails andn >= 16passes: that is exactlydelete_rest.rs:412's!keys_owned && key_count < 16— the clone-before-tombstone branch. A dynamically built receiver's private array is notGC_FLAG_SHAPE_SHARED, so it never enters the branch, anddeleteon 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
deletestarts. 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_dictionarytakes 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, soTAG_HOLEgoes 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.stringifyagreed 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:854is correct, and the proposed fix would have broken itWorth stating plainly, because I filed it myself and it is wrong.
object_keys_array(obj)returnsdescriptor.keyswhenever that word is nonzero and only otherwise consults the private list. Socurrent.keysandobject_keys_array(obj)are the same array for every receiver whose shape publishes keys, and they differ only for a dictionary — where0is not an accident but the invariantdebug_assert_dictionary_parityenforces (descriptor.keys == 0 && descriptor.logical_key_count == 0).Taking the count from
object_keys_array(obj)there would publishkeys = 0, logical_key_count = 4, whichshape_descriptor_ensure_with_holesrejects 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_counton a keyless shape. A dictionary receiver now ends up withhole_count = 1besidelogical_key_count = 0, which is coherent ifhole_countmeans "holes in the receiver's key list, wherever that list lives" — butrestamp_dictionary_shapehardcodes0, so a later live-bound change silently drops the count. No wrong value follows (enumeration readsTAG_HOLEout 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_SHAREDset, 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 copyNumbers
test_parity_dictionary_mode_order.tsagainst node, both arms byte-identical, 0 rows differing — latch off andPERRY_OBJECT_DICTIONARY_MIN_KEYS=0. That is the 7 rows (tomb×4,hole×3) gone, includingtomb 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_entryfails withdelete_rest.rspristine, fails on lane 8's kind-only tree atb858fe900with no #10938 at all, and passes at train 253 (0fa391529). It is a regression somewhere in the four commits onfeat/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-dict2on 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::Dictionaryclosed 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.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 aHashMapand 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:#[global_allocator]is mimalloc orstd::alloc::System(lib.rs:30-60), with no GC hook. A Rust-heap allocation cannot trigger a JS collection.alloc_shape_idis a pure atomic CAS (shapes.rs:558) — it does not allocate at all.arena_alloc_gc/js_array_alloc/js_object_alloc/js_string_*appear nowhere inshapes.rs,shapes_store.rsorshapes_slot_list.rsoutside#[test]code.mark_object_dynamic_shape_unknownflips header bits and a side-table entry;clear_array_subclass_named_prefix_tokenclears one meta word.
So
set_object_keys_arrayperforms 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 defect is feat(runtime): object dictionary mode — a receiver can carry its own keys (#10868 step 2.5 stage 1) #10938-gated, exactly as the issue's framing said. It should not jump the queue as a main-landable memory-safety item, and I am not proposing to land it on main: a fix for an unreachable bug, with a witness that cannot be built without the latch, is the thing this campaign rejects.
- It does not change the fix, which is still necessary and still correct.
- It does not weaken the case for the consuming publish (ShapeId exhaustion aborts the process after ~475 TypeScript transpiles (~2.26M ids minted per transpileModule, ~45 per AST node) #10868 stage 1c) — if anything it sharpens it. The reason nothing can replace the key list under a publish today is that the publish path does not allocate. Step 2.5 is the change that makes it replaceable. The ownership rule is being added at precisely the moment it becomes load-bearing, rather than retrofitted after.
The
restamp_dictionary_shapecomment 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.The
restamp_dictionary_shapehole_count = 0leftover: 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_shapepublisheshole_count = 0unconditionally. It has exactly two callers:latch_object_to_dictionary— refuses a receiver withhole_count != 0outright, so0is correct, anda_receiver_with_holes_is_refusedalready pins that.publish_keys, whenvalues_may_have_moved || live_changed—values_may_have_movedis the compacting delete, which squeezed the holes out, so0is correct;live_changedis structurally false for every non-birth call.set_object_keys_arraypassesobject_live_slot_count(obj), which isshape_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, viaset_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'shole_countmust equal the number ofTAG_HOLEslots 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 = swappedinpublish_keysreddens it withleft: 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.patchnow carries both tests (216 lines) and is verified to apply cleanly to the pre-rebasedictionary_tests.rs.combined.patchis0001+0002. The fix commit is unchanged atffba86288.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 7dictionary_testspass.
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 inObjectMeta::dictionary_keys.object::object_keys_arrayis 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()ordescriptor.logical_key_count == 0and 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_countconsumers before the first build:object/field_get_set/ic_miss.rs, theR::ObjectNoKeysarmobject/native_call_method.rs:~165, own-field shadowing scankey_count == 0makes "no own key shadows this method" vacuously true, so a vtable method beats an own field (obj.toString = …)object/native_call_method.rs:~2193, the same scan, second spellingobject/inherited_read_cache.rs, the prototype-hop own-key checkobject/field_set_by_name/fast_paths.rsobject_keys_array; for a dictionary receiver those name different arrays, and a write that lands is out of boundsobject/reserved_floor.rs, the floor restamplive = 0, destroying its real boundThree more found by running the whole parity file with the latch forced on for every receiver:
deleteholes the value slot and leaves the key in the list:Object.keysstill lists it,JSON.stringifyprintsnull,inreturnstrueSix-line repro for 7 (against PR #10938's branch,
PERRY_OBJECT_DICTIONARY_MIN_KEYS=0):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-1050js_shape_ordinary_inline_slot_for_key(a codegen PIC helper) — itshole_count == 0 && live_inline_slot_count == logical_key_counttest passes on0 == 0, and only then doeskeys_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 onlogical_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::Dictionarytoobject/shapes.rs:213. It is already one of the six components offacts_key, so it costs nothing new to carry, and it fixes the class by construction instead of by finding the consumers:keys.is_null()/logical_key_count == 0consumer keeps its meaning, because a dictionary receiver no longer looks like an empty one;object::object_is_regularanswersfalsefor it, so everyobject_is_regular-gated fast lane declines on its own instead of needing a hand-written guard at each site;is_dictionarybecomes one field of a descriptor the caller has already loaded, instead of atry_read_gc_headerplus 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.rsis 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_dictionaryis stubbed and can only answertruewhen explicitly armed (PERRY_OBJECT_DICTIONARY_MIN_KEYS,PERRY_OBJECT_DICTIONARY_LAYOUT_ID_BUDGET, ortest_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.