Auto merge of rust-lang#2940 - saethlin:use-after-free-spans, r=RalfJung

When reporting a heap use-after-free, say where the allocation was allocated and deallocated

This is a partial solution to: rust-lang/miri#2917

Currently in the interpreter, we only have accurate information for where heap allocations are allocated and deallocated (see rust-lang/miri#2940 (comment)). So this just implements support for allocations where the information is already available, and the full support will require more interpreter tweaks.

src/tools/miri/src/diagnostics.rs

@@ -304,11 +304,21 @@ pub fn report_error<'tcx, 'mir>(
                     (None, format!("this usually indicates that your program performed an invalid operation and caused Undefined Behavior")),
                     (None, format!("but due to `-Zmiri-symbolic-alignment-check`, alignment errors can also be false positives")),
                 ],
-            UndefinedBehavior(_) =>
-                vec![
+            UndefinedBehavior(info) => {
+                let mut helps = vec![
                     (None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")),
                     (None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")),
-                ],
+                ];
+                if let UndefinedBehaviorInfo::PointerUseAfterFree(alloc_id, _) = info {
+                    if let Some(span) = ecx.machine.allocated_span(*alloc_id) {
+                        helps.push((Some(span), format!("{:?} was allocated here:", alloc_id)));
+                    }
+                    if let Some(span) = ecx.machine.deallocated_span(*alloc_id) {
+                        helps.push((Some(span), format!("{:?} was deallocated here:", alloc_id)));
+                    }
+                }
+                helps
+            }
             InvalidProgram(
                 InvalidProgramInfo::AlreadyReported(_)
             ) => {

src/tools/miri/src/machine.rs

@@ -25,7 +25,7 @@ use rustc_middle::{
     },
 };
 use rustc_span::def_id::{CrateNum, DefId};
-use rustc_span::Symbol;
+use rustc_span::{Span, SpanData, Symbol};
 use rustc_target::abi::{Align, Size};
 use rustc_target::spec::abi::Abi;
 
@@ -135,6 +135,19 @@ impl MayLeak for MiriMemoryKind {
     }
 }
 
+impl MiriMemoryKind {
+    /// Whether we have a useful allocation span for an allocation of this kind.
+    fn should_save_allocation_span(self) -> bool {
+        use self::MiriMemoryKind::*;
+        match self {
+            // Heap allocations are fine since the `Allocation` is created immediately.
+            Rust | Miri | C | WinHeap | Mmap => true,
+            // Everything else is unclear, let's not show potentially confusing spans.
+            Machine | Global | ExternStatic | Tls | Runtime => false,
+        }
+    }
+}
+
 impl fmt::Display for MiriMemoryKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use self::MiriMemoryKind::*;
@@ -497,6 +510,10 @@ pub struct MiriMachine<'mir, 'tcx> {
 
     /// Whether to collect a backtrace when each allocation is created, just in case it leaks.
     pub(crate) collect_leak_backtraces: bool,
+
+    /// The spans we will use to report where an allocation was created and deallocated in
+    /// diagnostics.
+    pub(crate) allocation_spans: RefCell<FxHashMap<AllocId, (Span, Option<Span>)>>,
 }
 
 impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
@@ -621,6 +638,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
             stack_addr,
             stack_size,
             collect_leak_backtraces: config.collect_leak_backtraces,
+            allocation_spans: RefCell::new(FxHashMap::default()),
         }
     }
 
@@ -742,6 +760,21 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
     pub(crate) fn page_align(&self) -> Align {
         Align::from_bytes(self.page_size).unwrap()
     }
+
+    pub(crate) fn allocated_span(&self, alloc_id: AllocId) -> Option<SpanData> {
+        self.allocation_spans
+            .borrow()
+            .get(&alloc_id)
+            .map(|(allocated, _deallocated)| allocated.data())
+    }
+
+    pub(crate) fn deallocated_span(&self, alloc_id: AllocId) -> Option<SpanData> {
+        self.allocation_spans
+            .borrow()
+            .get(&alloc_id)
+            .and_then(|(_allocated, deallocated)| *deallocated)
+            .map(Span::data)
+    }
 }
 
 impl VisitTags for MiriMachine<'_, '_> {
@@ -791,6 +824,7 @@ impl VisitTags for MiriMachine<'_, '_> {
             stack_addr: _,
             stack_size: _,
             collect_leak_backtraces: _,
+            allocation_spans: _,
         } = self;
 
         threads.visit_tags(visit);
@@ -1051,6 +1085,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
             },
             |ptr| ecx.global_base_pointer(ptr),
         )?;
