Skip to content

Commit

Permalink
Auto merge of #2516 - RalfJung:read-pointer-as-bytes, r=RalfJung
Browse files Browse the repository at this point in the history
Adjust for supporting more implicit ptr-to-int transmutation

This is the Miri side of rust-lang/rust#101101.
Fixes #2456.
  • Loading branch information
bors committed Aug 31, 2022
2 parents da45adc + d936f3a commit 872bea4
Show file tree
Hide file tree
Showing 19 changed files with 126 additions and 78 deletions.
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
94b2b15e63c5d2b2a6a0910e3dae554ce9415bf9
0631ea5d73f4a3199c776687b12c20c50a91f0d2
7 changes: 4 additions & 3 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,13 +224,14 @@ pub fn report_error<'tcx, 'mir>(
let helps = match e.kind() {
Unsupported(
UnsupportedOpInfo::ThreadLocalStatic(_) |
UnsupportedOpInfo::ReadExternStatic(_)
UnsupportedOpInfo::ReadExternStatic(_) |
UnsupportedOpInfo::PartialPointerOverwrite(_) | // we make memory uninit instead
UnsupportedOpInfo::ReadPointerAsBytes
) =>
panic!("Error should never be raised by Miri: {kind:?}", kind = e.kind()),
Unsupported(
UnsupportedOpInfo::Unsupported(_) |
UnsupportedOpInfo::PartialPointerOverwrite(_) |
UnsupportedOpInfo::ReadPointerAsBytes
UnsupportedOpInfo::PartialPointerCopy(_)
) =>
vec![(None, format!("this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support"))],
UndefinedBehavior(UndefinedBehaviorInfo::AlignmentCheckFailed { .. })
Expand Down
2 changes: 1 addition & 1 deletion src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}

// Step 2: get the bytes.
this.read_bytes_ptr(ptr, len)
this.read_bytes_ptr_strip_provenance(ptr, len)
}

fn read_wide_str(&self, mut ptr: Pointer<Option<Provenance>>) -> InterpResult<'tcx, Vec<u16>> {
Expand Down
15 changes: 15 additions & 0 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,21 @@ impl interpret::Provenance for Provenance {
Provenance::Wildcard => None,
}
}

fn join(left: Option<Self>, right: Option<Self>) -> Option<Self> {
match (left, right) {
// If both are the *same* concrete tag, that is the result.
(
Some(Provenance::Concrete { alloc_id: left_alloc, sb: left_sb }),
Some(Provenance::Concrete { alloc_id: right_alloc, sb: right_sb }),
) if left_alloc == right_alloc && left_sb == right_sb => left,
// If one side is a wildcard, the best possible outcome is that it is equal to the other
// one, and we use that.
(Some(Provenance::Wildcard), o) | (o, Some(Provenance::Wildcard)) => o,
// Otherwise, fall back to `None`.
_ => None,
}
}
}

