Skip to content

Commit

Permalink
Auto merge of rust-lang#108012 - compiler-errors:issue-107999, r=oli-obk
Browse files Browse the repository at this point in the history
Don't ICE in `might_permit_raw_init` if reference is polymorphic

Emitting optimized MIR for a polymorphic function may require computing layout of a type that isn't (yet) known. This happens in the instcombine pass, for example. Let's fail gracefully in that condition.

cc `@saethlin`
fixes rust-lang#107999
  • Loading branch information
bors committed Feb 15, 2023
2 parents 2d14db3 + b096f0e commit c528357
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 56 deletions.
13 changes: 10 additions & 3 deletions compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,8 @@ fn codegen_regular_intrinsic_call<'tcx>(
sym::assert_inhabited | sym::assert_zero_valid | sym::assert_mem_uninitialized_valid => {
intrinsic_args!(fx, args => (); intrinsic);

let layout = fx.layout_of(substs.type_at(0));
let ty = substs.type_at(0);
let layout = fx.layout_of(ty);
if layout.abi.is_uninhabited() {
with_no_trimmed_paths!({
crate::base::codegen_panic_nounwind(
Expand All @@ -653,7 +654,10 @@ fn codegen_regular_intrinsic_call<'tcx>(
}

if intrinsic == sym::assert_zero_valid
&& !fx.tcx.permits_zero_init(fx.param_env().and(layout))
&& !fx
.tcx
.permits_zero_init(fx.param_env().and(ty))
.expect("expected to have layout during codegen")
{
with_no_trimmed_paths!({
crate::base::codegen_panic_nounwind(
Expand All @@ -669,7 +673,10 @@ fn codegen_regular_intrinsic_call<'tcx>(
}

if intrinsic == sym::assert_mem_uninitialized_valid
&& !fx.tcx.permits_uninit_init(fx.param_env().and(layout))
&& !fx
.tcx
.permits_uninit_init(fx.param_env().and(ty))
.expect("expected to have layout during codegen")
{
with_no_trimmed_paths!({
crate::base::codegen_panic_nounwind(
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,8 +674,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let layout = bx.layout_of(ty);
let do_panic = match intrinsic {
Inhabited => layout.abi.is_uninhabited(),
ZeroValid => !bx.tcx().permits_zero_init(bx.param_env().and(layout)),
MemUninitializedValid => !bx.tcx().permits_uninit_init(bx.param_env().and(layout)),
ZeroValid => !bx
.tcx()
.permits_zero_init(bx.param_env().and(ty))
.expect("expected to have layout during codegen"),
MemUninitializedValid => !bx
.tcx()
.permits_uninit_init(bx.param_env().and(ty))
.expect("expected to have layout during codegen"),
};
Some(if do_panic {
let msg_str = with_no_visible_paths!({
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

if intrinsic_name == sym::assert_zero_valid {
let should_panic = !self.tcx.permits_zero_init(self.param_env.and(layout));
let should_panic = !self
.tcx
.permits_zero_init(self.param_env.and(ty))
.map_err(|_| err_inval!(TooGeneric))?;

if should_panic {
M::abort(
Expand All @@ -462,7 +465,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

if intrinsic_name == sym::assert_mem_uninitialized_valid {
let should_panic = !self.tcx.permits_uninit_init(self.param_env.and(layout));
let should_panic = !self
.tcx
.permits_uninit_init(self.param_env.and(ty))
.map_err(|_| err_inval!(TooGeneric))?;

if should_panic {
M::abort(
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_const_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,8 @@ pub fn provide(providers: &mut Providers) {
const_eval::deref_mir_constant(tcx, param_env, value)
};
providers.permits_uninit_init = |tcx, param_env_and_ty| {
let (param_env, ty) = param_env_and_ty.into_parts();
util::might_permit_raw_init(tcx, param_env, ty, InitKind::UninitMitigated0x01Fill)
};
providers.permits_zero_init = |tcx, param_env_and_ty| {
let (param_env, ty) = param_env_and_ty.into_parts();
util::might_permit_raw_init(tcx, param_env, ty, InitKind::Zero)
util::might_permit_raw_init(tcx, param_env_and_ty, InitKind::UninitMitigated0x01Fill)
};
providers.permits_zero_init =
|tcx, param_env_and_ty| util::might_permit_raw_init(tcx, param_env_and_ty, InitKind::Zero);
}
35 changes: 17 additions & 18 deletions compiler/rustc_const_eval/src/util/might_permit_raw_init.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use rustc_middle::ty::layout::{LayoutCx, LayoutOf, TyAndLayout};
use rustc_middle::ty::{ParamEnv, TyCtxt};
use rustc_middle::ty::layout::{LayoutCx, LayoutError, LayoutOf, TyAndLayout};
use rustc_middle::ty::{ParamEnv, ParamEnvAnd, Ty, TyCtxt};
use rustc_session::Limit;
use rustc_target::abi::{Abi, FieldsShape, InitKind, Scalar, Variants};

Expand All @@ -20,15 +20,14 @@ use crate::interpret::{InterpCx, MemoryKind, OpTy};
/// to the full uninit check).
pub fn might_permit_raw_init<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
ty: TyAndLayout<'tcx>,
param_env_and_ty: ParamEnvAnd<'tcx, Ty<'tcx>>,
kind: InitKind,
) -> bool {
) -> Result<bool, LayoutError<'tcx>> {
if tcx.sess.opts.unstable_opts.strict_init_checks {
might_permit_raw_init_strict(ty, tcx, kind)
might_permit_raw_init_strict(tcx.layout_of(param_env_and_ty)?, tcx, kind)
} else {
let layout_cx = LayoutCx { tcx, param_env };
might_permit_raw_init_lax(ty, &layout_cx, kind)
let layout_cx = LayoutCx { tcx, param_env: param_env_and_ty.param_env };
might_permit_raw_init_lax(tcx.layout_of(param_env_and_ty)?, &layout_cx, kind)
}
}

Expand All @@ -38,7 +37,7 @@ fn might_permit_raw_init_strict<'tcx>(
ty: TyAndLayout<'tcx>,
tcx: TyCtxt<'tcx>,
kind: InitKind,
) -> bool {
) -> Result<bool, LayoutError<'tcx>> {
let machine = CompileTimeInterpreter::new(
Limit::new(0),
/*can_access_statics:*/ false,
Expand All @@ -65,7 +64,7 @@ fn might_permit_raw_init_strict<'tcx>(
// This does *not* actually check that references are dereferenceable, but since all types that
// require dereferenceability also require non-null, we don't actually get any false negatives
// due to this.
cx.validate_operand(&ot).is_ok()
Ok(cx.validate_operand(&ot).is_ok())
}

/// Implements the 'lax' (default) version of the `might_permit_raw_init` checks; see that function for
Expand All @@ -74,7 +73,7 @@ fn might_permit_raw_init_lax<'tcx>(
this: TyAndLayout<'tcx>,
cx: &LayoutCx<'tcx, TyCtxt<'tcx>>,
init_kind: InitKind,
) -> bool {
) -> Result<bool, LayoutError<'tcx>> {
let scalar_allows_raw_init = move |s: Scalar| -> bool {
match init_kind {
InitKind::Zero => {
Expand Down Expand Up @@ -103,20 +102,20 @@ fn might_permit_raw_init_lax<'tcx>(
};
if !valid {
// This is definitely not okay.
return false;
return Ok(false);
}

// Special magic check for references and boxes (i.e., special pointer types).
if let Some(pointee) = this.ty.builtin_deref(false) {
let pointee = cx.layout_of(pointee.ty).expect("need to be able to compute layouts");
let pointee = cx.layout_of(pointee.ty)?;
// We need to ensure that the LLVM attributes `aligned` and `dereferenceable(size)` are satisfied.
if pointee.align.abi.bytes() > 1 {
// 0x01-filling is not aligned.
return false;
return Ok(false);
}
if pointee.size.bytes() > 0 {
// A 'fake' integer pointer is not sufficiently dereferenceable.
return false;
return Ok(false);
}
}

Expand All @@ -129,9 +128,9 @@ fn might_permit_raw_init_lax<'tcx>(
}
FieldsShape::Arbitrary { offsets, .. } => {
for idx in 0..offsets.len() {
if !might_permit_raw_init_lax(this.field(cx, idx), cx, init_kind) {
if !might_permit_raw_init_lax(this.field(cx, idx), cx, init_kind)? {
// We found a field that is unhappy with this kind of initialization.
return false;
return Ok(false);
}
}
}
Expand All @@ -148,5 +147,5 @@ fn might_permit_raw_init_lax<'tcx>(
}
}

true
Ok(true)
}
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2143,12 +2143,12 @@ rustc_queries! {
separate_provide_extern
}

query permits_uninit_init(key: ty::ParamEnvAnd<'tcx, TyAndLayout<'tcx>>) -> bool {
desc { "checking to see if `{}` permits being left uninit", key.value.ty }
query permits_uninit_init(key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> Result<bool, ty::layout::LayoutError<'tcx>> {
desc { "checking to see if `{}` permits being left uninit", key.value }
}

query permits_zero_init(key: ty::ParamEnvAnd<'tcx, TyAndLayout<'tcx>>) -> bool {
desc { "checking to see if `{}` permits being left zeroed", key.value.ty }
query permits_zero_init(key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> Result<bool, ty::layout::LayoutError<'tcx>> {
desc { "checking to see if `{}` permits being left zeroed", key.value }
}

query compare_impl_const(
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_middle/src/ty/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use crate::traits::specialization_graph;
use crate::traits::{self, ImplSource};
use crate::ty::context::TyCtxtFeed;
use crate::ty::fast_reject::SimplifiedType;
use crate::ty::layout::TyAndLayout;
use crate::ty::subst::{GenericArg, SubstsRef};
use crate::ty::util::AlwaysRequiresDrop;
use crate::ty::GeneratorDiagnosticData;
Expand Down
44 changes: 24 additions & 20 deletions compiler/rustc_mir_transform/src/instcombine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use rustc_middle::mir::{
BinOp, Body, Constant, ConstantKind, LocalDecls, Operand, Place, ProjectionElem, Rvalue,
SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp,
};
use rustc_middle::ty::{self, layout::TyAndLayout, ParamEnv, ParamEnvAnd, SubstsRef, Ty, TyCtxt};
use rustc_middle::ty::layout::LayoutError;
use rustc_middle::ty::{self, ParamEnv, ParamEnvAnd, SubstsRef, Ty, TyCtxt};
use rustc_span::symbol::{sym, Symbol};

pub struct InstCombine;
Expand Down Expand Up @@ -230,38 +231,41 @@ impl<'tcx> InstCombineContext<'tcx, '_> {

// Check this is a foldable intrinsic before we query the layout of our generic parameter
let Some(assert_panics) = intrinsic_assert_panics(intrinsic_name) else { return; };
let Ok(layout) = self.tcx.layout_of(self.param_env.and(ty)) else { return; };
if assert_panics(self.tcx, self.param_env.and(layout)) {
// If we know the assert panics, indicate to later opts that the call diverges
*target = None;
} else {
// If we know the assert does not panic, turn the call into a Goto
terminator.kind = TerminatorKind::Goto { target: *target_block };
match assert_panics(self.tcx, self.param_env.and(ty)) {
// We don't know the layout, don't touch the assertion
Err(_) => {}
Ok(true) => {
// If we know the assert panics, indicate to later opts that the call diverges
*target = None;
}
Ok(false) => {
// If we know the assert does not panic, turn the call into a Goto
terminator.kind = TerminatorKind::Goto { target: *target_block };
}
}
}
}

fn intrinsic_assert_panics<'tcx>(
intrinsic_name: Symbol,
) -> Option<fn(TyCtxt<'tcx>, ParamEnvAnd<'tcx, TyAndLayout<'tcx>>) -> bool> {
) -> Option<fn(TyCtxt<'tcx>, ParamEnvAnd<'tcx, Ty<'tcx>>) -> Result<bool, LayoutError<'tcx>>> {
fn inhabited_predicate<'tcx>(
_tcx: TyCtxt<'tcx>,
param_env_and_layout: ParamEnvAnd<'tcx, TyAndLayout<'tcx>>,
) -> bool {
let (_param_env, layout) = param_env_and_layout.into_parts();
layout.abi.is_uninhabited()
tcx: TyCtxt<'tcx>,
param_env_and_ty: ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> Result<bool, LayoutError<'tcx>> {
Ok(tcx.layout_of(param_env_and_ty)?.abi.is_uninhabited())
}
fn zero_valid_predicate<'tcx>(
tcx: TyCtxt<'tcx>,
param_env_and_layout: ParamEnvAnd<'tcx, TyAndLayout<'tcx>>,
) -> bool {
!tcx.permits_zero_init(param_env_and_layout)
param_env_and_ty: ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> Result<bool, LayoutError<'tcx>> {
Ok(!tcx.permits_zero_init(param_env_and_ty)?)
}
fn mem_uninitialized_valid_predicate<'tcx>(
tcx: TyCtxt<'tcx>,
param_env_and_layout: ParamEnvAnd<'tcx, TyAndLayout<'tcx>>,
) -> bool {
!tcx.permits_uninit_init(param_env_and_layout)
param_env_and_ty: ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> Result<bool, LayoutError<'tcx>> {
Ok(!tcx.permits_uninit_init(param_env_and_ty)?)
}

match intrinsic_name {
Expand Down
23 changes: 23 additions & 0 deletions tests/mir-opt/dont_yeet_assert.generic.InstCombine.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
- // MIR for `generic` before InstCombine
+ // MIR for `generic` after InstCombine

fn generic() -> () {
let mut _0: (); // return place in scope 0 at $DIR/dont_yeet_assert.rs:+0:21: +0:21
let _1: (); // in scope 0 at $DIR/dont_yeet_assert.rs:+1:5: +1:61

bb0: {
StorageLive(_1); // scope 0 at $DIR/dont_yeet_assert.rs:+1:5: +1:61
_1 = assert_mem_uninitialized_valid::<&T>() -> bb1; // scope 0 at $DIR/dont_yeet_assert.rs:+1:5: +1:61
// mir::Constant
// + span: $DIR/dont_yeet_assert.rs:10:5: 10:59
// + user_ty: UserType(0)
// + literal: Const { ty: extern "rust-intrinsic" fn() {assert_mem_uninitialized_valid::<&T>}, val: Value(<ZST>) }
}

bb1: {
StorageDead(_1); // scope 0 at $DIR/dont_yeet_assert.rs:+1:61: +1:62
_0 = const (); // scope 0 at $DIR/dont_yeet_assert.rs:+0:21: +2:2
return; // scope 0 at $DIR/dont_yeet_assert.rs:+2:2: +2:2
}
}

11 changes: 11 additions & 0 deletions tests/mir-opt/dont_yeet_assert.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// compile-flags: --crate-type=lib
// unit-test: InstCombine

#![feature(core_intrinsics)]

// Want to make sure this assertion isn't compiled away in generic code.

// EMIT_MIR dont_yeet_assert.generic.InstCombine.diff
pub fn generic<T>() {
core::intrinsics::assert_mem_uninitialized_valid::<&T>();
}
8 changes: 8 additions & 0 deletions tests/ui/lint/invalid_value-polymorphic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// compile-flags: --crate-type=lib -Zmir-enable-passes=+InstCombine
// build-pass

#![feature(core_intrinsics)]

pub fn generic<T>() {
core::intrinsics::assert_mem_uninitialized_valid::<&T>();
}

0 comments on commit c528357

Please sign in to comment.