Skip to content

Commit 22f5aad

Browse files
borsbors
committed
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.
2 parents edc6fc1 + 6235604 commit 22f5aad

16 files changed

+229
-18
lines changed

‎src/tools/miri/src/diagnostics.rs

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,11 +304,21 @@ pub fn report_error<'tcx, 'mir>(
304304
(None, format!("this usually indicates that your program performed an invalid operation and caused Undefined Behavior")),
305305
(None, format!("but due to `-Zmiri-symbolic-alignment-check`, alignment errors can also be false positives")),
306306
],
307-
UndefinedBehavior(_) =>
308-
vec![
307+
UndefinedBehavior(info) => {
308+
let mut helps = vec![
309309
(None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")),
310310
(None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")),
311-
],
311+
];
312+
if let UndefinedBehaviorInfo::PointerUseAfterFree(alloc_id, _) = info {
313+
if let Some(span) = ecx.machine.allocated_span(*alloc_id) {
314+
helps.push((Some(span), format!("{:?} was allocated here:", alloc_id)));
315+
}
316+
if let Some(span) = ecx.machine.deallocated_span(*alloc_id) {
317+
helps.push((Some(span), format!("{:?} was deallocated here:", alloc_id)));
318+
}
319+
}
320+
helps
321+
}
312322
InvalidProgram(
313323
InvalidProgramInfo::AlreadyReported(_)
314324
) => {

‎src/tools/miri/src/machine.rs

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use rustc_middle::{
2525
},
2626
};
2727
use rustc_span::def_id::{CrateNum, DefId};
28-
use rustc_span::Symbol;
28+
use rustc_span::{Span, SpanData, Symbol};
2929
use rustc_target::abi::{Align, Size};
3030
use rustc_target::spec::abi::Abi;
3131

@@ -135,6 +135,19 @@ impl MayLeak for MiriMemoryKind {
135135
}
136136
}
137137

138+
impl MiriMemoryKind {
139+
/// Whether we have a useful allocation span for an allocation of this kind.
140+
fn should_save_allocation_span(self) -> bool {
141+
use self::MiriMemoryKind::*;
142+
match self {
143+
// Heap allocations are fine since the `Allocation` is created immediately.
144+
Rust | Miri | C | WinHeap | Mmap => true,
145+
// Everything else is unclear, let's not show potentially confusing spans.
146+
Machine | Global | ExternStatic | Tls | Runtime => false,
147+
}
148+
}
149+
}
150+
138151
impl fmt::Display for MiriMemoryKind {
139152
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
140153
use self::MiriMemoryKind::*;
@@ -497,6 +510,10 @@ pub struct MiriMachine<'mir, 'tcx> {
497510

498511
/// Whether to collect a backtrace when each allocation is created, just in case it leaks.
499512
pub(crate) collect_leak_backtraces: bool,
513+
514+
/// The spans we will use to report where an allocation was created and deallocated in
515+
/// diagnostics.
516+
pub(crate) allocation_spans: RefCell<FxHashMap<AllocId, (Span, Option<Span>)>>,
500517
}
501518

502519
impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
@@ -621,6 +638,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
621638
stack_addr,
622639
stack_size,
623640
collect_leak_backtraces: config.collect_leak_backtraces,
641+
allocation_spans: RefCell::new(FxHashMap::default()),
624642
}
625643
}
626644

@@ -742,6 +760,21 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> {
742760
pub(crate) fn page_align(&self) -> Align {
743761
Align::from_bytes(self.page_size).unwrap()
744762
}
763+
764+
pub(crate) fn allocated_span(&self, alloc_id: AllocId) -> Option<SpanData> {
765+
self.allocation_spans
766+
.borrow()
767+
.get(&alloc_id)
768+
.map(|(allocated, _deallocated)| allocated.data())
769+
}
770+
771+
pub(crate) fn deallocated_span(&self, alloc_id: AllocId) -> Option<SpanData> {
772+
self.allocation_spans
773+
.borrow()
774+
.get(&alloc_id)
775+
.and_then(|(_allocated, deallocated)| *deallocated)
776+
.map(Span::data)
777+
}
745778
}
746779

