diff options
| author | bors <bors@rust-lang.org> | 2023-08-25 13:27:21 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2023-08-25 13:27:21 +0000 |
| commit | 25ed43ddf3e03835e5a2f2c25463ceea17a66ae5 (patch) | |
| tree | 1f37e9ef91cf6f9d2d24c7e396893fccb08fe6bd | |
| parent | 738df13e8a73d6d95ddec81b7393b2d2a64b7e93 (diff) | |
| parent | 15a68610dd06351feac543afcd0802e6e2622ac8 (diff) | |
| download | rust-25ed43ddf3e03835e5a2f2c25463ceea17a66ae5.tar.gz rust-25ed43ddf3e03835e5a2f2c25463ceea17a66ae5.zip | |
Auto merge of #115138 - cjgillot:dse-move-packed, r=compiler-errors
Do not convert copies of packed projections to moves. This code path was introduced in https://github.com/rust-lang/rust/pull/113758 After seeing https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/Packed.20fields.20and.20in-place.20function.20argument.2Freturn.20passing, this may be UB, so should be disallowed. This should not appear in normally-built MIR, which introduces temporary copies for packed projections.
6 files changed, 64 insertions, 2 deletions
diff --git a/compiler/rustc_const_eval/src/util/alignment.rs b/compiler/rustc_const_eval/src/util/alignment.rs index 4f39dad205a..c1f0ff260d2 100644 --- a/compiler/rustc_const_eval/src/util/alignment.rs +++ b/compiler/rustc_const_eval/src/util/alignment.rs @@ -40,7 +40,7 @@ where } } -fn is_within_packed<'tcx, L>( +pub fn is_within_packed<'tcx, L>( tcx: TyCtxt<'tcx>, local_decls: &L, place: Place<'tcx>, diff --git a/compiler/rustc_const_eval/src/util/mod.rs b/compiler/rustc_const_eval/src/util/mod.rs index 289e3422595..0aef7fa469e 100644 --- a/compiler/rustc_const_eval/src/util/mod.rs +++ b/compiler/rustc_const_eval/src/util/mod.rs @@ -5,7 +5,7 @@ mod check_validity_requirement; mod compare_types; mod type_name; -pub use self::alignment::is_disaligned; +pub use self::alignment::{is_disaligned, is_within_packed}; pub use self::check_validity_requirement::check_validity_requirement; pub use self::compare_types::{is_equal_up_to_subtyping, is_subtype}; pub use self::type_name::type_name; diff --git a/compiler/rustc_mir_transform/src/dead_store_elimination.rs b/compiler/rustc_mir_transform/src/dead_store_elimination.rs index 3f988930b5e..ef14105041b 100644 --- a/compiler/rustc_mir_transform/src/dead_store_elimination.rs +++ b/compiler/rustc_mir_transform/src/dead_store_elimination.rs @@ -12,6 +12,7 @@ //! will still not cause any further changes. //! +use crate::util::is_within_packed; use rustc_index::bit_set::BitSet; use rustc_middle::mir::visit::Visitor; use rustc_middle::mir::*; @@ -49,6 +50,11 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitS && !place.is_indirect() && !borrowed.contains(place.local) && !state.contains(place.local) + // If `place` is a projection of a disaligned field in a packed ADT, + // the move may be codegened as a pointer to that field. + // Using that disaligned pointer may trigger UB in the callee, + // so do nothing. + && is_within_packed(tcx, body, place).is_none() { call_operands_to_move.push((bb, index)); } diff --git a/tests/mir-opt/dead-store-elimination/call_arg_copy.move_packed.DeadStoreElimination.panic-abort.diff b/tests/mir-opt/dead-store-elimination/call_arg_copy.move_packed.DeadStoreElimination.panic-abort.diff new file mode 100644 index 00000000000..dc0f9073416 --- /dev/null +++ b/tests/mir-opt/dead-store-elimination/call_arg_copy.move_packed.DeadStoreElimination.panic-abort.diff @@ -0,0 +1,15 @@ +- // MIR for `move_packed` before DeadStoreElimination ++ // MIR for `move_packed` after DeadStoreElimination + + fn move_packed(_1: Packed) -> () { + let mut _0: (); + + bb0: { + _0 = use_both(const 0_i32, (_1.1: i32)) -> [return: bb1, unwind unreachable]; + } + + bb1: { + return; + } + } + diff --git a/tests/mir-opt/dead-store-elimination/call_arg_copy.move_packed.DeadStoreElimination.panic-unwind.diff b/tests/mir-opt/dead-store-elimination/call_arg_copy.move_packed.DeadStoreElimination.panic-unwind.diff new file mode 100644 index 00000000000..86ef026ec5c --- /dev/null +++ b/tests/mir-opt/dead-store-elimination/call_arg_copy.move_packed.DeadStoreElimination.panic-unwind.diff @@ -0,0 +1,15 @@ +- // MIR for `move_packed` before DeadStoreElimination ++ // MIR for `move_packed` after DeadStoreElimination + + fn move_packed(_1: Packed) -> () { + let mut _0: (); + + bb0: { + _0 = use_both(const 0_i32, (_1.1: i32)) -> [return: bb1, unwind continue]; + } + + bb1: { + return; + } + } + diff --git a/tests/mir-opt/dead-store-elimination/call_arg_copy.rs b/tests/mir-opt/dead-store-elimination/call_arg_copy.rs index 41f91fc1306..f09cdee1482 100644 --- a/tests/mir-opt/dead-store-elimination/call_arg_copy.rs +++ b/tests/mir-opt/dead-store-elimination/call_arg_copy.rs @@ -2,6 +2,12 @@ // unit-test: DeadStoreElimination // compile-flags: -Zmir-enable-passes=+CopyProp +#![feature(core_intrinsics)] +#![feature(custom_mir)] +#![allow(internal_features)] + +use std::intrinsics::mir::*; + #[inline(never)] fn use_both(_: i32, _: i32) {} @@ -10,6 +16,26 @@ fn move_simple(x: i32) { use_both(x, x); } +#[repr(packed)] +struct Packed { + x: u8, + y: i32, +} + +// EMIT_MIR call_arg_copy.move_packed.DeadStoreElimination.diff +#[custom_mir(dialect = "analysis")] +fn move_packed(packed: Packed) { + mir!( + { + Call(RET = use_both(0, packed.y), ret) + } + ret = { + Return() + } + ) +} + fn main() { move_simple(1); + move_packed(Packed { x: 0, y: 1 }); } |