+
+        if matches!(kind, MemoryKind::Machine(kind) if kind.should_save_allocation_span()) {
+            ecx.machine
+                .allocation_spans
+                .borrow_mut()
+                .insert(id, (ecx.machine.current_span(), None));
+        }
+
         Ok(Cow::Owned(alloc))
     }
 
@@ -1181,6 +1223,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
         if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker {
             borrow_tracker.before_memory_deallocation(alloc_id, prove_extra, range, machine)?;
         }
+        if let Some((_, deallocated_at)) = machine.allocation_spans.borrow_mut().get_mut(&alloc_id)
+        {
+            *deallocated_at = Some(machine.current_span());
+        }
         Ok(())
     }
 

src/tools/miri/tests/fail/alloc/deallocate-twice.stderr

@@ -6,7 +6,17 @@ 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:
+help: ALLOC was allocated here:
+  --> $DIR/deallocate-twice.rs:LL:CC
+   |
+LL |         let x = alloc(Layout::from_size_align_unchecked(1, 1));
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/deallocate-twice.rs:LL:CC
+   |
+LL |         dealloc(x, Layout::from_size_align_unchecked(1, 1));
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: BACKTRACE (of the first span):
    = note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main`
   --> $DIR/deallocate-twice.rs:LL:CC

src/tools/miri/tests/fail/alloc/reallocate-change-alloc.stderr

@@ -6,7 +6,17 @@ 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:
+help: ALLOC was allocated here:
+  --> $DIR/reallocate-change-alloc.rs:LL:CC
+   |
+LL |         let x = alloc(Layout::from_size_align_unchecked(1, 1));
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/reallocate-change-alloc.rs:LL:CC
+   |
+LL |         let _y = realloc(x, Layout::from_size_align_unchecked(1, 1), 1);
+   |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: BACKTRACE (of the first span):
    = 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

src/tools/miri/tests/fail/alloc/reallocate-dangling.stderr

@@ -6,7 +6,17 @@ 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:
+help: ALLOC was allocated here:
+  --> $DIR/reallocate-dangling.rs:LL:CC
+   |
+LL |         let x = alloc(Layout::from_size_align_unchecked(1, 1));
+   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/reallocate-dangling.rs:LL:CC
+   |
+LL |         dealloc(x, Layout::from_size_align_unchecked(1, 1));
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = note: BACKTRACE (of the first span):
    = note: inside `std::alloc::realloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
 note: inside `main`
   --> $DIR/reallocate-dangling.rs:LL:CC

src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_addr_of.stderr

@@ -6,7 +6,17 @@ LL |     let x = unsafe { ptr::addr_of!(*p) };
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/dangling_pointer_addr_of.rs:LL:CC
+   |
+LL |         let b = Box::new(42);
+   |                 ^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/dangling_pointer_addr_of.rs:LL:CC
+   |
+LL |     };
+   |     ^
+   = note: BACKTRACE (of the first span):
    = note: inside `main` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
    = note: this error originates in the macro `ptr::addr_of` (in Nightly builds, run with -Z macro-backtrace for more info)
 

src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_deref.stderr

@@ -6,7 +6,17 @@ LL |     let x = unsafe { *p };
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/dangling_pointer_deref.rs:LL:CC
+   |
+LL |         let b = Box::new(42);
+   |                 ^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/dangling_pointer_deref.rs:LL:CC
+   |
+LL |     };
+   |     ^
+   = note: BACKTRACE (of the first span):
    = note: inside `main` at $DIR/dangling_pointer_deref.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_offset.stderr

@@ -6,7 +6,17 @@ LL |     let x = unsafe { p.offset(42) };
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/dangling_pointer_offset.rs:LL:CC
+   |
+LL |         let b = Box::new(42);
+   |                 ^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/dangling_pointer_offset.rs:LL:CC
+   |
+LL |     };
+   |     ^
+   = note: BACKTRACE (of the first span):
    = note: inside `main` at $DIR/dangling_pointer_offset.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

src/tools/miri/tests/fail/dangling_pointers/dangling_pointer_project_underscore.stderr

@@ -6,7 +6,17 @@ LL |         let _ = *p;
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/dangling_pointer_project_underscore.rs:LL:CC
+   |
+LL |         let b = Box::new(42);
+   |                 ^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/dangling_pointer_project_underscore.rs:LL:CC
+   |
+LL |     };
+   |     ^
+   = note: BACKTRACE (of the first span):
    = note: inside `main` at $DIR/dangling_pointer_project_underscore.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

