Auto merge of rust-lang#2308 - RalfJung:ptr-tacking, r=RalfJung

pointer tag tracking: on creation, log the offsets it is created for

Hopefully this makes things like Manishearth/triomphe#38 easier to diagnose.

src/diagnostics.rs

@@ -67,7 +67,7 @@ impl MachineStopType for TerminationInfo {}
 
 /// Miri specific diagnostics
 pub enum NonHaltingDiagnostic {
-    CreatedPointerTag(NonZeroU64),
+    CreatedPointerTag(NonZeroU64, Option<(AllocId, AllocRange)>),
     /// This `Item` was popped from the borrow stack, either due to an access with the given tag or
     /// a deallocation when the second argument is `None`.
     PoppedPointerTag(Item, Option<(SbTagExtra, AccessKind)>),
@@ -318,7 +318,7 @@ fn report_msg<'mir, 'tcx>(
     diag_level: DiagLevel,
     title: &str,
     span_msg: Vec<String>,
-    mut helps: Vec<(Option<SpanData>, String)>,
+    helps: Vec<(Option<SpanData>, String)>,
     stacktrace: &[FrameInfo<'tcx>],
 ) {
     let span = stacktrace.first().map_or(DUMMY_SP, |fi| fi.span);
@@ -344,15 +344,15 @@ fn report_msg<'mir, 'tcx>(
 
     // Show help messages.
     if !helps.is_empty() {
-        // Add visual separator before backtrace.
-        helps.last_mut().unwrap().1.push('\n');
         for (span_data, help) in helps {
             if let Some(span_data) = span_data {
                 err.span_help(span_data.span(), &help);
             } else {
                 err.help(&help);
             }
         }
+        // Add visual separator before backtrace.
+        err.note("backtrace:");
     }
     // Add backtrace
     for (idx, frame_info) in stacktrace.iter().enumerate() {
@@ -448,7 +448,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             for e in diagnostics.drain(..) {
                 use NonHaltingDiagnostic::*;
                 let msg = match e {
-                    CreatedPointerTag(tag) => format!("created tag {:?}", tag),
+                    CreatedPointerTag(tag, None) => format!("created tag {tag:?}"),
+                    CreatedPointerTag(tag, Some((alloc_id, range))) =>
+                        format!("created tag {tag:?} at {alloc_id}{}", HexRange(range)),
                     PoppedPointerTag(item, tag) =>
                         match tag {
                             None =>

src/stacked_borrows.rs

@@ -222,11 +222,9 @@ impl GlobalStateInner {
         }
     }
 
+    /// Generates a new pointer tag. Remember to also check track_pointer_tags and log its creation!
     fn new_ptr(&mut self) -> SbTag {
         let id = self.next_ptr_tag;
-        if self.tracked_pointer_tags.contains(&id) {
-            register_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(id.0));
-        }
         self.next_ptr_tag = SbTag(NonZeroU64::new(id.0.get() + 1).unwrap());
         id
     }
@@ -253,6 +251,9 @@ impl GlobalStateInner {
     pub fn base_ptr_tag(&mut self, id: AllocId) -> SbTag {
         self.base_ptr_tags.get(&id).copied().unwrap_or_else(|| {
             let tag = self.new_ptr();
+            if self.tracked_pointer_tags.contains(&tag) {
+                register_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(tag.0, None));
+            }
             trace!("New allocation {:?} has base tag {:?}", id, tag);
             self.base_ptr_tags.try_insert(id, tag).unwrap();
             tag
@@ -802,16 +803,30 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let this = self.eval_context_mut();
         let current_span = &mut this.machine.current_span();
 
+        // It is crucial that this gets called on all code paths, to ensure we track tag creation.
         let log_creation = |this: &MiriEvalContext<'mir, 'tcx>,
                             current_span: &mut CurrentSpan<'_, 'mir, 'tcx>,
-                            alloc_id,
-                            base_offset,
-                            orig_tag|
+                            loc: Option<(AllocId, Size, SbTagExtra)>| // alloc_id, base_offset, orig_tag
          -> InterpResult<'tcx> {
+            let global = this.machine.stacked_borrows.as_ref().unwrap().borrow();
+            if global.tracked_pointer_tags.contains(&new_tag) {
+                register_diagnostic(NonHaltingDiagnostic::CreatedPointerTag(
+                    new_tag.0,
+                    loc.map(|(alloc_id, base_offset, _)| (alloc_id, alloc_range(base_offset, size))),
+                ));
+            }
+            drop(global); // don't hold that reference any longer than we have to
+
+            let Some((alloc_id, base_offset, orig_tag)) = loc else {
+                return Ok(())
+            };
+
+            // The SB history tracking needs a parent tag, so skip if we come from a wildcard.
             let SbTagExtra::Concrete(orig_tag) = orig_tag else {
                 // FIXME: should we log this?
                 return Ok(())
             };
+
             let extra = this.get_alloc_extra(alloc_id)?;
             let mut stacked_borrows = extra
                 .stacked_borrows
@@ -846,14 +861,15 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             // Dangling slices are a common case here; it's valid to get their length but with raw
             // pointer tagging for example all calls to get_unchecked on them are invalid.
             if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr) {
-                log_creation(this, current_span, alloc_id, base_offset, orig_tag)?;
+                log_creation(this, current_span, Some((alloc_id, base_offset, orig_tag)))?;
                 return Ok(Some(alloc_id));
             }
             // This pointer doesn't come with an AllocId. :shrug:
+            log_creation(this, current_span, None)?;
             return Ok(None);
         }
         let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr)?;
-        log_creation(this, current_span, alloc_id, base_offset, orig_tag)?;
+        log_creation(this, current_span, Some((alloc_id, base_offset, orig_tag)))?;
 
         // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
         let (alloc_size, _) = this.get_live_alloc_size_and_align(alloc_id)?;

tests/fail/alloc/deallocate-bad-alignment.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/deallocate-bad-alignment.rs:LL:CC
   --> $DIR/deallocate-bad-alignment.rs:LL:CC

tests/fail/alloc/deallocate-bad-size.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/deallocate-bad-size.rs:LL:CC
   --> $DIR/deallocate-bad-size.rs:LL:CC

tests/fail/alloc/deallocate-twice.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/deallocate-twice.rs:LL:CC
   --> $DIR/deallocate-twice.rs:LL:CC

tests/fail/alloc/global_system_mixup.stderr

@@ -6,7 +6,7 @@ LL |         FREE();
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `std::sys::PLATFORM::alloc::<impl std::alloc::GlobalAlloc for std::alloc::System>::dealloc` at RUSTLIB/std/src/sys/PLATFORM/alloc.rs:LL:CC
    = note: inside `<std::alloc::System as std::alloc::Allocator>::deallocate` at RUSTLIB/std/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/global_system_mixup.rs:LL:CC

tests/fail/alloc/no_global_allocator.stderr

@@ -5,7 +5,7 @@ LL |         __rust_alloc(1, 1);
    |         ^^^^^^^^^^^^^^^^^^ can't call foreign function: __rust_alloc
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
-           
+   = note: backtrace:
    = note: inside `start` at $DIR/no_global_allocator.rs:LL:CC
 
 error: aborting due to previous error

tests/fail/alloc/reallocate-bad-size.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_realloc(ptr, layout.size(), layout.align(), new_size) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `std::alloc::realloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/reallocate-bad-size.rs:LL:CC
   --> $DIR/reallocate-bad-size.rs:LL:CC

tests/fail/alloc/reallocate-change-alloc.stderr

@@ -6,7 +6,7 @@ LL |         let _z = *x;
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/reallocate-change-alloc.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/alloc/reallocate-dangling.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_realloc(ptr, layout.size(), layout.align(), new_size) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `std::alloc::realloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main` at $DIR/reallocate-dangling.rs:LL:CC
   --> $DIR/reallocate-dangling.rs:LL:CC

tests/fail/alloc/stack_free.stderr

@@ -6,7 +6,7 @@ LL |     unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
    = note: inside `<std::alloc::Global as std::alloc::Allocator>::deallocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
    = note: inside `alloc::alloc::box_free::<i32, std::alloc::Global>` at RUSTLIB/alloc/src/alloc.rs:LL:CC

tests/fail/backtrace/bad-backtrace-decl.stderr

@@ -6,7 +6,7 @@ LL | ...   miri_resolve_frame(*frame, 0);
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/bad-backtrace-decl.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/backtrace/bad-backtrace-flags.stderr

@@ -5,7 +5,7 @@ LL |         miri_get_backtrace(2, std::ptr::null_mut());
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unknown `miri_get_backtrace` flags 2
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/bad-backtrace-flags.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/backtrace/bad-backtrace-ptr.stderr

@@ -6,7 +6,7 @@ LL |         miri_resolve_frame(std::ptr::null_mut(), 0);
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/bad-backtrace-ptr.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/backtrace/bad-backtrace-resolve-flags.stderr

@@ -5,7 +5,7 @@ LL |         miri_resolve_frame(buf[0], 2);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unknown `miri_resolve_frame` flags 2
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/bad-backtrace-resolve-flags.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/backtrace/bad-backtrace-resolve-names-flags.stderr

@@ -5,7 +5,7 @@ LL | ...   miri_resolve_frame_names(buf[0], 2, std::ptr::null_mut(), std::ptr::n
    |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unknown `miri_resolve_frame_names` flags 2
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/bad-backtrace-resolve-names-flags.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/backtrace/bad-backtrace-size-flags.stderr

@@ -5,7 +5,7 @@ LL |         miri_backtrace_size(2);
    |         ^^^^^^^^^^^^^^^^^^^^^^ unknown `miri_backtrace_size` flags 2
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/bad-backtrace-size-flags.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/box-cell-alias.stderr

@@ -19,6 +19,7 @@ help: <TAG> was later invalidated at offsets [0x0..0x1]
    |
 LL |     let res = helper(val, ptr);
    |                      ^^^
+   = note: backtrace:
    = note: inside `helper` at $DIR/box-cell-alias.rs:LL:CC
 note: inside `main` at $DIR/box-cell-alias.rs:LL:CC
   --> $DIR/box-cell-alias.rs:LL:CC

tests/fail/branchless-select-i128-pointer.stderr

@@ -10,7 +10,7 @@ LL | |             )
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/branchless-select-i128-pointer.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/concurrency/libc_pthread_join_detached.stderr

@@ -6,7 +6,7 @@ LL | ...   assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/libc_pthread_join_detached.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/concurrency/libc_pthread_join_joined.stderr

@@ -6,7 +6,7 @@ LL | ...   assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/libc_pthread_join_joined.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/concurrency/libc_pthread_join_main.stderr

@@ -6,7 +6,7 @@ LL | ...   assert_eq!(libc::pthread_join(thread_id, ptr::null_mut()), 0);
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside closure at $DIR/libc_pthread_join_main.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/concurrency/libc_pthread_join_multiple.stderr

@@ -6,7 +6,7 @@ LL | ...   assert_eq!(libc::pthread_join(native_copy, ptr::null_mut()), 0);
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside closure at $DIR/libc_pthread_join_multiple.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/concurrency/libc_pthread_join_self.stderr

@@ -6,7 +6,7 @@ LL |             assert_eq!(libc::pthread_join(native, ptr::null_mut()), 0);
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside closure at $DIR/libc_pthread_join_self.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/concurrency/thread-spawn.stderr

@@ -12,7 +12,7 @@ LL | |         );
    | |_________^ can't create threads on Windows
    |
    = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
-           
+   = note: backtrace:
    = note: inside `std::sys::PLATFORM::thread::Thread::new` at RUSTLIB/std/src/sys/PLATFORM/thread.rs:LL:CC
    = note: inside `std::thread::Builder::spawn_unchecked_::<[closure@$DIR/thread-spawn.rs:LL:CC], ()>` at RUSTLIB/std/src/thread/mod.rs:LL:CC
    = note: inside `std::thread::Builder::spawn_unchecked::<[closure@$DIR/thread-spawn.rs:LL:CC], ()>` at RUSTLIB/std/src/thread/mod.rs:LL:CC

tests/fail/concurrency/thread_local_static_dealloc.stderr

@@ -6,7 +6,7 @@ LL |         let _val = *dangling_ptr.0;
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `main` at $DIR/thread_local_static_dealloc.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

tests/fail/concurrency/too_few_args.stderr

@@ -6,7 +6,7 @@ LL |     panic!()
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `thread_start` at RUSTLIB/std/src/panic.rs:LL:CC
    = note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
 

tests/fail/concurrency/too_many_args.stderr

@@ -6,7 +6,7 @@ LL |     panic!()
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `thread_start` at RUSTLIB/std/src/panic.rs:LL:CC
    = note: this error originates in the macro `$crate::panic::panic_2015` (in Nightly builds, run with -Z macro-backtrace for more info)
 

tests/fail/concurrency/unwind_top_of_stack.stderr

@@ -11,7 +11,7 @@ LL | | }
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
-           
+   = note: backtrace:
    = note: inside `thread_start` at $DIR/unwind_top_of_stack.rs:LL:CC
 
 error: aborting due to previous error