Skip to content

Commit 6f3061b

Browse files
committed
Auto merge of #1945 - saethlin:better-sb-tracking, r=RalfJung
Provide slightly better notes when tracking a pointer tag I slapped this in as a sort of advanced println-based debugging when trying to figure out a track-raw-pointers finding in `smallvec`. Perhaps this looks like a good idea to you all? EDIT: User scenario Run `MIRIFLAGS=-Ztag-raw-pointers cargo miri test`, get a diagnostic that looks like ``` error: Undefined Behavior: trying to reborrow for SharedReadOnly at alloc99465+0x9, but parent tag <265507> does not have an appropriate item in the borrow stack ``` So now run `MIRIFLAGS=-Ztag-raw-pointers -Zmiri-track-pointer-tag=265507 cargo miri test` Old: ``` note: tracking was triggered --> src/lib.rs:822:36 | 822 | vec: NonNull::from(self), | ^^^^ popped tracked tag for item [SharedReadOnly for <265507>] ``` New: ``` note: tracking was triggered --> src/lib.rs:822:36 | 822 | vec: NonNull::from(self), | ^^^^ popped tracked tag for item [SharedReadOnly for <265507>] due to Write access for <265356> ``` So that if a user is now beginning to question their sanity because they don't really understand SB yet, they can then track the tag which caused the parent tag to be removed from the stack to be sure what's going on here: ``` --> src/lib.rs:792:5 | 792 | / pub fn drain<R>(&mut self, range: R) -> Drain<'_, A> 793 | | where 794 | | R: RangeBounds<usize>, 795 | | { ... | 824 | | } 825 | | } | |_____^ created tag 265356 ``` The existing diagnostic can tell you where the tag you'd need was invalidated, but it cannot tell you what and why that tag was invalidated.
2 parents e969615 + cd69219 commit 6f3061b

File tree

2 files changed

+31
-8
lines changed

2 files changed

+31
-8
lines changed

src/diagnostics.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use log::trace;
77
use rustc_middle::ty::{self, TyCtxt};
88
use rustc_span::{source_map::DUMMY_SP, Span, SpanData, Symbol};
99

10+
use crate::stacked_borrows::{AccessKind, SbTag};
1011
use crate::*;
1112

1213
/// Details of premature program termination.
@@ -58,7 +59,9 @@ impl MachineStopType for TerminationInfo {}
5859
/// Miri specific diagnostics
5960
pub enum NonHaltingDiagnostic {
6061
CreatedPointerTag(NonZeroU64),
61-
PoppedPointerTag(Item),
62+
/// This `Item` was popped from the borrow stack, either due to a grant of
63+
/// `AccessKind` to `SbTag` or a deallocation when the second argument is `None`.
64+
PoppedPointerTag(Item, Option<(SbTag, AccessKind)>),
6265
CreatedCallId(CallId),
6366
CreatedAlloc(AllocId),
6467
FreedAlloc(AllocId),
@@ -321,7 +324,20 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
321324
use NonHaltingDiagnostic::*;
322325
let msg = match e {
323326
CreatedPointerTag(tag) => format!("created tag {:?}", tag),
324-
PoppedPointerTag(item) => format!("popped tracked tag for item {:?}", item),
327+
PoppedPointerTag(item, tag) =>
328+
match tag {
329+
None =>
330+
format!(
331+
"popped tracked tag for item {:?} due to deallocation",
332+
item
333+
),
334+
Some((tag, access)) => {
335+
format!(
336+
"popped tracked tag for item {:?} due to {:?} access for {:?}",
337+
item, access, tag
338+
)
339+
}
340+
},
325341
CreatedCallId(id) => format!("function call with id {}", id),
326342
CreatedAlloc(AllocId(id)) => format!("created allocation with id {}", id),
327343
FreedAlloc(AllocId(id)) => format!("freed allocation with id {}", id),

src/stacked_borrows.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ pub struct GlobalState {
111111
pub type MemoryExtra = RefCell<GlobalState>;
112112

113113
/// Indicates which kind of access is being performed.
114-
#[derive(Copy, Clone, Hash, PartialEq, Eq)]
114+
#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
115115
pub enum AccessKind {
116116
Read,
117117
Write,
@@ -296,19 +296,26 @@ impl<'tcx> Stack {
296296
}
297297

298298
/// Check if the given item is protected.
299+
///
300+
/// The `provoking_access` argument is only used to produce diagnostics.
301+
/// It is `Some` when we are granting the contained access for said tag, and it is
302+
/// `None` during a deallocation.
299303
fn check_protector(
300304
item: &Item,
301-
tag: Option<SbTag>,
305+
provoking_access: Option<(SbTag, AccessKind)>,
302306
global: &GlobalState,
303307
) -> InterpResult<'tcx> {
304308
if let SbTag::Tagged(id) = item.tag {
305309
if Some(id) == global.tracked_pointer_tag {
306-
register_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(item.clone()));
310+
register_diagnostic(NonHaltingDiagnostic::PoppedPointerTag(
311+
item.clone(),
312+
provoking_access,
313+
));
307314
}
308315
}
309316
if let Some(call) = item.protector {
310317
if global.is_active(call) {
311-
if let Some(tag) = tag {
318+
if let Some((tag, _)) = provoking_access {
312319
Err(err_sb_ub(format!(
313320
"not granting access to tag {:?} because incompatible item is protected: {:?}",
314321
tag, item
@@ -348,7 +355,7 @@ impl<'tcx> Stack {
348355
let first_incompatible_idx = self.find_first_write_incompatible(granting_idx);
349356
for item in self.borrows.drain(first_incompatible_idx..).rev() {
350357
trace!("access: popping item {:?}", item);
351-
Stack::check_protector(&item, Some(tag), global)?;
358+
Stack::check_protector(&item, Some((tag, access)), global)?;
352359
}
353360
} else {
354361
// On a read, *disable* all `Unique` above the granting item. This ensures U2 for read accesses.
@@ -363,7 +370,7 @@ impl<'tcx> Stack {
363370
let item = &mut self.borrows[idx];
364371
if item.perm == Permission::Unique {
365372
trace!("access: disabling item {:?}", item);
366-
Stack::check_protector(item, Some(tag), global)?;
373+
Stack::check_protector(item, Some((tag, access)), global)?;
367374
item.perm = Permission::Disabled;
368375
}
369376
}

0 commit comments

Comments
 (0)