fix(gc): convert gc/census.rs to hot TLS and classify its holders (#9740)

changelog.d/9740-census-tls-and-window-verdict.md

@@ -0,0 +1,28 @@
+**Convert `gc/census.rs` to hot TLS, and give the root-holder inventory a
+verdict that fits its pass-1 snapshot.** `census.rs` was the last file keeping
+`tls-budget` / `self-test-checkers` red on `main`: it declared two shipping
+`thread_local!` blocks that `check_thread_locals.py` rejects, and every open PR
+inherited the red X.
+
+Converting them made `gc_runtime_root_holders.py` see four declarations for the
+first time — it enumerates `perry_thread_local!`, so a raw block escapes both
+gates, which is backwards: the declarations that skipped the convention are the
+ones whose GC contract nobody audited. `ARMED`, `SEQ` and `LABEL` are a flag, a
+counter and a `&'static str`, classified `not_a_gc_pointer`; the file's
+`#[cfg(test)]` block is folded into the macro too (`test_only`), so no
+declaration in `census.rs` is left outside the inventory.
+
+`PASS1_MARKED` is the one #9740 was filed for. It holds real GC header
+addresses, so `not_a_gc_pointer` — defined as an id, a counter, a code address,
+.rodata or Rust-owned state — would have been a false statement about it, and
+`covered_elsewhere` / `open_gap` / `unverified` fit no better. It is correct
+because it is *untraced*: written at the end of mark propagation and consumed at
+sweep entry of the same synchronous full cycle, under call-site guards that are
+the identical predicate, in a window with no evacuation and no mutator resume,
+and used only as `binary_search` keys. The new verdict
+`untraced_in_nonmoving_window` says exactly that, and — because the window is
+the whole safety argument — an entry must name the two functions that bound it
+in `window_opens` / `window_closes`. The gate checks against the holder's own
+source that both still exist and both still name the holder, so renaming a
+boundary, deleting one, or moving the write or the take out of it fails the
+gate. `--self-test` drives all three rejections.

crates/perry-runtime/src/gc/census.rs

@@ -43,17 +43,17 @@ static SIGNAL_PENDING: AtomicBool = AtomicBool::new(false);
 static SIGNAL_INSTALLED: AtomicBool = AtomicBool::new(false);
 static MAIN_THREAD: OnceLock<std::thread::ThreadId> = OnceLock::new();
 
-thread_local! {
+crate::perry_thread_local! {
     static ARMED: Cell<bool> = const { Cell::new(false) };
     static SEQ: Cell<u32> = const { Cell::new(0) };
     static LABEL: RefCell<&'static str> = const { RefCell::new("manual") };
 }
 
-#[cfg(test)]
-thread_local! {
+crate::perry_thread_local! {
     /// Test-only per-thread override of the output path, so a unit test can
     /// enable the census without touching the process env (the env read is a
     /// process-wide OnceLock that any earlier collection would latch).
+    #[cfg(test)]
     static TEST_PATH_OVERRIDE: RefCell<Option<&'static str>> = const { RefCell::new(None) };
 }
 
@@ -177,9 +177,29 @@ fn census_service_signal() {
     super::js_gc_collect();
 }
 
-thread_local! {
+crate::perry_thread_local! {
     /// Pass-1 snapshot: sorted header addresses that were marked when mark
     /// propagation finished (see the module docs).
+    ///
+    /// These are real GC header addresses and they are deliberately NOT traced:
+    /// the marked set is what the census observes, so tracing it would make the
+    /// observer a participant in the reachability it reports. That is sound
+    /// only inside one window — written by `census_pass1_if_armed` at the end
+    /// of mark propagation, `take()`n by
+    /// `census_take_if_armed_at_full_sweep_start` at sweep entry of the SAME
+    /// synchronous full cycle. Both call sites in `gc/cycle.rs` are guarded by
+    /// the identical predicate (`self.minor.is_none() && !is_budgeted()`), so
+    /// pass 1 running implies pass 2 consuming it; not-minor means no
+    /// evacuation in that cycle and not-budgeted means no mutator window, so
+    /// nothing moves and nothing is freed while the vector is alive. The
+    /// addresses are only ever `binary_search` keys — compared, never
+    /// dereferenced.
+    ///
+    /// KEEP THE WINDOW SHUT. If the write or the take ever leaves those two
+    /// functions the addresses can go stale, and the census silently
+    /// misclassifies live objects. `scripts/gc_runtime_root_holders.json`
+    /// records this as `untraced_in_nonmoving_window` and pins both boundary
+    /// names, so widening the window turns that gate red (#9740).
     static PASS1_MARKED: RefCell<Option<Vec<usize>>> = const { RefCell::new(None) };
 }
 

scripts/gc_runtime_root_holders.json

@@ -16,6 +16,16 @@
     "  not_a_gc_pointer   - the stored value is an id, a counter, an epoch, a code address, a",
     "                       .rodata object, or Rust-owned state. Nothing for the collector.",
     "  test_only          - #[cfg(test)] storage; never live in a shipped binary.",
+    "  untraced_in_nonmoving_window",
+    "                     - the stored value IS a GC heap address and is correct BECAUSE it is",
+    "                       untraced: written and consumed inside ONE window in which nothing",
+    "                       moves, nothing is freed and the mutator never runs, and used as a",
+    "                       key rather than dereferenced. `window_opens` / `window_closes` name",
+    "                       the two functions that bound it; the gate checks that both still",
+    "                       exist in the holder's file and both still name the holder, so",
+    "                       widening the window fails. Do NOT reach for this to avoid arguing",
+    "                       a holder is not a pointer \u2014 if the addresses outlive a collection,",
+    "                       the honest verdict is open_gap.",
     "  open_gap           - a real unrooted GC pointer. `issue` says where it is tracked; this",
     "                       verdict FAILS while old-page relocation ships enabled.",
     "  unverified         - enumerated, verdict NOT established. This also FAILS: an unknown",
@@ -237,6 +247,38 @@
       "verdict": "not_a_gc_pointer",
       "why": "Monotonic id counter for READ_LINES_REGISTRY keys. The registry's own heap values are visited by scan_filehandle_roots_mut (fs/filehandle.rs:65), reached from scan_fs_handle_roots_mut."
     },
+    {
+      "file": "crates/perry-runtime/src/gc/census.rs",
+      "name": "ARMED",
+      "verdict": "not_a_gc_pointer",
+      "why": "#9740: `Cell<bool>` saying whether a PERRY_GC_CENSUS census is armed on this thread. Set by census_arm, cleared by census_take_if_armed_at_full_sweep_start; false unless PERRY_GC_CENSUS is set. A flag, never an address."
+    },
+    {
+      "file": "crates/perry-runtime/src/gc/census.rs",
+      "name": "SEQ",
+      "verdict": "not_a_gc_pointer",
+      "why": "#9740: `Cell<u32>` monotonic census sequence number, incremented once per emitted census and written to the JSON line as `seq`. A counter, exactly what this verdict names."
+    },
+    {
+      "file": "crates/perry-runtime/src/gc/census.rs",
+      "name": "LABEL",
+      "verdict": "not_a_gc_pointer",
+      "why": "#9740: `RefCell<&'static str>` holding what triggered the armed census \u2014 \"manual\" (explicit gc()), \"signal\" (SIGUSR2), or a test literal. `&'static str` from string literals is .rodata, which this verdict names; the census never constructs one from the JS heap."
+    },
+    {
+      "file": "crates/perry-runtime/src/gc/census.rs",
+      "name": "PASS1_MARKED",
+      "verdict": "untraced_in_nonmoving_window",
+      "window_opens": "census_pass1_if_armed",
+      "window_closes": "census_take_if_armed_at_full_sweep_start",
+      "why": "#9740: sorted GC header addresses of the objects marked when mark propagation finished. These ARE heap addresses, so `not_a_gc_pointer` (an id, a counter, a code address, .rodata or Rust-owned state) would be a false statement about them; they are correct because they are untraced, not because they are not pointers. Written at the end of mark propagation and `take()`n at sweep entry of the SAME cycle \u2014 both call sites are guarded by the identical predicate `self.minor.is_none() && !self.progress_kind.is_budgeted()` (gc/cycle.rs:1182 and :1670, where `full_trace = self.minor.is_none()`), so pass 1 running implies pass 2 consuming it. Not minor means no evacuation and no rewrite_forwarded_references in that cycle; not budgeted means run_to_completion drives every phase on an unbounded budget, so no mutator window exists between the two. Nothing moves, nothing is freed, and the vector is used only as `binary_search` keys at census.rs:389 \u2014 compared, never dereferenced. Deliberately untraced: the marked set is the census's observation, and tracing it would make the observer a participant in the reachability it reports. Distinct from map.rs's MAP_COMPACTION_LOG, which is also address-keyed but LONG-LIVED and stays valid across moves via map_header_moved_for_gc; this holder has no such maintenance and needs none, because it cannot survive a move. The window is the whole safety argument, so the window is what is pinned."
+    },
+    {
+      "file": "crates/perry-runtime/src/gc/census.rs",
+      "name": "TEST_PATH_OVERRIDE",
+      "verdict": "test_only",
+      "why": "#9740: `#[cfg(test)]` per-thread override of the PERRY_GC_CENSUS output path, so a unit test can enable a census without latching the process-wide OnceLock that reads the env var. Holds `Option<&'static str>` \u2014 a leaked path string \u2014 and is absent from shipped binaries."
+    },
     {
       "file": "crates/perry-runtime/src/gc/trace.rs",
       "name": "FORWARDED_STUB_MEMBERSHIP_RECOVERIES",

scripts/gc_runtime_root_holders.py

@@ -83,6 +83,16 @@
    (a stale exemption is how these gates rot — same rule as
    `scripts/gc_root_dominance_allowlist.json`).
 
+   One verdict carries a machine-checked side condition rather than only prose.
+   `untraced_in_nonmoving_window` (#9740) covers the holder whose stored value
+   really is a GC heap address and is correct BECAUSE it is untraced — written
+   and consumed inside one window in which nothing moves, nothing is freed and
+   the mutator never runs, and used as a key rather than dereferenced. Such an
+   entry must name the window with `window_opens` / `window_closes`, and both
+   functions must still exist in the holder's own file and still name the
+   holder. The safety argument is entirely "the window is shut", so the window
+   is what the gate pins; see `VERDICTS` for why no older verdict fit.
+
 The identity-pinned frontier
 ----------------------------
 
@@ -125,6 +135,9 @@
 * an inventory entry that no longer matches a declaration -> exit 1
 * an `open_gap` or `unverified` verdict -> exit 1; old-page relocation ships
   enabled, so a known or unevaluated movable-address holder cannot be exempted
+* an `untraced_in_nonmoving_window` verdict whose named window no longer bounds
+  the holder — a renamed or deleted boundary, or a boundary that no longer
+  touches the holder -> exit 1
 * a frontier/rule-T holder not in the pinned `frontier` list -> exit 1 (ratchet up)
 * a `frontier` entry matching no holder -> exit 1 (ratchet down / stale)
 * fewer than MIN_HOLDERS declarations matched -> exit 2, because a regex that
@@ -1037,10 +1050,35 @@ def reachable_text(call_pattern: re.Pattern) -> dict[Path, str]:
     "covered_elsewhere",  # a registered scanner in ANOTHER file visits it
     "not_a_gc_pointer",  # id, counter, epoch, code address, .rodata, Rust-owned
     "test_only",  # #[cfg(test)] storage
+    "untraced_in_nonmoving_window",  # GC addresses, untraced, bounded by a named window
     "open_gap",  # a real unrooted GC pointer, with an issue
     "unverified",  # enumerated, verdict not established — a dated TODO
 }
 
+# `untraced_in_nonmoving_window` (#9740) is for the holder whose stored value IS
+# a GC heap address and which is nonetheless correct BECAUSE it is untraced.
+#
+# `gc/census.rs`'s `PASS1_MARKED` is the case that forced it: a sorted vector of
+# header addresses snapshotted at the end of mark propagation and consumed at
+# sweep entry OF THE SAME synchronous full cycle, used only as `binary_search`
+# keys — compared, never dereferenced, and deliberately not traced (tracing the
+# marked set would make the census a participant in the reachability it exists
+# to observe). Every other verdict would have been a false statement about it:
+# `not_a_gc_pointer` is defined as an id/counter/code address/Rust-owned state
+# and a heap address is none of those; `covered_elsewhere` names a scanner, and
+# no root scan can even observe this holder, which is empty outside the window;
+# `open_gap` and `unverified` assert a defect or an unanswered question, and
+# both fail the gate.
+#
+# The verdict is only worth more than an exemption if the WINDOW is the thing
+# pinned, so `window_opens` / `window_closes` name the two functions that bound
+# it and `window_problems` checks, against the holder's own source, that both
+# still exist and both still name the holder. Renaming a boundary, deleting one,
+# or moving the write or the take out of it turns this gate red — which is the
+# regression the verdict exists to catch, since a holder of stale heap addresses
+# is safe only for as long as its window stays shut.
+WINDOW_FIELDS = ("window_opens", "window_closes")
+
 # `unverified` is the one verdict that classifies nothing. It exists so a hole
 # the gate CAN see is named rather than silent, and it is capped so the list
 # cannot quietly become the whole inventory — at which point the gate would be a
@@ -1106,7 +1144,43 @@ def apply_frontier(
     return unpinned, stale
 
 
-def inventory_problems(inventory: list[dict]) -> list[str]:
+def window_problems(entry: dict, root: Path, label: str) -> list[str]:
+    """Check a window verdict against the holder's own source.
+
+    The claim `untraced_in_nonmoving_window` makes is not "this value is
+    harmless" but "this value never outlives the window between these two
+    functions". That is checkable in the same shallow, same-file way the rest of
+    this script computes coverage: both boundaries must still be functions in
+    the holder's file, and both must still name the holder. A `why` string
+    cannot notice that the take moved; this can.
+    """
+    problems: list[str] = []
+    path = root / entry["file"]
+    if not path.exists():
+        return [
+            f"{label}: {entry['file']} does not exist, so the window this verdict "
+            f"names cannot be checked"
+        ]
+    bodies = function_bodies(path.read_text(encoding="utf-8", errors="replace"))
+    name = entry["name"]
+    for field in WINDOW_FIELDS:
+        fn = entry[field].strip()
+        body = bodies.get(fn)
+        if body is None:
+            problems.append(
+                f"{label}: {field} names `{fn}`, which is not a function in "
+                f"{entry['file']} — the window's boundary was renamed or removed, so "
+                f"this verdict no longer describes the holder"
+            )
+        elif not re.search(rf"\b{re.escape(name)}\b", body):
+            problems.append(
+                f"{label}: `{fn}` ({field}) no longer mentions `{name}` — the holder "
+                f"outlives the window the verdict was granted for"
+            )
+    return problems
+
+
+def inventory_problems(inventory: list[dict], root: Path | None = None) -> list[str]:
     """Structural checks on the inventory itself.
 
     Without these, `apply_inventory` accepts any object carrying a matching
@@ -1140,6 +1214,16 @@ def inventory_problems(inventory: list[dict]) -> list[str]:
             )
         if verdict == "open_gap" and not (entry.get("issue") or "").strip():
             problems.append(f"{label}: open_gap must cite an `issue`")
+        if verdict == "untraced_in_nonmoving_window":
+            missing = [f for f in WINDOW_FIELDS if not (entry.get(f) or "").strip()]
+            if missing:
+                problems.append(
+                    f"{label}: untraced_in_nonmoving_window must name the window that "
+                    f"bounds the holder — missing {', '.join(missing)}. Without both "
+                    f"boundaries this verdict is an exemption, not a contract"
+                )
+            elif root is not None:
+                problems.extend(window_problems(entry, root, label))
         if verdict in {"open_gap", "unverified"}:
             problems.append(
                 f"{label}: `{verdict}` is not a shippable old-page relocation verdict. "
@@ -1221,7 +1305,7 @@ def report(root: Path, quiet: bool = False) -> int:
 
     inventory = load_inventory(INVENTORY_PATH)
     unclassified, stale = apply_inventory(holders, inventory)
-    malformed = inventory_problems(inventory)
+    malformed = inventory_problems(inventory, root)
     frontier = load_frontier(INVENTORY_PATH)
     frontier_new, frontier_stale = apply_frontier(
         holders, frontier, registered_scanners, inventory
@@ -1893,7 +1977,7 @@ def expect_absent(rel: str, name: str, why: str) -> None:
                 ", ".join(f"{e['file']}:{e['name']}" for e in frontier_stale[:5]),
             )
         )
-    failures.extend(inventory_problems(inventory))
+    failures.extend(inventory_problems(inventory, REPO_ROOT))
     # …and the structural checker must itself be able to fail.
     long_why = "x" * 30
     for bad, expect in (
@@ -1906,6 +1990,48 @@ def expect_absent(rel: str, name: str, why: str) -> None:
             failures.append(
                 f"inventory_problems did not reject the malformed {expect!r} entry: {bad}"
             )
+    # The window verdict's boundaries are checked against real source, so its
+    # rejections need a real tree. Planting one is what keeps the check from
+    # degrading into "the field is non-empty".
+    with tempfile.TemporaryDirectory() as tmp:
+        window_root = Path(tmp)
+        (window_root / "src").mkdir()
+        (window_root / "src" / "w.rs").write_text(
+            "crate::perry_thread_local! {\n"
+            "    static SNAP: RefCell<Option<Vec<usize>>> = const { RefCell::new(None) };\n"
+            "}\n"
+            "fn opens() {\n    SNAP.with(|p| *p.borrow_mut() = Some(Vec::new()));\n}\n"
+            "fn closes() {\n    SNAP.with(|p| p.borrow_mut().take());\n}\n"
+            "fn unrelated() {\n    let _ = 1;\n}\n",
+            encoding="utf-8",
+        )
+        bounded = {
+            "file": "src/w.rs",
+            "name": "SNAP",
+            "verdict": "untraced_in_nonmoving_window",
+            "why": long_why,
+            "window_opens": "opens",
+            "window_closes": "closes",
+        }
+        if inventory_problems([bounded], window_root):
+            failures.append(
+                "the window verdict rejected a holder its own named window bounds"
+            )
+        for field, value, expect in (
+            ("window_opens", "", "window_opens"),
+            ("window_closes", "renamed", "not a function"),
+            ("window_closes", "unrelated", "no longer mentions"),
+        ):
+            broken = dict(bounded, **{field: value})
+            if not any(
+                expect in problem
+                for problem in inventory_problems([broken], window_root)
+            ):
+                failures.append(
+                    f"the window verdict accepted {field}={value!r}, which widens the "
+                    f"window it claims to be bounded by"
+                )
+
     over_cap = [
         {"file": f"f{i}", "name": "N", "verdict": "unverified", "why": long_why}
         for i in range(MAX_UNVERIFIED + 1)