Skip to content

Commit 2e2a309

Browse files
committed
Auto merge of #144081 - RalfJung:const-ptr-fragments, r=oli-obk
const-eval: full support for pointer fragments This fixes rust-lang/const-eval#72 and makes `swap_nonoverlapping` fully work in const-eval by enhancing per-byte provenance tracking with tracking of *which* of the bytes of the pointer this one is. Later, if we see all the same bytes in the exact same order, we can treat it like a whole pointer again without ever risking a leak of the data bytes (that encode the offset into the allocation). This lifts the limitation that was discussed quite a bit in rust-lang/rust#137280. For a concrete piece of code that used to fail and now works properly consider this example doing a byte-for-byte memcpy in const without using intrinsics: ```rust use std::{mem::{self, MaybeUninit}, ptr}; type Byte = MaybeUninit<u8>; const unsafe fn memcpy(dst: *mut Byte, src: *const Byte, n: usize) { let mut i = 0; while i < n { *dst.add(i) = *src.add(i); i += 1; } } const _MEMCPY: () = unsafe { let ptr = &42; let mut ptr2 = ptr::null::<i32>(); // Copy from ptr to ptr2. memcpy(&mut ptr2 as *mut _ as *mut _, &ptr as *const _ as *const _, mem::size_of::<&i32>()); assert!(*ptr2 == 42); }; ``` What makes this code tricky is that pointers are "opaque blobs" in const-eval, we cannot just let people look at the individual bytes since *we don't know what those bytes look like* -- that depends on the absolute address the pointed-to object will be placed at. The code above "breaks apart" a pointer into individual bytes, and then puts them back together in the same order elsewhere. This PR implements the logic to properly track how those individual bytes relate to the original pointer, and to recognize when they are in the right order again. We still reject constants where the final value contains a not-fully-put-together pointer: I have no idea how one could construct an LLVM global where one byte is defined as "the 3rd byte of a pointer to that other global over there" -- and even if LLVM supports this somehow, we can leave implementing that to a future PR. It seems unlikely to me anyone would even want this, but who knows.^^ This also changes the behavior of Miri, by tracking the order of bytes with provenance and only considering a pointer to have valid provenance if all bytes are in the original order again. This is related to rust-lang/unsafe-code-guidelines#558. It means one cannot implement XOR linked lists with strict provenance any more, which is however only of theoretical interest. Practically I am curious if anyone will show up with any code that Miri now complains about - that would be interesting data. Cc `@rust-lang/opsem`
2 parents 04b0ab6 + a05f76c commit 2e2a309

File tree

11 files changed

+104
-69
lines changed

11 files changed

+104
-69
lines changed

src/machine.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,16 +280,16 @@ impl interpret::Provenance for Provenance {
280280
Ok(())
281281
}
282282

