Skip to content

lint: gc_store_site_inventory has 19 unaudited store sites on main #7258

Description

@proggeramlug

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions