Conversation
ciaranra
marked this pull request as ready for review
September 16, 2026 00:56
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
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.
Fix macOS dynamic-library resolution failures and a Windows QIS recovery ABI mismatch uncovered by cross-platform verification.
panic(i32, ptr)to macOS libSystem's unrelatedpanicfunction, causing process exit/abort. Explicitly link the selected PECOS QIS FFI dylib on macOS so Mach-O's two-level namespace selects the correct runtime. Include the link-policy revision in compiled-program cache keys so Python extension upgrades cannot reuse old incorrectly linked programs.extern "C". UseC-unwindon those boundaries to permit recovery without Rust's abort-on-unwind barriers. Keep releasing owned values and locks before transfer because Unix longjmp does not run destructors.Evidence and validation
Compiler regression-only commit
00b51575failed as expected: https://github.com/PECOS-packages/PECOS/actions/runs/35016292928/job/104540441757 (rustc SIGSEGV in LLVM initialization).After the LLVM fix, final Python CI at
10fe72dapassed, including stable macOS builds and installed-wheel LLVM checks on Python 3.12 and 3.14: https://github.com/PECOS-packages/PECOS/actions/runs/35019112455.The full Rust run exposed the additional QIS abort: https://github.com/PECOS-packages/PECOS/actions/runs/35021706111/job/104560321159. Reproduced the termination-test failure on unchanged local dev without DYLD variables.
dyld_info -importsshowed_panic (from libSystem); after the fix it shows_panic (from libpecos_qis_ffi).New regression test checks the generated program's actual panic function address against the selected FFI library. Verified it fails with the old link policy and passes with the fix.
All 57 QIS unit tests pass locally, including exit/panic recovery, heap cleanup, and same-named program isolation. QIS Clippy with warnings denied, formatting, and whitespace checks pass.
Earlier LLVM validation: complete local Python build with no manual overrides, 16 LLVM IR/binding tests, module-version checks, and CLI Clippy all pass.
Final regular PR CI at
714a1a38is green, including the new macOS QIS executor regression step and LLVM wheel runtime checks: https://github.com/PECOS-packages/PECOS/actions/runs/35026623706.Full Rust verification at
714a1a38passed on macOS and Linux: https://github.com/PECOS-packages/PECOS/actions/runs/35026678187. Windows failed inwrappers_free_program_heap_on_return_exit_panic_and_invalid_input: invalidheap_alloctransfers via the C shim's longjmp, Windows invokesRtlUnwindEx, and Rust aborts withpanic in a function that cannot unwindat theextern "C"heap_alloc boundary.Windows follow-up
6904c224: a new host-independent ABI regression aborts with the old heap_alloc ABI and passes with C-unwind. All 57 QIS and 129 FFI unit tests pass locally; Clippy with warnings denied, formatting, and whitespace checks pass. Regular macOS and Windows PR jobs now run both FFI and executor regressions, including actual C longjmp recovery, heap cleanup, and subsequent execution. Fresh PR CI and a full cross-platform Rust run are pending; keep this PR in draft until actual Windows verification passes.Windows CI at
6904c224now passes all 129 FFI tests and 12 executor tests, including real longjmp recovery, program heap cleanup, and successful execution after termination: https://github.com/PECOS-packages/PECOS/actions/runs/35034496393/job/104600374131. The corresponding macOS regression job also passed. Full cross-platform Rust verification remains pending: https://github.com/PECOS-packages/PECOS/actions/runs/35034531567.PR-visible full-suite opt-in
Commit
a7502509adds theci:full-rustlabel: adding it launches full Linux/macOS/Windows Rust validation as normal PR checks, and subsequent pushes rerun it while the label remains. Removing it restores the lightweight PR path. Unrelated label events do not cancel validation; post-merge and manual full runs remain unchanged. Usage is documented in.github/CI.md. Actionlint passes; zizmor reports only the two existing unpinned setup-just tool warnings.The label is applied to this PR. GitHub confirmed a
pull_requestrun ona7502509, titled Rust validation (full): https://github.com/PECOS-packages/PECOS/actions/runs/35042473499. This latest full run and regular PR checks are pending. The prior merged heade93ab326had green regular checks and a code review with no actionable findings, but those results do not replace validation of this latest workflow change.Latest verification on
1e132847The label-triggered full run is visible in PR checks and passed Linux and macOS: https://github.com/PECOS-packages/PECOS/actions/runs/35043360816. Windows now fails in
pecos-simulatorstestfast_corpus_matches_oracle_and_reference_bits:crossover_8t_256_termsamplitude[0].re is3fa0000000000004versus reference3fa0000000000009(five ULPs). This PR has no changes tocrates/pecos-simulatorsrelative to dev; the reference test originated in #716 and was later touched by #717. This is a separate numerical-reference issue, not the prior QIS unwind abort. Do not treat the latest full run as green; diagnosis/fix scope needs a separate decision rather than silently relaxing exact-reference assertions.Supersedes #640. Related: #566.