fix handling of spurious accesses during retag #2694
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
RalfJung
force-pushed
the
retag-deref-check
branch
from
7b55507
to
8c600ac
Compare
|
While I was touching retagging anyway, I also added a fix for #2648. |
RalfJung
force-pushed
the
retag-deref-check
branch
from
8c600ac
to
f6d1a2b
Compare
src/stacked_borrows/diagnostics.rs
Outdated
| let Operation::Dealloc(op) = &self.operation else { | ||
| unreachable!("dealloc_error should only be called during a deallocation") | ||
| }; | ||
| err_sb_ub( | ||
| format!( | ||
| "no item granting write access for deallocation to tag {:?} at {:?} found in borrow stack", | ||
| op.tag, self.history.id, | ||
| "attemtping deallocation using {tag:?} at {alloc_id:?}{cause}", |
There was a problem hiding this comment.
| "attemtping deallocation using {tag:?} at {alloc_id:?}{cause}", | |
| "attempting deallocation using {tag:?} at {alloc_id:?}{cause}", |
src/stacked_borrows/mod.rs
Outdated
| })?; | ||
| drop(global); | ||
| if let Some(access) = access { | ||
| assert!(access == AccessKind::Read); |
There was a problem hiding this comment.
If I recall correctly, assert_eq! produces better panic messages, so I always use it when possible
There was a problem hiding this comment.
It prints what the values actually were. But here there are only two options.
But yeah probably better to get in the habit of using assert_eq.
|
I skimmed over this but I'm a bit fried right now, I'll look over it for real tomorrow. (some of the changes look like they are worth me understanding properly, and right now I do not) |
also fix ICE on deallocation error and avoid redundant find_granting on retag
RalfJung
force-pushed
the
retag-deref-check
branch
from
f6d1a2b
to
c75f23d
Compare
|
I think I understand this decently well now. r=me whenever you want to merge (we have a number of outstanding PRs which may be disruptive so I'm not going to just send this) |
|
We'll have to land them in some order... 🤷 |
|
☀️ Test successful - checks-actions |
for now, do not do fake reads on non-Unpin mutable references Work-around for rust-lang/unsafe-code-guidelines#381, needed to make the new test pass. Undoes parts of #2694.
for now, do not do fake reads on non-Unpin mutable references Work-around for rust-lang/unsafe-code-guidelines#381, needed to make the new test pass. Undoes parts of rust-lang/miri#2694.
The
dereferenceableattribute 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 #2648.
Also fixes #2693 since the same issue arose for retagging as well.
r? @saethlin