Skip to content

Commit

Permalink
Auto merge of #98112 - saethlin:mir-alignment-checks, r=oli-obk
Browse files Browse the repository at this point in the history
Insert alignment checks for pointer dereferences when debug assertions are enabled

Closes rust-lang/rust#54915

- [x] Jake tells me this sounds like a place to use `MirPatch`, but I can't figure out how to insert a new basic block with a new terminator in the middle of an existing basic block, using `MirPatch`. (if nobody else backs up this point I'm checking this as "not actually a good idea" because the code looks pretty clean to me after rearranging it a bit)
- [x] Using `CastKind::PointerExposeAddress` is definitely wrong, we don't want to expose. Calling a function to get the pointer address seems quite excessive. ~I'll see if I can add a new `CastKind`.~ `CastKind::Transmute` to the rescue!
- [x] Implement a more helpful panic message like slice bounds checking.

r? `@oli-obk`
  • Loading branch information
bors committed Mar 31, 2023
2 parents d061a3b + 1115f04 commit c221618
Show file tree
Hide file tree
Showing 16 changed files with 53 additions and 11 deletions.
28 changes: 28 additions & 0 deletions src/shims/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,34 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
},
)?;
}
MisalignedPointerDereference { required, found } => {
// Forward to `panic_misaligned_pointer_dereference` lang item.

// First arg: required.
let required = this.read_scalar(&this.eval_operand(required, None)?)?;
// Second arg: found.
let found = this.read_scalar(&this.eval_operand(found, None)?)?;

// Call the lang item.
let panic_misaligned_pointer_dereference =
this.tcx.lang_items().panic_misaligned_pointer_dereference_fn().unwrap();
let panic_misaligned_pointer_dereference =
ty::Instance::mono(this.tcx.tcx, panic_misaligned_pointer_dereference);
this.call_function(
panic_misaligned_pointer_dereference,
Abi::Rust,
&[required.into(), found.into()],
None,
StackPopCleanup::Goto {
ret: None,
unwind: match unwind {
Some(cleanup) => StackPopUnwind::Cleanup(cleanup),
None => StackPopUnwind::Skip,
},
},
)?;
}

_ => {
// Forward everything else to `panic` lang item.
this.start_panic(
Expand Down
1 change: 1 addition & 0 deletions tests/fail/unaligned_pointers/alignment.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//@normalize-stderr-test: "\| +\^+" -> "| ^"
//@compile-flags: -Cdebug-assertions=no

fn main() {
// No retry needed, this fails reliably.
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/atomic_unaligned.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@compile-flags: -Zmiri-symbolic-alignment-check
//@compile-flags: -Zmiri-symbolic-alignment-check -Cdebug-assertions=no
#![feature(core_intrinsics)]

fn main() {
Expand Down
2 changes: 2 additions & 0 deletions tests/fail/unaligned_pointers/drop_in_place.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//@compile-flags: -Cdebug-assertions=no

#[repr(transparent)]
struct HasDrop(u8);

Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/dyn_alignment.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// should find the bug even without validation and stacked borrows, but gets masked by optimizations
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Zmir-opt-level=0
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Zmir-opt-level=0 -Cdebug-assertions=no

#[repr(align(256))]
#[derive(Debug)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@compile-flags: -Zmiri-symbolic-alignment-check -Zmiri-permissive-provenance
//@compile-flags: -Zmiri-symbolic-alignment-check -Zmiri-permissive-provenance -Cdebug-assertions=no
// With the symbolic alignment check, even with intptrcast and without
// validation, we want to be *sure* to catch bugs that arise from pointers being
// insufficiently aligned. The only way to achieve that is not not let programs
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/reference_to_packed.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// This should fail even without validation/SB
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Cdebug-assertions=no

#![allow(dead_code, unused_variables)]

Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/unaligned_ptr1.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// This should fail even without validation or Stacked Borrows.
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Cdebug-assertions=no

fn main() {
// Try many times as this might work by chance.
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/unaligned_ptr2.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// This should fail even without validation or Stacked Borrows.
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Cdebug-assertions=no

fn main() {
// No retry needed, this fails reliably.
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/unaligned_ptr3.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// This should fail even without validation or Stacked Borrows.
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Cdebug-assertions=no

fn main() {
// Try many times as this might work by chance.
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/unaligned_ptr4.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// This should fail even without validation or Stacked Borrows.
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Cdebug-assertions=no

fn main() {
// Make sure we notice when a u16 is loaded at offset 1 into a u8 allocation.
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/unaligned_ptr_addr_of.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// This should fail even without validation or Stacked Borrows.
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows
//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Cdebug-assertions=no
use std::ptr;

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion tests/fail/unaligned_pointers/unaligned_ptr_zst.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This should fail even without validation
// Some optimizations remove ZST accesses, thus masking this UB.
//@compile-flags: -Zmir-opt-level=0 -Zmiri-disable-validation
//@compile-flags: -Zmir-opt-level=0 -Zmiri-disable-validation -Cdebug-assertions=no

fn main() {
// Try many times as this might work by chance.
Expand Down
9 changes: 9 additions & 0 deletions tests/panic/alignment-assertion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@compile-flags: -Zmiri-disable-alignment-check -Cdebug-assertions=yes

fn main() {
let mut x = [0u32; 2];
let ptr: *mut u8 = x.as_mut_ptr().cast::<u8>();
unsafe {
*(ptr.add(1).cast::<u32>()) = 42;
}
}
2 changes: 2 additions & 0 deletions tests/panic/alignment-assertion.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
thread 'main' panicked at 'misaligned pointer dereference: address must be a multiple of 0x4 but is $HEX', $DIR/alignment-assertion.rs:LL:CC
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
2 changes: 1 addition & 1 deletion tests/pass/disable-alignment-check.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@revisions: stack tree
//@[tree]compile-flags: -Zmiri-tree-borrows
//@compile-flags: -Zmiri-disable-alignment-check
//@compile-flags: -Zmiri-disable-alignment-check -Cdebug-assertions=no

fn main() {
let mut x = [0u8; 20];
Expand Down

0 comments on commit c221618

Please sign in to comment.