Skip to content

Commit edf0b25

Browse files
committed
properly track why we checked whether a pointer is in-bounds
also simplify the in-bounds checking in Miri's borrow trackers
1 parent 4771879 commit edf0b25

36 files changed

+102
-102
lines changed

src/borrow_tracker/stacked_borrows/mod.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use rustc_middle::ty::{
1818
layout::{HasParamEnv, LayoutOf},
1919
Ty,
2020
};
21-
use rustc_target::abi::{Abi, Size};
21+
use rustc_target::abi::{Abi, Align, Size};
2222

2323
use crate::borrow_tracker::{
2424
stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder},
@@ -619,6 +619,8 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
619619
retag_info: RetagInfo, // diagnostics info about this retag
620620
) -> InterpResult<'tcx, Option<AllocId>> {
621621
let this = self.eval_context_mut();
622+
// Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
623+
this.check_ptr_access_align(place.ptr, size, Align::ONE, CheckInAllocMsg::InboundsTest)?;
622624

623625
// It is crucial that this gets called on all code paths, to ensure we track tag creation.
624626
let log_creation = |this: &MiriInterpCx<'mir, 'tcx>,
@@ -707,18 +709,6 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
707709
let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr)?;
708710
log_creation(this, Some((alloc_id, base_offset, orig_tag)))?;
709711

710-
// Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
711-
let (alloc_size, _) = this.get_live_alloc_size_and_align(alloc_id)?;
712-
if base_offset + size > alloc_size {
713-
throw_ub!(PointerOutOfBounds {
714-
alloc_id,
715-
alloc_size,
716-
ptr_offset: this.target_usize_to_isize(base_offset.bytes()),
717-
ptr_size: size,
718-
msg: CheckInAllocMsg::InboundsTest
719-
});
720-
}
721-
722712
trace!(
723713
"reborrow: reference {:?} derived from {:?} (pointee {}): {:?}, size {}",
724714
new_tag,

src/borrow_tracker/tree_borrows/mod.rs

Lines changed: 23 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use log::trace;
22

3-
use rustc_target::abi::{Abi, Size};
3+
use rustc_target::abi::{Abi, Align, Size};
44

55
use crate::borrow_tracker::{AccessKind, GlobalStateInner, ProtectorKind, RetagFields};
66
use rustc_middle::{
@@ -182,6 +182,8 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
182182
new_tag: BorTag,
183183
) -> InterpResult<'tcx, Option<(AllocId, BorTag)>> {
184184
let this = self.eval_context_mut();
185+
// Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
186+
this.check_ptr_access_align(place.ptr, ptr_size, Align::ONE, CheckInAllocMsg::InboundsTest)?;
185187

186188
// It is crucial that this gets called on all code paths, to ensure we track tag creation.
187189
let log_creation = |this: &MiriInterpCx<'mir, 'tcx>,
@@ -202,51 +204,33 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'
202204
};
203205

204206
trace!("Reborrow of size {:?}", ptr_size);
205-
let (alloc_id, base_offset, parent_prov) = if ptr_size > Size::ZERO {
206-
this.ptr_get_alloc_id(place.ptr)?
207-
} else {
208-
match this.ptr_try_get_alloc_id(place.ptr) {
209-
Ok(data) => data,
210-
Err(_) => {
211-
// This pointer doesn't come with an AllocId, so there's no
212-
// memory to do retagging in.
213-
trace!(
214-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
215-
new_tag,
216-
place.ptr,
217-
place.layout.ty,
218-
);
219-
log_creation(this, None)?;
220-
return Ok(None);
221-
}
207+
let (alloc_id, base_offset, parent_prov) = match this.ptr_try_get_alloc_id(place.ptr) {
208+
Ok(data) => {
209+
// Unlike SB, we *do* a proper retag for size 0 if can identify the allocation.
210+
// After all, the pointer may be lazily initialized outside this initial range.
211+
data
212+
},
213+
Err(_) => {
214+
assert_eq!(ptr_size, Size::ZERO); // we did the deref check above, size has to be 0 here
215+
// This pointer doesn't come with an AllocId, so there's no
216+
// memory to do retagging in.
217+
trace!(
218+
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
219+
new_tag,
220+
place.ptr,
221+
place.layout.ty,
222+
);
223+
log_creation(this, None)?;
224+
return Ok(None);
222225
}
223226
};
227+
log_creation(this, Some((alloc_id, base_offset, parent_prov)))?;
228+
224229
let orig_tag = match parent_prov {
225230
ProvenanceExtra::Wildcard => return Ok(None), // TODO: handle wildcard pointers
226231
ProvenanceExtra::Concrete(tag) => tag,
227232
};
228233

229-
// Protection against trying to get a reference to a vtable:
230-
// vtables do not have an alloc_extra so the call to
231-
// `get_alloc_extra` that follows fails.
232-
let (alloc_size, _align, alloc_kind) = this.get_alloc_info(alloc_id);
233-
if ptr_size == Size::ZERO && !matches!(alloc_kind, AllocKind::LiveData) {
234-
return Ok(Some((alloc_id, orig_tag)));
235-
}
236-
237-
log_creation(this, Some((alloc_id, base_offset, parent_prov)))?;
238-
239-
// Ensure we bail out if the pointer goes out-of-bounds (see miri#1050).
240-
if base_offset + ptr_size > alloc_size {
241-
throw_ub!(PointerOutOfBounds {
242-
alloc_id,
243-
alloc_size,
244-
ptr_offset: this.target_usize_to_isize(base_offset.bytes()),
245-
ptr_size,
246-
msg: CheckInAllocMsg::InboundsTest
247-
});
248-
}
249-
250234
trace!(
251235
"reborrow: reference {:?} derived from {:?} (pointee {}): {:?}, size {}",
252236
new_tag,

tests/fail/alloc/deallocate-twice.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::alloc::{alloc, dealloc, Layout};
22

3-
//@error-in-other-file: dereferenced after this allocation got freed
3+
//@error-in-other-file: has been freed
44

55
fn main() {
66
unsafe {

tests/fail/alloc/deallocate-twice.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: pointer to ALLOC was dereferenced after this allocation got freed
1+
error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
22
--> RUSTLIB/alloc/src/alloc.rs:LL:CC
33
|
44
LL | unsafe { __rust_dealloc(ptr, layout.size(), layout.align()) }
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pointer to ALLOC was dereferenced after this allocation got freed
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: ALLOC has been freed, so this pointer is dangling
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

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,6 @@ fn main() {
44
unsafe {
55
let x = alloc(Layout::from_size_align_unchecked(1, 1));
66
let _y = realloc(x, Layout::from_size_align_unchecked(1, 1), 1);
7-
let _z = *x; //~ ERROR: dereferenced after this allocation got freed
7+
let _z = *x; //~ ERROR: has been freed
88
}
99
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: pointer to ALLOC was dereferenced after this allocation got freed
1+
error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
22
--> $DIR/reallocate-change-alloc.rs:LL:CC
33
|
44
LL | let _z = *x;
5-
| ^^ pointer to ALLOC was dereferenced after this allocation got freed
5+
| ^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
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

tests/fail/alloc/reallocate-dangling.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::alloc::{alloc, dealloc, realloc, Layout};
22

3-
//@error-in-other-file: dereferenced after this allocation got freed
3+
//@error-in-other-file: has been freed
44

55
fn main() {
66
unsafe {

tests/fail/alloc/reallocate-dangling.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: pointer to ALLOC was dereferenced after this allocation got freed
1+
error: Undefined Behavior: memory access failed: ALLOC has been freed, so this pointer is dangling
22
--> RUSTLIB/alloc/src/alloc.rs:LL:CC
33
|
44
LL | unsafe { __rust_realloc(ptr, layout.size(), layout.align(), new_size) }
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pointer to ALLOC was dereferenced after this allocation got freed
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ memory access failed: ALLOC has been freed, so this pointer is dangling
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

tests/fail/concurrency/thread_local_static_dealloc.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ unsafe impl Send for SendRaw {}
1111
fn main() {
1212
unsafe {
1313
let dangling_ptr = std::thread::spawn(|| SendRaw(&TLS as *const u8)).join().unwrap();
14-
let _val = *dangling_ptr.0; //~ ERROR: dereferenced after this allocation got freed
14+
let _val = *dangling_ptr.0; //~ ERROR: has been freed
1515
}
1616
}

tests/fail/concurrency/thread_local_static_dealloc.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
error: Undefined Behavior: pointer to ALLOC was dereferenced after this allocation got freed
1+
error: Undefined Behavior: dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
22
--> $DIR/thread_local_static_dealloc.rs:LL:CC
33
|
44
LL | let _val = *dangling_ptr.0;
5-
| ^^^^^^^^^^^^^^^ pointer to ALLOC was dereferenced after this allocation got freed
5+
| ^^^^^^^^^^^^^^^ dereferencing pointer failed: ALLOC has been freed, so this pointer is dangling
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

0 commit comments

Comments
 (0)