Skip to content

Commit

Permalink
Rollup merge of #117441 - cjgillot:diag-noassert, r=oli-obk,RalfJung
Browse files Browse the repository at this point in the history
Do not assert in op_to_const.

`op_to_const` is used in `try_destructure_mir_constant_for_diagnostics`, which may encounter invalid constants created by optimizations and debugging.

r? ``@oli-obk``

Fixes #117368
  • Loading branch information
matthiaskrgr authored Nov 1, 2023
2 parents 3087b63 + f512f91 commit 4e437be
Show file tree
Hide file tree
Showing 12 changed files with 468 additions and 11 deletions.
16 changes: 14 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,16 @@ pub(crate) fn mk_eval_cx<'mir, 'tcx>(
}

/// This function converts an interpreter value into a MIR constant.
///
/// The `for_diagnostics` flag turns the usual rules for returning `ConstValue::Scalar` into a
/// best-effort attempt. This is not okay for use in const-eval sine it breaks invariants rustc
/// relies on, but it is okay for diagnostics which will just give up gracefully when they
/// encounter an `Indirect` they cannot handle.
#[instrument(skip(ecx), level = "debug")]
pub(super) fn op_to_const<'tcx>(
ecx: &CompileTimeEvalContext<'_, 'tcx>,
op: &OpTy<'tcx>,
for_diagnostics: bool,
) -> ConstValue<'tcx> {
// Handle ZST consistently and early.
if op.layout.is_zst() {
Expand All @@ -133,7 +139,13 @@ pub(super) fn op_to_const<'tcx>(
_ => false,
};
let immediate = if force_as_immediate {
Right(ecx.read_immediate(op).expect("normalization works on validated constants"))
match ecx.read_immediate(op) {
Ok(imm) => Right(imm),
Err(err) if !for_diagnostics => {
panic!("normalization works on validated constants: {err:?}")
}
_ => op.as_mplace_or_imm(),
}
} else {
op.as_mplace_or_imm()
};
Expand Down Expand Up @@ -205,7 +217,7 @@ pub(crate) fn turn_into_const_value<'tcx>(
);

// Turn this into a proper constant.
op_to_const(&ecx, &mplace.into())
op_to_const(&ecx, &mplace.into(), /* for diagnostics */ false)
}

#[instrument(skip(tcx), level = "debug")]
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub(crate) fn eval_to_valtree<'tcx>(
}

#[instrument(skip(tcx), level = "debug")]
pub(crate) fn try_destructure_mir_constant_for_diagnostics<'tcx>(
pub(crate) fn try_destructure_mir_constant_for_user_output<'tcx>(
tcx: TyCtxtAt<'tcx>,
val: mir::ConstValue<'tcx>,
ty: Ty<'tcx>,
Expand All @@ -99,7 +99,7 @@ pub(crate) fn try_destructure_mir_constant_for_diagnostics<'tcx>(
let fields_iter = (0..field_count)
.map(|i| {
let field_op = ecx.project_field(&down, i).ok()?;
let val = op_to_const(&ecx, &field_op);
let val = op_to_const(&ecx, &field_op, /* for diagnostics */ true);
Some((val, field_op.layout.ty))
})
.collect::<Option<Vec<_>>>()?;
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ pub fn valtree_to_const_value<'tcx>(
let mut ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No);
let imm = valtree_to_ref(&mut ecx, valtree, *inner_ty);
let imm = ImmTy::from_immediate(imm, tcx.layout_of(param_env_ty).unwrap());
op_to_const(&ecx, &imm.into())
op_to_const(&ecx, &imm.into(), /* for diagnostics */ false)
}
ty::Tuple(_) | ty::Array(_, _) | ty::Adt(..) => {
let layout = tcx.layout_of(param_env_ty).unwrap();
Expand Down Expand Up @@ -265,7 +265,7 @@ pub fn valtree_to_const_value<'tcx>(
dump_place(&ecx, &place);
intern_const_alloc_recursive(&mut ecx, InternKind::Constant, &place).unwrap();

op_to_const(&ecx, &place.into())
op_to_const(&ecx, &place.into(), /* for diagnostics */ false)
}
ty::Never
| ty::Error(_)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ pub fn provide(providers: &mut Providers) {
let (param_env, raw) = param_env_and_value.into_parts();
const_eval::eval_to_valtree(tcx, param_env, raw)
};
providers.hooks.try_destructure_mir_constant_for_diagnostics =
const_eval::try_destructure_mir_constant_for_diagnostics;
providers.hooks.try_destructure_mir_constant_for_user_output =
const_eval::try_destructure_mir_constant_for_user_output;
providers.valtree_to_const_val = |tcx, (ty, valtree)| {
const_eval::valtree_to_const_value(tcx, ty::ParamEnv::empty().and(ty), valtree)
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/hooks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ macro_rules! declare_hooks {
declare_hooks! {
/// Tries to destructure an `mir::Const` ADT or array into its variant index
/// and its field values. This should only be used for pretty printing.
hook try_destructure_mir_constant_for_diagnostics(val: mir::ConstValue<'tcx>, ty: Ty<'tcx>) -> Option<mir::DestructuredConstant<'tcx>>;
hook try_destructure_mir_constant_for_user_output(val: mir::ConstValue<'tcx>, ty: Ty<'tcx>) -> Option<mir::DestructuredConstant<'tcx>>;

/// Getting a &core::panic::Location referring to a span.
hook const_caller_location(file: rustc_span::Symbol, line: u32, col: u32) -> mir::ConstValue<'tcx>;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1713,7 +1713,7 @@ fn pretty_print_const_value_tcx<'tcx>(
(_, ty::Array(..) | ty::Tuple(..) | ty::Adt(..)) if !ty.has_non_region_param() => {
let ct = tcx.lift(ct).unwrap();
let ty = tcx.lift(ty).unwrap();
if let Some(contents) = tcx.try_destructure_mir_constant_for_diagnostics(ct, ty) {
if let Some(contents) = tcx.try_destructure_mir_constant_for_user_output(ct, ty) {
let fields: Vec<(ConstValue<'_>, Ty<'_>)> = contents.fields.to_vec();
match *ty.kind() {
ty::Array(..) => {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ fn field_of_struct<'tcx>(
field: &Ident,
) -> Option<mir::Const<'tcx>> {
if let mir::Const::Val(result, ty) = result
&& let Some(dc) = lcx.tcx.try_destructure_mir_constant_for_diagnostics(result, ty)
&& let Some(dc) = lcx.tcx.try_destructure_mir_constant_for_user_output(result, ty)
&& let Some(dc_variant) = dc.variant
&& let Some(variant) = adt_def.variants().get(dc_variant)
&& let Some(field_idx) = variant.fields.iter().position(|el| el.name == field.name)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
- // MIR for `main` before GVN
+ // MIR for `main` after GVN

fn main() -> () {
let mut _0: ();
let _1: std::alloc::Layout;
let mut _2: std::option::Option<std::alloc::Layout>;
let mut _3: *mut u8;
let mut _4: *mut [u8];
let mut _5: std::ptr::NonNull<[u8]>;
let mut _6: std::result::Result<std::ptr::NonNull<[u8]>, std::alloc::AllocError>;
let mut _7: &std::alloc::Global;
let mut _8: std::alloc::Layout;
scope 1 {
debug layout => _1;
let mut _9: &std::alloc::Global;
scope 2 {
debug ptr => _3;
}
scope 5 (inlined <std::alloc::Global as Allocator>::allocate) {
debug self => _9;
debug layout => _1;
}
scope 6 (inlined #[track_caller] Result::<NonNull<[u8]>, std::alloc::AllocError>::unwrap) {
debug self => _6;
let mut _12: isize;
let _13: std::alloc::AllocError;
let mut _14: !;
let _15: &str;
let mut _16: &dyn std::fmt::Debug;
let mut _17: &std::alloc::AllocError;
scope 7 {
debug t => _5;
}
scope 8 {
debug e => const std::alloc::AllocError;
}
}
scope 9 (inlined NonNull::<[u8]>::as_ptr) {
debug self => _5;
let mut _18: *const [u8];
}
}
scope 3 (inlined #[track_caller] Option::<Layout>::unwrap) {
debug self => _2;
let mut _10: isize;
let mut _11: !;
scope 4 {
debug val => _1;
}
}

bb0: {
StorageLive(_2);
- _2 = Option::<Layout>::None;
+ _2 = const Option::<Layout>::None;
StorageLive(_10);
_10 = const 0_isize;
switchInt(const 0_isize) -> [0: bb1, 1: bb3, otherwise: bb2];
}

bb1: {
_11 = core::panicking::panic(const "called `Option::unwrap()` on a `None` value") -> unwind unreachable;
}

bb2: {
unreachable;
}

bb3: {
- _1 = move ((_2 as Some).0: std::alloc::Layout);
+ _1 = const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): ptr::alignment::AlignmentEnum32) }};
StorageDead(_10);
StorageDead(_2);
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_9 = const _;
- _6 = std::alloc::Global::alloc_impl(_9, _1, const false) -> [return: bb4, unwind unreachable];
+ _6 = std::alloc::Global::alloc_impl(const {ALLOC1: &std::alloc::Global}, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): ptr::alignment::AlignmentEnum32) }}, const false) -> [return: bb4, unwind unreachable];
}

bb4: {
StorageLive(_12);
StorageLive(_15);
_12 = discriminant(_6);
switchInt(move _12) -> [0: bb6, 1: bb5, otherwise: bb2];
}

bb5: {
_15 = const "called `Result::unwrap()` on an `Err` value";
StorageLive(_16);
StorageLive(_17);
_17 = &_13;
_16 = move _17 as &dyn std::fmt::Debug (PointerCoercion(Unsize));
StorageDead(_17);
_14 = result::unwrap_failed(move _15, move _16) -> unwind unreachable;
}

bb6: {
_5 = move ((_6 as Ok).0: std::ptr::NonNull<[u8]>);
StorageDead(_15);
StorageDead(_12);
StorageDead(_6);
StorageLive(_18);
_18 = (_5.0: *const [u8]);
_4 = move _18 as *mut [u8] (PtrToPtr);
StorageDead(_18);
StorageDead(_5);
_3 = move _4 as *mut u8 (PtrToPtr);
StorageDead(_4);
StorageDead(_3);
return;
}
}
+
+ ALLOC0 (size: 8, align: 4) {
+ 00 00 00 00 __ __ __ __ │ ....░░░░
+ }
+
+ ALLOC1 (size: 0, align: 1) {}

Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
- // MIR for `main` before GVN
+ // MIR for `main` after GVN

fn main() -> () {
let mut _0: ();
let _1: std::alloc::Layout;
let mut _2: std::option::Option<std::alloc::Layout>;
let mut _3: *mut u8;
let mut _4: *mut [u8];
let mut _5: std::ptr::NonNull<[u8]>;
let mut _6: std::result::Result<std::ptr::NonNull<[u8]>, std::alloc::AllocError>;
let mut _7: &std::alloc::Global;
let mut _8: std::alloc::Layout;
scope 1 {
debug layout => _1;
let mut _9: &std::alloc::Global;
scope 2 {
debug ptr => _3;
}
scope 5 (inlined <std::alloc::Global as Allocator>::allocate) {
debug self => _9;
debug layout => _1;
}
scope 6 (inlined NonNull::<[u8]>::as_ptr) {
debug self => _5;
let mut _12: *const [u8];
}
}
scope 3 (inlined #[track_caller] Option::<Layout>::unwrap) {
debug self => _2;
let mut _10: isize;
let mut _11: !;
scope 4 {
debug val => _1;
}
}

bb0: {
StorageLive(_2);
- _2 = Option::<Layout>::None;
+ _2 = const Option::<Layout>::None;
StorageLive(_10);
_10 = const 0_isize;
switchInt(const 0_isize) -> [0: bb2, 1: bb4, otherwise: bb3];
}

bb1: {
StorageDead(_6);
StorageLive(_12);
_12 = (_5.0: *const [u8]);
_4 = move _12 as *mut [u8] (PtrToPtr);
StorageDead(_12);
StorageDead(_5);
_3 = move _4 as *mut u8 (PtrToPtr);
StorageDead(_4);
StorageDead(_3);
return;
}

bb2: {
_11 = core::panicking::panic(const "called `Option::unwrap()` on a `None` value") -> unwind continue;
}

bb3: {
unreachable;
}

bb4: {
- _1 = move ((_2 as Some).0: std::alloc::Layout);
+ _1 = const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): ptr::alignment::AlignmentEnum32) }};
StorageDead(_10);
StorageDead(_2);
StorageLive(_3);
StorageLive(_4);
StorageLive(_5);
StorageLive(_6);
_9 = const _;
- _6 = std::alloc::Global::alloc_impl(_9, _1, const false) -> [return: bb5, unwind continue];
+ _6 = std::alloc::Global::alloc_impl(const {ALLOC1: &std::alloc::Global}, const Layout {{ size: Indirect { alloc_id: ALLOC0, offset: Size(4 bytes) }: usize, align: std::ptr::Alignment(Scalar(0x00000000): ptr::alignment::AlignmentEnum32) }}, const false) -> [return: bb5, unwind continue];
}

bb5: {
_5 = Result::<NonNull<[u8]>, std::alloc::AllocError>::unwrap(move _6) -> [return: bb1, unwind continue];
}
}
+
+ ALLOC0 (size: 8, align: 4) {
+ 00 00 00 00 __ __ __ __ │ ....░░░░
+ }
+
+ ALLOC1 (size: 0, align: 1) {}

Loading

0 comments on commit 4e437be

Please sign in to comment.