747780
impl VisitTags for MiriMachine<'_, '_> {
@@ -791,6 +824,7 @@ impl VisitTags for MiriMachine<'_, '_> {
791824
stack_addr: _,
792825
stack_size: _,
793826
collect_leak_backtraces: _,
827+
allocation_spans: _,
794828
} = self;
795829

796830
threads.visit_tags(visit);
@@ -1051,6 +1085,14 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
10511085
},
10521086
|ptr| ecx.global_base_pointer(ptr),
10531087
)?;
1088+
1089+
if matches!(kind, MemoryKind::Machine(kind) if kind.should_save_allocation_span()) {
1090+
ecx.machine
1091+
.allocation_spans
1092+
.borrow_mut()
1093+
.insert(id, (ecx.machine.current_span(), None));
1094+
}
1095+
10541096
Ok(Cow::Owned(alloc))
10551097
}
10561098

@@ -1181,6 +1223,10 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> {
11811223
if let Some(borrow_tracker) = &mut alloc_extra.borrow_tracker {
11821224
borrow_tracker.before_memory_deallocation(alloc_id, prove_extra, range, machine)?;
11831225
}
1226+
if let Some((_, deallocated_at)) = machine.allocation_spans.borrow_mut().get_mut(&alloc_id)
1227+
{
1228+
*deallocated_at = Some(machine.current_span());
1229+
}
11841230
Ok(())
11851231
}
11861232

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/deallocate-twice.rs:LL:CC
11+
|
12+
LL | let x = alloc(Layout::from_size_align_unchecked(1, 1));
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/deallocate-twice.rs:LL:CC
16+
|
17+
LL | dealloc(x, Layout::from_size_align_unchecked(1, 1));
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `std::alloc::dealloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
1121
note: inside `main`
1222
--> $DIR/deallocate-twice.rs:LL:CC

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | let _z = *x;
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/reallocate-change-alloc.rs:LL:CC
11+
|
12+
LL | let x = alloc(Layout::from_size_align_unchecked(1, 1));
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/reallocate-change-alloc.rs:LL:CC
16+
|
17+
LL | let _y = realloc(x, Layout::from_size_align_unchecked(1, 1), 1);
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `main` at $DIR/reallocate-change-alloc.rs:LL:CC
1121

1222
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | unsafe { __rust_realloc(ptr, layout.size(), layout.align(), new_size) }
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/reallocate-dangling.rs:LL:CC
11+
|
12+
LL | let x = alloc(Layout::from_size_align_unchecked(1, 1));
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/reallocate-dangling.rs:LL:CC
16+
|
17+
LL | dealloc(x, Layout::from_size_align_unchecked(1, 1));
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `std::alloc::realloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
1121
note: inside `main`
1222
--> $DIR/reallocate-dangling.rs:LL:CC

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | let x = unsafe { ptr::addr_of!(*p) };
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/dangling_pointer_addr_of.rs:LL:CC
11+
|
12+
LL | let b = Box::new(42);
13+
| ^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/dangling_pointer_addr_of.rs:LL:CC
16+
|
17+
LL | };
18+
| ^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `main` at RUSTLIB/core/src/ptr/mod.rs:LL:CC
1121
= note: this error originates in the macro `ptr::addr_of` (in Nightly builds, run with -Z macro-backtrace for more info)
1222

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | let x = unsafe { *p };
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/dangling_pointer_deref.rs:LL:CC
11+
|
12+
LL | let b = Box::new(42);
13+
| ^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/dangling_pointer_deref.rs:LL:CC
16+
|
17+
LL | };
18+
| ^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `main` at $DIR/dangling_pointer_deref.rs:LL:CC
1121

1222
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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | let x = unsafe { p.offset(42) };
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/dangling_pointer_offset.rs:LL:CC
11+
|
12+
LL | let b = Box::new(42);
13+
| ^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/dangling_pointer_offset.rs:LL:CC
16+
|
17+
LL | };
18+
| ^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `main` at $DIR/dangling_pointer_offset.rs:LL:CC
1121

1222
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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | let _ = *p;
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/dangling_pointer_project_underscore.rs:LL:CC
11+
|
12+
LL | let b = Box::new(42);
13+
| ^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/dangling_pointer_project_underscore.rs:LL:CC
16+
|
17+
LL | };
18+
| ^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `main` at $DIR/dangling_pointer_project_underscore.rs:LL:CC
1121