283-
fn join(left: Option<Self>, right: Option<Self>) -> Option<Self> {
283+
fn join(left: Self, right: Self) -> Option<Self> {
284284
match (left, right) {
285285
// If both are the *same* concrete tag, that is the result.
286286
(
287-
Some(Provenance::Concrete { alloc_id: left_alloc, tag: left_tag }),
288-
Some(Provenance::Concrete { alloc_id: right_alloc, tag: right_tag }),
289-
) if left_alloc == right_alloc && left_tag == right_tag => left,
287+
Provenance::Concrete { alloc_id: left_alloc, tag: left_tag },
288+
Provenance::Concrete { alloc_id: right_alloc, tag: right_tag },
289+
) if left_alloc == right_alloc && left_tag == right_tag => Some(left),
290290
// If one side is a wildcard, the best possible outcome is that it is equal to the other
291291
// one, and we use that.
292-
(Some(Provenance::Wildcard), o) | (o, Some(Provenance::Wildcard)) => o,
292+
(Provenance::Wildcard, o) | (o, Provenance::Wildcard) => Some(o),
293293
// Otherwise, fall back to `None`.
294294
_ => None,
295295
}

src/shims/native_lib/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ trait EvalContextExtPriv<'tcx>: crate::MiriInterpCxExt<'tcx> {
246246
let p_map = alloc.provenance();
247247
for idx in overlap {
248248
// If a provenance was read by the foreign code, expose it.
249-
if let Some(prov) = p_map.get(Size::from_bytes(idx), this) {
249+
if let Some((prov, _idx)) = p_map.get_byte(Size::from_bytes(idx), this) {
250250
this.expose_provenance(prov)?;
251251
}
252252
}

tests/fail/provenance/mix-ptrs1.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
use std::{mem, ptr};
2+
3+
const PTR_SIZE: usize = mem::size_of::<&i32>();
4+
5+
fn main() {
6+
unsafe {
7+
let ptr = &0 as *const i32;
8+
let arr = [ptr; 2];
9+
// We want to do a scalar read of a pointer at offset PTR_SIZE/2 into this array. But we
10+
// cannot use a packed struct or `read_unaligned`, as those use the memcpy code path in
11+
// Miri. So instead we shift the entire array by a bit and then the actual read we want to
12+
// do is perfectly aligned.
13+
let mut target_arr = [ptr::null::<i32>(); 3];
14+
let target = target_arr.as_mut_ptr().cast::<u8>();
15+
target.add(PTR_SIZE / 2).cast::<[*const i32; 2]>().write_unaligned(arr);
16+
// Now target_arr[1] is a mix of the two `ptr` we had stored in `arr`.
17+
// They all have the same provenance, but not in the right order, so we reject this.
18+
let strange_ptr = target_arr[1];
19+
assert_eq!(*strange_ptr.with_addr(ptr.addr()), 0); //~ERROR: no provenance
20+
}
21+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: Undefined Behavior: pointer not dereferenceable: pointer must be dereferenceable for 4 bytes, but got $HEX[noalloc] which is a dangling pointer (it has no provenance)
2+
--> tests/fail/provenance/mix-ptrs1.rs:LL:CC
3+
|
4+
LL | assert_eq!(*strange_ptr.with_addr(ptr.addr()), 0);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
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 RUSTLIB/core/src/macros/mod.rs:LL:CC
11+
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
12+
13+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
14+
15+
error: aborting due to 1 previous error
16+

tests/fail/provenance/mix-ptrs2.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
use std::mem;
2+
3+
const PTR_SIZE: usize = mem::size_of::<&i32>();
4+
5+
/// Overwrite one byte of a pointer, then restore it.
6+
fn main() {
7+
unsafe fn ptr_bytes<'x>(ptr: &'x mut *const i32) -> &'x mut [mem::MaybeUninit<u8>; PTR_SIZE] {
8+
mem::transmute(ptr)
9+
}
10+
11+
// Returns a value with the same provenance as `x` but 0 for the integer value.
12+
// `x` must be initialized.
13+
unsafe fn zero_with_provenance(x: mem::MaybeUninit<u8>) -> mem::MaybeUninit<u8> {
14+
let ptr = [x; PTR_SIZE];
15+
let ptr: *const i32 = mem::transmute(ptr);
16+
let mut ptr = ptr.with_addr(0);
17+
ptr_bytes(&mut ptr)[0]
18+
}
19+
20+
unsafe {
21+
let ptr = &42;
22+
let mut ptr = ptr as *const i32;
23+
// Get a bytewise view of the pointer.
24+
let ptr_bytes = ptr_bytes(&mut ptr);
25+
26+
// The highest bytes must be 0 for this to work.
27+
let hi = if cfg!(target_endian = "little") { ptr_bytes.len() - 1 } else { 0 };
28+
assert_eq!(*ptr_bytes[hi].as_ptr().cast::<u8>(), 0);
29+
// Overwrite provenance on the last byte.
30+
ptr_bytes[hi] = mem::MaybeUninit::new(0);
31+
// Restore it from the another byte.
32+
ptr_bytes[hi] = zero_with_provenance(ptr_bytes[1]);
33+
34+
// Now ptr is almost good, except the provenance fragment indices do not work out...
35+
assert_eq!(*ptr, 42); //~ERROR: no provenance
36+
}
37+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: Undefined Behavior: pointer not dereferenceable: pointer must be dereferenceable for 4 bytes, but got $HEX[noalloc] which is a dangling pointer (it has no provenance)
2+
--> tests/fail/provenance/mix-ptrs2.rs:LL:CC
3+
|
4+
LL | assert_eq!(*ptr, 42);
5+
| ^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
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 RUSTLIB/core/src/macros/mod.rs:LL:CC
11+
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
12+
13+
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
14+
15+
error: aborting due to 1 previous error
16+

tests/fail/uninit/uninit_alloc_diagnostic_with_provenance.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,21 @@ use std::alloc::{Layout, alloc, dealloc};
99
use std::mem::{self, MaybeUninit};
1010
use std::slice::from_raw_parts;
1111

12-
fn byte_with_provenance<T>(val: u8, prov: *const T) -> MaybeUninit<u8> {
13-
let ptr = prov.with_addr(val as usize);
12+
fn byte_with_provenance<T>(val: u8, prov: *const T, frag_idx: usize) -> MaybeUninit<u8> {
13+
let ptr = prov.with_addr(usize::from_ne_bytes([val; _]));
1414
let bytes: [MaybeUninit<u8>; mem::size_of::<*const ()>()] = unsafe { mem::transmute(ptr) };
15-
let lsb = if cfg!(target_endian = "little") { 0 } else { bytes.len() - 1 };
16-
bytes[lsb]
15+
bytes[frag_idx]
1716
}
1817

1918
fn main() {
2019
let layout = Layout::from_size_align(16, 8).unwrap();
2120
unsafe {
2221
let ptr = alloc(layout);
2322
let ptr_raw = ptr.cast::<MaybeUninit<u8>>();
24-
*ptr_raw.add(0) = byte_with_provenance(0x42, &42u8);
23+
*ptr_raw.add(0) = byte_with_provenance(0x42, &42u8, 0);
2524
*ptr.add(1) = 0x12;
2625
*ptr.add(2) = 0x13;
27-
*ptr_raw.add(3) = byte_with_provenance(0x43, &0u8);
26+
*ptr_raw.add(3) = byte_with_provenance(0x43, &0u8, 1);
2827
let slice1 = from_raw_parts(ptr, 8);
2928
let slice2 = from_raw_parts(ptr.add(8), 8);
3029
drop(slice1.cmp(slice2));

tests/fail/uninit/uninit_alloc_diagnostic_with_provenance.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ LL | drop(slice1.cmp(slice2));
1717

1818
Uninitialized memory occurred at ALLOC[0x4..0x8], in this allocation:
1919
ALLOC (Rust heap, size: 16, align: 8) {
20-
╾42[ALLOC]<TAG> (1 ptr byte)╼ 12 13 ╾43[ALLOC]<TAG> (1 ptr byte)╼ __ __ __ __ __ __ __ __ __ __ __ __ │ ━..━░░░░░░░░░░░░
20+
╾42[ALLOC]<TAG> (ptr fragment 0)╼ 12 13 ╾43[ALLOC]<TAG> (ptr fragment 1)╼ __ __ __ __ __ __ __ __ __ __ __ __ │ ━..━░░░░░░░░░░░░
2121
}
2222
ALLOC (global (static or const), size: 1, align: 1) {
2323
2a │ *

tests/native-lib/fail/tracing/partial_init.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ LL | partial_init();
3535

3636
Uninitialized memory occurred at ALLOC[0x2..0x3], in this allocation:
3737
ALLOC (stack variable, size: 3, align: 1) {
38-
╾00[wildcard] (1 ptr byte)╼ ╾00[wildcard] (1 ptr byte)╼ __ │ ━━░
38+
╾00[wildcard] (ptr fragment 0)╼ ╾00[wildcard] (ptr fragment 0)╼ __ │ ━━░
3939
}
4040

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

tests/pass/provenance.rs

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ const PTR_SIZE: usize = mem::size_of::<&i32>();
66

77
fn main() {
88
basic();
9-
partial_overwrite_then_restore();
109
bytewise_ptr_methods();
1110
bytewise_custom_memcpy();
1211
bytewise_custom_memcpy_chunked();
@@ -29,40 +28,6 @@ fn basic() {
2928
assert_eq!(unsafe { *ptr_back }, 42);
3029
}
3130

32-
/// Overwrite one byte of a pointer, then restore it.
33-
fn partial_overwrite_then_restore() {
34-
unsafe fn ptr_bytes<'x>(ptr: &'x mut *const i32) -> &'x mut [mem::MaybeUninit<u8>; PTR_SIZE] {
35-
mem::transmute(ptr)
36-
}
37-
38-
// Returns a value with the same provenance as `x` but 0 for the integer value.
39-
// `x` must be initialized.
40-
unsafe fn zero_with_provenance(x: mem::MaybeUninit<u8>) -> mem::MaybeUninit<u8> {
41-
let ptr = [x; PTR_SIZE];
42-
let ptr: *const i32 = mem::transmute(ptr);
43-
let mut ptr = ptr.with_addr(0);
44-
ptr_bytes(&mut ptr)[0]
45-
}
46-
47-
unsafe {
48-
let ptr = &42;
49-
let mut ptr = ptr as *const i32;
50-
// Get a bytewise view of the pointer.
51-
let ptr_bytes = ptr_bytes(&mut ptr);
52-
53-
// The highest bytes must be 0 for this to work.
54-
let hi = if cfg!(target_endian = "little") { ptr_bytes.len() - 1 } else { 0 };
55-
assert_eq!(*ptr_bytes[hi].as_ptr().cast::<u8>(), 0);
56-
// Overwrite provenance on the last byte.
57-
ptr_bytes[hi] = mem::MaybeUninit::new(0);
58-
// Restore it from the another byte.
59-
ptr_bytes[hi] = zero_with_provenance(ptr_bytes[1]);
60-
61-
// Now ptr should be good again.
62-
assert_eq!(*ptr, 42);
63-
}
64-
}
65-
6631
fn bytewise_ptr_methods() {
6732
let mut ptr1 = &1;
6833
let mut ptr2 = &2;

0 commit comments

Comments
 (0)