Cover aliasing, panic safety, and cross-thread use under miri - #135
Open
cf-rhett wants to merge 1 commit into
Open
Cover aliasing, panic safety, and cross-thread use under miri#135cf-rhett wants to merge 1 commit into
cf-rhett wants to merge 1 commit into
Conversation
The suite has no test that holds a read grant and a write grant into the same allocation at once, none that unwinds out of a commit or release, and none that puts the two halves on different threads. Each of those is a place where a stale pointer, a double release, or a torn coordinator update would pass a normal run and only show up under miri. Six tests, gated on `std` for threads and `catch_unwind`, so the no-std builds are unaffected. A panicking `Notifier` is the lever for the unwind cases: it makes `commit` and `release` fail partway through, after the coordinator has been updated but before the grant has finished tidying up.
cf-rhett
marked this pull request as ready for review
July 31, 2026 19:34
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.
Summary
Three gaps in the current suite, all of them things that pass a normal
cargo testand only bite under miri:commitorrelease, so a grant that panics partway through its teardown is untestedSix tests covering those, gated on
stdforstd::threadandcatch_unwind. The no-std and embedded builds are unaffected —cargo testwith no features still passes, as does the full matrix below.The unwind cases use a
Notifierwhose wakes panic. That is a convenient lever: it makescommitandreleasefail after the coordinator has been updated but before the grant has finished releasing its queue handle, which is exactly where a double release or a leakedArcwould hide. The tests assert the strong count returns to its baseline and that aWeakcannot be upgraded afterwards, and that the frame is still published and the write slot reopened.Testing
Passes under both borrow models, 16 tests either way:
./miri.shMIRIFLAGS="-Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-tree-borrows" cargo +nightly miri test --target x86_64-unknown-linux-gnu --features=stdcargo test --features=stdcargo test(no features)cargo build --no-default-featurescargo build --features=stdcargo build --no-default-features --target=thumbv6m-none-eabicargo build --target=thumbv6m-none-eabi --features=portable-atomic-unsafe-assume-single-coreNote
Independent of my other open PRs, and touches a file none of them do. Worth mentioning that these leak under
-Zmiri-disable-isolationalone on currentmain— that is the same leak #131 fixes, not anything new here, and they are clean once #131 lands.