Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adjust for better provenance control #2183

Merged
merged 6 commits into from
Jun 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
8 changes: 4 additions & 4 deletions tests/fail/transmute_fat1.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// This should fail even without validation
// compile-flags: -Zmiri-disable-validation
// error-pattern: type validation failed: encountered a pointer
// normalize-stderr-test: "\[u8; (08|16)\]" -> "$$ARRAY"

fn main() {
#[cfg(target_pointer_width="64")]
Expand All @@ -8,7 +8,7 @@ fn main() {
};
#[cfg(target_pointer_width="32")]
let bad = unsafe {
std::mem::transmute::<&[u8], [u8; 8]>(&[1u8])
std::mem::transmute::<&[u8], [u8; 08]>(&[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], $ARRAY>(&[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
Loading