Skip to content

Commit

Permalink
Auto merge of #115138 - cjgillot:dse-move-packed, r=compiler-errors
Browse files Browse the repository at this point in the history
Do not convert copies of packed projections to moves.

This code path was introduced in #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.
  • Loading branch information
bors committed Aug 25, 2023
2 parents 738df13 + 15a6861 commit 25ed43d
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 2 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/util/alignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_mir_transform/src/dead_store_elimination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}

Original file line number Diff line number Diff line change
@@ -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;
}
}

26 changes: 26 additions & 0 deletions tests/mir-opt/dead-store-elimination/call_arg_copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand All @@ -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 });
}

0 comments on commit 25ed43d

Please sign in to comment.