From e0816c87aacd579e1f2a14168e5843e7a7ec5fe5 Mon Sep 17 00:00:00 2001 From: Jan Sommer Date: Thu, 17 Oct 2024 11:03:45 +0200 Subject: [PATCH 1/5] Bump libc to 0.2.161 (cherry picked from commit a09c54d4d34627444d4d1416930c11ffeebd0d2e) --- library/Cargo.lock | 6 +++--- library/std/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/library/Cargo.lock b/library/Cargo.lock index 59b76d8d4427d..268367cd9c144 100644 --- a/library/Cargo.lock +++ b/library/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "addr2line" @@ -158,9 +158,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.159" +version = "0.2.161" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "561d97a539a36e26a9a5fad1ea11a3039a67714694aaa379433e580854bc3dc5" +checksum = "8e9489c2807c139ffd9c1794f4af0ebe86a828db53ecdc7fea2111d0fed085d1" dependencies = [ "rustc-std-workspace-core", ] diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index f2669326065d5..5f8dcde189bcf 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -39,7 +39,7 @@ miniz_oxide = { version = "0.7.0", optional = true, default-features = false } addr2line = { version = "0.22.0", optional = true, default-features = false } [target.'cfg(not(all(windows, target_env = "msvc")))'.dependencies] -libc = { version = "0.2.156", default-features = false, features = [ +libc = { version = "0.2.161", default-features = false, features = [ 'rustc-dep-of-std', ], public = true } From 1766bc36f57794846519425248821fec601fe75a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:12:11 +0800 Subject: [PATCH 2/5] Avoid shadowing user provided types or type aliases in `thread_local!` By using qualified imports, i.e. `$crate::...::LocalKey`. (cherry picked from commit 7b2320c3df9e57e8920a8eeec94e907e3d3e6347) --- .../std/src/sys/thread_local/native/mod.rs | 31 +++++++++---------- library/std/src/sys/thread_local/os.rs | 17 ++++++---- library/std/tests/thread.rs | 23 ++++++++++++++ 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/library/std/src/sys/thread_local/native/mod.rs b/library/std/src/sys/thread_local/native/mod.rs index f498dee0899f9..a5dffe3c45883 100644 --- a/library/std/src/sys/thread_local/native/mod.rs +++ b/library/std/src/sys/thread_local/native/mod.rs @@ -49,20 +49,21 @@ pub use lazy::Storage as LazyStorage; #[unstable(feature = "thread_local_internals", issue = "none")] #[rustc_macro_transparency = "semitransparent"] pub macro thread_local_inner { - // used to generate the `LocalKey` value for const-initialized thread locals + // NOTE: we cannot import `LocalKey`, `LazyStorage` or `EagerStorage` with a `use` because that + // can shadow user provided type or type alias with a matching name. Please update the shadowing + // test in `tests/thread.rs` if these types are renamed. + + // Used to generate the `LocalKey` value for const-initialized thread locals. (@key $t:ty, const $init:expr) => {{ const __INIT: $t = $init; unsafe { - use $crate::mem::needs_drop; - use $crate::thread::LocalKey; - use $crate::thread::local_impl::EagerStorage; - - LocalKey::new(const { - if needs_drop::<$t>() { + $crate::thread::LocalKey::new(const { + if $crate::mem::needs_drop::<$t>() { |_| { #[thread_local] - static VAL: EagerStorage<$t> = EagerStorage::new(__INIT); + static VAL: $crate::thread::local_impl::EagerStorage<$t> + = $crate::thread::local_impl::EagerStorage::new(__INIT); VAL.get() } } else { @@ -84,21 +85,19 @@ pub macro thread_local_inner { } unsafe { - use $crate::mem::needs_drop; - use $crate::thread::LocalKey; - use $crate::thread::local_impl::LazyStorage; - - LocalKey::new(const { - if needs_drop::<$t>() { + $crate::thread::LocalKey::new(const { + if $crate::mem::needs_drop::<$t>() { |init| { #[thread_local] - static VAL: LazyStorage<$t, ()> = LazyStorage::new(); + static VAL: $crate::thread::local_impl::LazyStorage<$t, ()> + = $crate::thread::local_impl::LazyStorage::new(); VAL.get_or_init(init, __init) } } else { |init| { #[thread_local] - static VAL: LazyStorage<$t, !> = LazyStorage::new(); + static VAL: $crate::thread::local_impl::LazyStorage<$t, !> + = $crate::thread::local_impl::LazyStorage::new(); VAL.get_or_init(init, __init) } } diff --git a/library/std/src/sys/thread_local/os.rs b/library/std/src/sys/thread_local/os.rs index 26ce3322a16e3..f5a2aaa6c6a3f 100644 --- a/library/std/src/sys/thread_local/os.rs +++ b/library/std/src/sys/thread_local/os.rs @@ -15,19 +15,24 @@ pub macro thread_local_inner { $crate::thread::local_impl::thread_local_inner!(@key $t, { const INIT_EXPR: $t = $init; INIT_EXPR }) }, - // used to generate the `LocalKey` value for `thread_local!` + // NOTE: we cannot import `Storage` or `LocalKey` with a `use` because that can shadow user + // provided type or type alias with a matching name. Please update the shadowing test in + // `tests/thread.rs` if these types are renamed. + + // used to generate the `LocalKey` value for `thread_local!`. (@key $t:ty, $init:expr) => {{ #[inline] fn __init() -> $t { $init } + // NOTE: this cannot import `LocalKey` or `Storage` with a `use` because that can shadow + // user provided type or type alias with a matching name. Please update the shadowing test + // in `tests/thread.rs` if these types are renamed. unsafe { - use $crate::thread::LocalKey; - use $crate::thread::local_impl::Storage; - // Inlining does not work on windows-gnu due to linking errors around // dllimports. See https://github.com/rust-lang/rust/issues/109797. - LocalKey::new(#[cfg_attr(windows, inline(never))] |init| { - static VAL: Storage<$t> = Storage::new(); + $crate::thread::LocalKey::new(#[cfg_attr(windows, inline(never))] |init| { + static VAL: $crate::thread::local_impl::Storage<$t> + = $crate::thread::local_impl::Storage::new(); VAL.get(init, __init) }) } diff --git a/library/std/tests/thread.rs b/library/std/tests/thread.rs index 83574176186a4..1bb17d149fa10 100644 --- a/library/std/tests/thread.rs +++ b/library/std/tests/thread.rs @@ -38,6 +38,29 @@ fn thread_local_containing_const_statements() { assert_eq!(REFCELL.take(), 1); } +#[test] +fn thread_local_hygeiene() { + // Previously `thread_local_inner!` had use imports for `LocalKey`, `Storage`, `EagerStorage` + // and `LazyStorage`. The use imports will shadow a user-provided type or type alias if the + // user-provided type or type alias has the same name. Make sure that this does not happen. See + // . + // + // NOTE: if the internal implementation details change (i.e. get renamed), this test should be + // updated. + + #![allow(dead_code)] + type LocalKey = (); + type Storage = (); + type LazyStorage = (); + type EagerStorage = (); + thread_local! { + static A: LocalKey = const { () }; + static B: Storage = const { () }; + static C: LazyStorage = const { () }; + static D: EagerStorage = const { () }; + } +} + #[test] // Include an ignore list on purpose, so that new platforms don't miss it #[cfg_attr( From 90a1f5f7896f4ce0e21b40e52553dd5cc0fa6316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 30 Oct 2024 20:17:42 +0800 Subject: [PATCH 3/5] Add a regression test for #132353 To catch at least one pattern that was miscompiled. The test is a minimization of the MCVE reported in . (cherry picked from commit 4d8bda335e23b7ff10ff0c645c825a90fc2646bb) --- ...lone-canonicalization-miscompile-132353.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 tests/ui/mir/clone-canonicalization-miscompile-132353.rs diff --git a/tests/ui/mir/clone-canonicalization-miscompile-132353.rs b/tests/ui/mir/clone-canonicalization-miscompile-132353.rs new file mode 100644 index 0000000000000..ba740c10f9060 --- /dev/null +++ b/tests/ui/mir/clone-canonicalization-miscompile-132353.rs @@ -0,0 +1,25 @@ +//! The mir-opt added in unfortunately seems to lead +//! to a miscompile (reported in , 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 { + 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(); +} From b5bc13a36741c9a5199eada4baf331ff7bdfcc05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 30 Oct 2024 21:11:37 +0800 Subject: [PATCH 4/5] Mark `simplify_aggregate_to_copy` mir-opt as unsound Co-authored-by: DianQK (cherry picked from commit 10b8ba4ecb19ac2eb7be97a1a1eb1ffae9fec534) --- compiler/rustc_mir_transform/src/gvn.rs | 4 ++- tests/codegen/clone_as_copy.rs | 2 ++ tests/codegen/try_question_mark_nop.rs | 14 +++------ tests/mir-opt/gvn_clone.rs | 2 ++ .../mir-opt/gvn_clone.{impl#0}-clone.GVN.diff | 6 ++-- tests/mir-opt/gvn_copy_aggregate.rs | 2 ++ tests/mir-opt/pre-codegen/clone_as_copy.rs | 2 ++ ..._clone.{impl#0}-clone.PreCodegen.after.mir | 6 +++- .../try_identity.old.PreCodegen.after.mir | 8 ++--- ..._to_slice.PreCodegen.after.panic-abort.mir | 30 +++++++++++-------- ...to_slice.PreCodegen.after.panic-unwind.mir | 30 +++++++++++-------- 11 files changed, 63 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index daf868559bcae..de53ebfcd0ee2 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -1079,7 +1079,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> { } } - if let AggregateTy::Def(_, _) = ty + // unsound: https://github.com/rust-lang/rust/issues/132353 + if tcx.sess.opts.unstable_opts.unsound_mir_opts + && let AggregateTy::Def(_, _) = ty && let Some(value) = self.simplify_aggregate_to_copy(rvalue, location, &fields, variant_index) { diff --git a/tests/codegen/clone_as_copy.rs b/tests/codegen/clone_as_copy.rs index 36a59ae56b72b..6ba198297e226 100644 --- a/tests/codegen/clone_as_copy.rs +++ b/tests/codegen/clone_as_copy.rs @@ -1,4 +1,6 @@ //@ revisions: DEBUGINFO NODEBUGINFO +//@ compile-flags: -Zunsound-mir-opts +// FIXME: see //@ compile-flags: -O -Cno-prepopulate-passes //@ [DEBUGINFO] compile-flags: -Cdebuginfo=full diff --git a/tests/codegen/try_question_mark_nop.rs b/tests/codegen/try_question_mark_nop.rs index 65167f5c5af97..bbab0d9eb1dbf 100644 --- a/tests/codegen/try_question_mark_nop.rs +++ b/tests/codegen/try_question_mark_nop.rs @@ -1,10 +1,7 @@ //@ compile-flags: -O -Z merge-functions=disabled --edition=2021 //@ only-x86_64 // FIXME: Remove the `min-llvm-version`. -//@ revisions: NINETEEN TWENTY -//@[NINETEEN] min-llvm-version: 19 -//@[NINETEEN] ignore-llvm-version: 20-99 -//@[TWENTY] min-llvm-version: 20 +//@ min-llvm-version: 19 #![crate_type = "lib"] #![feature(try_blocks)] @@ -16,12 +13,9 @@ use std::ptr::NonNull; #[no_mangle] pub fn option_nop_match_32(x: Option) -> Option { // CHECK: start: - // NINETEEN-NEXT: [[TRUNC:%.*]] = trunc nuw i32 %0 to i1 - // NINETEEN-NEXT: [[FIRST:%.*]] = select i1 [[TRUNC]], i32 %0 - // NINETEEN-NEXT: insertvalue { i32, i32 } poison, i32 [[FIRST]], 0 - // TWENTY-NEXT: insertvalue { i32, i32 } poison, i32 %0, 0 - // CHECK-NEXT: insertvalue { i32, i32 } - // CHECK-NEXT: ret { i32, i32 } + // CHECK-NEXT: [[REG1:%.*]] = insertvalue { i32, i32 } poison, i32 %0, 0 + // CHECK-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } [[REG1]], i32 %1, 1 + // CHECK-NEXT: ret { i32, i32 } [[REG2]] match x { Some(x) => Some(x), None => None, diff --git a/tests/mir-opt/gvn_clone.rs b/tests/mir-opt/gvn_clone.rs index 08938c0e1b427..c16b665fbd39c 100644 --- a/tests/mir-opt/gvn_clone.rs +++ b/tests/mir-opt/gvn_clone.rs @@ -1,3 +1,5 @@ +//@ compile-flags: -Zunsound-mir-opts +// FIXME: see //@ test-mir-pass: GVN //@ compile-flags: -Zmir-enable-passes=+InstSimplify-before-inline diff --git a/tests/mir-opt/gvn_clone.{impl#0}-clone.GVN.diff b/tests/mir-opt/gvn_clone.{impl#0}-clone.GVN.diff index 57b0980a0bd17..8d5991872e180 100644 --- a/tests/mir-opt/gvn_clone.{impl#0}-clone.GVN.diff +++ b/tests/mir-opt/gvn_clone.{impl#0}-clone.GVN.diff @@ -1,7 +1,7 @@ -- // MIR for `::clone` before GVN -+ // MIR for `::clone` after GVN +- // MIR for `::clone` before GVN ++ // MIR for `::clone` after GVN - fn ::clone(_1: &AllCopy) -> AllCopy { + fn ::clone(_1: &AllCopy) -> AllCopy { debug self => _1; let mut _0: AllCopy; let mut _2: i32; diff --git a/tests/mir-opt/gvn_copy_aggregate.rs b/tests/mir-opt/gvn_copy_aggregate.rs index c9473025a15f2..7c181d1ad3784 100644 --- a/tests/mir-opt/gvn_copy_aggregate.rs +++ b/tests/mir-opt/gvn_copy_aggregate.rs @@ -1,3 +1,5 @@ +//@ compile-flags: -Zunsound-mir-opts +// FIXME: see //@ compile-flags: -Cdebuginfo=full // Check if we have transformed the nested clone to the copy in the complete pipeline. diff --git a/tests/mir-opt/pre-codegen/no_inlined_clone.{impl#0}-clone.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/no_inlined_clone.{impl#0}-clone.PreCodegen.after.mir index 9020cf1ef37f2..62a9cd9131f0b 100644 --- a/tests/mir-opt/pre-codegen/no_inlined_clone.{impl#0}-clone.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/no_inlined_clone.{impl#0}-clone.PreCodegen.after.mir @@ -3,9 +3,13 @@ fn ::clone(_1: &Foo) -> Foo { debug self => _1; let mut _0: Foo; + let mut _2: i32; bb0: { - _0 = copy (*_1); + StorageLive(_2); + _2 = copy ((*_1).0: i32); + _0 = Foo { a: move _2 }; + StorageDead(_2); return; } } diff --git a/tests/mir-opt/pre-codegen/try_identity.old.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/try_identity.old.PreCodegen.after.mir index 889e80d26e1cc..ac485f485b1cc 100644 --- a/tests/mir-opt/pre-codegen/try_identity.old.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/try_identity.old.PreCodegen.after.mir @@ -19,14 +19,14 @@ fn old(_1: Result) -> Result { } bb1: { - _3 = copy ((_1 as Ok).0: T); - _0 = copy _1; + _3 = move ((_1 as Ok).0: T); + _0 = Result::::Ok(copy _3); goto -> bb3; } bb2: { - _4 = copy ((_1 as Err).0: E); - _0 = copy _1; + _4 = move ((_1 as Err).0: E); + _0 = Result::::Err(copy _4); goto -> bb3; } diff --git a/tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-abort.mir index 4d964b0afb78c..c3091bd439576 100644 --- a/tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-abort.mir +++ b/tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-abort.mir @@ -7,7 +7,7 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { debug self => _1; scope 2 (inlined Vec::::as_slice) { debug self => _1; - let mut _6: usize; + let mut _7: usize; scope 3 (inlined Vec::::as_ptr) { debug self => _1; let mut _2: &alloc::raw_vec::RawVec; @@ -16,6 +16,7 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { let mut _3: &alloc::raw_vec::RawVecInner; scope 5 (inlined alloc::raw_vec::RawVecInner::ptr::) { debug self => _3; + let mut _6: std::ptr::NonNull; scope 6 (inlined alloc::raw_vec::RawVecInner::non_null::) { debug self => _3; let mut _4: std::ptr::NonNull; @@ -31,20 +32,20 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { } } scope 10 (inlined Unique::::as_non_null_ptr) { - debug ((self: Unique).0: std::ptr::NonNull) => _4; + debug ((self: Unique).0: std::ptr::NonNull) => _6; debug ((self: Unique).1: std::marker::PhantomData) => const PhantomData::; } } scope 11 (inlined NonNull::::as_ptr) { - debug self => _4; + debug self => _6; } } } } scope 12 (inlined std::slice::from_raw_parts::<'_, u8>) { debug data => _5; - debug len => _6; - let _7: *const [u8]; + debug len => _7; + let _8: *const [u8]; scope 13 (inlined core::ub_checks::check_language_ub) { scope 14 (inlined core::ub_checks::check_language_ub::runtime) { } @@ -55,10 +56,10 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { } scope 17 (inlined slice_from_raw_parts::) { debug data => _5; - debug len => _6; + debug len => _7; scope 18 (inlined std::ptr::from_raw_parts::<[u8], u8>) { debug data_pointer => _5; - debug metadata => _6; + debug metadata => _7; } } } @@ -70,17 +71,22 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { _2 = &((*_1).0: alloc::raw_vec::RawVec); StorageLive(_3); _3 = &(((*_1).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner); + StorageLive(_6); + StorageLive(_4); _4 = copy (((((*_1).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); _5 = copy (_4.0: *const u8); + _6 = NonNull:: { pointer: copy _5 }; + StorageDead(_4); + StorageDead(_6); StorageDead(_3); StorageDead(_2); - StorageLive(_6); - _6 = copy ((*_1).1: usize); StorageLive(_7); - _7 = *const [u8] from (copy _5, copy _6); - _0 = &(*_7); + _7 = copy ((*_1).1: usize); + StorageLive(_8); + _8 = *const [u8] from (copy _5, copy _7); + _0 = &(*_8); + StorageDead(_8); StorageDead(_7); - StorageDead(_6); return; } } diff --git a/tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-unwind.mir index 4d964b0afb78c..c3091bd439576 100644 --- a/tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-unwind.mir +++ b/tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-unwind.mir @@ -7,7 +7,7 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { debug self => _1; scope 2 (inlined Vec::::as_slice) { debug self => _1; - let mut _6: usize; + let mut _7: usize; scope 3 (inlined Vec::::as_ptr) { debug self => _1; let mut _2: &alloc::raw_vec::RawVec; @@ -16,6 +16,7 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { let mut _3: &alloc::raw_vec::RawVecInner; scope 5 (inlined alloc::raw_vec::RawVecInner::ptr::) { debug self => _3; + let mut _6: std::ptr::NonNull; scope 6 (inlined alloc::raw_vec::RawVecInner::non_null::) { debug self => _3; let mut _4: std::ptr::NonNull; @@ -31,20 +32,20 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { } } scope 10 (inlined Unique::::as_non_null_ptr) { - debug ((self: Unique).0: std::ptr::NonNull) => _4; + debug ((self: Unique).0: std::ptr::NonNull) => _6; debug ((self: Unique).1: std::marker::PhantomData) => const PhantomData::; } } scope 11 (inlined NonNull::::as_ptr) { - debug self => _4; + debug self => _6; } } } } scope 12 (inlined std::slice::from_raw_parts::<'_, u8>) { debug data => _5; - debug len => _6; - let _7: *const [u8]; + debug len => _7; + let _8: *const [u8]; scope 13 (inlined core::ub_checks::check_language_ub) { scope 14 (inlined core::ub_checks::check_language_ub::runtime) { } @@ -55,10 +56,10 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { } scope 17 (inlined slice_from_raw_parts::) { debug data => _5; - debug len => _6; + debug len => _7; scope 18 (inlined std::ptr::from_raw_parts::<[u8], u8>) { debug data_pointer => _5; - debug metadata => _6; + debug metadata => _7; } } } @@ -70,17 +71,22 @@ fn vec_deref_to_slice(_1: &Vec) -> &[u8] { _2 = &((*_1).0: alloc::raw_vec::RawVec); StorageLive(_3); _3 = &(((*_1).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner); + StorageLive(_6); + StorageLive(_4); _4 = copy (((((*_1).0: alloc::raw_vec::RawVec).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique).0: std::ptr::NonNull); _5 = copy (_4.0: *const u8); + _6 = NonNull:: { pointer: copy _5 }; + StorageDead(_4); + StorageDead(_6); StorageDead(_3); StorageDead(_2); - StorageLive(_6); - _6 = copy ((*_1).1: usize); StorageLive(_7); - _7 = *const [u8] from (copy _5, copy _6); - _0 = &(*_7); + _7 = copy ((*_1).1: usize); + StorageLive(_8); + _8 = *const [u8] from (copy _5, copy _7); + _0 = &(*_8); + StorageDead(_8); StorageDead(_7); - StorageDead(_6); return; } } From 0f95b22c12bf01116622ffff58573a3220bd5528 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Wed, 30 Oct 2024 23:02:18 +0800 Subject: [PATCH 5/5] Add a mir-opt GVN test for #128299 Co-authored-by: DianQK (cherry picked from commit cfb4c05d77df4a6bcc53924eddb3a35102b65da9) --- ..._aggregate_to_copy_miscompile.foo.GVN.diff | 72 +++++++++++++++++++ .../simplify_aggregate_to_copy_miscompile.rs | 32 +++++++++ 2 files changed, 104 insertions(+) create mode 100644 tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff create mode 100644 tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs diff --git a/tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff new file mode 100644 index 0000000000000..22d4277ee4515 --- /dev/null +++ b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.foo.GVN.diff @@ -0,0 +1,72 @@ +- // MIR for `foo` before GVN ++ // MIR for `foo` after GVN + + fn foo(_1: &mut Option) -> Option { + debug v => _1; + let mut _0: std::option::Option; + let mut _2: &std::option::Option; + let mut _3: &std::option::Option; + let _4: &&mut std::option::Option; + let mut _5: isize; + let mut _7: !; + let mut _8: std::option::Option; + let mut _9: i32; + let mut _10: !; + let mut _11: &mut std::option::Option; + 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::::None; +- (*_1) = move _8; ++ _8 = const Option::::None; ++ (*_1) = const Option::::None; + StorageDead(_8); + StorageLive(_9); + _9 = copy _6; +- _0 = Option::::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 __ __ __ __ │ ....░░░░ + } + diff --git a/tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs new file mode 100644 index 0000000000000..47721b768be79 --- /dev/null +++ b/tests/mir-opt/simplify_aggregate_to_copy_miscompile.rs @@ -0,0 +1,32 @@ +//! The `simplify_aggregate_to_copy` mir-opt introduced in +//! 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) -> Option { + 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) -> &Option { + v +}