mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #107971 - saethlin:mir-opt-ub, r=cjgillot

Browse Source 
Clearly document intentional UB in mir-opt tests

All of the changed mir-opt test input files did not pass Miri. Now they do.

r? `@cjgillot` because there's a CopyProp test in here that I do not fully understand
This commit is contained in:
Matthias Krüger 2023-02-13 23:25:13 +01:00 committed by GitHub
commit 5915309645
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 49 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
tests/mir-opt/copy-prop/mutate_through_pointer.rs
View File

@ -1,3 +1,13 @@
// This attempts to mutate `a` via a pointer derived from `addr_of!(a)`. That is UB
// according to Miri. However, the decision to make this UB - and to allow
// rustc to rely on that fact for the purpose of optimizations - has not been
// finalized.
//
// As such, we include this test to ensure that copy prop does not rely on that
// fact. Specifically, if `addr_of!(a)` could not be used to modify a, it would
// be correct for CopyProp to replace all occurrences of `a` with `c` - but that
// would cause `f(true)` to output `false` instead of `true`.
#![feature(custom_mir, core_intrinsics)]
#![allow(unused_assignments)]
extern crate core;

2
tests/mir-opt/dataflow-const-prop/sibling_ptr.main.DataflowConstProp.diff
View File

@ -32,7 +32,7 @@
_5 = _3; // scope 3 at $DIR/sibling_ptr.rs:+4:10: +4:11
_4 = ptr::mut_ptr::<impl *mut u8>::add(move _5, const 1_usize) -> bb1; // scope 3 at $DIR/sibling_ptr.rs:+4:10: +4:18
// mir::Constant
// + span: $DIR/sibling_ptr.rs:8:12: 8:15
// + span: $DIR/sibling_ptr.rs:15:12: 15:15
// + literal: Const { ty: unsafe fn(*mut u8, usize) -> *mut u8 {ptr::mut_ptr::<impl *mut u8>::add}, val: Value(<ZST>) }
}

9
tests/mir-opt/dataflow-const-prop/sibling_ptr.rs
View File

@ -1,3 +1,10 @@
// This attempts to modify `x.1` via a pointer derived from `addr_of_mut!(x.0)`.
// According to Miri, that is UB. However, T-opsem has not finalized that
// decision and as such we cannot rely on it in optimizations. Consequently,
// DataflowConstProp must treat the `addr_of_mut!(x.0)` as potentially being
// used to modify `x.1` - if it did not, then it might incorrectly assume that it
// can infer the value of `x.1` at the end of this function.
// unit-test: DataflowConstProp
// EMIT_MIR sibling_ptr.main.DataflowConstProp.diff
@ -7,5 +14,5 @@ fn main() {
let p = std::ptr::addr_of_mut!(x.0);
*p.add(1) = 1;
}
let x1 = x.1; // should not be propagated
let x1 = x.1; // should not be propagated
}

48
tests/mir-opt/sroa.escaping.ScalarReplacementOfAggregates.diff
View File

