Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions changelog.d/7824-thread-local-policy-gate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
**The thread-local policy gate is green again, and now measures what it claims to.**
`scripts/check_thread_locals.py` was failing on `main`, which blocked
`tls-budget`'s documented promotion to a required context — a gate that runs,
reports failure, and cannot block anything. Two of the eight raw `thread_local!`
blocks were real, and they were not the ones reported: `array/indexing.rs`,
`map.rs`, `set.rs` and `registry_latch_probes.rs` are all `#[cfg(test)]` and do
not exist in a shipping build, while `gc/schedule.rs`'s `SAFEPOINT_COUNTER`
(every safepoint) and `SCHEDULE_NEXT_CANDIDATE_BYTES` (every poll) were live and
paying `_tlv_get_addr` on Darwin. Those two now use
`crate::perry_thread_local!`. The scan no longer counts `#[cfg(test)]`
declarations at all — recording one as "cold" records the wrong fact, since it
is not cold but absent — covering the attribute directly above a block, an
inline `#[cfg(test)] mod`, and a whole file declared `#[cfg(test)] mod <stem>;`
(closed transitively, so `gc/tests/mod.rs` carries its subtree). Because an
over-broad exclusion would make the gate pass *by seeing less*, `--self-test`
now checks each shape in both directions — gated is skipped, and removing the
gate makes the same declaration fail again — proving six rejection directions
instead of four. The allowlist regeneration is provably one-way (101 → 91
entries, 157 → 129 blocks, nothing added and no count increased), which also
retires three already-stale entries: `gc/zeal.rs` from #7741's removal of
`PERRY_GC_ZEAL`, plus `arena/quarantine.rs` and `gc/oldgen_defrag.rs`. (#7814)
4 changes: 2 additions & 2 deletions crates/perry-runtime/src/gc/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ static SCHEDULE_FORCED: AtomicU64 = AtomicU64::new(0);
/// other test's `before + 1` flaky.
static SCHEDULE_SAFEPOINTS: AtomicU64 = AtomicU64::new(0);

thread_local! {
crate::perry_thread_local! {
/// The monotonically increasing safepoint ordinal this thread's schedule is
/// a function of. Thread-local on purpose — see the determinism note above.
static SAFEPOINT_COUNTER: std::cell::Cell<u64> = const { std::cell::Cell::new(0) };
Expand Down Expand Up @@ -512,7 +512,7 @@ impl Drop for ScheduleStrideGuard {
}
}

thread_local! {
crate::perry_thread_local! {
/// From-space high-water mark at or above which the next poll-arm candidate
/// is due. Per-thread because the arena it measures is.
///
Expand Down
147 changes: 144 additions & 3 deletions scripts/check_thread_locals.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,24 @@
* `--self-test` runs both directions against synthetic trees, so the checker
itself cannot quietly stop being able to say no.

`#[cfg(test)]` DECLARATIONS ARE OUT OF SCOPE BY CONSTRUCTION
===========================================================

A `thread_local!` that only exists under `#[cfg(test)]` cannot cost
`_tlv_get_addr` in a shipping build, because it is not in one. Recording such a
declaration as "cold" would be recording the wrong fact — it is not cold, it is
*absent* — and it would spend the allowlist's credibility on entries no one can
ever act on. Three gated shapes are therefore not counted at all:

* `#[cfg(test)]` immediately above the block,
* the block nested inside a `#[cfg(test)] mod … { … }` in the same file,
* the block's whole FILE being a test module, i.e. some `lib.rs`/`mod.rs`
declares it `#[cfg(test)] mod <stem>;`.

This is a *static* fact the scan can see, unlike hot-vs-cold. It is also the
direction that can go wrong quietly, so `--self-test` checks that removing the
gate makes the same declaration fail again.

WHAT IT DOES NOT DO
===================

Expand Down Expand Up @@ -77,6 +95,17 @@
# macro without infinite regress.
EXCLUDED = {"crates/perry-runtime/src/tls_hot.rs"}

# `#[cfg(test)] mod <stem>;` — the whole file is a test module.
CFG_TEST_MOD_RE = re.compile(
r"(?m)^[ \t]*#\[cfg\(test\)\]\s*\n[ \t]*(?:pub(?:\([^)]*\))?\s+)?mod\s+([A-Za-z_0-9]+)\s*;"
)
# Any out-of-line `mod <stem>;`, gated or not — the edges of the module tree.
ANY_MOD_RE = re.compile(r"(?m)^[ \t]*(?:pub(?:\([^)]*\))?\s+)?mod\s+([A-Za-z_0-9]+)\s*;")
# `#[cfg(test)] mod <name> {` — an inline test module, whose body is skipped.
CFG_TEST_INLINE_MOD_RE = re.compile(
r"(?m)^[ \t]*#\[cfg\(test\)\]\s*\n[ \t]*(?:pub(?:\([^)]*\))?\s+)?mod\s+[A-Za-z_0-9]+\s*\{"
)


def block_bodies(src: str, pattern: re.Pattern[str]) -> list[str]:
"""Bodies of every macro block `pattern` starts, brace-matched."""
Expand All @@ -97,10 +126,86 @@ def block_bodies(src: str, pattern: re.Pattern[str]) -> list[str]:
return bodies


def brace_span(src: str, start: int) -> tuple[int, int]:
"""`(open, close)` offsets of the brace-matched block opening at/after `start`."""
i = src.index("{", start)
depth = 0
j = i
while j < len(src):
if src[j] == "{":
depth += 1
elif src[j] == "}":
depth -= 1
if depth == 0:
break
j += 1
return i, j
Comment on lines +129 to +142

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Parse Rust block boundaries without counting braces in literals or comments.

brace_span treats every { and } as syntax. A } in a string before a test-only thread_local! ends the inline module span early. The scanner then counts a declaration that cannot ship. A { can also extend the span and hide a shipping declaration.

Use a Rust-aware lexical scan that ignores comments and literals before matching braces. Add a --self-test case with braces inside a string or comment.

Also applies to: 191-201, 363-395

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/check_thread_locals.py` around lines 129 - 142, The brace_span
scanner must ignore braces inside Rust comments and literals when finding block
boundaries. Replace its character-by-character matching with Rust-aware lexical
scanning while preserving the returned opening and closing offsets, and add a
--self-test case covering braces in a string or comment so declarations outside
the intended block are classified correctly.



def cfg_test_module_files(root: Path, crates: list[str]) -> set[str]:
"""Files whose entire module cannot exist in a shipping build.

Seeded from every `#[cfg(test)] mod <stem>;`, then closed transitively: a
plain `mod y;` inside an already-test-only file is test-only too, which is
what makes `gc/tests/mod.rs` carry its whole subtree. A `mod x;` in `a/b.rs`
(or `a/b/mod.rs`) resolves to `a/b/x.rs` or `a/b/x/mod.rs`; both spellings
are recorded, and a miss is simply a file that stays in scope.
"""
declares: dict[str, list[tuple[str, bool]]] = {}
for crate in crates:
base = root / crate
for dirpath, _dirs, files in os.walk(base):
for name in sorted(files):
if not name.endswith(".rs"):
continue
path = Path(dirpath) / name
rel = str(path.relative_to(root))
src = path.read_text()
parent = path.parent if name in ("lib.rs", "mod.rs") else path.with_suffix("")
gated = set(CFG_TEST_MOD_RE.findall(src))
edges = []
for stem in set(ANY_MOD_RE.findall(src)):
for candidate in (parent / f"{stem}.rs", parent / stem / "mod.rs"):
if candidate.exists():
edges.append((str(candidate.relative_to(root)), stem in gated))
declares[rel] = edges

test_only = {child for edges in declares.values() for child, gated in edges if gated}
changed = True
while changed:
changed = False
for rel in test_only & declares.keys():
for child, _gated in declares[rel]:
if child not in test_only:
test_only.add(child)
changed = True
return test_only


def shipping_raw_blocks(src: str) -> int:
"""Raw `thread_local!` blocks that survive into a non-test build.

Skips a block carrying `#[cfg(test)]` directly above it, and any block
inside an inline `#[cfg(test)] mod … { … }`.
"""
gated_spans = [brace_span(src, m.start()) for m in CFG_TEST_INLINE_MOD_RE.finditer(src)]
count = 0
for m in RAW_RE.finditer(src):
if any(open_at < m.start() < close_at for open_at, close_at in gated_spans):
continue
preceding = src[: m.start()].rstrip()
line = preceding[preceding.rfind("\n") + 1 :].strip()
if line == "#[cfg(test)]":
continue
count += 1
return count


def scan(root: Path, crates: list[str]) -> tuple[dict[str, int], int]:
"""Raw `thread_local!` blocks per file, and total hot declarations."""
raw: dict[str, int] = {}
hot_declarations = 0
test_only_files = cfg_test_module_files(root, crates)
for crate in crates:
base = root / crate
for dirpath, _dirs, files in os.walk(base):
Expand All @@ -113,9 +218,9 @@ def scan(root: Path, crates: list[str]) -> tuple[dict[str, int], int]:
hot_declarations += sum(
len(DECL_RE.findall(body)) for body in block_bodies(src, HOT_RE)
)
if rel in EXCLUDED:
if rel in EXCLUDED or rel in test_only_files:
continue
count = len(RAW_RE.findall(src))
count = shipping_raw_blocks(src)
if count:
raw[rel] = count
return raw, hot_declarations
Expand Down Expand Up @@ -251,12 +356,48 @@ def self_test() -> int:
)
if not any("HOT_SLOT_CAPACITY" in p for p in verify(root, CRATES, allowlist)):
failures.append("exceeding HOT_SLOT_CAPACITY passed")
(src_dir / "hot.rs").write_text(
"crate::perry_thread_local! { static B: u8 = const { 0 }; }\n"
)

# 5. The three `#[cfg(test)]` shapes are out of scope — and, the half
# that can go wrong quietly, REMOVING the gate puts them back in it.
(src_dir / "cold.rs").write_text("thread_local! { static A: u8 = const { 0 }; }\n")
write_allowlist(root, CRATES, allowlist)
gated = {
"attribute": "#[cfg(test)]\nthread_local! { static G: u8 = const { 0 }; }\n",
"inline mod": (
"#[cfg(test)]\nmod t {\n"
" thread_local! { static G: u8 = const { 0 }; }\n"
"}\n"
),
}
for shape, body in gated.items():
(src_dir / "gated.rs").write_text(body)
if verify(root, CRATES, allowlist):
failures.append(f"a `#[cfg(test)]` {shape} declaration was counted")
(src_dir / "gated.rs").write_text(body.replace("#[cfg(test)]\n", ""))
if not verify(root, CRATES, allowlist):
failures.append(f"an UNGATED {shape} declaration passed")
(src_dir / "gated.rs").unlink()

# 6. A whole file declared `#[cfg(test)] mod <stem>;` is out of scope,
# and drops back in when the parent stops gating it.
(src_dir / "probes.rs").write_text(
"thread_local! { static G: u8 = const { 0 }; }\n"
)
(src_dir / "lib.rs").write_text("#[cfg(test)]\nmod probes;\n")
if verify(root, CRATES, allowlist):
failures.append("a `#[cfg(test)] mod <stem>;` file was counted")
(src_dir / "lib.rs").write_text("mod probes;\n")
if not verify(root, CRATES, allowlist):
failures.append("an ungated `mod <stem>;` file passed")

for f in failures:
print(f"SELF-TEST FAILED: {f}", file=sys.stderr)
if failures:
return 1
print("self-test: the checker can fail in all four directions")
print("self-test: the checker can fail in all six directions")
return 0


Expand Down
32 changes: 11 additions & 21 deletions scripts/thread_local_cold_allowlist.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
{
"_comment": "Files still declaring raw `thread_local!`. Every entry is a declaration that pays `_tlv_get_addr` on Darwin; the count is a ratchet, so adding one to an already-listed file fails too. New code should use `crate::perry_thread_local!` \u2014 see crates/perry-runtime/src/tls_hot.rs. Regenerate with scripts/check_thread_locals.py --update.",
"_hot_declarations": 157,
"_hot_declarations": 160,
"files": {
"crates/perry-runtime/src/agent.rs": 1,
"crates/perry-runtime/src/arena/block.rs": 3,
"crates/perry-runtime/src/arena/block.rs": 2,
"crates/perry-runtime/src/arena/page_meta.rs": 2,
"crates/perry-runtime/src/arena/quarantine.rs": 1,
"crates/perry-runtime/src/async_context.rs": 2,
"crates/perry-runtime/src/async_hooks.rs": 3,
"crates/perry-runtime/src/builtins/arithmetic.rs": 1,
Expand Down Expand Up @@ -33,36 +32,29 @@
"crates/perry-runtime/src/fs/stream.rs": 1,
"crates/perry-runtime/src/gc/barrier.rs": 2,
"crates/perry-runtime/src/gc/barrier_arming.rs": 1,
"crates/perry-runtime/src/gc/cycle.rs": 2,
"crates/perry-runtime/src/gc/cycle.rs": 1,
"crates/perry-runtime/src/gc/dirty_page_cache.rs": 1,
"crates/perry-runtime/src/gc/fromspace_scan.rs": 1,
"crates/perry-runtime/src/gc/layout.rs": 2,
"crates/perry-runtime/src/gc/layout.rs": 1,
"crates/perry-runtime/src/gc/layout_tables.rs": 1,
"crates/perry-runtime/src/gc/malloc.rs": 2,
"crates/perry-runtime/src/gc/mod.rs": 3,
"crates/perry-runtime/src/gc/mod.rs": 2,
"crates/perry-runtime/src/gc/old_free.rs": 1,
"crates/perry-runtime/src/gc/oldgen_defrag.rs": 1,
"crates/perry-runtime/src/gc/policy.rs": 10,
"crates/perry-runtime/src/gc/promote_in_place.rs": 2,
"crates/perry-runtime/src/gc/policy.rs": 6,
"crates/perry-runtime/src/gc/promote_in_place.rs": 1,
"crates/perry-runtime/src/gc/roots/scan_mode.rs": 1,
"crates/perry-runtime/src/gc/roots/shadow_stack.rs": 2,
"crates/perry-runtime/src/gc/roots/temp_roots.rs": 1,
"crates/perry-runtime/src/gc/scan_fallback.rs": 1,
"crates/perry-runtime/src/gc/shape_install.rs": 2,
"crates/perry-runtime/src/gc/telemetry.rs": 3,
"crates/perry-runtime/src/gc/shape_install.rs": 1,
"crates/perry-runtime/src/gc/telemetry.rs": 2,
"crates/perry-runtime/src/gc/tenuring.rs": 1,
"crates/perry-runtime/src/gc/tests/copying.rs": 1,
"crates/perry-runtime/src/gc/tests/roots.rs": 1,
"crates/perry-runtime/src/gc/tests/runtime_roots.rs": 4,
"crates/perry-runtime/src/gc/tests/runtime_roots/transient_handles.rs": 1,
"crates/perry-runtime/src/gc/tests/support.rs": 1,
"crates/perry-runtime/src/gc/trace.rs": 2,
"crates/perry-runtime/src/gc/zeal.rs": 3,
"crates/perry-runtime/src/intl/number_format.rs": 1,
"crates/perry-runtime/src/iter_result.rs": 1,
"crates/perry-runtime/src/json/mod.rs": 1,
"crates/perry-runtime/src/json/raw_json.rs": 1,
"crates/perry-runtime/src/json_tape.rs": 3,
"crates/perry-runtime/src/json_tape.rs": 2,
"crates/perry-runtime/src/json_tape_store.rs": 1,
"crates/perry-runtime/src/media_playback.rs": 1,
"crates/perry-runtime/src/native_arena.rs": 1,
Expand All @@ -78,13 +70,11 @@
"crates/perry-runtime/src/node_submodules/test.rs": 1,
"crates/perry-runtime/src/node_submodules/test_once_unit_tests.rs": 1,
"crates/perry-runtime/src/node_submodules/test_property.rs": 1,
"crates/perry-runtime/src/node_submodules/tests.rs": 1,
"crates/perry-runtime/src/node_submodules/trace_events.rs": 1,
"crates/perry-runtime/src/object/native_module/callable_exports.rs": 2,
"crates/perry-runtime/src/object/spill.rs": 2,
"crates/perry-runtime/src/object/spill.rs": 1,
"crates/perry-runtime/src/os/os_process_emitter.rs": 1,
"crates/perry-runtime/src/os_process_streams.rs": 1,
"crates/perry-runtime/src/per_test_global.rs": 1,
"crates/perry-runtime/src/perf_hooks.rs": 3,
"crates/perry-runtime/src/process.rs": 2,
"crates/perry-runtime/src/process/env_misc.rs": 3,
Expand Down