From 4932bd7e1cb0098d8be2dfefb5ea0408fa793e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 2 Aug 2026 21:00:02 +0200 Subject: [PATCH 1/3] fix(lint): audit the 19 unaudited GC store sites (#7258) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comment-only in the runtime/codegen, plus one rule-precision fix in the scanner. No generated code and no runtime behaviour changes. Triage of the 19: * 5 were PHANTOM. `classify_rust_store` searched the atomic-store regexes against `call_window` (6 lines forward) unanchored, so one `cache.store(...)` was re-reported from each of the five lines above it — including `continue;` and a bare `}`. Anchoring the match to the head line keeps split-call coverage and reports the site once. * 3 were genuinely new runtime sites needing a class: - array/alloc.rs, array/push_pop.rs (#7138): TAG_HOLE fill of freshly allocated, not-yet-published capacity -> INIT. - array/indexing.rs (#7071): the collector's own rewrite of a registered side-table root slot, mutator stopped -> ROOT. * 11 were codegen stores. Four already carried a POINTER_FREE marker that the #6915 `value_is_canonical_raw_f64` branch split pushed out of the +/-6-line window; the other seven (finite-check-guarded field stores, typed-array backing-store element stores) had prose rationale but not the canonical marker form. Self-tests: the new `proto_cache_scan` case fails on the pre-fix unanchored logic with exactly the five phantom lines, so the regression guard is live rather than merely non-throwing. --- crates/perry-codegen/src/expr/index.rs | 6 ++ crates/perry-codegen/src/expr/index_set.rs | 3 + .../perry-codegen/src/expr/instance_misc1.rs | 7 ++ crates/perry-codegen/src/expr/property_set.rs | 12 +-- .../src/expr/proven_view_access.rs | 12 +++ crates/perry-runtime/src/array/alloc.rs | 4 + crates/perry-runtime/src/array/indexing.rs | 6 ++ crates/perry-runtime/src/array/push_pop.rs | 6 ++ scripts/gc_store_site_inventory.py | 87 +++++++++++++++++-- 9 files changed, 132 insertions(+), 11 deletions(-) diff --git a/crates/perry-codegen/src/expr/index.rs b/crates/perry-codegen/src/expr/index.rs index ae62d77845..3ab8d017e6 100644 --- a/crates/perry-codegen/src/expr/index.rs +++ b/crates/perry-codegen/src/expr/index.rs @@ -336,6 +336,8 @@ pub(crate) fn lower_index_set_fast( // arithmetic / Math.* / coerce chain) — store verbatim. blk.store(DOUBLE, val_double, &element_ptr); } else { + // GC_STORE_AUDIT(POINTER_FREE): js_array_numeric_value_to_raw_f64 + // returns a plain unboxed f64 — no GC pointer, so no barrier. let numeric_value = canonicalize_raw_f64_numeric_store_value(blk, val_double); blk.store(DOUBLE, &numeric_value, &element_ptr); } @@ -484,6 +486,8 @@ pub(crate) fn lower_index_set_fast( if value_is_canonical_raw_f64 { blk.store(DOUBLE, val_double, &element_ptr); } else { + // GC_STORE_AUDIT(POINTER_FREE): js_array_numeric_value_to_raw_f64 + // returns a plain unboxed f64 — no GC pointer, so no barrier. let numeric_value = canonicalize_raw_f64_numeric_store_value(blk, val_double); blk.store(DOUBLE, &numeric_value, &element_ptr); } @@ -519,6 +523,8 @@ pub(crate) fn lower_index_set_fast( if value_is_canonical_raw_f64 { blk.store(DOUBLE, val_double, &element_ptr); } else { + // GC_STORE_AUDIT(POINTER_FREE): js_array_numeric_value_to_raw_f64 + // returns a plain unboxed f64 — no GC pointer, so no barrier. let numeric_value = canonicalize_raw_f64_numeric_store_value(blk, val_double); blk.store(DOUBLE, &numeric_value, &element_ptr); } diff --git a/crates/perry-codegen/src/expr/index_set.rs b/crates/perry-codegen/src/expr/index_set.rs index 2d55d7bc96..44bcde5006 100644 --- a/crates/perry-codegen/src/expr/index_set.rs +++ b/crates/perry-codegen/src/expr/index_set.rs @@ -1211,6 +1211,9 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } else { let numeric_value = canonicalize_raw_f64_numeric_store_value(blk, &val_double); + // GC_STORE_AUDIT(POINTER_FREE): the + // canonicalizer returns a plain unboxed + // f64, never a GC pointer — no barrier. blk.store(DOUBLE, &numeric_value, &element_ptr); } blk.br(&merge_label); diff --git a/crates/perry-codegen/src/expr/instance_misc1.rs b/crates/perry-codegen/src/expr/instance_misc1.rs index 7c6d179058..f9f5c73110 100644 --- a/crates/perry-codegen/src/expr/instance_misc1.rs +++ b/crates/perry-codegen/src/expr/instance_misc1.rs @@ -1497,7 +1497,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } ctx.current_block = store_idx; { + // Reached only when the finite check above + // proved `new`'s exponent is NOT all-ones; + // every NaN-box tag (INT32/STRING/POINTER/ + // BIGINT) has an all-ones exponent. let blk = ctx.block(); + // GC_STORE_AUDIT(POINTER_FREE): a genuine + // unboxed double by the proof above, never + // a GC pointer — no edge, so no barrier. blk.store(DOUBLE, &new, &field_ptr); blk.br(&merge_label); } diff --git a/crates/perry-codegen/src/expr/property_set.rs b/crates/perry-codegen/src/expr/property_set.rs index b8e6666ed3..4e2558084b 100644 --- a/crates/perry-codegen/src/expr/property_set.rs +++ b/crates/perry-codegen/src/expr/property_set.rs @@ -590,12 +590,14 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } ctx.current_block = store_idx; { - // The finite check proved a genuine - // unboxed double (INT32-boxed and every - // NaN-box tag share the all-ones - // exponent) — no canonicalization call, - // no barrier (pointer-free by proof). + // The finite check proved a genuine unboxed + // double (INT32-boxed and every NaN-box tag + // share the all-ones exponent), so no + // canonicalization call is needed. let blk = ctx.block(); + // GC_STORE_AUDIT(POINTER_FREE): pointer-free + // by that proof — no GC pointer reaches the + // slot, so no write barrier. blk.store(DOUBLE, &val_double, &field_ptr); blk.br(&merge_label); } diff --git a/crates/perry-codegen/src/expr/proven_view_access.rs b/crates/perry-codegen/src/expr/proven_view_access.rs index 703c63f1d0..ce4e20e3c5 100644 --- a/crates/perry-codegen/src/expr/proven_view_access.rs +++ b/crates/perry-codegen/src/expr/proven_view_access.rs @@ -388,23 +388,35 @@ pub(crate) fn try_lower_proven_view_checked_store( idx_i64 }; let elem_ptr = blk.gep(I8, &data_ptr, &[(I64, &byte_off)]); + // Every arm below stores into `elem_ptr`, which addresses the view's + // BACKING STORE (`view.data_slot`). Typed-array elements are raw + // numeric bytes and can never hold a JSValue, so none of these stores + // creates a heap edge and none needs a write barrier. This is the + // codegen-side counterpart of the runtime carve-out for the + // `typedarray` / `typedarray_view` / `buffer` modules + // (`is_pointer_free_module` in scripts/gc_store_site_inventory.py). match view.elem { BufferElem::I8 | BufferElem::U8 => { let byte = blk.trunc(I32, &value_native.value, I8); + // GC_STORE_AUDIT(POINTER_FREE): typed-array backing store. blk.store(I8, &byte, &elem_ptr); } BufferElem::I16 | BufferElem::U16 => { let half = blk.trunc(I32, &value_native.value, I16); + // GC_STORE_AUDIT(POINTER_FREE): typed-array backing store. blk.store(I16, &half, &elem_ptr); } BufferElem::I32 | BufferElem::U32 => { + // GC_STORE_AUDIT(POINTER_FREE): typed-array backing store. blk.store(I32, &value_native.value, &elem_ptr); } BufferElem::F32 => { let narrow = blk.fptrunc(DOUBLE, &value_native.value, F32); + // GC_STORE_AUDIT(POINTER_FREE): typed-array backing store. blk.store(F32, &narrow, &elem_ptr); } BufferElem::F64 => { + // GC_STORE_AUDIT(POINTER_FREE): typed-array backing store. blk.store(DOUBLE, &value_native.value, &elem_ptr); } BufferElem::U8Clamped => unreachable!("gated above"), diff --git a/crates/perry-runtime/src/array/alloc.rs b/crates/perry-runtime/src/array/alloc.rs index 9286cf2ce1..13047b3124 100644 --- a/crates/perry-runtime/src/array/alloc.rs +++ b/crates/perry-runtime/src/array/alloc.rs @@ -52,6 +52,10 @@ pub extern "C" fn js_array_alloc(capacity: u32) -> *mut ArrayHeader { // scan misreads as live from-space pointers. let elements_ptr = (ptr as *mut u8).add(std::mem::size_of::()) as *mut u64; for i in 0..actual_capacity as usize { + // GC_STORE_AUDIT(INIT): initialization of a just-allocated array + // that is not yet reachable from any root, and TAG_HOLE is a + // non-pointer sentinel — there is no old value to remember and no + // new edge to record, so no write barrier. std::ptr::write(elements_ptr.add(i), crate::value::TAG_HOLE); } set_array_numeric_layout(ptr, NumericArrayLayout::RawF64); diff --git a/crates/perry-runtime/src/array/indexing.rs b/crates/perry-runtime/src/array/indexing.rs index 92618fe1c1..76024eb13d 100644 --- a/crates/perry-runtime/src/array/indexing.rs +++ b/crates/perry-runtime/src/array/indexing.rs @@ -108,6 +108,12 @@ pub fn scan_prototype_addr_cache_roots_mut(visitor: &mut crate::gc::RuntimeRootV } let mut addr = cached; if visitor.visit_usize_slot(&mut addr) { + // GC_STORE_AUDIT(ROOT): this IS the collector's root-rewrite of a + // registered side-table slot, running inside a root scan with the + // mutator stopped. `visit_usize_slot` returns true only when it + // relocated the object, and the value written is the visitor's own + // to-space address — barriering it would push an edge into the + // remembered set that this very cycle is rebuilding. cache.store(addr, Ordering::Relaxed); } } diff --git a/crates/perry-runtime/src/array/push_pop.rs b/crates/perry-runtime/src/array/push_pop.rs index 0f4daba730..f60e35a212 100644 --- a/crates/perry-runtime/src/array/push_pop.rs +++ b/crates/perry-runtime/src/array/push_pop.rs @@ -98,6 +98,12 @@ pub extern "C" fn js_array_grow(arr: *mut ArrayHeader, min_capacity: u32) -> *mu let new_elems = (new_ptr as *mut u8).add(std::mem::size_of::()) as *mut u64; for i in old_capacity as usize..new_capacity as usize { + // GC_STORE_AUDIT(INIT): initialization of the freshly grown + // array's added [old_capacity, new_capacity) slack — storage + // this allocation has never published, written with the + // non-pointer TAG_HOLE sentinel. No edge is created, so no + // write barrier (the copied prefix replays its own barriers + // via replay_array_growth_write_barriers below). ptr::write(new_elems.add(i), crate::value::TAG_HOLE); } } diff --git a/scripts/gc_store_site_inventory.py b/scripts/gc_store_site_inventory.py index 9946cab034..945820090c 100644 --- a/scripts/gc_store_site_inventory.py +++ b/scripts/gc_store_site_inventory.py @@ -318,6 +318,33 @@ def call_window(lines: list[str], index: int) -> str: return " ".join(line.strip() for line in lines[start:end]) +def window_head_len(lines: list[str], index: int) -> int: + """Length of the window's first line as `call_window` renders it.""" + + return len(lines[index].strip()) + + +def anchored_search( + pattern: re.Pattern[str], window: str, head_len: int +) -> re.Match[str] | None: + """Match `pattern` in `window` only when the match STARTS on the head line. + + `call_window` exists so a call whose arguments are split across lines is + still classified from its opening line (`CACHE.store(` … `);`). Searching + the whole window unanchored, however, re-reports that one store from each + of the ~6 lines ABOVE it: the `array/indexing.rs` finding was six lines + wide and listed `continue;`, a bare `}` and two `if` lines as "raw atomic + cache/global pointer store" (#7258). Requiring the match to begin inside + the head line keeps the split-call coverage and reports each site once, at + the line that actually contains the store. + """ + + match = pattern.search(window) + if match is None or match.start() >= head_len: + return None + return match + + def has_nearby_marker(lines: list[str], index: int) -> bool: start = max(0, index - 6) end = min(len(lines), index + 7) @@ -337,13 +364,14 @@ def is_risky_codegen_store(line: str) -> bool: def classify_rust_store(path: Path, lines: list[str], index: int) -> str | None: line = lines[index] window = call_window(lines, index) - atomic_store = RUST_ATOMIC_STORE_RE.search(window) + head_len = window_head_len(lines, index) + atomic_store = anchored_search(RUST_ATOMIC_STORE_RE, window, head_len) if atomic_store and is_risky_atomic_root_store( atomic_store.group("target"), atomic_store.group("value") ): return "raw atomic cache/global pointer store" - atomic_cas = RUST_ATOMIC_COMPARE_EXCHANGE_RE.search(window) + atomic_cas = anchored_search(RUST_ATOMIC_COMPARE_EXCHANGE_RE, window, head_len) if atomic_cas and is_risky_atomic_root_store( atomic_cas.group("target"), atomic_cas.group("value") ): @@ -446,13 +474,19 @@ def scan_file(path: Path) -> list[Finding]: def run_self_tests() -> int: failures: list[str] = [] - def check(rel_path: str, lines: list[str], expected: str | None) -> None: - reason = classify_rust_store(REPO_ROOT / rel_path, lines, 0) + def check_at( + rel_path: str, lines: list[str], index: int, expected: str | None + ) -> None: + reason = classify_rust_store(REPO_ROOT / rel_path, lines, index) + where = f"{rel_path}[{index}]" if expected is None: if reason is not None: - failures.append(f"{rel_path}: expected clean, got {reason!r}") + failures.append(f"{where}: expected clean, got {reason!r}") elif reason is None or expected not in reason: - failures.append(f"{rel_path}: expected {expected!r}, got {reason!r}") + failures.append(f"{where}: expected {expected!r}, got {reason!r}") + + def check(rel_path: str, lines: list[str], expected: str | None) -> None: + check_at(rel_path, lines, 0, expected) check( "crates/perry-runtime/src/array.rs", @@ -620,6 +654,47 @@ def check(rel_path: str, lines: list[str], expected: str | None) -> None: None, ) + # The call window must classify a SPLIT call from its opening line, but must + # NOT re-report that one store from the lines above it. Before #7258 the + # unanchored window search made `scan_prototype_addr_cache_roots_mut` a + # SIX-line finding whose first five lines (`continue;`, a bare `}`, two + # `if`s, a `let`) contain no store at all. + split_atomic = [ + "GLOBAL_CACHE.store(", + " addr,", + " Ordering::Relaxed,", + ");", + ] + check_at( + "crates/perry-runtime/src/object/mod.rs", + split_atomic, + 0, + "raw atomic cache/global pointer store", + ) + for above in range(1, len(split_atomic)): + check_at("crates/perry-runtime/src/object/mod.rs", split_atomic, above, None) + + proto_cache_scan = [ + "for cache in [&ARRAY_PROTO_ADDR, &OBJECT_PROTO_ADDR] {", + " let cached = cache.load(Ordering::Relaxed);", + " if cached == usize::MAX || cached == 0 {", + " continue;", + " }", + " let mut addr = cached;", + " if visitor.visit_usize_slot(&mut addr) {", + " cache.store(addr, Ordering::Relaxed);", + " }", + "}", + ] + store_index = 7 + for index in range(len(proto_cache_scan)): + check_at( + "crates/perry-runtime/src/array/indexing.rs", + proto_cache_scan, + index, + "raw atomic cache/global pointer store" if index == store_index else None, + ) + # store_aligned/store_volatile are store emitters too. if not is_risky_codegen_store('.store_aligned(I64, &val, &field_ptr, 8);'): failures.append("codegen: store_aligned with heap dest not flagged") From e8692139940867b2522046fd14bd263912c40b61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 2 Aug 2026 21:07:06 +0200 Subject: [PATCH 2/3] fix(lint): route the new handle-floor sites through addr_class (#7259) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three hand-rolled address floors landed on 2026-07-30 without going through `value::addr_class`, tripping the ratchet: child_process/value_util.rs `> 0x10000` / `<= 0x10000` (8fed172cf) fs/dirent.rs `< 0x1000` (7a4a86677) Both are real, not bookkeeping. `js_get_string_pointer_unified` forwards a POINTER_TAG payload verbatim, and `JSValue::as_pointer` does the same — those payloads carry registry handle ids as well as heap pointers. A `0x1000` / `0x10000` floor sits an order of magnitude below `HANDLE_BAND_MAX`, so a fetch (0x40000..0xE0000), zlib (0xE0000..0xF0000) or proxy (0xF0000..0x100000) id sailed through into a `StringHeader`/`ObjectHeader` dereference — the Linux-only fault class of #1843/#4004/#6271 that macOS's high allocation base hides. Converted every handle-floor site in both files (including the two grandfathered ones, same defect, same fix) to `is_handle_band` / `is_above_handle_band`, which also subsumes the redundant null checks that followed. `cp_raw_slot_is_heap_ptr` keeps an explicit `raw > 0` so a negative i64 slot is not turned into a huge "above the band" value by `as usize`. Separately, the `O_SYMLINK` allowlist entry was keyed on ONE of the two spellings in constants.rs, so `("O_SYMLINK", (0x200000) as f64),` failed the gate while `"O_SYMLINK" => Some(0x200000),` was suppressed. Re-keyed on the constant name; a self-test now asserts BOTH spellings are covered and fails against the old entry. Ratchet baseline regenerated: 566 -> 544 sites, 249 entries. Zero increases and zero new keys (verified by diffing the key/count maps) — purely the slack that had accumulated while lint was red. 3 of the 22 are this change; 11 are `perry-stdlib/src/http.rs`, whose sites moved into `crates/perry-ext-http/` when #6826 merged it into the HTTP extension. `SCAN_ROOTS` covers only perry-runtime and perry-stdlib, so those sites are no longer audited at all — tracked separately, not silently absorbed here. --- .../src/child_process/value_util.rs | 26 ++++++++++++++-- crates/perry-runtime/src/fs/dirent.rs | 24 +++++++-------- scripts/addr_class_allowlist.txt | 6 ++-- scripts/addr_class_inventory.py | 30 +++++++++++++++++++ scripts/addr_class_ratchet_baseline.txt | 11 ++----- 5 files changed, 71 insertions(+), 26 deletions(-) diff --git a/crates/perry-runtime/src/child_process/value_util.rs b/crates/perry-runtime/src/child_process/value_util.rs index 5e2b81d544..01081ce904 100644 --- a/crates/perry-runtime/src/child_process/value_util.rs +++ b/crates/perry-runtime/src/child_process/value_util.rs @@ -100,7 +100,13 @@ pub(crate) fn cp_box_string(s: &str) -> f64 { /// route through the unified accessor which materializes SSO bytes. pub(crate) fn cp_value_to_string(value: f64) -> Option { let ptr = crate::value::js_get_string_pointer_unified(value) as *const StringHeader; - if ptr.is_null() || (ptr as usize) < 0x1000 { + // #7259: `js_get_string_pointer_unified` forwards a POINTER_TAG payload + // verbatim, and those payloads carry registry handle ids as well as heap + // pointers. The old `< 0x1000` floor sat an order of magnitude below + // `HANDLE_BAND_MAX`, so a fetch/zlib/proxy id sailed through into the + // `(*ptr).byte_len` read below — the Linux-only segfault class of #1843 / + // #4004 / #6271, which macOS's high allocation base hides. + if !crate::value::addr_class::is_above_handle_band(ptr as usize) { return None; } unsafe { @@ -215,14 +221,28 @@ pub(crate) fn cp_args_from_value(value: f64) -> Vec { } } +/// True when a codegen-supplied raw argument slot holds a dereferenceable heap +/// pointer, rather than an absent argument or a small registry handle id. +/// +/// #7259: the previous `> 0x10000` / `<= 0x10000` floors sat an order of +/// magnitude below `HANDLE_BAND_MAX`, so a fetch (0x40000..0xE0000), zlib +/// (0xE0000..0xF0000) or proxy (0xF0000..0x100000) id in the slot was boxed as +/// a pointer and then dereferenced by `cp_array_ptr` / `cp_object_ptr`. The +/// explicit `raw > 0` keeps the old signed comparison's rejection of negative +/// slots, which `as usize` alone would turn into a huge "above the band" value. +#[inline] +fn cp_raw_slot_is_heap_ptr(raw: i64) -> bool { + raw > 0 && crate::value::addr_class::is_above_handle_band(raw as usize) +} + /// Normalize `spawn*`/`fork`'s optional `(args, options)` slots after codegen /// has unboxed them: when the third argument is absent, a plain object in the /// second slot is the options object, not an argv list. pub(crate) fn cp_options_from_raw_args(args_ptr: i64, opts_ptr: i64) -> f64 { - if opts_ptr > 0x10000 { + if cp_raw_slot_is_heap_ptr(opts_ptr) { return cp_box_ptr(opts_ptr as *const u8); } - if args_ptr <= 0x10000 { + if !cp_raw_slot_is_heap_ptr(args_ptr) { return cp_undefined(); } let args = cp_box_ptr(args_ptr as *const u8); diff --git a/crates/perry-runtime/src/fs/dirent.rs b/crates/perry-runtime/src/fs/dirent.rs index c3605ec9ed..95c4d1e929 100644 --- a/crates/perry-runtime/src/fs/dirent.rs +++ b/crates/perry-runtime/src/fs/dirent.rs @@ -129,13 +129,15 @@ pub(crate) unsafe fn options_with_file_types(options_value: f64) -> bool { } else { return false; }; - if raw_ptr < 0x1000 { + // #7259: a POINTER_TAG payload can be a registry handle id rather than a + // heap address, and `< 0x1000` sits an order of magnitude below + // `HANDLE_BAND_MAX` — fetch/zlib/proxy ids passed it and were dereferenced + // as an ObjectHeader (the Linux-only fault class of #1843/#4004/#6271). + // `is_handle_band` also subsumes the null check that used to follow. + if crate::value::addr_class::is_handle_band(raw_ptr) { return false; } let obj_ptr = raw_ptr as *const crate::object::ObjectHeader; - if obj_ptr.is_null() { - return false; - } let key = crate::string::js_string_from_bytes(b"withFileTypes".as_ptr(), 13); let val = crate::object::js_object_get_field_by_name(obj_ptr, key); crate::value::js_is_truthy(f64::from_bits(val.bits())) != 0 @@ -181,13 +183,13 @@ pub(crate) unsafe fn options_field_value( } else { return None; }; - if raw_ptr < 0x1000 { + // #7259: see `options_with_file_types` — a POINTER_TAG payload can be a + // registry handle id, and `is_handle_band` (not `< 0x1000`) is the floor + // that rejects the fetch/zlib/proxy bands. It subsumes the null check too. + if crate::value::addr_class::is_handle_band(raw_ptr) { return None; } let obj_ptr = raw_ptr as *const crate::object::ObjectHeader; - if obj_ptr.is_null() { - return None; - } let keys = (*obj_ptr).keys_array; if !keys.is_null() { let key_count = crate::array::js_array_length(keys) as usize; @@ -214,13 +216,11 @@ pub(crate) unsafe fn options_field_value( } else { return None; }; - if refreshed_ptr < 0x1000 { + // #7259: same handle-band floor after the GC-safe re-read of the handle. + if crate::value::addr_class::is_handle_band(refreshed_ptr) { return None; } let refreshed_obj_ptr = refreshed_ptr as *const crate::object::ObjectHeader; - if refreshed_obj_ptr.is_null() { - return None; - } let val = crate::object::js_object_get_field_by_name(refreshed_obj_ptr, key); if val.bits() == crate::value::TAG_UNDEFINED { None diff --git a/scripts/addr_class_allowlist.txt b/scripts/addr_class_allowlist.txt index 6619b6dd5b..16d375e44f 100644 --- a/scripts/addr_class_allowlist.txt +++ b/scripts/addr_class_allowlist.txt @@ -15,7 +15,9 @@ # # Band-literal exceptions (deliberate, not band classification): crates/perry-runtime/src/closure/dynamic_props.rs | for handle in [0x10000usize, | #4740 regression-test fixture: raw in-band addresses asserted NOT to be dereferenced -crates/perry-runtime/src/object/native_module.rs | "O_SYMLINK" => Some(0x200000), | fs.constants O_SYMLINK flag value; unrelated to the handle bands +# (The pre-split `object/native_module.rs` O_SYMLINK entry was dropped in +# #7259: that file now holds zero band literals -- the constant table moved to +# native_module/constants.rs, which carries its own entry further down.) # # Grandfathered GcHeader-cast files: crates/perry-runtime/src/arena/allocators.rs | * | arena allocator/walker internals: header addresses come from block iteration or fresh allocation, never from NaN-box payloads @@ -142,7 +144,7 @@ crates/perry-runtime/src/object/collection_proto_thunks.rs | * | GcHeader obj_ty crates/perry-runtime/src/symbol/ | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of symbol.rs) crates/perry-runtime/src/typedarray/ | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of typedarray/mod.rs) crates/perry-runtime/src/process/ | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of process.rs) -crates/perry-runtime/src/object/native_module/constants.rs | "O_SYMLINK" => Some(0x200000), | fs.constants O_SYMLINK flag value; unrelated to the handle bands (split of native_module.rs) +crates/perry-runtime/src/object/native_module/constants.rs | O_SYMLINK | fs.constants O_SYMLINK flag value (POSIX fcntl bit 0x200000); unrelated to the handle bands. Keyed on the CONSTANT NAME rather than one spelling: constants.rs writes the same flag two ways -- `("O_SYMLINK", (0x200000) as f64),` in the table and `"O_SYMLINK" => Some(0x200000),` in the lookup -- and the old `=> Some(...)` substring suppressed only the second (#7259). Any OTHER band literal in this file still fails the gate. crates/perry-runtime/src/child_process/value_util.rs | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of child_process/mod.rs) crates/perry-runtime/src/closure/dispatch/ | * | pre-existing GcHeader probe predating addr_class; address validated by call-site guards (magnitude/registry/is_valid_obj_ptr) -- migrate to addr_class::try_read_gc_header in a follow-up (split of closure/dispatch.rs) crates/perry-runtime/src/bun_compat/string_width.rs | 0xE0000..=0xE007F | Unicode "Tags" codepoint block (U+E0000..U+E007F) tested against a char, not a handle-band address diff --git a/scripts/addr_class_inventory.py b/scripts/addr_class_inventory.py index 0bb1f10931..101cef9323 100644 --- a/scripts/addr_class_inventory.py +++ b/scripts/addr_class_inventory.py @@ -449,6 +449,36 @@ def expect(cond: bool, message: str) -> None: "gc/ must be exempt", ) + # A hand-rolled floor rewritten through the addr_class predicates must + # clear the rule — otherwise the gate would block its own fix (#7259). + for fixed in ( + "if crate::value::addr_class::is_handle_band(raw_ptr) {\n", + "raw > 0 && crate::value::addr_class::is_above_handle_band(raw as usize)\n", + ): + expect( + not any(f.rule == "handle-floor" for f in scan_text(runtime, fixed)), + f"handle-floor must not fire on the addr_class fix shape: {fixed.strip()}", + ) + + # fs.constants spells O_SYMLINK two ways; an allowlist entry keyed on one + # spelling silently stops covering the other (#7259). Both forms are the + # POSIX fcntl flag, and both must be suppressed by the single entry. + constants = "crates/perry-runtime/src/object/native_module/constants.rs" + o_symlink_entries = [ + e for e in load_allowlist(DEFAULT_ALLOWLIST) if e.path_prefix == constants + ] + for spelling in ( + ' ("O_SYMLINK", (0x200000) as f64),\n', + ' "O_SYMLINK" => Some(0x200000),\n', + ): + hits = scan_text(constants, spelling) + expect( + bool(hits) and all( + any(e.matches(h) for e in o_symlink_entries) for h in hits + ), + f"O_SYMLINK allowlist must cover the spelling: {spelling.strip()}", + ) + # Allowlist matching: prefix + substring, prefix + wildcard. finding = Finding(runtime, 1, "gcheader-cast", "x as *const GcHeader") expect( diff --git a/scripts/addr_class_ratchet_baseline.txt b/scripts/addr_class_ratchet_baseline.txt index 4b0750f2a0..12ff1ecfa8 100644 --- a/scripts/addr_class_ratchet_baseline.txt +++ b/scripts/addr_class_ratchet_baseline.txt @@ -57,11 +57,8 @@ handle-floor | crates/perry-runtime/src/builtins/formatting/typed_array_equality handle-floor | crates/perry-runtime/src/builtins/globals.rs | 9 handle-floor | crates/perry-runtime/src/builtins/numbers.rs | 2 handle-floor | crates/perry-runtime/src/builtins/table.rs | 1 -handle-floor | crates/perry-runtime/src/child_process/fork.rs | 1 -handle-floor | crates/perry-runtime/src/child_process/reactor.rs | 1 handle-floor | crates/perry-runtime/src/child_process/registry.rs | 1 handle-floor | crates/perry-runtime/src/child_process/v8_serde.rs | 1 -handle-floor | crates/perry-runtime/src/child_process/value_util.rs | 1 handle-floor | crates/perry-runtime/src/closure/dispatch/validate.rs | 1 handle-floor | crates/perry-runtime/src/cluster.rs | 1 handle-floor | crates/perry-runtime/src/collection_iter_object.rs | 1 @@ -69,7 +66,6 @@ handle-floor | crates/perry-runtime/src/date.rs | 3 handle-floor | crates/perry-runtime/src/dgram.rs | 1 handle-floor | crates/perry-runtime/src/dns.rs | 4 handle-floor | crates/perry-runtime/src/exception.rs | 2 -handle-floor | crates/perry-runtime/src/fs/dirent.rs | 2 handle-floor | crates/perry-runtime/src/fs/filehandle.rs | 4 handle-floor | crates/perry-runtime/src/fs/mod.rs | 2 handle-floor | crates/perry-runtime/src/fs/stream.rs | 1 @@ -90,13 +86,12 @@ handle-floor | crates/perry-runtime/src/node_stream.rs | 1 handle-floor | crates/perry-runtime/src/node_stream_constructors/introspection.rs | 1 handle-floor | crates/perry-runtime/src/node_stream_event_emitter.rs | 1 handle-floor | crates/perry-runtime/src/node_stream_json.rs | 1 -handle-floor | crates/perry-runtime/src/node_stream_readable_read.rs | 1 handle-floor | crates/perry-runtime/src/node_stream_readwrite.rs | 2 handle-floor | crates/perry-runtime/src/node_submodules/blob.rs | 1 handle-floor | crates/perry-runtime/src/node_submodules/consumers.rs | 1 handle-floor | crates/perry-runtime/src/node_submodules/diagnostics.rs | 1 handle-floor | crates/perry-runtime/src/node_submodules/fs_promises.rs | 1 -handle-floor | crates/perry-runtime/src/node_submodules/test.rs | 2 +handle-floor | crates/perry-runtime/src/node_submodules/test.rs | 1 handle-floor | crates/perry-runtime/src/node_submodules/timers.rs | 1 handle-floor | crates/perry-runtime/src/node_submodules/trace_events.rs | 1 handle-floor | crates/perry-runtime/src/node_submodules/zlib.rs | 3 @@ -155,7 +150,7 @@ handle-floor | crates/perry-runtime/src/os/signal.rs | 1 handle-floor | crates/perry-runtime/src/path.rs | 1 handle-floor | crates/perry-runtime/src/perf_hooks.rs | 2 handle-floor | crates/perry-runtime/src/process.rs | 2 -handle-floor | crates/perry-runtime/src/process/env_misc.rs | 3 +handle-floor | crates/perry-runtime/src/process/env_misc.rs | 1 handle-floor | crates/perry-runtime/src/process/node_module.rs | 2 handle-floor | crates/perry-runtime/src/promise/combinators.rs | 1 handle-floor | crates/perry-runtime/src/proxy.rs | 5 @@ -194,7 +189,6 @@ handle-floor | crates/perry-runtime/src/value/dynamic_object.rs | 2 handle-floor | crates/perry-runtime/src/value/equality.rs | 2 handle-floor | crates/perry-runtime/src/value/nanbox.rs | 1 handle-floor | crates/perry-runtime/src/value/to_string.rs | 3 -handle-floor | crates/perry-runtime/src/wasi.rs | 2 handle-floor | crates/perry-runtime/src/weakref.rs | 2 handle-floor | crates/perry-stdlib/src/axios.rs | 1 handle-floor | crates/perry-stdlib/src/container/mod.rs | 1 @@ -209,7 +203,6 @@ handle-floor | crates/perry-stdlib/src/events.rs | 1 handle-floor | crates/perry-stdlib/src/exponential_backoff.rs | 1 handle-floor | crates/perry-stdlib/src/fetch/dispatch.rs | 4 handle-floor | crates/perry-stdlib/src/fetch/mod.rs | 1 -handle-floor | crates/perry-stdlib/src/http.rs | 11 handle-floor | crates/perry-stdlib/src/jsonwebtoken.rs | 1 handle-floor | crates/perry-stdlib/src/querystring.rs | 5 handle-floor | crates/perry-stdlib/src/readline/mod.rs | 1 From 5c0b30f1c254efc70612cc2bde19122e140820a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 2 Aug 2026 21:14:08 +0200 Subject: [PATCH 3/3] docs: changelog fragment for #7273 --- ...7273-lint-gc-store-and-addr-class-gates.md | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 changelog.d/7273-lint-gc-store-and-addr-class-gates.md diff --git a/changelog.d/7273-lint-gc-store-and-addr-class-gates.md b/changelog.d/7273-lint-gc-store-and-addr-class-gates.md new file mode 100644 index 0000000000..6d560f0926 --- /dev/null +++ b/changelog.d/7273-lint-gc-store-and-addr-class-gates.md @@ -0,0 +1,79 @@ +**Cleared the two `lint` steps that had been failing on `main` for days — +`gc_store_site_inventory.py` (19 unaudited GC store sites) and +`addr_class_inventory.py` (2 ratchet regressions plus a stale allowlist +substring).** Both were invisible in CI: the `lint` job stops at its first +failing step and step 9 fails ahead of them, so neither had ever executed. + +Of the 19 GC store-site findings, **5 were phantom**. `classify_rust_store` +searched the atomic-store regexes against `call_window` — a 6-line *forward* +window — without anchoring, so a single `cache.store(...)` was re-reported from +each of the five lines above it; `continue;` and a bare `}` were both listed as +"raw atomic cache/global pointer store". `anchored_search` now requires the +match to begin inside the head line, preserving the split-call coverage the +window exists for (`CACHE.store(` … `);`) while reporting each site once. + +Three were genuinely new runtime sites and got a class each: the `TAG_HOLE` +capacity fills in `array/alloc.rs` and `array/push_pop.rs` (#7138) are `INIT` — +freshly allocated, never-published storage written with a non-pointer sentinel, +so there is no old value to remember and no edge to record; the `cache.store` in +`array/indexing.rs`'s `scan_prototype_addr_cache_roots_mut` (#7071) is `ROOT` — +it *is* the collector rewriting a registered side-table slot with the mutator +stopped, and `visit_usize_slot` returns true only when it relocated the object. + +The remaining 11 were codegen stores. Four already carried a +`GC_STORE_AUDIT(POINTER_FREE)` marker that #6915's `value_is_canonical_raw_f64` +branch split had pushed 9–10 lines below it, outside the ±6-line window. The +other seven had prose rationale but not the canonical marker form: the +`instance_misc1.rs` / `property_set.rs` field stores are reached only after +`emit_plain_finite_number_check` proves the value's exponent is *not* all-ones +(every NaN-box tag shares an all-ones exponent, so the value is a genuine +unboxed double), and the five `proven_view_access.rs` arms write the typed +array's backing store, whose elements are raw numeric bytes — the codegen-side +counterpart of the `is_pointer_free_module` carve-out the script already makes +for `typedarray`/`typedarray_view`/`buffer`. That half of the change is +comments only; no generated code and no runtime behaviour moved. + +The address-classification failures were **real defects, not bookkeeping**. +Three hand-rolled address floors landed on 2026-07-30 without going through +`value::addr_class`: `> 0x10000` / `<= 0x10000` in +`child_process/value_util.rs` and `< 0x1000` in `fs/dirent.rs`. +`js_get_string_pointer_unified` forwards a `POINTER_TAG` payload verbatim and +`JSValue::as_pointer` does the same, and those payloads carry registry handle +ids as well as heap pointers — so a fetch (`0x40000..0xE0000`), zlib +(`0xE0000..0xF0000`) or proxy (`0xF0000..0x100000`) id passed a floor an order +of magnitude too low and was dereferenced as a `StringHeader`/`ObjectHeader`. +That is the Linux-only fault class of #1843 / #4004 / #6271, which macOS's high +allocation base hides. Every handle-floor site in both files was converted to +`is_handle_band` / `is_above_handle_band` — including the two grandfathered +ones, same defect and same one-line fix — so both baseline entries drop to zero +instead of being re-pinned lower. `is_handle_band` also subsumes the redundant +null checks that followed, and `cp_raw_slot_is_heap_ptr` keeps an explicit +`raw > 0` so a negative `i64` slot is not turned into a huge "above the band" +value by `as usize`. + +Separately, the `O_SYMLINK` allowlist entry was keyed on one of the two +spellings in `native_module/constants.rs`, so `("O_SYMLINK", (0x200000) as +f64),` failed the gate while `"O_SYMLINK" => Some(0x200000),` was suppressed. +Re-keyed on the constant name; it remains path-prefixed to that file, so any +other band literal there still fails. The pre-split `object/native_module.rs` +entry was dropped — that file now holds zero band literals. + +The ratchet baseline was regenerated: 566 → 544 sites across 256 → 249 entries, +verified to contain **zero increases and zero new keys** by diffing the +`(rule, path) → count` maps, so it is purely the slack that accumulated while +`lint` was red. Only 3 of the 22 come from this change. + +Both new regression tests were confirmed to fail *without* the fix rather than +merely pass with it: the `proto_cache_scan` case fails on the pre-anchoring +logic with exactly the five phantom lines, and the `O_SYMLINK` case fails +against the pre-fix allowlist with exactly the uncovered spelling. + +One finding was deliberately left open. `addr_class_inventory.py`'s +`SCAN_ROOTS` covers only `perry-runtime` and `perry-stdlib`, while its sibling +`gc_store_site_inventory.py` also globs `crates/perry-ext-*`. When #6826 moved +`perry-stdlib/src/http.rs` into the HTTP extension its 11 `handle-floor` sites +moved out of the gate's field of view rather than being fixed — 18 ext-crate +sites are unaudited, 8 of them `band-literal`, which is allowlist-governed and +would hard-fail until each is individually justified. Filed as #7272 rather +than silently absorbed. This change also does not by itself turn `lint` green: +step 9 (#7257) still fails ahead of these two.