@ -3,42 +3,42 @@
fn escaping() -> () {
let mut _0: (); // return place in scope 0 at $DIR/sroa.rs:+0:19: +0:19
let _1: (); // in scope 0 at $DIR/sroa.rs:+2:5: +2:42
let mut _2: *const u32; // in scope 0 at $DIR/sroa.rs:+2:7: +2:41
let _3: &u32; // in scope 0 at $DIR/sroa.rs:+2:7: +2:41
let _4: Escaping; // in scope 0 at $DIR/sroa.rs:+2:8: +2:39
let mut _5: u32; // in scope 0 at $DIR/sroa.rs:+2:34: +2:37
let _1: (); // in scope 0 at $DIR/sroa.rs:+1:5: +1:42
let mut _2: *const u32; // in scope 0 at $DIR/sroa.rs:+1:7: +1:41
let _3: &u32; // in scope 0 at $DIR/sroa.rs:+1:7: +1:41
let _4: Escaping; // in scope 0 at $DIR/sroa.rs:+1:8: +1:39
let mut _5: u32; // in scope 0 at $DIR/sroa.rs:+1:34: +1:37
bb0: {
StorageLive(_1); // scope 0 at $DIR/sroa.rs:+2:5: +2:42
StorageLive(_2); // scope 0 at $DIR/sroa.rs:+2:7: +2:41
StorageLive(_3); // scope 0 at $DIR/sroa.rs:+2:7: +2:41
StorageLive(_4); // scope 0 at $DIR/sroa.rs:+2:8: +2:39
StorageLive(_5); // scope 0 at $DIR/sroa.rs:+2:34: +2:37
_5 = g() -> bb1; // scope 0 at $DIR/sroa.rs:+2:34: +2:37
StorageLive(_1); // scope 0 at $DIR/sroa.rs:+1:5: +1:42
StorageLive(_2); // scope 0 at $DIR/sroa.rs:+1:7: +1:41
StorageLive(_3); // scope 0 at $DIR/sroa.rs:+1:7: +1:41
StorageLive(_4); // scope 0 at $DIR/sroa.rs:+1:8: +1:39
StorageLive(_5); // scope 0 at $DIR/sroa.rs:+1:34: +1:37
_5 = g() -> bb1; // scope 0 at $DIR/sroa.rs:+1:34: +1:37
// mir::Constant
// + span: $DIR/sroa.rs:73:34: 73:35
// + span: $DIR/sroa.rs:78:34: 78:35
// + literal: Const { ty: fn() -> u32 {g}, val: Value(<ZST>) }
}
bb1: {
_4 = Escaping { a: const 1_u32, b: const 2_u32, c: move _5 }; // scope 0 at $DIR/sroa.rs:+2:8: +2:39
StorageDead(_5); // scope 0 at $DIR/sroa.rs:+2:38: +2:39
_3 = &(_4.0: u32); // scope 0 at $DIR/sroa.rs:+2:7: +2:41
_2 = &raw const (*_3); // scope 0 at $DIR/sroa.rs:+2:7: +2:41
_1 = f(move _2) -> bb2; // scope 0 at $DIR/sroa.rs:+2:5: +2:42
_4 = Escaping { a: const 1_u32, b: const 2_u32, c: move _5 }; // scope 0 at $DIR/sroa.rs:+1:8: +1:39
StorageDead(_5); // scope 0 at $DIR/sroa.rs:+1:38: +1:39
_3 = &(_4.0: u32); // scope 0 at $DIR/sroa.rs:+1:7: +1:41
_2 = &raw const (*_3); // scope 0 at $DIR/sroa.rs:+1:7: +1:41
_1 = f(move _2) -> bb2; // scope 0 at $DIR/sroa.rs:+1:5: +1:42
// mir::Constant
// + span: $DIR/sroa.rs:73:5: 73:6
// + span: $DIR/sroa.rs:78:5: 78:6
// + literal: Const { ty: fn(*const u32) {f}, val: Value(<ZST>) }
}
bb2: {
StorageDead(_2); // scope 0 at $DIR/sroa.rs:+2:41: +2:42
StorageDead(_4); // scope 0 at $DIR/sroa.rs:+2:42: +2:43
StorageDead(_3); // scope 0 at $DIR/sroa.rs:+2:42: +2:43
StorageDead(_1); // scope 0 at $DIR/sroa.rs:+2:42: +2:43
_0 = const (); // scope 0 at $DIR/sroa.rs:+0:19: +3:2
return; // scope 0 at $DIR/sroa.rs:+3:2: +3:2
StorageDead(_2); // scope 0 at $DIR/sroa.rs:+1:41: +1:42
StorageDead(_4); // scope 0 at $DIR/sroa.rs:+1:42: +1:43
StorageDead(_3); // scope 0 at $DIR/sroa.rs:+1:42: +1:43
StorageDead(_1); // scope 0 at $DIR/sroa.rs:+1:42: +1:43
_0 = const (); // scope 0 at $DIR/sroa.rs:+0:19: +2:2
return; // scope 0 at $DIR/sroa.rs:+2:2: +2:2
}
}

7
tests/mir-opt/sroa.rs
View File

@ -68,8 +68,13 @@ fn f(a: *const u32) {
println!("{}", unsafe { *a.add(2) });
}
// `f` uses the `&e.a` to access `e.c`. This is UB according to Miri today; however,
// T-opsem has not finalized that decision and as such rustc should not rely on
// it. If SROA were to rely on it, it would be (almost) correct to turn `e` into
// three distinct locals - one for each field - and pass a reference to only one
// of them to `f`. However, this would lead to a miscompilation because `b` and `c`
// might no longer appear right after `a` in memory.
pub fn escaping() {
// Verify this struct is not flattened.
f(&Escaping { a: 1, b: 2, c: g() }.a);
}