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

Stacked Borrows retags should also be accesses for the data race model #2648

Closed
RalfJung opened this issue Nov 5, 2022 · 0 comments · Fixed by #2694
Closed

Stacked Borrows retags should also be accesses for the data race model #2648

RalfJung opened this issue Nov 5, 2022 · 0 comments · Fixed by #2694
Labels
A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) A-data-race Area: data race detector C-bug Category: This is a bug. I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings)

Comments

@RalfJung
Copy link
Member

RalfJung commented Nov 5, 2022

Currently, many Stacked Borrows retags are effectively read/write accesses for the aliasing model:

// A "safe" reborrow for a pointer that actually expects some aliasing guarantees.
// Here, creating a reference actually counts as an access.
// This ensures F2b for `Unique`, by removing offending `SharedReadOnly`.
self.access(access, derived_from, global, dcx, exposed_tags)?;

However, the data race model never hears about them, which as @JakobDegen points out leads us to accept some code that we probably should not accept.

@RalfJung RalfJung added A-aliasing Area: This affects the aliasing model (Stacked/Tree Borrows) A-data-race Area: data race detector C-bug Category: This is a bug. I-misses-UB Impact: makes Miri miss UB, i.e., a false negative (with default settings) labels Nov 5, 2022
@bors bors closed this as completed in 9f30f00 Nov 27, 2022
RalfJung pushed a commit to RalfJung/rust that referenced this issue Nov 27, 2022
fix handling of spurious accesses during retag

The `dereferenceable` attribute we emit for LLVM is checked during retag in Stacked Borrows.
However, we currently don't properly do that for retagging of `&mut !Unpin`, which this PR fixes.
Also this adjusts retagging to inform the data race model of the accesses as well.

Fixes rust-lang/miri#2648.
Also fixes rust-lang/miri#2693 since the same issue arose for retagging as well.

r? `@saethlin`
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) A-data-race Area: data race detector C-bug Category: This is a bug. 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