Skip to content

Don't validate constants in const propagation #109521

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,14 @@ macro_rules! throw_validation_failure {
msg.push_str(", but expected ");
write!(&mut msg, $($expected_fmt)*).unwrap();
)?
let path = rustc_middle::ty::print::with_no_trimmed_paths!({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I hadn't even realized that this was added in the validity checker. Happy to see it gone!

let where_ = &$where;
if !where_.is_empty() {
let mut path = String::new();
write_path(&mut path, where_);
Some(path)
} else {
None
}
});
let where_ = &$where;
let path = if !where_.is_empty() {
let mut path = String::new();
write_path(&mut path, where_);
Some(path)
} else {
None
};
throw_ub!(ValidationFailure { path, msg })
}};
}
Expand Down
28 changes: 3 additions & 25 deletions compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ use rustc_trait_selection::traits;

use crate::MirPass;
use rustc_const_eval::interpret::{
self, compile_time_machine, AllocId, ConstAllocation, ConstValue, CtfeValidationMode, Frame,
ImmTy, Immediate, InterpCx, InterpResult, LocalValue, MemoryKind, OpTy, PlaceTy, Pointer,
Scalar, StackPopCleanup,
self, compile_time_machine, AllocId, ConstAllocation, ConstValue, Frame, ImmTy, Immediate,
InterpCx, InterpResult, LocalValue, MemoryKind, OpTy, PlaceTy, Pointer, Scalar,
StackPopCleanup,
};

/// The maximum number of bytes that we'll allocate space for a local or the return value.
Expand Down Expand Up @@ -501,16 +501,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {

return None;
}
// Do not try creating references, nor any types with potentially-complex
// invariants. This avoids an issue where checking validity would do a
// bunch of work generating a nice message about the invariant violation,
// only to not show it to anyone (since this isn't the lint).
Rvalue::Cast(CastKind::Transmute, op, dst_ty) if !dst_ty.is_primitive() => {
trace!("skipping Transmute of {:?} to {:?}", op, dst_ty);

return None;
}

// There's no other checking to do at this time.
Rvalue::Aggregate(..)
| Rvalue::Use(..)
Expand Down Expand Up @@ -628,18 +618,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}

trace!("attempting to replace {:?} with {:?}", rval, value);
if let Err(e) = self.ecx.const_validate_operand(
value,
vec![],
// FIXME: is ref tracking too expensive?
// FIXME: what is the point of ref tracking if we do not even check the tracked refs?
&mut interpret::RefTracking::empty(),
CtfeValidationMode::Regular,
) {
trace!("validation error, attempt failed: {:?}", e);
return;
}

// FIXME> figure out what to do when read_immediate_raw fails
let imm = self.ecx.read_immediate_raw(value).ok();

Expand Down
15 changes: 15 additions & 0 deletions tests/mir-opt/const_prop/transmute.from_char.ConstProp.64bit.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- // MIR for `from_char` before ConstProp
+ // MIR for `from_char` after ConstProp

