Misoptimisation of complex control flow only under low mir-opt-level
#112061
Comments
Executing the program trough |
It does, and since the UB involves |
saethlin
added
I-unsound
rustbot
added
the
I-prioritize
|
(defensively labeled this as unsound because that seems more likely than not) |
|
|
|
Here's the diff that ConstProp produces. Does this look relevant? diff --git a/mir_dump/main.fn12_rs.005-002.ConstProp.before.mir b/mir_dump/main.fn12_rs.005-002.ConstProp.after.mir
index 038b484..44ab18e 100644
--- a/mir_dump/main.fn12_rs.005-002.ConstProp.before.mir
+++ b/mir_dump/main.fn12_rs.005-002.ConstProp.after.mir
@@ -1,4 +1,4 @@
-// MIR for `fn12_rs` before ConstProp
+// MIR for `fn12_rs` after ConstProp
fn fn12_rs() -> ([u128; 7], *mut i8, *mut bool) {
let mut _0: ([u128; 7], *mut i8, *mut bool); // return place in scope 0 at main.rs:5:28: 5:59
@@ -213,7 +213,7 @@ fn fn12_rs() -> ([u128; 7], *mut i8, *mut bool) {
StorageLive(_26); // scope 11 at /rustc/1c53407e8c7cc922d718bde61ca34f47b6d2120f/library/core/src/ptr/mod.rs:2034:5: 2034:20
StorageLive(_27); // scope 11 at main.rs:20:43: 20:45
_27 = _3; // scope 11 at main.rs:20:43: 20:45
- _28 = Len(_6); // scope 11 at main.rs:20:39: 20:46
+ _28 = const 8_usize; // scope 11 at main.rs:20:39: 20:46
_29 = Lt(_27, _28); // scope 11 at main.rs:20:39: 20:46
assert(move _29, "index out of bounds: the length is {} but the index is {}", move _28, _27) -> bb12; // scope 11 at main.rs:20:39: 20:46
}
@@ -274,7 +274,7 @@ fn fn12_rs() -> ([u128; 7], *mut i8, *mut bool) {
bb20: {
((_13.2: ([u32; 6], usize, *mut [u32; 6])).0: [u32; 6]) = [const 2262110980_u32; 6]; // scope 11 at main.rs:40:45: 40:75
- _2 = Not(const 13152832795211590855_u64); // scope 11 at main.rs:41:45: 41:75
+ _2 = const 5293911278497960760_u64; // scope 11 at main.rs:41:45: 41:75
(_13.0: usize) = const 6_usize; // scope 11 at main.rs:42:45: 42:54
StorageLive(_35); // scope 11 at main.rs:43:51: 43:56
_35 = (_9.2: *mut bool); // scope 11 at main.rs:43:51: 43:56
@@ -293,7 +293,7 @@ fn fn12_rs() -> ([u128; 7], *mut i8, *mut bool) {
_38 = _7; // scope 11 at main.rs:47:51: 47:54
_6 = move _38; // scope 11 at main.rs:47:45: 47:54
StorageDead(_38); // scope 11 at main.rs:47:53: 47:54
- switchInt((_13.0: usize)) -> [6: bb22, 0: bb24, otherwise: bb21]; // scope 11 at main.rs:48:45: 48:56
+ switchInt(const 6_usize) -> [6: bb22, 0: bb24, otherwise: bb21]; // scope 11 at main.rs:48:45: 48:56
}
bb21: {
@@ -329,7 +329,7 @@ fn fn12_rs() -> ([u128; 7], *mut i8, *mut bool) {
StorageLive(_45); // scope 11 at /rustc/1c53407e8c7cc922d718bde61ca34f47b6d2120f/library/core/src/ptr/mod.rs:2034:5: 2034:20
StorageLive(_46); // scope 11 at main.rs:66:55: 66:57
_46 = _3; // scope 11 at main.rs:66:55: 66:57
- _47 = Len(_6); // scope 11 at main.rs:66:51: 66:58
+ _47 = const 8_usize; // scope 11 at main.rs:66:51: 66:58
_48 = Lt(_46, _47); // scope 11 at main.rs:66:51: 66:58
assert(move _48, "index out of bounds: the length is {} but the index is {}", move _47, _46) -> bb26; // scope 11 at main.rs:66:51: 66:58
}Relevant stuff is buried down in the pile of scopes ugh. The relevant diff smells like - switchInt((_13.0: usize)) -> [6: bb22, 0: bb24, otherwise: bb21]; // scope 11 at main.rs:48:45: 48:56
+ switchInt(const 6_usize) -> [6: bb22, 0: bb24, otherwise: bb21]; // scope 11 at main.rs:48:45: 48:56 |
- switchInt((_13.0: usize)) -> [6: bb22, 0: bb24, otherwise: bb21]; // scope 11 at main.rs:48:45: 48:56
+ switchInt(const 6_usize) -> [6: bb22, 0: bb24, otherwise: bb21]; // scope 11 at main.rs:48:45: 48:56This is the relevant diff, but ConstProp is correct here: |
|
Slightly edited to make the executed control flow simpler: no loop is taken more than once except for Seeuse std::ptr;
pub fn print_var(v: u8) {
println!("{v}");
}
pub unsafe fn fn12_rs() -> ([u128; 7], *mut i8, *mut bool) {
let mut v2: bool = false;
let mut v8: u64 = 0;
let mut v9: usize = 0;
let mut v12: *mut u8 = ptr::null_mut();
let mut v17: *mut bool = ptr::null_mut();
let mut v20: [u8; 8] = Default::default();
let mut v21: [u8; 8] = Default::default();
let mut v31: (bool, u8, usize, f32) = Default::default();
let mut v33: ([u128; 7], *mut i8, *mut bool) = ([0; 7], ptr::null_mut(), ptr::null_mut());
let mut v39: (usize, [u128; 7], ([u32; 6], usize, *mut [u32; 6]), [u32; 2]) =
(0, [0; 7], ([0; 6], 0, ptr::null_mut()), [0; 2]);
let mut ret: ([u128; 7], *mut i8, *mut bool) = ([0; 7], ptr::null_mut(), ptr::null_mut());
ret.2 = core::ptr::addr_of_mut!(v2);
'l0: loop {
v12 = core::ptr::addr_of_mut!(v20[v9]);
v20 = [197_u8; 8];
v9 = 2_usize;
'l1: loop {
match *v12 {
197 => {
// Taken
v8 = 13978819448286864680_u64;
v33.2 = ret.2;
match v39.0 {
0 => {
// Taken
'l2: loop {
v20 = [11_u8; 8]; // What LLVM with low mir-opt prints
(*v12) = 22; // What Miri prints
'l3: loop {
v21 = v20;
match v8 {
13978819448286864680 => {
// Taken
v39.2 .0 = [2262110980_u32; 6];
v8 = 2;
v39.0 = 6;
v17 = v33.2;
v33.2 = core::ptr::addr_of_mut!(v31.0);
v31.1 = *v12;
(*v17) = true;
v20 = v21;
match v39.0 {
6 => {
// Taken
print_var(v31.1);
// Loop on 'l3, then since v8 == 2 we return
}
0 => continue 'l2,
_ => return ret,
}
}
2 => return ret,
_ => continue 'l0,
}
}
}
}
_ => return ret,
}
}
4 => {
v12 = core::ptr::addr_of_mut!(v20[v9]);
}
_ => return ret,
}
}
}
}
pub fn main() {
unsafe {
fn12_rs();
}
} |
|
I put this together to look through what LLVM is doing with and without ConstProp enabled: https://godbolt.org/z/4ohsGae5z |
saethlin
added
A-LLVM
|
Regression in nightly-2022-08-13 |
|
WG-prioritization assigning priority (Zulip discussion). Provisionally assigning a P-high priority, will be discussed in the next compiler meeting. @rustbot label -I-prioritize +P-high +I-compiler-nominated |
rustbot
added
I-compiler-nominated
apiraino
added
the
T-compiler
Note that the compiler is not intended to exploit SB yet so this is still a bug (either in rustc, or in TB failing to represent some UB that we really need). |
|
Since this only happens on low optimization levels, and based on the regression range, looks like an LLVM issue masked by rustc ConstProp? Cc @rust-lang/wg-llvm |
|
This version has no UB under Stacked Borrows but still triggers the misoptimisation use std::ptr;
pub fn print_var(v: u8) {
println!("{v}");
}
pub unsafe fn fn12_rs() -> ([u128; 7], *mut i8, *mut bool) {
let mut v2: bool = false;
let mut v8: u64 = 0;
let mut v9: usize = 0;
let mut v12: *mut u8 = ptr::null_mut();
let mut v17: *mut bool = ptr::null_mut();
let mut v20: [u8; 8] = Default::default();
let mut v21: [u8; 8] = Default::default();
let mut v31: (bool, u8, usize, f32) = Default::default();
let mut v33: ([u128; 7], *mut i8, *mut bool) = ([0; 7], ptr::null_mut(), ptr::null_mut());
let mut v39: (usize, [u128; 7], ([u32; 6], usize, *mut [u32; 6]), [u32; 2]) =
(0, [0; 7], ([0; 6], 0, ptr::null_mut()), [0; 2]);
let mut ret: ([u128; 7], *mut i8, *mut bool) = ([0; 7], ptr::null_mut(), ptr::null_mut());
ret.2 = core::ptr::addr_of_mut!(v2);
'l0: loop {
v20 = [197_u8; 8];
let v20_ptr = ptr::addr_of_mut!(v20);
v12 = core::ptr::addr_of_mut!((*v20_ptr)[v9]);
v9 = 2_usize;
'l1: loop {
match *v12 {
197 => {
// Taken
v8 = 13978819448286864680_u64;
v33.2 = ret.2;
match v39.0 {
0 => {
// Taken
'l2: loop {
(*v20_ptr) = [11_u8; 8]; // What LLVM with low mir-opt prints
(*v12) = 22; // What Miri prints
'l3: loop {
v21 = *v20_ptr;
match v8 {
13978819448286864680 => {
// Taken
v39.2 .0 = [2262110980_u32; 6];
v8 = 2;
v39.0 = 6;
v17 = v33.2;
v33.2 = core::ptr::addr_of_mut!(v31.0);
v31.1 = *v12;
(*v17) = true;
(*v20_ptr) = v21;
match v39.0 {
6 => {
// Taken
print_var(v31.1);
}
0 => continue 'l2,
_ => return ret,
}
}
2 => return ret,
_ => continue 'l0,
}
}
}
}
_ => return ret,
}
}
4 => {
v12 = core::ptr::addr_of_mut!((*v20_ptr)[v9]);
}
_ => return ret,
}
}
}
}
pub fn main() {
unsafe {
fn12_rs();
}
} |
|
I've started to minimize it further: use std::ptr;
#[inline(never)]
pub fn print_var(v: u8) {
println!("{v}");
}
pub unsafe fn fn12_rs() {
let mut bool_storage: bool = false;
let mut v9: usize = 0;
'l0: loop {
let mut v20 = [197_u8; 8];
let v20_ptr = ptr::addr_of_mut!(v20);
let mut v12: *mut u8 = core::ptr::addr_of_mut!((*v20_ptr)[v9]);
v9 = 2_usize; // unused but necessary write
loop { // only runs once, but necessary
match *v12 {
197 => {
let mut match_condition: u64 = 0;
let mut v33: *mut bool = core::ptr::addr_of_mut!(bool_storage);
let mut key_read: (bool, u8) = (false, 0);
let mut v39: (usize, [u32; 6]) = (0, [0; 6]);
// Taken
'l2: loop {
(*v20_ptr) = [11_u8; 8]; // What LLVM with low mir-opt prints
(*v12) = 22; // What Miri prints
loop {
let v21 = *v20_ptr;
match match_condition {
0 => {
// Taken
v39.1 = [1; 6];
match_condition = 2;
v39.0 = 6;
let v17 = v33;
v33 = core::ptr::addr_of_mut!(key_read.0);
key_read.1 = *v12;
(*v17) = true;
(*v20_ptr) = v21;
match v39.0 {
6 => {
// Taken
print_var(key_read.1);
}
0 => continue 'l2,
_ => return,
}
}
2 => return,
_ => continue 'l0,
}
}
}
}
_ => {
// Dead code but necessary
v12 = core::ptr::addr_of_mut!((*v20_ptr)[2]);
}
}
}
}
}
pub fn main() {
unsafe {
fn12_rs();
}
}more updates will come later |
|
@Nilstrieb I don't have a compiled LLVM lying around, could you run your reduction with an LLVM built with ASan? My fuzzer was also running into this segfault llvm/llvm-project#63013, and on this repro when I tried |
|
I don't have an LLVM built with ASan around either right now, but I do have alive2 yelling at me when I feed in a reduced version of this test case. I'll create an issue soon I guess. But I can look into ASan as well. |
|
I checked with ASan and it all looked fine. As backlinked already, I also created an LLVM issue: llvm/llvm-project#63019 |
apiraino
removed
the
I-compiler-nominated
|
The bug has been fixed in llvm/llvm-project@97f0e7b |
Apologies for the not-very-readable reproduction. It was generated by a fuzzer and this is the best I can minimise it to.
The program has no UB under
-Zmiri-tree-borrows, and Miri prints 22, this is also the result with-Zmir-opt-level>=2 -Copt-level=3Very strangely, if you drop
mir-opt-levelto 0 or 1, the result becomes 11 which is wrong.Reproducible on both Apple Silicon macOS and x86_64 Linux
cc @RalfJung @nikic
The text was updated successfully, but these errors were encountered: