fix(tooling): make GC structural audits portable (#7878) - #7882
Merged
Conversation
📝 WalkthroughWalkthroughThe pull request adds Windows GC audit steps, normalizes repository-relative paths to POSIX separators, removes an unused Unix-only import, and prevents measurement without ChangesWindows GC audits
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
proggeramlug
marked this pull request as ready for review
August 11, 2026 18:34
proggeramlug
marked this pull request as ready for review
August 11, 2026 18:34
proggeramlug
pushed a commit
that referenced
this pull request
Aug 11, 2026
proggeramlug
added a commit
that referenced
this pull request
Aug 12, 2026
…s green, gc-native-roots (#7977, #7971, #7970) (#7986) * fix(ci): read Rust sources as UTF-8 in the Windows GC audits (#7977) `scripts/check_thread_locals.py` walked `crates/perry-runtime/src` with a bare `Path.read_text()`, which decodes with `locale.getencoding()` — cp1252 on a GitHub Windows runner. Fifteen runtime sources carry a byte cp1252 cannot map (0x81/0x8d/0x8f/0x90/0x9d); `i18n.rs` is reached first, at offset 31552 with 923 newlines before it, so `core.autocrlf` puts the failure at position 32475 — matching the reported traceback to the byte. That is the FIRST step of `windows-build`, so the seven steps behind it were `skipped` on every PR: no Windows build of the runtime, stdlib or either Windows UI crate, no Windows run of the `perry-runtime` unit tests, no Windows parity smoke, no VERSIONINFO check, no COFF-trimming test. #7882 fixed the path-separator half of this class in this same file and the encoding in `gc_runtime_root_holders.py`; these four readers were missed. - route all reads/writes through `read_source()` / `write_source()` helpers that pass `encoding="utf-8"` (and `newline=""`, so `--update` is byte-stable across hosts). The verified `files` map is unchanged. - fix a latent instance in `gc_runtime_root_holders.py`'s fixture writer and a bare `read_text()` in `tests/test_gc_ratchet.py`. - add `scripts/check_locale_independent_io.py`: an AST scan of exactly the six Python files the Windows audit step runs, failing on locale-defaulted text I/O. Static rather than `PYTHONWARNDEFAULTENCODING` because that only fires on executed calls, and because `test_gc_ratchet.py` embeds `open(...)` inside probe source *literals* that an AST correctly ignores and a grep would not. - run it in `lint` (Linux, per-PR, already required) so the class is caught before it reaches Windows, and set `PYTHONUTF8=1` on the Windows step as belt-and-braces. Validated: main's version exits 1 with `UnicodeDecodeError` under a non-UTF-8 locale and the fixed version exits 0; the full eight-command Windows audit sequence passes under that locale; and replanting the exact defect makes the new gate exit 1 naming the call site, so a green run means the detector works. * fix(ci): make llvm-inprocess say what it did, and stop it greening on a skip (#7971) Three problems, all of which let the gate report success while asserting nothing. 1. VACUOUS GREEN. On a PR that touched no IR-affecting path the workflow ran `changes=success, native-backend=skipped` and concluded `success`. Sampled runs 31505530279, 31499833415 and 31476724152 are all that shape. "Green" meant "not relevant", but read as "the in-process backend passed". Add a `llvm-inprocess-complete` fan-in that states the verdict in the log and the step summary — EXERCISED vs NOT EXERCISED — and that can fail in two directions: `native-backend` failing or being cancelled, and `native-backend` being SKIPPED on a non-PR event. Every non-PR event sets `relevant=true` unconditionally, so a skip there means the post-merge anchor has stopped anchoring, which is how #7856 starved this gate for eight days unnoticed. 2. NO DIAGNOSTIC. `Native-mode smoke` ran bare `grep -q` / `cmp` under `set -euo pipefail`, with each compiler's stderr redirected to a file nothing ever printed. The three 2026-08-11 `main` failures therefore ended at "Generating code..." with a naked `exit 1`. Rewrite it around `run` / `assert_grep` / `assert_same` helpers: the failing command is named with its real exit status, captured stdout and stderr are dumped, a parity failure prints the diff, and every liveness assert says what it is protecting. (The status is captured after the command rather than inside an `if !` branch, where `$?` is the negated status and always 0.) 3. STALE CORPORA, INVISIBLE. The unit gate asserts `corpus_spike ... ok` to prove the corpus tests ran, but not that the corpora are current. All three were frozen on 2026-08-03, 151 codegen commits ago, and contain zero `addrspace(1)` — so they stayed green while the end-to-end arm could not build a single RS4GC root slot. Print corpus age and the IR-affecting commit count so that gap is visible instead of rediscovered. The underlying backend defect this arm was correctly reporting is filed as #7982: `PERRY_LLVM_INPROCESS=native` cannot construct `ptr addrspace(1)` roots. It is reproduced locally and is not a one-line fix, so it is left to that issue rather than folded into a CI change. * fix(ci): repair the gc-native-roots macOS arm; separate its real defects (#7970) `gc-native-roots` had never had a green run on any branch. Three of its four arms failed for three unrelated reasons; they are not one bug. macos-14 — GATE DEFECT, fixed. The in-process step asserted evacuation liveness without setting `PERRY_GC_DIAG=1`, and `[gc-copy-minor]` — the only input `gc_evacuation_liveness_assert.py` reads — is printed only under that flag. The assert saw an empty trace and reported "the forced-evacuation arm evacuated NOTHING (0 copying minors, 0 objects copied)" on every run since the arm was written, so it could never pass. Measured on macOS aarch64: without the flag, stderr is 98 bytes of `#gcmetric` and 0 copying minors; with it, the same binary under the same GC env reports 75 copying minors and 16277 objects copied, and the whole step passes (gcmap section present, stackmaps absent, control-vs-forced stdout identical). #7970 wondered whether this was #7965-shaped. It is not — the collector was evacuating the whole time. `--probe` was missing too, which is why the logs say the literal `<probe>`. To stop that misdiagnosis recurring, `gc_evacuation_liveness_assert.py` now distinguishes "the instrument was off" from "the subject was dead": a trace containing no `[gc-...]` marker at all is reported as `PERRY_GC_DIAG=1` not being set, with "fix the RUN, not the collector", instead of being blamed on the GC. It gains a `--self-test` covering five directions, run in `lint`. windows-latest — the second of its two failures was a workflow bug: Git-bash GNU tar reads `$RUNNER_TEMP` (`D:\a\_temp`) as a remote `host:path` and tries to connect to a host named `D`. Fixed with `--force-local` and a post-extraction check so a truncated download reports itself rather than surfacing later as "no matched opt+clang pair". The two remaining failures are REAL DEFECTS and are filed, not silenced: * #7984 — aarch64-linux: under `PERRY_STACKMAP_WALKER=verify` the fast fp-chain walker and the unwinder resolve the same root to addresses 96 bytes apart. The fast walker is what runs when verify is off, so this is a potential live rooting bug on that target. The arm is red because it found the bug it exists to find, and must stay red until fixed. * #7985 — windows: `perry.exe` cannot link against the official LLVM 22 release (/MT-vs-/MD CRT mismatch, bundled rpmalloc redefining malloc, and inkwell referencing target backends the release does not build). Likely also latent in `windows-build`, whose build step has not run since #7977. A STATUS block at the top of the workflow records which arm is which, and states that this gate must NOT be promoted to a required context while #7984/#7985 are open — two legitimately-red arms would block every PR. * docs(changelog): record the CI gate repairs (#7986) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
os.wait4is unavailableTests
python3 scripts/gc_runtime_root_holders.py --self-testpython3 scripts/gc_runtime_root_holders.pypython3 scripts/check_thread_locals.py --self-testpython3 scripts/check_thread_locals.pypython3 -m unittest discover -s tests -p 'test_gc_ratchet.py' -v(93 passed)python3 benchmarks/gc_ratchet/gc_ratchet.py validate --scope structuralpython3 -m compileall -q scripts/gc_runtime_root_holders.py scripts/check_thread_locals.py benchmarks/gc_ratchet/gc_ratchet.py tests/test_gc_ratchet.pybash scripts/check_file_size.sh.github/workflows/test.ymlCloses #7878
Summary by CodeRabbit
New Features
Bug Fixes