impl fmt::Debug for ProvenanceExtra {
Expand Down
8 changes: 4 additions & 4 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,8 +560,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let n = Size::from_bytes(this.read_scalar(n)?.to_machine_usize(this)?);

let result = {
let left_bytes = this.read_bytes_ptr(left, n)?;
let right_bytes = this.read_bytes_ptr(right, n)?;
let left_bytes = this.read_bytes_ptr_strip_provenance(left, n)?;
let right_bytes = this.read_bytes_ptr_strip_provenance(right, n)?;

use std::cmp::Ordering::*;
match left_bytes.cmp(right_bytes) {
Expand All @@ -583,7 +583,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let val = val as u8;

if let Some(idx) = this
.read_bytes_ptr(ptr, Size::from_bytes(num))?
.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))?
.iter()
.rev()
.position(|&c| c == val)
Expand All @@ -606,7 +606,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let val = val as u8;

let idx = this
.read_bytes_ptr(ptr, Size::from_bytes(num))?
.read_bytes_ptr_strip_provenance(ptr, Size::from_bytes(num))?
.iter()
.position(|&c| c == val);
if let Some(idx) = idx {
Expand Down
2 changes: 1 addition & 1 deletion src/shims/unix/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let communicate = this.machine.communicate();

if let Some(file_descriptor) = this.machine.file_handler.handles.get(&fd) {
let bytes = this.read_bytes_ptr(buf, Size::from_bytes(count))?;
let bytes = this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(count))?;
let result =
file_descriptor.write(communicate, bytes)?.map(|c| i64::try_from(c).unwrap());
this.try_unwrap_io_result(result)
Expand Down
3 changes: 2 additions & 1 deletion src/shims/windows/dlsym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// stdout/stderr
use std::io::{self, Write};

let buf_cont = this.read_bytes_ptr(buf, Size::from_bytes(u64::from(n)))?;
let buf_cont =
this.read_bytes_ptr_strip_provenance(buf, Size::from_bytes(u64::from(n)))?;
let res = if this.machine.mute_stdout_stderr {
Ok(buf_cont.len())
} else if handle == -11 {
Expand Down
20 changes: 20 additions & 0 deletions tests/fail/copy_half_a_pointer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![allow(dead_code)]

// We use packed structs to get around alignment restrictions
#[repr(packed)]
struct Data {
pad: u8,
ptr: &'static i32,
}

static G: i32 = 0;

fn main() {
let mut d = Data { pad: 0, ptr: &G };

// Get a pointer to the beginning of the Data struct (one u8 byte, then the pointer bytes).
let d_alias = &mut d as *mut _ as *mut *const u8;
unsafe {
let _x = d_alias.read_unaligned(); //~ERROR: unable to copy parts of a pointer
}
}
14 changes: 14 additions & 0 deletions tests/fail/copy_half_a_pointer.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: unsupported operation: unable to copy parts of a pointer from memory at ALLOC+0x8
--> $DIR/copy_half_a_pointer.rs:LL:CC
|
LL | let _x = d_alias.read_unaligned();
| ^^^^^^^^^^^^^^^^^^^^^^^^ unable to copy parts of a pointer from memory at ALLOC+0x8
|
= help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
= note: backtrace:
= note: inside `main` at $DIR/copy_half_a_pointer.rs:LL:CC

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

error: aborting due to previous error

3 changes: 1 addition & 2 deletions tests/fail/intrinsics/raw_eq_on_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@ extern "rust-intrinsic" {

fn main() {
let x = &0;
// FIXME: the error message is not great (should be UB rather than 'unsupported')
unsafe { raw_eq(&x, &x) }; //~ERROR: unsupported operation
unsafe { raw_eq(&x, &x) }; //~ERROR: `raw_eq` on bytes with provenance
}
7 changes: 4 additions & 3 deletions tests/fail/intrinsics/raw_eq_on_ptr.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
error: unsupported operation: unable to turn pointer into raw bytes
error: Undefined Behavior: `raw_eq` on bytes with provenance
--> $DIR/raw_eq_on_ptr.rs:LL:CC
|
LL | unsafe { raw_eq(&x, &x) };
| ^^^^^^^^^^^^^^ unable to turn pointer into raw bytes
| ^^^^^^^^^^^^^^ `raw_eq` on bytes with provenance
|
= help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
= 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:
= note: inside `main` at $DIR/raw_eq_on_ptr.rs:LL:CC

Expand Down
3 changes: 2 additions & 1 deletion tests/fail/reading_half_a_pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ fn main() {
// starts 1 byte to the right, so using it would actually be wrong!
let d_alias = &mut w.data as *mut _ as *mut *const u8;
unsafe {
let _x = *d_alias; //~ ERROR: unable to turn pointer into raw bytes
let x = *d_alias;
let _val = *x; //~ERROR: is a dangling pointer (it has no provenance)
}
}
9 changes: 5 additions & 4 deletions tests/fail/reading_half_a_pointer.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
error: unsupported operation: unable to turn pointer into raw bytes
error: Undefined Behavior: dereferencing pointer failed: $HEX[noalloc] is a dangling pointer (it has no provenance)
--> $DIR/reading_half_a_pointer.rs:LL:CC
|
LL | let _x = *d_alias;
| ^^^^^^^^ unable to turn pointer into raw bytes
LL | let _val = *x;
| ^^ dereferencing pointer failed: $HEX[noalloc] is a dangling pointer (it has no provenance)
|
= help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support
= 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:
= note: inside `main` at $DIR/reading_half_a_pointer.rs:LL:CC

Expand Down
13 changes: 0 additions & 13 deletions tests/fail/transmute_fat1.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/transmute_fat1.stderr

This file was deleted.

4 changes: 0 additions & 4 deletions tests/fail/validity/ptr_integer_array_transmute.rs

This file was deleted.

15 changes: 0 additions & 15 deletions tests/fail/validity/ptr_integer_array_transmute.stderr

This file was deleted.

10 changes: 0 additions & 10 deletions tests/pass/transmute_fat.rs

This file was deleted.

52 changes: 52 additions & 0 deletions tests/pass/transmute_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#![feature(strict_provenance)]
use std::{mem, ptr};

fn t1() {
// If we are careful, we can exploit data layout...
// This is a tricky case since we are transmuting a ScalarPair type to a non-ScalarPair type.
let raw = unsafe { mem::transmute::<&[u8], [*const u8; 2]>(&[42]) };
let ptr: *const u8 = unsafe { mem::transmute_copy(&raw) };
assert_eq!(unsafe { *ptr }, 42);
}

#[cfg(target_pointer_width = "64")]
const PTR_SIZE: usize = 8;
#[cfg(target_pointer_width = "32")]
const PTR_SIZE: usize = 4;

fn t2() {
let bad = unsafe { mem::transmute::<&[u8], [u8; 2 * PTR_SIZE]>(&[1u8]) };
let _val = bad[0] + bad[bad.len() - 1];
}

fn ptr_integer_array() {
let r = &mut 42;
let _i: [usize; 1] = unsafe { mem::transmute(r) };

let _x: [u8; PTR_SIZE] = unsafe { mem::transmute(&0) };
}

fn ptr_in_two_halves() {
unsafe {
let ptr = &0 as *const i32;
let arr = [ptr; 2];
// We want to do a scalar read of a pointer at offset PTR_SIZE/2 into this array. But we
// cannot use a packed struct or `read_unaligned`, as those use the memcpy code path in
// Miri. So instead we shift the entire array by a bit and then the actual read we want to
// do is perfectly aligned.
let mut target_arr = [ptr::null::<i32>(); 3];
let target = target_arr.as_mut_ptr().cast::<u8>();
target.add(PTR_SIZE / 2).cast::<[*const i32; 2]>().write_unaligned(arr);
// Now target_arr[1] is a mix of the two `ptr` we had stored in `arr`.
let strange_ptr = target_arr[1];
// Check that the provenance works out.
assert_eq!(*strange_ptr.with_addr(ptr.addr()), 0);
}
}

fn main() {
t1();
t2();
ptr_integer_array();
ptr_in_two_halves();
}

0 comments on commit 872bea4

Please sign in to comment.