Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Miri does not properly check dereferenceable on &mut !Unpin #2714

Closed
RalfJung opened this issue Dec 3, 2022 · 0 comments · Fixed by rust-lang/rust#106180
Closed

Miri does not properly check dereferenceable on &mut !Unpin #2714

RalfJung opened this issue Dec 3, 2022 · 0 comments · Fixed by rust-lang/rust#106180
Labels
A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) C-spec-question Category: it is unclear what the intended behavior of Miri for this case is I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings)

Comments

@RalfJung
Copy link
Member

RalfJung commented Dec 3, 2022

#2713 works around rust-lang/unsafe-code-guidelines#381 by just making Miri check fewer things, to avoid blocking people that gate their CI on Miri. This almost certainly means Miri currently misses some LLVM UB. So while the UCG discusses how the issue is to be resolved, let's track on the Miri side that we are missing some UB here. (#2713 removes a fail test that should possibly be reinstated once we figure out what to do here, or maybe be turned into a pass test.)

@RalfJung RalfJung added A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) C-spec-question Category: it is unclear what the intended behavior of Miri for this case is labels Dec 3, 2022
bors added a commit that referenced this issue Feb 7, 2023
make &mut !Unpin not dereferenceable, and Box<!Unpin> not noalias

See rust-lang/unsafe-code-guidelines#381 and [this LLVM discussion](https://discourse.llvm.org/t/interaction-of-noalias-and-dereferenceable/66979). The exact semantics of how `noalias` and `dereferenceable` interact are unclear, and `@comex` found a case of LLVM actually exploiting that ambiguity for optimizations. I think for now we should treat LLVM `dereferenceable` as implying a "fake read" to happen immediately at the top of the function (standing in for the spurious reads that LLVM might introduce), and that fake read is subject to all the usual `noalias` restrictions. This means we cannot put `dereferenceable` on `&mut !Unpin` references as those references can alias with other references that are being read and written inside the function (e.g. for self-referential generators), meaning the fake read introduces aliasing conflicts with those other accesses.

For `&` this is already not a problem due to rust-lang/rust#98017 which removed the `dereferenceable` attribute for other reasons.

Regular `&mut Unpin` references are unaffected, so I hope the impact of this is going to be tiny.

The first commit does some refactoring of the `PointerKind` enum since I found the old code very confusing each time I had to touch it. It doesn't change behavior.

Fixes #2714

EDIT: Turns out our `Box<!Unpin>` treatment was incorrect, too, so the PR also fixes that now (in codegen and Miri): we do not put `noalias` on these boxes any more.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) C-spec-question Category: it is unclear what the intended behavior of Miri for this case is I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant