Skip to content

Commit 4c77f86

Browse files
committed
interpret: reset padding during validation
1 parent fd02b78 commit 4c77f86

16 files changed

+249
-15
lines changed

src/machine.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,9 @@ pub struct MiriMachine<'tcx> {
572572
/// Invariant: the promised alignment will never be less than the native alignment of the
573573
/// allocation.
574574
pub(crate) symbolic_alignment: RefCell<FxHashMap<AllocId, (Size, Align)>>,
575+
576+
/// A cache of "data range" computations for unions (i.e., the offsets of non-padding bytes).
577+
union_data_ranges: FxHashMap<Ty<'tcx>, RangeSet>,
575578
}
576579

577580
impl<'tcx> MiriMachine<'tcx> {
@@ -714,6 +717,7 @@ impl<'tcx> MiriMachine<'tcx> {
714717
allocation_spans: RefCell::new(FxHashMap::default()),
715718
const_cache: RefCell::new(FxHashMap::default()),
716719
symbolic_alignment: RefCell::new(FxHashMap::default()),
720+
union_data_ranges: FxHashMap::default(),
717721
}
718722
}
719723

@@ -826,6 +830,7 @@ impl VisitProvenance for MiriMachine<'_> {
826830
allocation_spans: _,
827831
const_cache: _,
828832
symbolic_alignment: _,
833+
union_data_ranges: _,
829834
} = self;
830835

831836
threads.visit_provenance(visit);
@@ -1627,4 +1632,12 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
16271632
ecx.machine.rng.borrow_mut().gen::<usize>() % ADDRS_PER_ANON_GLOBAL
16281633
}
16291634
}
1635+
1636+
fn cached_union_data_range<'e>(
1637+
ecx: &'e mut InterpCx<'tcx, Self>,
1638+
ty: Ty<'tcx>,
1639+
compute_range: impl FnOnce() -> RangeSet,
1640+
) -> Cow<'e, RangeSet> {
1641+
Cow::Borrowed(ecx.machine.union_data_ranges.entry(ty).or_insert_with(compute_range))
1642+
}
16301643
}

tests/fail/provenance/ptr_copy_loses_partial_provenance0.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
fn main() {
2-
unsafe {
3-
let mut bytes = [1u8; 16];
4-
let bytes = bytes.as_mut_ptr();
2+
let half_ptr = std::mem::size_of::<*const ()>() / 2;
3+
let mut bytes = [1u8; 16];
4+
let bytes = bytes.as_mut_ptr();
55

6+
unsafe {
67
// Put a pointer in the middle.
7-
bytes.add(4).cast::<&i32>().write_unaligned(&42);
8+
bytes.add(half_ptr).cast::<&i32>().write_unaligned(&42);
89
// Typed copy of the entire thing as two pointers, but not perfectly
910
// overlapping with the pointer we have in there.
1011
let copy = bytes.cast::<[*const (); 2]>().read_unaligned();
1112
let copy_bytes = copy.as_ptr().cast::<u8>();
1213
// Now go to the middle of the copy and get the pointer back out.
13-
let ptr = copy_bytes.add(4).cast::<*const i32>().read_unaligned();
14+
let ptr = copy_bytes.add(half_ptr).cast::<*const i32>().read_unaligned();
1415
// Dereferencing this should fail as the copy has removed the provenance.
1516
let _val = *ptr; //~ERROR: dangling
1617
}

tests/fail/provenance/ptr_copy_loses_partial_provenance1.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
fn main() {
2-
unsafe {
3-
let mut bytes = [1u8; 16];
4-
let bytes = bytes.as_mut_ptr();
2+
let half_ptr = std::mem::size_of::<*const ()>() / 2;
3+
let mut bytes = [1u8; 16];
4+
let bytes = bytes.as_mut_ptr();
55

6+
unsafe {
67
// Put a pointer in the middle.
7-
bytes.add(4).cast::<&i32>().write_unaligned(&42);
8+
bytes.add(half_ptr).cast::<&i32>().write_unaligned(&42);
89
// Typed copy of the entire thing as two *function* pointers, but not perfectly
910
// overlapping with the pointer we have in there.
1011
let copy = bytes.cast::<[fn(); 2]>().read_unaligned();
1112
let copy_bytes = copy.as_ptr().cast::<u8>();
1213
// Now go to the middle of the copy and get the pointer back out.
13-
let ptr = copy_bytes.add(4).cast::<*const i32>().read_unaligned();
14+
let ptr = copy_bytes.add(half_ptr).cast::<*const i32>().read_unaligned();
1415
// Dereferencing this should fail as the copy has removed the provenance.
1516
let _val = *ptr; //~ERROR: dangling
1617
}

tests/fail/uninit/padding-enum.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
use std::mem;
2+
3+
// We have three fields to avoid the ScalarPair optimization.
4+
#[allow(unused)]
5+
enum E {
6+
None,
7+
Some(&'static (), &'static (), usize),
8+
}
9+
10+
fn main() { unsafe {
11+
let mut p: mem::MaybeUninit<E> = mem::MaybeUninit::zeroed();
12+
// The copy when `E` is returned from `transmute` should destroy padding
13+
// (even when we use `write_unaligned`, which under the hood uses an untyped copy).
14+
p.as_mut_ptr().write_unaligned(mem::transmute((0usize, 0usize, 0usize)));
15+
// This is a `None`, so everything but the discriminant is padding.
16+
assert!(matches!(*p.as_ptr(), E::None));
17+
18+
// Turns out the discriminant is (currently) stored
19+
// in the 2nd pointer, so the first half is padding.
20+
let c = &p as *const _ as *const u8;
21+
let _val = *c.add(0); // Get the padding byte.
22+
//~^ERROR: uninitialized
23+
} }

tests/fail/uninit/padding-enum.stderr

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
2+
--> $DIR/padding-enum.rs:LL:CC
3+
|
4+
LL | let _val = *c.add(0); // Get the padding byte.
5+
| ^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/padding-enum.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to 1 previous error
15+

tests/fail/uninit/padding-pair.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#![feature(core_intrinsics)]
2+
3+
use std::mem::{self, MaybeUninit};
4+
5+
fn main() {
6+
// This constructs a `(usize, bool)` pair: 9 bytes initialized, the rest not.
7+
// Ensure that these 9 bytes are indeed initialized, and the rest is indeed not.
8+
// This should be the case even if we write into previously initialized storage.
9+
let mut x: MaybeUninit<Box<[u8]>> = MaybeUninit::zeroed();
10+
let z = std::intrinsics::add_with_overflow(0usize, 0usize);
11+
unsafe { x.as_mut_ptr().cast::<(usize, bool)>().write(z) };
12+
// Now read this bytewise. There should be (`ptr_size + 1`) def bytes followed by
13+
// (`ptr_size - 1`) undef bytes (the padding after the bool) in there.
14+
let z: *const u8 = &x as *const _ as *const _;
15+
let first_undef = mem::size_of::<usize>() as isize + 1;
16+
for i in 0..first_undef {
17+
let byte = unsafe { *z.offset(i) };
18+
assert_eq!(byte, 0);
19+
}
20+
let v = unsafe { *z.offset(first_undef) };
21+
//~^ ERROR: uninitialized
22+
if v == 0 {
23+
println!("it is zero");
24+
}
25+
}

tests/fail/uninit/padding-pair.stderr

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
2+
--> $DIR/padding-pair.rs:LL:CC
3+
|
4+
LL | let v = unsafe { *z.offset(first_undef) };
5+
| ^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/padding-pair.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to 1 previous error
15+
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#[repr(C)]
2+
#[derive(Debug, Copy, Clone)]
3+
struct Foo {
4+
val16: u16,
5+
// Padding bytes go here!
6+
val32: u32,
7+
}
8+
9+
#[repr(C)]
10+
#[derive(Debug, Copy, Clone)]
11+
struct Bar {
12+
bytes: [u8; 8],
13+
}
14+
15+
#[repr(C)]
16+
union FooBar {
17+
foo: Foo,
18+
bar: Bar,
19+
}
20+
21+
pub fn main() {
22+
// Initialize as u8 to ensure padding bytes are zeroed.
23+
let mut foobar = FooBar { bar: Bar { bytes: [0u8; 8] } };
24+
// Reading either field is ok.
25+
let _val = unsafe { (foobar.foo, foobar.bar) };
26+
// Does this assignment copy the uninitialized padding bytes
27+
// over the initialized padding bytes? miri doesn't seem to think so.
28+
foobar.foo = Foo { val16: 1, val32: 2 };
29+
// This resets the padding to uninit.
30+
let _val = unsafe { (foobar.foo, foobar.bar) };
31+
//~^ ERROR: uninitialized
32+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
error: Undefined Behavior: constructing invalid value at .bytes[2]: encountered uninitialized memory, but expected an integer
2+
--> $DIR/padding-struct-in-union.rs:LL:CC
3+
|
4+
LL | let _val = unsafe { (foobar.foo, foobar.bar) };
5+
| ^^^^^^^^^^ constructing invalid value at .bytes[2]: encountered uninitialized memory, but expected an integer
6+
|
7+
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
8+
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
9+
= note: BACKTRACE:
10+
= note: inside `main` at $DIR/padding-struct-in-union.rs:LL:CC
11+
12+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
13+
14+
error: aborting due to 1 previous error
15+

tests/fail/uninit/padding-struct.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
use std::mem;
2+
3+
#[repr(C)]
4+
struct Pair(u8, u16);
5+
6+
fn main() { unsafe {
7+
let p: Pair = mem::transmute(0u32); // The copy when `Pair` is returned from `transmute` should destroy padding.
8+
let c = &p as *const _ as *const u8;
9+
let _val = *c.add(1); // Get the padding byte.
10+
//~^ERROR: uninitialized
11+
} }

0 commit comments

Comments
 (0)