forked from rust-lang/rust
-
Notifications
You must be signed in to change notification settings - Fork 6
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of rust-lang#132356 - jieyouxu:unsound-simplify_aggregate_…
…to_copy, r=cjgillot Mark `simplify_aggregate_to_copy` mir-opt as unsound Mark the `simplify_aggregate_to_copy` mir-opt added in rust-lang#128299 as unsound as it seems to miscompile the MCVE reported in rust-lang#132353. The mir-opt can be re-enabled once this case is fixed. ```rs fn pop_min(mut score2head: Vec<Option<usize>>) -> Option<usize> { loop { if let Some(col) = score2head[0] { score2head[0] = None; return Some(col); } } } fn main() { let min = pop_min(vec![Some(1)]); println!("min: {:?}", min); // panic happens here on beta in release mode // but not in debug mode min.unwrap(); } ``` This MCVE is included as a `run-pass` ui regression test in the first commit. I built the ui test with a nightly manually, and can reproduce the behavioral difference with `-C opt-level=0` and `-C opt-level=1`. Locally, this ui test will fail unless it was run on a compiler built with the second commit marking the mir-opt as unsound thus disabling it by default. This PR **partially reverts** commit e7386b3, reversing changes made to 02b1be1. The mir-opt implementation is just marked as unsound but **not** reverted to make reland reviews easier. The test changes are **fully** reverted. cc `@DianQK` `@cjgillot` (PR author and reviewer of rust-lang#128299)
- Loading branch information
Showing
13 changed files
with
188 additions
and
33 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
72 changes: 72 additions & 0 deletions
72
tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| - // MIR for `foo` before GVN | ||
| + // MIR for `foo` after GVN | ||
|
|
||
| fn foo(_1: &mut Option<i32>) -> Option<i32> { | ||
| debug v => _1; | ||
| let mut _0: std::option::Option<i32>; | ||
| let mut _2: &std::option::Option<i32>; | ||
| let mut _3: &std::option::Option<i32>; | ||
| let _4: &&mut std::option::Option<i32>; | ||
| let mut _5: isize; | ||
| let mut _7: !; | ||
| let mut _8: std::option::Option<i32>; | ||
| let mut _9: i32; | ||
| let mut _10: !; | ||
| let mut _11: &mut std::option::Option<i32>; | ||
| scope 1 { | ||
| debug col => _6; | ||
| let _6: i32; | ||
| } | ||
|
|
||
| bb0: { | ||
| - StorageLive(_2); | ||
| + nop; | ||
| StorageLive(_3); | ||
| StorageLive(_4); | ||
| _4 = &_1; | ||
| - _11 = deref_copy (*_4); | ||
| - _3 = &(*_11); | ||
| + _11 = copy _1; | ||
| + _3 = &(*_1); | ||
| _2 = get(move _3) -> [return: bb1, unwind unreachable]; | ||
| } | ||
|
|
||
| bb1: { | ||
| StorageDead(_3); | ||
| _5 = discriminant((*_2)); | ||
| switchInt(move _5) -> [1: bb2, otherwise: bb3]; | ||
| } | ||
|
|
||
| bb2: { | ||
| - StorageLive(_6); | ||
| + nop; | ||
| _6 = copy (((*_2) as Some).0: i32); | ||
| StorageLive(_8); | ||
| - _8 = Option::<i32>::None; | ||
| - (*_1) = move _8; | ||
| + _8 = const Option::<i32>::None; | ||
| + (*_1) = const Option::<i32>::None; | ||
| StorageDead(_8); | ||
| StorageLive(_9); | ||
| _9 = copy _6; | ||
| - _0 = Option::<i32>::Some(move _9); | ||
| + _0 = copy (*_2); | ||
| StorageDead(_9); | ||
| - StorageDead(_6); | ||
| + nop; | ||
| StorageDead(_4); | ||
| - StorageDead(_2); | ||
| + nop; | ||
| return; | ||
| } | ||
|
|
||
| bb3: { | ||
| StorageLive(_10); | ||
| unreachable; | ||
| } | ||
| + } | ||
| + | ||
| + ALLOC0 (size: 8, align: 4) { | ||
| + 00 00 00 00 __ __ __ __ │ ....░░░░ | ||
| } | ||
|
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| //! The `simplify_aggregate_to_copy` mir-opt introduced in | ||
| //! <https://github.com/rust-lang/rust/pull/128299> caused a miscompile because the initial | ||
| //! implementation | ||
| //! | ||
| //! > introduce[d] new dereferences without checking for aliasing | ||
| //! | ||
| //! This test demonstrates the behavior, and should be adjusted or removed when fixing and relanding | ||
| //! the mir-opt. | ||
| #![crate_type = "lib"] | ||
| // skip-filecheck | ||
| //@ compile-flags: -O -Zunsound-mir-opts | ||
| //@ test-mir-pass: GVN | ||
| #![allow(internal_features)] | ||
| #![feature(rustc_attrs, core_intrinsics)] | ||
|
|
||
| // EMIT_MIR simplify_aggregate_to_copy_miscompile.foo.GVN.diff | ||
| #[no_mangle] | ||
| fn foo(v: &mut Option<i32>) -> Option<i32> { | ||
| if let &Some(col) = get(&v) { | ||
| *v = None; | ||
| return Some(col); | ||
| } else { | ||
| unsafe { std::intrinsics::unreachable() } | ||
| } | ||
| } | ||
|
|
||
| #[no_mangle] | ||
| #[inline(never)] | ||
| #[rustc_nounwind] | ||
| fn get(v: &Option<i32>) -> &Option<i32> { | ||
| v | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| //! The mir-opt added in <https://github.com/rust-lang/rust/pull/128299> unfortunately seems to lead | ||
| //! to a miscompile (reported in <https://github.com/rust-lang/rust/issues/132353>, minimization | ||
| //! reproduced in this test file). | ||
| //@ revisions: release debug | ||
| // Note: it's not strictly cargo's release profile, but any non-zero opt-level was sufficient to | ||
| // reproduce the miscompile. | ||
| //@[release] compile-flags: -C opt-level=1 | ||
| //@[debug] compile-flags: -C opt-level=0 | ||
| //@ run-pass | ||
|
|
||
| fn pop_min(mut score2head: Vec<Option<usize>>) -> Option<usize> { | ||
| loop { | ||
| if let Some(col) = score2head[0] { | ||
| score2head[0] = None; | ||
| return Some(col); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn main() { | ||
| let min = pop_min(vec![Some(1)]); | ||
| println!("min: {:?}", min); | ||
| // panic happened on 1.83.0 beta in release mode but not debug mode. | ||
| let _ = min.unwrap(); | ||
| } |