fn from_char() -> i32 {
let mut _0: i32; // return place in scope 0 at $DIR/transmute.rs:+0:23: +0:26
scope 1 {
}

bb0: {
- _0 = const 'R' as i32 (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:28
+ _0 = const 82_i32; // scope 1 at $DIR/transmute.rs:+1:14: +1:28
return; // scope 0 at $DIR/transmute.rs:+2:2: +2:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
}

bb0: {
_0 = const -1_i8 as bool (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:30
- _0 = const -1_i8 as bool (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:30
+ _0 = const {transmute(0xff): bool}; // scope 1 at $DIR/transmute.rs:+1:14: +1:30
return; // scope 0 at $DIR/transmute.rs:+2:2: +2:2
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- // MIR for `invalid_bool` before ConstProp
+ // MIR for `invalid_bool` after ConstProp

fn invalid_bool() -> bool {
let mut _0: bool; // return place in scope 0 at $DIR/transmute.rs:+0:33: +0:37
scope 1 {
}

bb0: {
- _0 = const -1_i8 as bool (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:30
+ _0 = const {transmute(0xff): bool}; // scope 1 at $DIR/transmute.rs:+1:14: +1:30
return; // scope 0 at $DIR/transmute.rs:+2:2: +2:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
}

bb0: {
_0 = const _ as char (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:33
- _0 = const _ as char (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:33
+ _0 = const {transmute(0x7fffffff): char}; // scope 1 at $DIR/transmute.rs:+1:14: +1:33
return; // scope 0 at $DIR/transmute.rs:+2:2: +2:2
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- // MIR for `invalid_char` before ConstProp
+ // MIR for `invalid_char` after ConstProp

fn invalid_char() -> char {
let mut _0: char; // return place in scope 0 at $DIR/transmute.rs:+0:33: +0:37
scope 1 {
}

bb0: {
- _0 = const _ as char (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:33
+ _0 = const {transmute(0x7fffffff): char}; // scope 1 at $DIR/transmute.rs:+1:14: +1:33
return; // scope 0 at $DIR/transmute.rs:+2:2: +2:2
}
}

23 changes: 23 additions & 0 deletions tests/mir-opt/const_prop/transmute.less_as_i8.ConstProp.64bit.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
- // MIR for `less_as_i8` before ConstProp
+ // MIR for `less_as_i8` after ConstProp

fn less_as_i8() -> i8 {
let mut _0: i8; // return place in scope 0 at $DIR/transmute.rs:+0:24: +0:26
let mut _1: std::cmp::Ordering; // in scope 0 at $DIR/transmute.rs:+1:24: +1:48
scope 1 {
}

bb0: {
StorageLive(_1); // scope 1 at $DIR/transmute.rs:+1:24: +1:48
- _1 = Less; // scope 1 at $DIR/transmute.rs:+1:24: +1:48
- _0 = move _1 as i8 (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:49
+ _1 = const Less; // scope 1 at $DIR/transmute.rs:+1:24: +1:48
+ // mir::Constant
+ // + span: no-location
+ // + literal: Const { ty: std::cmp::Ordering, val: Value(Scalar(0xff)) }
+ _0 = const -1_i8; // scope 1 at $DIR/transmute.rs:+1:14: +1:49
StorageDead(_1); // scope 1 at $DIR/transmute.rs:+1:48: +1:49
return; // scope 0 at $DIR/transmute.rs:+2:2: +2:2
}
}

2 changes: 2 additions & 0 deletions tests/mir-opt/const_prop/transmute.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// unit-test: ConstProp
// compile-flags: -O --crate-type=lib
// ignore-endian-big
// EMIT_MIR_FOR_EACH_BIT_WIDTH

use std::mem::transmute;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
- // MIR for `undef_union_as_integer` before ConstProp
+ // MIR for `undef_union_as_integer` after ConstProp

fn undef_union_as_integer() -> u32 {
let mut _0: u32; // return place in scope 0 at $DIR/transmute.rs:+0:43: +0:46
let mut _1: undef_union_as_integer::Union32; // in scope 0 at $DIR/transmute.rs:+2:24: +2:44
let mut _2: (); // in scope 0 at $DIR/transmute.rs:+2:40: +2:42
scope 1 {
}

bb0: {
StorageLive(_1); // scope 1 at $DIR/transmute.rs:+2:24: +2:44
StorageLive(_2); // scope 1 at $DIR/transmute.rs:+2:40: +2:42
_2 = (); // scope 1 at $DIR/transmute.rs:+2:40: +2:42
_1 = Union32 { value: move _2 }; // scope 1 at $DIR/transmute.rs:+2:24: +2:44
StorageDead(_2); // scope 1 at $DIR/transmute.rs:+2:43: +2:44
_0 = move _1 as u32 (Transmute); // scope 1 at $DIR/transmute.rs:+2:14: +2:45
StorageDead(_1); // scope 1 at $DIR/transmute.rs:+2:44: +2:45
return; // scope 0 at $DIR/transmute.rs:+3:2: +3:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
bb0: {
StorageLive(_1); // scope 0 at $DIR/transmute.rs:+0:38: +3:2
StorageLive(_2); // scope 0 at $DIR/transmute.rs:+1:9: +1:10
_2 = const 1_usize as std::boxed::Box<Never> (Transmute); // scope 2 at $DIR/transmute.rs:+1:34: +1:52
- _2 = const 1_usize as std::boxed::Box<Never> (Transmute); // scope 2 at $DIR/transmute.rs:+1:34: +1:52
+ _2 = const Box::<Never>(Unique::<Never> {{ pointer: NonNull::<Never> {{ pointer: {0x1 as *const Never} }}, _marker: PhantomData::<Never> }}, std::alloc::Global); // scope 2 at $DIR/transmute.rs:+1:34: +1:52
+ // mir::Constant
+ // + span: no-location
+ // + literal: Const { ty: Box<Never>, val: Value(Scalar(0x00000001)) }
StorageLive(_3); // scope 1 at $DIR/transmute.rs:+2:5: +2:16
unreachable; // scope 1 at $DIR/transmute.rs:+2:11: +2:13
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
- // MIR for `unreachable_box` before ConstProp
+ // MIR for `unreachable_box` after ConstProp

fn unreachable_box() -> ! {
let mut _0: !; // return place in scope 0 at $DIR/transmute.rs:+0:36: +0:37
let mut _1: !; // in scope 0 at $DIR/transmute.rs:+0:38: +3:2
let _2: std::boxed::Box<Never>; // in scope 0 at $DIR/transmute.rs:+1:9: +1:10
let mut _3: !; // in scope 0 at $DIR/transmute.rs:+2:5: +2:16
scope 1 {
debug x => _2; // in scope 1 at $DIR/transmute.rs:+1:9: +1:10
}
scope 2 {
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/transmute.rs:+0:38: +3:2
StorageLive(_2); // scope 0 at $DIR/transmute.rs:+1:9: +1:10
- _2 = const 1_usize as std::boxed::Box<Never> (Transmute); // scope 2 at $DIR/transmute.rs:+1:34: +1:52
+ _2 = const Box::<Never>(Unique::<Never> {{ pointer: NonNull::<Never> {{ pointer: {0x1 as *const Never} }}, _marker: PhantomData::<Never> }}, std::alloc::Global); // scope 2 at $DIR/transmute.rs:+1:34: +1:52
+ // mir::Constant
+ // + span: no-location
+ // + literal: Const { ty: Box<Never>, val: Value(Scalar(0x0000000000000001)) }
StorageLive(_3); // scope 1 at $DIR/transmute.rs:+2:5: +2:16
unreachable; // scope 1 at $DIR/transmute.rs:+2:11: +2:13
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
- // MIR for `unreachable_direct` before ConstProp
+ // MIR for `unreachable_direct` after ConstProp

fn unreachable_direct() -> ! {
let mut _0: !; // return place in scope 0 at $DIR/transmute.rs:+0:39: +0:40
let mut _1: !; // in scope 0 at $DIR/transmute.rs:+0:41: +3:2
let _2: Never; // in scope 0 at $DIR/transmute.rs:+1:9: +1:10
let mut _3: (); // in scope 0 at $DIR/transmute.rs:+1:39: +1:41
let mut _4: !; // in scope 0 at $DIR/transmute.rs:+2:5: +2:15
scope 1 {
debug x => _2; // in scope 1 at $DIR/transmute.rs:+1:9: +1:10
}
scope 2 {
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/transmute.rs:+0:41: +3:2
StorageLive(_2); // scope 0 at $DIR/transmute.rs:+1:9: +1:10
StorageLive(_3); // scope 2 at $DIR/transmute.rs:+1:39: +1:41
_3 = (); // scope 2 at $DIR/transmute.rs:+1:39: +1:41
_2 = move _3 as Never (Transmute); // scope 2 at $DIR/transmute.rs:+1:29: +1:42
unreachable; // scope 2 at $DIR/transmute.rs:+1:29: +1:42
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
StorageLive(_1); // scope 0 at $DIR/transmute.rs:+0:38: +3:2
StorageLive(_2); // scope 0 at $DIR/transmute.rs:+1:9: +1:10
StorageLive(_3); // scope 0 at $DIR/transmute.rs:+1:34: +1:52
_3 = const 1_usize as &mut Never (Transmute); // scope 2 at $DIR/transmute.rs:+1:34: +1:52
- _3 = const 1_usize as &mut Never (Transmute); // scope 2 at $DIR/transmute.rs:+1:34: +1:52
+ _3 = const {0x1 as &mut Never}; // scope 2 at $DIR/transmute.rs:+1:34: +1:52
+ // mir::Constant
+ // + span: no-location
+ // + literal: Const { ty: &mut Never, val: Value(Scalar(0x00000001)) }
_2 = &mut (*_3); // scope 0 at $DIR/transmute.rs:+1:34: +1:52
StorageDead(_3); // scope 0 at $DIR/transmute.rs:+1:54: +1:55
StorageLive(_4); // scope 1 at $DIR/transmute.rs:+2:5: +2:16
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
- // MIR for `unreachable_mut` before ConstProp
+ // MIR for `unreachable_mut` after ConstProp

fn unreachable_mut() -> ! {
let mut _0: !; // return place in scope 0 at $DIR/transmute.rs:+0:36: +0:37
let mut _1: !; // in scope 0 at $DIR/transmute.rs:+0:38: +3:2
let _2: &mut Never; // in scope 0 at $DIR/transmute.rs:+1:9: +1:10
let mut _3: &mut Never; // in scope 0 at $DIR/transmute.rs:+1:34: +1:52
let mut _4: !; // in scope 0 at $DIR/transmute.rs:+2:5: +2:16
scope 1 {
debug x => _2; // in scope 1 at $DIR/transmute.rs:+1:9: +1:10
}
scope 2 {
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/transmute.rs:+0:38: +3:2
StorageLive(_2); // scope 0 at $DIR/transmute.rs:+1:9: +1:10
StorageLive(_3); // scope 0 at $DIR/transmute.rs:+1:34: +1:52
- _3 = const 1_usize as &mut Never (Transmute); // scope 2 at $DIR/transmute.rs:+1:34: +1:52
+ _3 = const {0x1 as &mut Never}; // scope 2 at $DIR/transmute.rs:+1:34: +1:52
+ // mir::Constant
+ // + span: no-location
+ // + literal: Const { ty: &mut Never, val: Value(Scalar(0x0000000000000001)) }
_2 = &mut (*_3); // scope 0 at $DIR/transmute.rs:+1:34: +1:52
StorageDead(_3); // scope 0 at $DIR/transmute.rs:+1:54: +1:55
StorageLive(_4); // scope 1 at $DIR/transmute.rs:+2:5: +2:16
unreachable; // scope 1 at $DIR/transmute.rs:+2:11: +2:13
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@
bb0: {
StorageLive(_1); // scope 0 at $DIR/transmute.rs:+0:38: +3:2
StorageLive(_2); // scope 0 at $DIR/transmute.rs:+1:9: +1:10
_2 = const 1_usize as &Never (Transmute); // scope 2 at $DIR/transmute.rs:+1:30: +1:48
- _2 = const 1_usize as &Never (Transmute); // scope 2 at $DIR/transmute.rs:+1:30: +1:48
+ _2 = const {0x1 as &Never}; // scope 2 at $DIR/transmute.rs:+1:30: +1:48
+ // mir::Constant
+ // + span: no-location
+ // + literal: Const { ty: &Never, val: Value(Scalar(0x00000001)) }
StorageLive(_3); // scope 1 at $DIR/transmute.rs:+2:5: +2:16
unreachable; // scope 1 at $DIR/transmute.rs:+2:11: +2:13
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
- // MIR for `unreachable_ref` before ConstProp
+ // MIR for `unreachable_ref` after ConstProp

fn unreachable_ref() -> ! {
let mut _0: !; // return place in scope 0 at $DIR/transmute.rs:+0:36: +0:37
let mut _1: !; // in scope 0 at $DIR/transmute.rs:+0:38: +3:2
let _2: &Never; // in scope 0 at $DIR/transmute.rs:+1:9: +1:10
let mut _3: !; // in scope 0 at $DIR/transmute.rs:+2:5: +2:16
scope 1 {
debug x => _2; // in scope 1 at $DIR/transmute.rs:+1:9: +1:10
}
scope 2 {
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/transmute.rs:+0:38: +3:2
StorageLive(_2); // scope 0 at $DIR/transmute.rs:+1:9: +1:10
- _2 = const 1_usize as &Never (Transmute); // scope 2 at $DIR/transmute.rs:+1:30: +1:48
+ _2 = const {0x1 as &Never}; // scope 2 at $DIR/transmute.rs:+1:30: +1:48
+ // mir::Constant
+ // + span: no-location
+ // + literal: Const { ty: &Never, val: Value(Scalar(0x0000000000000001)) }
StorageLive(_3); // scope 1 at $DIR/transmute.rs:+2:5: +2:16
unreachable; // scope 1 at $DIR/transmute.rs:+2:11: +2:13
}
}

15 changes: 15 additions & 0 deletions tests/mir-opt/const_prop/transmute.valid_char.ConstProp.64bit.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
- // MIR for `valid_char` before ConstProp
+ // MIR for `valid_char` after ConstProp

fn valid_char() -> char {
let mut _0: char; // return place in scope 0 at $DIR/transmute.rs:+0:24: +0:28
scope 1 {
}

bb0: {
- _0 = const 82_u32 as char (Transmute); // scope 1 at $DIR/transmute.rs:+1:14: +1:33
+ _0 = const 'R'; // scope 1 at $DIR/transmute.rs:+1:14: +1:33
return; // scope 0 at $DIR/transmute.rs:+2:2: +2:2
}
}