You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
lint: gc_store_site_inventory has 19 unaudited store sites on main #7258
Step 6 of the lint job, scripts/gc_store_site_inventory.py, exits 1 on main (cdb3934) with 19 findings. It is currently masked: the job stops at step 3 (#7257), so this has not been visible in CI.
Confirmed unrelated to #7256 (the file-size split) — that PR's finding set is byte-identical to main's.
Current findings
GC store-site inventory failed; add nearby GC_STORE_AUDIT markers:
crates/perry-codegen/src/expr/index.rs:340: raw generated heap/global store: blk.store(DOUBLE, &numeric_value, &element_ptr);
crates/perry-codegen/src/expr/index.rs:488: raw generated heap/global store: blk.store(DOUBLE, &numeric_value, &element_ptr);
crates/perry-codegen/src/expr/index.rs:523: raw generated heap/global store: blk.store(DOUBLE, &numeric_value, &element_ptr);
crates/perry-codegen/src/expr/index_set.rs:1214: raw generated heap/global store: blk.store(DOUBLE, &numeric_value, &element_ptr);
crates/perry-codegen/src/expr/instance_misc1.rs:1501: raw generated heap/global store: blk.store(DOUBLE, &new, &field_ptr);
crates/perry-codegen/src/expr/property_set.rs:599: raw generated heap/global store: blk.store(DOUBLE, &val_double, &field_ptr);
crates/perry-codegen/src/expr/proven_view_access.rs:394: raw generated heap/global store: blk.store(I8, &byte, &elem_ptr);
crates/perry-codegen/src/expr/proven_view_access.rs:398: raw generated heap/global store: blk.store(I16, &half, &elem_ptr);
crates/perry-codegen/src/expr/proven_view_access.rs:401: raw generated heap/global store: blk.store(I32, &value_native.value, &elem_ptr);
crates/perry-codegen/src/expr/proven_view_access.rs:405: raw generated heap/global store: blk.store(F32, &narrow, &elem_ptr);
crates/perry-codegen/src/expr/proven_view_access.rs:408: raw generated heap/global store: blk.store(DOUBLE, &value_native.value, &elem_ptr);
crates/perry-runtime/src/array/alloc.rs:55: raw slot write: std::ptr::write(elements_ptr.add(i), crate::value::TAG_HOLE);
crates/perry-runtime/src/array/indexing.rs:106: raw atomic cache/global pointer store: if cached == usize::MAX || cached == 0 {
crates/perry-runtime/src/array/indexing.rs:107: raw atomic cache/global pointer store: continue;
crates/perry-runtime/src/array/indexing.rs:108: raw atomic cache/global pointer store: }
crates/perry-runtime/src/array/indexing.rs:109: raw atomic cache/global pointer store: let mut addr = cached;
crates/perry-runtime/src/array/indexing.rs:110: raw atomic cache/global pointer store: if visitor.visit_usize_slot(&mut addr) {
crates/perry-runtime/src/array/indexing.rs:111: raw atomic cache/global pointer store: cache.store(addr, Ordering::Relaxed);
crates/perry-runtime/src/array/push_pop.rs:101: raw slot write: ptr::write(new_elems.add(i), crate::value::TAG_HOLE);
Accepted marker form: // GC_STORE_AUDIT(BARRIERED): reason, with class one of BARRIERED, EXTERNAL_BARRIERED, INIT, POINTER_FREE, ROOT, STACK
Or add a justified entry to scripts/gc_store_site_allowlist.txt.
Notes for whoever picks this up
Each site needs a per-site audit-class judgement (BARRIERED, EXTERNAL_BARRIERED, INIT, POINTER_FREE, ROOT, STACK) rather than a blanket allowlist — mislabelling one is worse than leaving the gate red, which is why this was not folded into #7256.
Two specific observations:
crates/perry-runtime/src/array/indexing.rs:106-111 is scan_prototype_addr_cache_roots_mut, added by fix(gc): resolve the memoized Array.prototype address across relocation (#6981) #7071 ("resolve the memoized Array.prototype address across relocation"). The flagged cache.store(addr, Ordering::Relaxed)is the GC's own rewrite of a side-table slot, so GC_STORE_AUDIT(ROOT) looks right — but note the script reports a 6-line window including continue; and }, which are not stores. The multi-line reporting makes the finding list look larger and less precise than it is; worth a look at the rule's window logic.
crates/perry-runtime/src/array/alloc.rs:55 and push_pop.rs:101 are ptr::write(..., TAG_HOLE) — writing a non-pointer sentinel into fresh storage, i.e. INIT / POINTER_FREE.
The perry-codegen sites are generated stores (blk.store(DOUBLE, &numeric_value, &element_ptr)), where the question is whether the emitted code path is barriered — a codegen question, not a runtime one.
scripts/gc_store_site_allowlist.txt already has a whole-module policy entry for crates/perry-runtime/src/gc/; the accepted inline form is // GC_STORE_AUDIT(<CLASS>): reason next to the store.
All 19 findings reproduce byte-identically to the list in this issue — the 11 perry-codegen generated-store sites (expr/index.rs ×3, index_set.rs, instance_misc1.rs, property_set.rs, proven_view_access.rs ×5), array/alloc.rs:55, the 6-line array/indexing.rs:106-111 window, and array/push_pop.rs:101.
The observation about the reporting window still holds and is worth acting on alongside the audit: array/indexing.rs:106-111 reports six lines including continue; and }, which are not stores. That makes the finding list read as 19 when the number of actual store sites is smaller, and it makes the gate's output less usable for exactly the per-site judgement it is asking for.
Step 6 of the
lintjob,scripts/gc_store_site_inventory.py, exits 1 onmain(cdb3934) with 19 findings. It is currently masked: the job stops at step 3 (#7257), so this has not been visible in CI.Confirmed unrelated to #7256 (the file-size split) — that PR's finding set is byte-identical to
main's.Current findings
Notes for whoever picks this up
Each site needs a per-site audit-class judgement (
BARRIERED,EXTERNAL_BARRIERED,INIT,POINTER_FREE,ROOT,STACK) rather than a blanket allowlist — mislabelling one is worse than leaving the gate red, which is why this was not folded into #7256.Two specific observations:
crates/perry-runtime/src/array/indexing.rs:106-111isscan_prototype_addr_cache_roots_mut, added by fix(gc): resolve the memoized Array.prototype address across relocation (#6981) #7071 ("resolve the memoizedArray.prototypeaddress across relocation"). The flaggedcache.store(addr, Ordering::Relaxed)is the GC's own rewrite of a side-table slot, soGC_STORE_AUDIT(ROOT)looks right — but note the script reports a 6-line window includingcontinue;and}, which are not stores. The multi-line reporting makes the finding list look larger and less precise than it is; worth a look at the rule's window logic.crates/perry-runtime/src/array/alloc.rs:55andpush_pop.rs:101areptr::write(..., TAG_HOLE)— writing a non-pointer sentinel into fresh storage, i.e.INIT/POINTER_FREE.perry-codegensites are generated stores (blk.store(DOUBLE, &numeric_value, &element_ptr)), where the question is whether the emitted code path is barriered — a codegen question, not a runtime one.scripts/gc_store_site_allowlist.txtalready has a whole-module policy entry forcrates/perry-runtime/src/gc/; the accepted inline form is// GC_STORE_AUDIT(<CLASS>): reasonnext to the store.