src/tools/miri/tests/fail/dangling_pointers/dangling_zst_deref.stderr

@@ -6,7 +6,17 @@ LL |     let _x = unsafe { *p };
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/dangling_zst_deref.rs:LL:CC
+   |
+LL |         let b = Box::new(42);
+   |                 ^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/dangling_zst_deref.rs:LL:CC
+   |
+LL |     };
+   |     ^
+   = note: BACKTRACE (of the first span):
    = note: inside `main` at $DIR/dangling_zst_deref.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

src/tools/miri/tests/fail/data_race/dealloc_read_race2.stderr

@@ -6,7 +6,21 @@ LL |             *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:
+help: ALLOC was allocated here:
+  --> $DIR/dealloc_read_race2.rs:LL:CC
+   |
+LL |     let pointer: *mut usize = Box::into_raw(Box::new(0usize));
+   |                                             ^^^^^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/dealloc_read_race2.rs:LL:CC
+   |
+LL | /             __rust_dealloc(
+LL | |                 ptr.0 as *mut _,
+LL | |                 std::mem::size_of::<usize>(),
+LL | |                 std::mem::align_of::<usize>(),
+LL | |             )
+   | |_____________^
+   = note: BACKTRACE (of the first span):
    = note: inside closure at $DIR/dealloc_read_race2.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

src/tools/miri/tests/fail/data_race/dealloc_write_race2.stderr

@@ -6,7 +6,21 @@ LL |             *ptr.0 = 2;
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/dealloc_write_race2.rs:LL:CC
+   |
+LL |     let pointer: *mut usize = Box::into_raw(Box::new(0usize));
+   |                                             ^^^^^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/dealloc_write_race2.rs:LL:CC
+   |
+LL | /             __rust_dealloc(
+LL | |                 ptr.0 as *mut _,
+LL | |                 std::mem::size_of::<usize>(),
+LL | |                 std::mem::align_of::<usize>(),
+LL | |             );
+   | |_____________^
+   = note: BACKTRACE (of the first span):
    = note: inside closure at $DIR/dealloc_write_race2.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

src/tools/miri/tests/fail/generator-pinned-moved.stderr

@@ -6,7 +6,17 @@ LL |         *num += 1;
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/generator-pinned-moved.rs:LL:CC
+   |
+LL |         let mut generator_iterator = Box::new(GeneratorIteratorAdapter(firstn()));
+   |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/generator-pinned-moved.rs:LL:CC
+   |
+LL |     }; // *deallocate* generator_iterator
+   |     ^
+   = note: BACKTRACE (of the first span):
    = note: inside closure at $DIR/generator-pinned-moved.rs:LL:CC
 note: inside `<GeneratorIteratorAdapter<[static generator@$DIR/generator-pinned-moved.rs:LL:CC]> as std::iter::Iterator>::next`
   --> $DIR/generator-pinned-moved.rs:LL:CC

src/tools/miri/tests/fail/rc_as_ptr.stderr

@@ -6,7 +6,17 @@ LL |     assert_eq!(42, **unsafe { &*Weak::as_ptr(&weak) });
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/rc_as_ptr.rs:LL:CC
+   |
+LL |     let strong = Rc::new(Box::new(42));
+   |                          ^^^^^^^^^^^^
+help: ALLOC was deallocated here:
+  --> $DIR/rc_as_ptr.rs:LL:CC
+   |
+LL |     drop(strong);
+   |     ^^^^^^^^^^^^
+   = note: BACKTRACE (of the first span):
    = note: inside `main` at RUSTLIB/core/src/macros/mod.rs:LL:CC
    = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
 

src/tools/miri/tests/fail/shims/mmap_use_after_munmap.stderr

@@ -21,7 +21,24 @@ LL |         let _x = *(ptr as *mut u8);
    |
    = 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:
+help: ALLOC was allocated here:
+  --> $DIR/mmap_use_after_munmap.rs:LL:CC
+   |
+LL |           let ptr = libc::mmap(
+   |  ___________________^
+LL | |             std::ptr::null_mut(),
+LL | |             4096,
+LL | |             libc::PROT_READ | libc::PROT_WRITE,
+...  |
+LL | |             0,
+LL | |         );
+   | |_________^
+help: ALLOC was deallocated here:
+  --> $DIR/mmap_use_after_munmap.rs:LL:CC
+   |
+LL |         libc::munmap(ptr, 4096);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^
+   = note: BACKTRACE (of the first span):
    = note: inside `main` at $DIR/mmap_use_after_munmap.rs:LL:CC
 
 note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace