ci(gc): the macOS in-process RS4GC arm could never pass - #7414
Merged
Conversation
added 2 commits
August 5, 2026 07:21
`RS4GC works on a stock toolchain via the in-process backend` asserts that a copying minor actually moved objects, by counting `[gc-copy-minor] ran copied_objects=` lines in the probe's stderr. Both of those prints are gated on PERRY_GC_DIAG (gc/copying.rs:993 and :1246), and this step never set it -- the sibling walker step does. So the trace held nothing but the probe's own #gcmetric lines, the assert read 0 copying minors / 0 objects copied off an effectively empty file, and the step failed regardless of how the collector behaved. The inverse of the usual hazard: not a gate that cannot fail, but one that cannot PASS. It shipped with the step in #7339 and had never executed, because three of the four arms in this matrix were permanently queued until #7393 added a concurrency group. Also makes the assert say what actually happened. A trace with no [gc-*] diagnostics at all is indistinguishable, by counts alone, from a collector that moved nothing, and the old message asserted the latter. That misdiagnosis is what made this cost a build to identify. Reproduced on macOS aarch64 against current main (cd29706, which contains #7398 and #7400, so it was not already fixed): the step as written reproduces the CI error byte-for-byte with a 3-line stderr; the same binary with PERRY_GC_DIAG=1 reports 2 copying minors / 10892 objects copied and the full step exits 0, stdout unchanged so the control diff still holds. The new branch is capable of failing: it fires on the pre-fix trace, passes on the post-fix one, and two negative controls (diagnostics present but zero copies; a synthetic manual_collect trace) still fail with the original message. This does not make the workflow green -- the other three arms fail earlier in "Probe matrix" for unrelated reasons.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe macOS in-process RS4GC workflow now enables GC diagnostics. The liveness assertion reports missing diagnostics separately from diagnostics that show zero moved objects. ChangesRS4GC diagnostic validation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
✨ Finishing Touches📝 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
pushed a commit
that referenced
this pull request
Aug 5, 2026
Audit follow-up on this PR. Moving the malloc-trim counter to the top of run_malloc_trim made the gate satisfiable on Windows/musl -- correct, and #6180's subject really is "did budgeted reclaim REACH the trim call", since the bug was ordinary_budgeted skipping it. But it also dropped the stronger property on glibc/macOS. The portable counter witnesses only reaching, so it would pass with the platform arm deleted, while the assertion still read "must invoke allocator trim". Split into two counters: the portable ..._CALLS for "reached", and a cfg-gated ..._EXECUTED incremented inside BOTH the glibc and Darwin arms for "a trim primitive ran". Instrumenting only glibc would have made the macOS gate impossible to satisfy -- the same shape as #7414, where an assertion could not pass because its input was never produced. Verified the new assertion can fail: removing the Darwin instrumentation fails with "on a target with a trim primitive, budgeted reclaim must EXECUTE it".
proggeramlug
added a commit
that referenced
this pull request
Aug 5, 2026
* fix(runtime): green perry-runtime --lib suite on Windows (#7356) Two production bugs and four platform-shape test fixes; the suite goes from three process-killing stoppers to 1635/1635 single-threaded. - js_throw's longjmp was UB on windows-msvc: MSVC longjmp performs a real RtlUnwindEx when _JUMP_BUFFER.Frame (first 8 bytes) is nonzero, and the one-arg setjmp extern leaves that slot as garbage RDX (the CRT _setjmp stores its SECOND parameter there). Measured STATUS_BAD_STACK (0xC0000028) in a compiled probe and _report_gsfailure aborts under the panic=unwind test harness. Zero the slot before jumping: the non-unwinding POSIX semantics are exactly what the savepoint restores assume. A/B: probe (throwing .then / Array.from mapper / Promise.all member / 1000-throw churn) crashes 0xC0000028 without the fix, is byte-identical to the Node oracle with it. - The conservative-scan register snapshot buffer ([u64; 32], 8-aligned) and the ffi::setjmp test buffers violated MSVC _setjmp's 16-byte alignment contract (aligned XMM stores) - an AV whenever the stack lands 8-mod-16. All are repr(align(16)) now; the extern documents both MSVC contracts. - date: TZ-isolation child uses PST8PDT on Windows (UCRT TZ parser silently degrades IANA ids to UTC, failing the subject-is-live guard). - gc malloc-trim test: count that budgeted reclaim REACHED the trim call (the #6180 subject) instead of counting only the glibc/macOS executing arms, which made the gate unsatisfiable where trim is unsupported. - child_process: spawnSync test spawns `cmd /c echo hi` on Windows (echo is a cmd builtin; ENOENT under Node too). - CI: windows-build now runs RUST_TEST_THREADS=1 cargo test --profile perry-dev --lib -p perry-runtime - the arm that keeps the suite from rotting back to unmeasurable. Closes #7356 * changelog: fragment for #7419 (Windows runtime suite green, #7356) * test(gc): assert both trim claims, not just the portable one Audit follow-up on this PR. Moving the malloc-trim counter to the top of run_malloc_trim made the gate satisfiable on Windows/musl -- correct, and #6180's subject really is "did budgeted reclaim REACH the trim call", since the bug was ordinary_budgeted skipping it. But it also dropped the stronger property on glibc/macOS. The portable counter witnesses only reaching, so it would pass with the platform arm deleted, while the assertion still read "must invoke allocator trim". Split into two counters: the portable ..._CALLS for "reached", and a cfg-gated ..._EXECUTED incremented inside BOTH the glibc and Darwin arms for "a trim primitive ran". Instrumenting only glibc would have made the macOS gate impossible to satisfy -- the same shape as #7414, where an assertion could not pass because its input was never produced. Verified the new assertion can fail: removing the Darwin instrumentation fails with "on a target with a trim primitive, budgeted reclaim must EXECUTE it". --------- Co-authored-by: Ralph Kuepper <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.
The inverse of the usual hazard: not a gate that cannot fail, but one that cannot pass.
What happened
The step
RS4GC works on a stock toolchain via the in-process backendasserts that a copying minor actually moved objects, by counting[gc-copy-minor] ran copied_objects=lines in the probe's stderr.Both of those prints are gated on
PERRY_GC_DIAG(gc/copying.rs:993and:1246) — and this step never set it. The sibling walker step does. So the trace contained nothing but the probe's own#gcmetriclines, the assert read 0 copying minors / 0 objects copied off an effectively empty file, and the step failed no matter how the collector behaved.It shipped with the step in #7339 and had never executed, because three of four arms in this matrix were permanently queued until #7393 added a
concurrencygroup.Reproduced on current main
Against
cd29706c0— which contains #7398 and #7400, so this was not already fixed by the walker work:#gcmetricPERRY_GC_DIAG=1--only-backend rs4gc(9 functions)stdout is unchanged, so the shadow-stack control diff still holds — diagnostics go to stderr only.
Second change: make the assert say what happened
A trace with no
[gc-*]diagnostics at all is indistinguishable, by counts alone, from a collector that moved nothing — and the old message asserted the latter ("evacuated NOTHING… the arm is vacuous"). That misdiagnosis is precisely what made this cost a build to identify rather than a glance.The new branch is verified capable of failing: it fires on the pre-fix trace, passes on the post-fix one, and two negative controls (diagnostics present but zero copies; a synthetic
manual_collecttrace) still fail with the original message and hint.Not a green workflow
The other three arms fail earlier, in "Probe matrix", for unrelated reasons — windows is a build failure, and the two ELF arms were failing on a run that predates #7398/#7400. Those are separate.
Summary by CodeRabbit
Bug Fixes
Documentation