Skip to content
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

Don't ICE in might_permit_raw_init if reference is polymorphic #108012

Merged
merged 2 commits into from
Feb 16, 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
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);
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -2138,12 +2138,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>();
}