1222
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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | let _x = unsafe { *p };
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/dangling_zst_deref.rs:LL:CC
11+
|
12+
LL | let b = Box::new(42);
13+
| ^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/dangling_zst_deref.rs:LL:CC
16+
|
17+
LL | };
18+
| ^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `main` at $DIR/dangling_zst_deref.rs:LL:CC
1121

1222
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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,21 @@ LL | *ptr.0
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/dealloc_read_race2.rs:LL:CC
11+
|
12+
LL | let pointer: *mut usize = Box::into_raw(Box::new(0usize));
13+
| ^^^^^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/dealloc_read_race2.rs:LL:CC
16+
|
17+
LL | / __rust_dealloc(
18+
LL | | ptr.0 as *mut _,
19+
LL | | std::mem::size_of::<usize>(),
20+
LL | | std::mem::align_of::<usize>(),
21+
LL | | )
22+
| |_____________^
23+
= note: BACKTRACE (of the first span):
1024
= note: inside closure at $DIR/dealloc_read_race2.rs:LL:CC
1125

1226
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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,21 @@ LL | *ptr.0 = 2;
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/dealloc_write_race2.rs:LL:CC
11+
|
12+
LL | let pointer: *mut usize = Box::into_raw(Box::new(0usize));
13+
| ^^^^^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/dealloc_write_race2.rs:LL:CC
16+
|
17+
LL | / __rust_dealloc(
18+
LL | | ptr.0 as *mut _,
19+
LL | | std::mem::size_of::<usize>(),
20+
LL | | std::mem::align_of::<usize>(),
21+
LL | | );
22+
| |_____________^
23+
= note: BACKTRACE (of the first span):
1024
= note: inside closure at $DIR/dealloc_write_race2.rs:LL:CC
1125

1226
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | *num += 1;
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/generator-pinned-moved.rs:LL:CC
11+
|
12+
LL | let mut generator_iterator = Box::new(GeneratorIteratorAdapter(firstn()));
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/generator-pinned-moved.rs:LL:CC
16+
|
17+
LL | }; // *deallocate* generator_iterator
18+
| ^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside closure at $DIR/generator-pinned-moved.rs:LL:CC
1121
note: inside `<GeneratorIteratorAdapter<[static generator@$DIR/generator-pinned-moved.rs:LL:CC]> as std::iter::Iterator>::next`
1222
--> $DIR/generator-pinned-moved.rs:LL:CC

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ LL | assert_eq!(42, **unsafe { &*Weak::as_ptr(&weak) });
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9-
= note: BACKTRACE:
9+
help: ALLOC was allocated here:
10+
--> $DIR/rc_as_ptr.rs:LL:CC
11+
|
12+
LL | let strong = Rc::new(Box::new(42));
13+
| ^^^^^^^^^^^^
14+
help: ALLOC was deallocated here:
15+
--> $DIR/rc_as_ptr.rs:LL:CC
16+
|
17+
LL | drop(strong);
18+
| ^^^^^^^^^^^^
19+
= note: BACKTRACE (of the first span):
1020
= note: inside `main` at RUSTLIB/core/src/macros/mod.rs:LL:CC
1121
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
1222

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,24 @@ LL | let _x = *(ptr as *mut u8);
2121
|
2222
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
2323
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
24-
= note: BACKTRACE:
24+
help: ALLOC was allocated here:
25+
--> $DIR/mmap_use_after_munmap.rs:LL:CC
26+
|
27+
LL | let ptr = libc::mmap(
28+
| ___________________^
29+
LL | | std::ptr::null_mut(),
30+
LL | | 4096,
31+
LL | | libc::PROT_READ | libc::PROT_WRITE,
32+
... |
33+
LL | | 0,
34+
LL | | );
35+
| |_________^
36+
help: ALLOC was deallocated here:
37+
--> $DIR/mmap_use_after_munmap.rs:LL:CC
38+
|
39+
LL | libc::munmap(ptr, 4096);
40+
| ^^^^^^^^^^^^^^^^^^^^^^^
41+
= note: BACKTRACE (of the first span):
2542
= note: inside `main` at $DIR/mmap_use_after_munmap.rs:LL:CC
2643

2744
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

0 commit comments

Comments
 (0)