From 63625b03974bdb41f7629045ebfa239328fdcf02 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 3 Jun 2022 08:46:22 -0400 Subject: [PATCH] adjust for better provenance control --- src/diagnostics.rs | 14 +++++++--- src/helpers.rs | 4 +-- tests/fail/branchless-select-i128-pointer.rs | 6 ++--- .../branchless-select-i128-pointer.stderr | 8 +++--- tests/fail/pointer_partial_overwrite.rs | 2 +- tests/fail/pointer_partial_read.rs | 9 ------- tests/fail/pointer_partial_read.stderr | 14 ---------- .../permissive_provenance_transmute.rs | 27 +++++++++++++++++++ .../permissive_provenance_transmute.stderr | 20 ++++++++++++++ .../provenance/strict_provenance_transmute.rs | 4 +-- .../strict_provenance_transmute.stderr | 6 ++--- tests/fail/transmute_fat1.rs | 5 ++-- tests/fail/transmute_fat1.stderr | 9 ++++--- tests/fail/validity/ptr_integer_transmute.rs | 4 --- .../validity/ptr_integer_transmute.stderr | 15 ----------- ..._provenance.rs => ptr_int_from_exposed.rs} | 0 tests/pass/ptr_int_transmute.rs | 22 +++++++++++++++ 17 files changed, 103 insertions(+), 66 deletions(-) delete mode 100644 tests/fail/pointer_partial_read.rs delete mode 100644 tests/fail/pointer_partial_read.stderr create mode 100644 tests/fail/provenance/permissive_provenance_transmute.rs create mode 100644 tests/fail/provenance/permissive_provenance_transmute.stderr delete mode 100644 tests/fail/validity/ptr_integer_transmute.rs delete mode 100644 tests/fail/validity/ptr_integer_transmute.stderr rename tests/pass/{ptr_int_permissive_provenance.rs => ptr_int_from_exposed.rs} (100%) create mode 100644 tests/pass/ptr_int_transmute.rs diff --git a/src/diagnostics.rs b/src/diagnostics.rs index ace521f1bed..52f93a6cea9 100644 --- a/src/diagnostics.rs +++ b/src/diagnostics.rs @@ -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 @@ -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) } diff --git a/src/helpers.rs b/src/helpers.rs index cb4c3c293e9..4c79633c72d 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -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 { @@ -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 { diff --git a/tests/fail/branchless-select-i128-pointer.rs b/tests/fail/branchless-select-i128-pointer.rs index 61fd57f8f05..20fbcd1de78 100644 --- a/tests/fail/branchless-select-i128-pointer.rs +++ b/tests/fail/branchless-select-i128-pointer.rs @@ -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); diff --git a/tests/fail/branchless-select-i128-pointer.stderr b/tests/fail/branchless-select-i128-pointer.stderr index 2e0f8139830..f37dcf955e3 100644 --- a/tests/fail/branchless-select-i128-pointer.stderr +++ b/tests/fail/branchless-select-i128-pointer.stderr @@ -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 diff --git a/tests/fail/pointer_partial_overwrite.rs b/tests/fail/pointer_partial_overwrite.rs index 8bee58d20a5..1bbb33aa2bb 100644 --- a/tests/fail/pointer_partial_overwrite.rs +++ b/tests/fail/pointer_partial_overwrite.rs @@ -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 . +// Also see . fn main() { let mut p = &42; diff --git a/tests/fail/pointer_partial_read.rs b/tests/fail/pointer_partial_read.rs deleted file mode 100644 index a4a5071f5da..00000000000 --- a/tests/fail/pointer_partial_read.rs +++ /dev/null @@ -1,9 +0,0 @@ -// Test what happens when we read parts of a pointer. -// Related to . -fn main() { - let x = 13; - let y = &x; - let z = &y as *const &i32 as *const u8; - // the deref fails, because we are reading only a part of the pointer - let _val = unsafe { *z }; //~ ERROR unable to turn pointer into raw bytes -} diff --git a/tests/fail/pointer_partial_read.stderr b/tests/fail/pointer_partial_read.stderr deleted file mode 100644 index dc35f7e109a..00000000000 --- a/tests/fail/pointer_partial_read.stderr +++ /dev/null @@ -1,14 +0,0 @@ -error: unsupported operation: unable to turn pointer into raw bytes - --> $DIR/pointer_partial_read.rs:LL:CC - | -LL | let _val = unsafe { *z }; - | ^^ unable to turn pointer into raw 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 - - = note: inside `main` at $DIR/pointer_partial_read.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to previous error - diff --git a/tests/fail/provenance/permissive_provenance_transmute.rs b/tests/fail/provenance/permissive_provenance_transmute.rs new file mode 100644 index 00000000000..dbfc5732ed3 --- /dev/null +++ b/tests/fail/provenance/permissive_provenance_transmute.rs @@ -0,0 +1,27 @@ +// compile-flags: -Zmiri-permissive-provenance -Zmiri-disable-stacked-borrows +#![feature(strict_provenance)] + +use std::mem; + +// This is the example from +// . + +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())); + } +} diff --git a/tests/fail/provenance/permissive_provenance_transmute.stderr b/tests/fail/provenance/permissive_provenance_transmute.stderr new file mode 100644 index 00000000000..12f3562011a --- /dev/null +++ b/tests/fail/provenance/permissive_provenance_transmute.stderr @@ -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 + diff --git a/tests/fail/provenance/strict_provenance_transmute.rs b/tests/fail/provenance/strict_provenance_transmute.rs index a684d65b2ce..12a141e9ddf 100644 --- a/tests/fail/provenance/strict_provenance_transmute.rs +++ b/tests/fail/provenance/strict_provenance_transmute.rs @@ -7,13 +7,13 @@ use std::mem; // . 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 } } diff --git a/tests/fail/provenance/strict_provenance_transmute.stderr b/tests/fail/provenance/strict_provenance_transmute.stderr index 544431815c1..8df94d50bba 100644 --- a/tests/fail/provenance/strict_provenance_transmute.stderr +++ b/tests/fail/provenance/strict_provenance_transmute.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: type validation failed: encountered pointer to $HEX[ALLOC], 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], 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 diff --git a/tests/fail/transmute_fat1.rs b/tests/fail/transmute_fat1.rs index da45dad7b7d..22fb4c6fdcc 100644 --- a/tests/fail/transmute_fat1.rs +++ b/tests/fail/transmute_fat1.rs @@ -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")] @@ -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]; } diff --git a/tests/fail/transmute_fat1.stderr b/tests/fail/transmute_fat1.stderr index 2966f042001..ea83dd442d2 100644 --- a/tests/fail/transmute_fat1.stderr +++ b/tests/fail/transmute_fat1.stderr @@ -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 diff --git a/tests/fail/validity/ptr_integer_transmute.rs b/tests/fail/validity/ptr_integer_transmute.rs deleted file mode 100644 index b23ccbbb1b0..00000000000 --- a/tests/fail/validity/ptr_integer_transmute.rs +++ /dev/null @@ -1,4 +0,0 @@ -fn main() { - let r = &mut 42; - let _i: usize = unsafe { std::mem::transmute(r) }; //~ ERROR expected plain (non-pointer) bytes -} diff --git a/tests/fail/validity/ptr_integer_transmute.stderr b/tests/fail/validity/ptr_integer_transmute.stderr deleted file mode 100644 index cad53d71f4d..00000000000 --- a/tests/fail/validity/ptr_integer_transmute.stderr +++ /dev/null @@ -1,15 +0,0 @@ -error: Undefined Behavior: type validation failed: encountered pointer to $HEX[ALLOC], but expected plain (non-pointer) bytes - --> $DIR/ptr_integer_transmute.rs:LL:CC - | -LL | let _i: usize = unsafe { std::mem::transmute(r) }; - | ^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered pointer to $HEX[ALLOC], but expected plain (non-pointer) 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 - - = note: inside `main` at $DIR/ptr_integer_transmute.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to previous error - diff --git a/tests/pass/ptr_int_permissive_provenance.rs b/tests/pass/ptr_int_from_exposed.rs similarity index 100% rename from tests/pass/ptr_int_permissive_provenance.rs rename to tests/pass/ptr_int_from_exposed.rs diff --git a/tests/pass/ptr_int_transmute.rs b/tests/pass/ptr_int_transmute.rs new file mode 100644 index 00000000000..ba50480c539 --- /dev/null +++ b/tests/pass/ptr_int_transmute.rs @@ -0,0 +1,22 @@ +// Test what happens when we read parts of a pointer. +// Related to . +fn ptr_partial_read() { + let x = 13; + let y = &x; + let z = &y as *const &i32 as *const u8; + + // This just strips provenance, but should work fine otherwise. + let _val = unsafe { *z }; +} + +fn transmute_strip_provenance() { + let r = &mut 42; + let addr = r as *mut _ as usize; + let i: usize = unsafe { std::mem::transmute(r) }; + assert_eq!(i, addr); +} + +fn main() { + ptr_partial_read(); + transmute_strip_provenance(); +}