Skip to content

Commit

Permalink
Auto merge of #2183 - RalfJung:better-provenance-control, r=RalfJung
Browse files Browse the repository at this point in the history
adjust for better provenance control

This is the Miri side of rust-lang/rust#97684.
  • Loading branch information
bors committed Jun 6, 2022
2 parents 1c711a1 + b39e4c7 commit 9376978
Show file tree
Hide file tree
Showing 29 changed files with 136 additions and 109 deletions.
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
4e725bad73747a4c93a3ac53106e4b4006edc665
9d20fd109809f20c049d6895a5be27a1fbd39daa
2 changes: 1 addition & 1 deletion rustup-toolchain
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ else
NEW_COMMIT="$1"
fi
echo "$NEW_COMMIT" > rust-version
shift
shift || true # don't fail if shifting fails

# Check if we already are at that commit.
CUR_COMMIT=$(rustc +miri --version -v 2>/dev/null | egrep "^commit-hash: " | cut -d " " -f 2)
Expand Down
14 changes: 11 additions & 3 deletions src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,16 @@ pub fn report_error<'tcx, 'mir>(
};
#[rustfmt::skip]
let helps = match e.kind() {
Unsupported(UnsupportedOpInfo::ThreadLocalStatic(_) | UnsupportedOpInfo::ReadExternStatic(_)) =>
Unsupported(
UnsupportedOpInfo::ThreadLocalStatic(_) |
UnsupportedOpInfo::ReadExternStatic(_)
) =>
panic!("Error should never be raised by Miri: {:?}", e.kind()),
Unsupported(_) =>
Unsupported(
UnsupportedOpInfo::Unsupported(_) |
UnsupportedOpInfo::PartialPointerOverwrite(_) |
UnsupportedOpInfo::ReadPointerAsBytes
) =>
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 { .. })
if ecx.machine.check_alignment == AlignmentCheck::Symbolic
Expand All @@ -245,7 +252,8 @@ pub fn report_error<'tcx, 'mir>(
(None, format!("this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior")),
(None, format!("see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information")),
],
_ => vec![],
InvalidProgram(_) | ResourceExhaustion(_) | MachineStop(_) =>
vec![],
};
(Some(title), helps)
}
Expand Down
4 changes: 2 additions & 2 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// FIXME: We are re-getting the allocation each time around the loop.
// Would be nice if we could somehow "extend" an existing AllocRange.
let alloc = this.get_ptr_alloc(ptr.offset(len, this)?, size1, Align::ONE)?.unwrap(); // not a ZST, so we will get a result
let byte = alloc.read_scalar(alloc_range(Size::ZERO, size1))?.to_u8()?;
let byte = alloc.read_integer(Size::ZERO, size1)?.to_u8()?;
if byte == 0 {
break;
} else {
Expand All @@ -703,7 +703,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// FIXME: We are re-getting the allocation each time around the loop.
// Would be nice if we could somehow "extend" an existing AllocRange.
let alloc = this.get_ptr_alloc(ptr, size2, align2)?.unwrap(); // not a ZST, so we will get a result
let wchar = alloc.read_scalar(alloc_range(Size::ZERO, size2))?.to_u16()?;
let wchar = alloc.read_integer(Size::ZERO, size2)?.to_u16()?;
if wchar == 0 {
break;
} else {
Expand Down
6 changes: 3 additions & 3 deletions tests/fail/branchless-select-i128-pointer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ fn main() {
for &my_bool in &[true, false] {
let mask = -(my_bool as TwoPtrs); // false -> 0, true -> -1 aka !0
// This is branchless code to select one or the other pointer.
// For now, Miri brafs on it, but if this code ever passes we better make sure it behaves correctly.
// However, it drops provenance when transmuting to TwoPtrs, so this is UB.
let val = unsafe {
transmute::<_, &str>(
!mask & transmute::<_, TwoPtrs>("false !") | mask & transmute::<_, TwoPtrs>("true !"), //~ERROR encountered (potentially part of) a pointer, but expected plain (non-pointer) bytes
transmute::<_, &str>( //~ERROR type validation failed: encountered a dangling reference
!mask & transmute::<_, TwoPtrs>("false !") | mask & transmute::<_, TwoPtrs>("true !"),
)
};
println!("{}", val);
Expand Down
8 changes: 5 additions & 3 deletions tests/fail/branchless-select-i128-pointer.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
error: Undefined Behavior: type validation failed: encountered (potentially part of) a pointer, but expected plain (non-pointer) bytes
error: Undefined Behavior: type validation failed: encountered a dangling reference (address $HEX is unallocated)
--> $DIR/branchless-select-i128-pointer.rs:LL:CC
|
LL | !mask & transmute::<_, TwoPtrs>("false !") | mask & transmute::<_, TwoPtrs>("true !"),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered (potentially part of) a pointer, but expected plain (non-pointer) bytes
LL | / transmute::<_, &str>(
LL | | !mask & transmute::<_, TwoPtrs>("false !") | mask & transmute::<_, TwoPtrs>("true !"),
LL | | )
| |_____________^ type validation failed: encountered a dangling reference (address $HEX is unallocated)
|
= 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
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/pointer_partial_overwrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// compile-flags: -Zmiri-disable-alignment-check -Zmiri-disable-stacked-borrows -Zmiri-disable-validation

// Test what happens when we overwrite parts of a pointer.
// Also see <https://github.com/rust-lang/rust/issues/87184>.
// Also see <https://github.com/rust-lang/miri/issues/2181>.

fn main() {
let mut p = &42;
Expand Down
9 changes: 0 additions & 9 deletions tests/fail/pointer_partial_read.rs

This file was deleted.

14 changes: 0 additions & 14 deletions tests/fail/pointer_partial_read.stderr

This file was deleted.

27 changes: 27 additions & 0 deletions tests/fail/provenance/permissive_provenance_transmute.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows
#![feature(strict_provenance)]

use std::mem;

// This is the example from
// <https://github.com/rust-lang/unsafe-code-guidelines/issues/286#issuecomment-1085144431>.

unsafe fn deref(left: *const u8, right: *const u8) {
let left_int: usize = mem::transmute(left);
let right_int: usize = mem::transmute(right);
if left_int == right_int {
// The compiler is allowed to replace `left_int` by `right_int` here...
let left_ptr: *const u8 = mem::transmute(left_int);
// ...which however means here it could be dereferencing the wrong pointer.
let _val = *left_ptr; //~ERROR dereferencing pointer failed
}
}

fn main() {
let ptr1 = &0u8 as *const u8;
let ptr2 = &1u8 as *const u8;
unsafe {
// Two pointers with the same address but different provenance.
deref(ptr1, ptr2.with_addr(ptr1.addr()));
}
}
20 changes: 20 additions & 0 deletions tests/fail/provenance/permissive_provenance_transmute.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: Undefined Behavior: dereferencing pointer failed: $HEX is not a valid pointer
--> $DIR/permissive_provenance_transmute.rs:LL:CC
|
LL | let _val = *left_ptr;
| ^^^^^^^^^ dereferencing pointer failed: $HEX is not a valid pointer
|
= 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: inside `deref` at $DIR/permissive_provenance_transmute.rs:LL:CC
note: inside `main` at $DIR/permissive_provenance_transmute.rs:LL:CC
--> $DIR/permissive_provenance_transmute.rs:LL:CC
|
LL | deref(ptr1, ptr2.with_addr(ptr1.addr()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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

error: aborting due to previous error

10 changes: 5 additions & 5 deletions tests/fail/provenance/ptr_int_unexposed.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows -Zmiri-allow-ptr-int-transmute
// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows
#![feature(strict_provenance)]

fn main() {
let x: i32 = 3;
let x_ptr = &x as *const i32;

// TODO: switch this to addr() once we intrinsify it
let x_usize: usize = unsafe { std::mem::transmute(x_ptr) };
// Cast back a pointer that did *not* get exposed.
let ptr = x_usize as *const i32;
let x_usize: usize = x_ptr.addr();
// Cast back an address that did *not* get exposed.
let ptr = std::ptr::from_exposed_addr::<i32>(x_usize);
assert_eq!(unsafe { *ptr }, 3); //~ ERROR Undefined Behavior: dereferencing pointer failed
}
4 changes: 2 additions & 2 deletions tests/fail/provenance/strict_provenance_transmute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ use std::mem;
// <https://github.com/rust-lang/unsafe-code-guidelines/issues/286#issuecomment-1085144431>.

unsafe fn deref(left: *const u8, right: *const u8) {
let left_int: usize = mem::transmute(left); //~ERROR expected plain (non-pointer) bytes
let left_int: usize = mem::transmute(left);
let right_int: usize = mem::transmute(right);
if left_int == right_int {
// The compiler is allowed to replace `left_int` by `right_int` here...
let left_ptr: *const u8 = mem::transmute(left_int);
// ...which however means here it could be dereferencing the wrong pointer.
let _val = *left_ptr;
let _val = *left_ptr; //~ERROR dereferencing pointer failed
}
}

Expand Down
6 changes: 3 additions & 3 deletions tests/fail/provenance/strict_provenance_transmute.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: type validation failed: encountered pointer to $HEX[ALLOC]<TAG>, but expected plain (non-pointer) bytes
error: Undefined Behavior: dereferencing pointer failed: $HEX is not a valid pointer
--> $DIR/strict_provenance_transmute.rs:LL:CC
|
LL | let left_int: usize = mem::transmute(left);
| ^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to $HEX[ALLOC]<TAG>, but expected plain (non-pointer) bytes
LL | let _val = *left_ptr;
| ^^^^^^^^^ dereferencing pointer failed: $HEX is not a valid pointer
|
= 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
Expand Down
20 changes: 8 additions & 12 deletions tests/fail/stacked_borrows/illegal_read3.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
// compile-flags: -Zmiri-allow-ptr-int-transmute
// A callee may not read the destination of our `&mut` without us noticing.
// Thise code got carefully checked to not introduce any reborrows
// that are not explicit in the source. Let's hope the compiler does not break this later!

#![feature(untagged_unions)]

use std::mem;

union HiddenRef {
// We avoid retagging at this type, so shared vs mutable does not matter.
r: &'static i32,
}

fn main() {
let mut x: i32 = 15;
let xref1 = &mut x;
let xref1_sneaky: usize = unsafe { mem::transmute_copy(&xref1) };
let xref1_sneaky: HiddenRef = unsafe { mem::transmute_copy(&xref1) };
// Derived from `xref1`, so using raw value is still ok, ...
let xref2 = &mut *xref1;
callee(xref1_sneaky);
Expand All @@ -19,14 +21,8 @@ fn main() {
//~^ ERROR: borrow stack
}

fn callee(xref1: usize) {
// Transmuting through a union to avoid retagging.
union UsizeToRef {
from: usize,
to: &'static mut i32,
}
let xref1 = UsizeToRef { from: xref1 };
fn callee(xref1: HiddenRef) {
// Doing the deref and the transmute (through the union) in the same place expression
// should avoid retagging.
let _val = unsafe { *xref1.to };
let _val = unsafe { *xref1.r };
}
4 changes: 2 additions & 2 deletions tests/fail/stacked_borrows/illegal_read3.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ LL | let xref2 = &mut *xref1;
help: <TAG> was later invalidated at offsets [0x0..0x4]
--> $DIR/illegal_read3.rs:LL:CC
|
LL | let _val = unsafe { *xref1.to };
| ^^^^^^^^^
LL | let _val = unsafe { *xref1.r };
| ^^^^^^^^
= note: inside `main` at $DIR/illegal_read3.rs:LL:CC

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
Expand Down
3 changes: 1 addition & 2 deletions tests/fail/transmute-pair-uninit.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// compile-flags: -Zmiri-allow-uninit-numbers
#![feature(core_intrinsics)]

use std::mem;
Expand All @@ -18,6 +17,6 @@ fn main() {
assert_eq!(byte, 0);
}
let v = unsafe { *z.offset(first_undef) };
//~^ ERROR uninitialized
if v == 0 { println!("it is zero"); }
//~^ ERROR this operation requires initialized memory
}
6 changes: 3 additions & 3 deletions tests/fail/transmute-pair-uninit.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
error: Undefined Behavior: type validation failed: encountered uninitialized bytes, but expected initialized bytes
--> $DIR/transmute-pair-uninit.rs:LL:CC
|
LL | if v == 0 { println!("it is zero"); }
| ^^^^^^ using uninitialized data, but this operation requires initialized memory
LL | let v = unsafe { *z.offset(first_undef) };
| ^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized bytes
|
= 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
Expand Down
5 changes: 2 additions & 3 deletions tests/fail/transmute_fat1.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// This should fail even without validation
// compile-flags: -Zmiri-disable-validation
// error-pattern: type validation failed: encountered a pointer

fn main() {
#[cfg(target_pointer_width="64")]
Expand All @@ -10,5 +9,5 @@ fn main() {
let bad = unsafe {
std::mem::transmute::<&[u8], [u8; 8]>(&[1u8])
};
let _val = bad[0] + bad[bad.len()-1]; //~ ERROR unable to turn pointer into raw bytes
let _val = bad[0] + bad[bad.len()-1];
}
9 changes: 5 additions & 4 deletions tests/fail/transmute_fat1.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: type validation failed: encountered a pointer, but expected plain (non-pointer) bytes
--> $DIR/transmute_fat1.rs:LL:CC
|
LL | let _val = bad[0] + bad[bad.len()-1];
| ^^^^^^ unable to turn pointer into raw bytes
LL | std::mem::transmute::<&[u8], [u8; 16]>(&[1u8])
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected plain (non-pointer) bytes
|
= 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: inside `main` at $DIR/transmute_fat1.rs:LL:CC

Expand Down
5 changes: 2 additions & 3 deletions tests/fail/uninit_byte_read.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// compile-flags: -Zmiri-allow-uninit-numbers
fn main() {
let v: Vec<u8> = Vec::with_capacity(10);
let undef = unsafe { *v.get_unchecked(5) };
let x = undef + 1; //~ ERROR this operation requires initialized memory
let undef = unsafe { *v.get_unchecked(5) }; //~ ERROR uninitialized
let x = undef + 1;
panic!("this should never print: {}", x);
}
6 changes: 3 additions & 3 deletions tests/fail/uninit_byte_read.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
error: Undefined Behavior: type validation failed: encountered uninitialized bytes, but expected initialized bytes
--> $DIR/uninit_byte_read.rs:LL:CC
|
LL | let x = undef + 1;
| ^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
LL | let undef = unsafe { *v.get_unchecked(5) };
| ^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected initialized bytes
|
= 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
Expand Down
1 change: 1 addition & 0 deletions tests/fail/validity/invalid_enum_tag_256variants_uninit.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Even when uninit numbers are allowed, this enum is not.
// compile-flags: -Zmiri-allow-uninit-numbers
#![allow(unused, deprecated, invalid_value)]

Expand Down
4 changes: 0 additions & 4 deletions tests/fail/validity/ptr_integer_transmute.rs

This file was deleted.

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

This file was deleted.

6 changes: 2 additions & 4 deletions tests/pass/intptrcast.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
// compile-flags: -Zmiri-allow-ptr-int-transmute

// This returns a miri pointer at type usize, if the argument is a proper pointer
// This strips provenance
fn transmute_ptr_to_int<T>(x: *const T) -> usize {
unsafe { std::mem::transmute(x) }
}
Expand Down Expand Up @@ -39,7 +37,7 @@ fn transmute() {
// transmuting.
let a: *const i32 = &42;
let b = transmute_ptr_to_int(a) as u8;
let c = a as usize as u8;
let c = a as u8;
assert_eq!(b, c);
}

Expand Down
File renamed without changes.
Loading

0 comments on commit 9376978

Please sign in to comment.