Rollup merge of #97323 - 5225225:strict_init_checks, r=oli-obk
Introduce stricter checks for might_permit_raw_init under a debug flag This is intended to be a version of the strict checks tried out in #79296, but also checking number validity (under the assumption that `let _ = std::mem::uninitialized::<u32>()` is UB, which seems to be what https://github.com/rust-lang/unsafe-code-guidelines/issues/71 is leaning towards.)
This commit is contained in:
commit
02c0c768c1
@ -58,6 +58,7 @@ macro_rules! intrinsic_match {
|
||||
use rustc_middle::ty::print::with_no_trimmed_paths;
|
||||
use rustc_middle::ty::subst::SubstsRef;
|
||||
use rustc_span::symbol::{kw, sym, Symbol};
|
||||
use rustc_target::abi::InitKind;
|
||||
|
||||
use crate::prelude::*;
|
||||
use cranelift_codegen::ir::AtomicRmwOp;
|
||||
@ -671,7 +672,12 @@ fn swap(bcx: &mut FunctionBuilder<'_>, v: Value) -> Value {
|
||||
return;
|
||||
}
|
||||
|
||||
if intrinsic == sym::assert_zero_valid && !layout.might_permit_raw_init(fx, /*zero:*/ true) {
|
||||
if intrinsic == sym::assert_zero_valid
|
||||
&& !layout.might_permit_raw_init(
|
||||
fx,
|
||||
InitKind::Zero,
|
||||
fx.tcx.sess.opts.debugging_opts.strict_init_checks) {
|
||||
|
||||
with_no_trimmed_paths!({
|
||||
crate::base::codegen_panic(
|
||||
fx,
|
||||
@ -682,7 +688,12 @@ fn swap(bcx: &mut FunctionBuilder<'_>, v: Value) -> Value {
|
||||
return;
|
||||
}
|
||||
|
||||
if intrinsic == sym::assert_uninit_valid && !layout.might_permit_raw_init(fx, /*zero:*/ false) {
|
||||
if intrinsic == sym::assert_uninit_valid
|
||||
&& !layout.might_permit_raw_init(
|
||||
fx,
|
||||
InitKind::Uninit,
|
||||
fx.tcx.sess.opts.debugging_opts.strict_init_checks) {
|
||||
|
||||
with_no_trimmed_paths!({
|
||||
crate::base::codegen_panic(
|
||||
fx,
|
||||
|
@ -22,7 +22,7 @@
|
||||
use rustc_span::{sym, Symbol};
|
||||
use rustc_symbol_mangling::typeid_for_fnabi;
|
||||
use rustc_target::abi::call::{ArgAbi, FnAbi, PassMode};
|
||||
use rustc_target::abi::{self, HasDataLayout, WrappingRange};
|
||||
use rustc_target::abi::{self, HasDataLayout, InitKind, WrappingRange};
|
||||
use rustc_target::spec::abi::Abi;
|
||||
|
||||
/// Used by `FunctionCx::codegen_terminator` for emitting common patterns
|
||||
@ -521,6 +521,7 @@ fn codegen_panic_intrinsic(
|
||||
source_info: mir::SourceInfo,
|
||||
target: Option<mir::BasicBlock>,
|
||||
cleanup: Option<mir::BasicBlock>,
|
||||
strict_validity: bool,
|
||||
) -> bool {
|
||||
// Emit a panic or a no-op for `assert_*` intrinsics.
|
||||
// These are intrinsics that compile to panics so that we can get a message
|
||||
@ -543,8 +544,8 @@ enum AssertIntrinsic {
|
||||
let layout = bx.layout_of(ty);
|
||||
let do_panic = match intrinsic {
|
||||
Inhabited => layout.abi.is_uninhabited(),
|
||||
ZeroValid => !layout.might_permit_raw_init(bx, /*zero:*/ true),
|
||||
UninitValid => !layout.might_permit_raw_init(bx, /*zero:*/ false),
|
||||
ZeroValid => !layout.might_permit_raw_init(bx, InitKind::Zero, strict_validity),
|
||||
UninitValid => !layout.might_permit_raw_init(bx, InitKind::Uninit, strict_validity),
|
||||
};
|
||||
if do_panic {
|
||||
let msg_str = with_no_visible_paths!({
|
||||
@ -678,6 +679,7 @@ fn codegen_call_terminator(
|
||||
source_info,
|
||||
target,
|
||||
cleanup,
|
||||
self.cx.tcx().sess.opts.debugging_opts.strict_init_checks,
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
@ -15,7 +15,7 @@
|
||||
use rustc_middle::ty::subst::SubstsRef;
|
||||
use rustc_middle::ty::{Ty, TyCtxt};
|
||||
use rustc_span::symbol::{sym, Symbol};
|
||||
use rustc_target::abi::{Abi, Align, Primitive, Size};
|
||||
use rustc_target::abi::{Abi, Align, InitKind, Primitive, Size};
|
||||
|
||||
use super::{
|
||||
util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, Machine, OpTy, PlaceTy,
|
||||
@ -408,7 +408,11 @@ pub fn emulate_intrinsic(
|
||||
)?;
|
||||
}
|
||||
if intrinsic_name == sym::assert_zero_valid
|
||||
&& !layout.might_permit_raw_init(self, /*zero:*/ true)
|
||||
&& !layout.might_permit_raw_init(
|
||||
self,
|
||||
InitKind::Zero,
|
||||
self.tcx.sess.opts.debugging_opts.strict_init_checks,
|
||||
)
|
||||
{
|
||||
M::abort(
|
||||
self,
|
||||
@ -419,7 +423,11 @@ pub fn emulate_intrinsic(
|
||||
)?;
|
||||
}
|
||||
if intrinsic_name == sym::assert_uninit_valid
|
||||
&& !layout.might_permit_raw_init(self, /*zero:*/ false)
|
||||
&& !layout.might_permit_raw_init(
|
||||
self,
|
||||
InitKind::Uninit,
|
||||
self.tcx.sess.opts.debugging_opts.strict_init_checks,
|
||||
)
|
||||
{
|
||||
M::abort(
|
||||
self,
|
||||
|
@ -1495,6 +1495,8 @@ pub(crate) fn parse_branch_protection(
|
||||
"hash algorithm of source files in debug info (`md5`, `sha1`, or `sha256`)"),
|
||||
stack_protector: StackProtector = (StackProtector::None, parse_stack_protector, [TRACKED],
|
||||
"control stack smash protection strategy (`rustc --print stack-protector-strategies` for details)"),
|
||||
strict_init_checks: bool = (false, parse_bool, [TRACKED],
|
||||
"control if mem::uninitialized and mem::zeroed panic on more UB"),
|
||||
strip: Strip = (Strip::None, parse_strip, [UNTRACKED],
|
||||
"tell the linker which information to strip (`none` (default), `debuginfo` or `symbols`)"),
|
||||
split_dwarf_kind: SplitDwarfKind = (SplitDwarfKind::Split, parse_split_dwarf_kind, [UNTRACKED],
|
||||
|
@ -894,6 +894,15 @@ pub fn is_always_valid<C: HasDataLayout>(&self, cx: &C) -> bool {
|
||||
Scalar::Union { .. } => true,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns `true` if this type can be left uninit.
|
||||
#[inline]
|
||||
pub fn is_uninit_valid(&self) -> bool {
|
||||
match *self {
|
||||
Scalar::Initialized { .. } => false,
|
||||
Scalar::Union { .. } => true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Describes how the fields of a type are located in memory.
|
||||
@ -1355,6 +1364,14 @@ pub struct PointeeInfo {
|
||||
pub address_space: AddressSpace,
|
||||
}
|
||||
|
||||
/// Used in `might_permit_raw_init` to indicate the kind of initialisation
|
||||
/// that is checked to be valid
|
||||
#[derive(Copy, Clone, Debug)]
|
||||
pub enum InitKind {
|
||||
Zero,
|
||||
Uninit,
|
||||
}
|
||||
|
||||
/// Trait that needs to be implemented by the higher-level type representation
|
||||
/// (e.g. `rustc_middle::ty::Ty`), to provide `rustc_target::abi` functionality.
|
||||
pub trait TyAbiInterface<'a, C>: Sized {
|
||||
@ -1461,26 +1478,37 @@ pub fn is_zst(&self) -> bool {
|
||||
|
||||
/// Determines if this type permits "raw" initialization by just transmuting some
|
||||
/// memory into an instance of `T`.
|
||||
/// `zero` indicates if the memory is zero-initialized, or alternatively
|
||||
/// left entirely uninitialized.
|
||||
///
|
||||
/// `init_kind` indicates if the memory is zero-initialized or left uninitialized.
|
||||
///
|
||||
/// `strict` is an opt-in debugging flag added in #97323 that enables more checks.
|
||||
///
|
||||
/// This is conservative: in doubt, it will answer `true`.
|
||||
///
|
||||
/// FIXME: Once we removed all the conservatism, we could alternatively
|
||||
/// create an all-0/all-undef constant and run the const value validator to see if
|
||||
/// this is a valid value for the given type.
|
||||
pub fn might_permit_raw_init<C>(self, cx: &C, zero: bool) -> bool
|
||||
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind, strict: bool) -> bool
|
||||
where
|
||||
Self: Copy,
|
||||
Ty: TyAbiInterface<'a, C>,
|
||||
C: HasDataLayout,
|
||||
{
|
||||
let scalar_allows_raw_init = move |s: Scalar| -> bool {
|
||||
if zero {
|
||||
// The range must contain 0.
|
||||
s.valid_range(cx).contains(0)
|
||||
} else {
|
||||
// The range must include all values.
|
||||
s.is_always_valid(cx)
|
||||
match init_kind {
|
||||
InitKind::Zero => {
|
||||
// The range must contain 0.
|
||||
s.valid_range(cx).contains(0)
|
||||
}
|
||||
InitKind::Uninit => {
|
||||
if strict {
|
||||
// The type must be allowed to be uninit (which means "is a union").
|
||||
s.is_uninit_valid()
|
||||
} else {
|
||||
// The range must include all values.
|
||||
s.is_always_valid(cx)
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
@ -1500,12 +1528,19 @@ pub fn might_permit_raw_init<C>(self, cx: &C, zero: bool) -> bool
|
||||
// If we have not found an error yet, we need to recursively descend into fields.
|
||||
match &self.fields {
|
||||
FieldsShape::Primitive | FieldsShape::Union { .. } => {}
|
||||
FieldsShape::Array { .. } => {
|
||||
// FIXME(#66151): For now, we are conservative and do not check arrays.
|
||||
FieldsShape::Array { count, .. } => {
|
||||
// FIXME(#66151): For now, we are conservative and do not check arrays by default.
|
||||
if strict
|
||||
&& *count > 0
|
||||
&& !self.field(cx, 0).might_permit_raw_init(cx, init_kind, strict)
|
||||
{
|
||||
// Found non empty array with a type that is unhappy about this kind of initialization
|
||||
return false;
|
||||
}
|
||||
}
|
||||
FieldsShape::Arbitrary { offsets, .. } => {
|
||||
for idx in 0..offsets.len() {
|
||||
if !self.field(cx, idx).might_permit_raw_init(cx, zero) {
|
||||
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind, strict) {
|
||||
// We found a field that is unhappy with this kind of initialization.
|
||||
return false;
|
||||
}
|
||||
|
@ -1,8 +1,9 @@
|
||||
// run-pass
|
||||
// needs-unwind
|
||||
// ignore-wasm32-bare compiled with panic=abort by default
|
||||
// revisions: mir thir
|
||||
// revisions: mir thir strict
|
||||
// [thir]compile-flags: -Zthir-unsafeck
|
||||
// [strict]compile-flags: -Zstrict-init-checks
|
||||
// ignore-tidy-linelength
|
||||
|
||||
// This test checks panic emitted from `mem::{uninitialized,zeroed}`.
|
||||
@ -54,6 +55,8 @@ enum LR_NonZero {
|
||||
Right(num::NonZeroI64),
|
||||
}
|
||||
|
||||
struct ZeroSized;
|
||||
|
||||
fn test_panic_msg<T>(op: impl (FnOnce() -> T) + panic::UnwindSafe, msg: &str) {
|
||||
let err = panic::catch_unwind(op).err();
|
||||
assert_eq!(
|
||||
@ -228,11 +231,40 @@ fn main() {
|
||||
let _val = mem::zeroed::<[!; 0]>();
|
||||
let _val = mem::uninitialized::<MaybeUninit<bool>>();
|
||||
let _val = mem::uninitialized::<[!; 0]>();
|
||||
let _val = mem::uninitialized::<()>();
|
||||
let _val = mem::uninitialized::<ZeroSized>();
|
||||
|
||||
// These are UB because they have not been officially blessed, but we await the resolution
|
||||
// of <https://github.com/rust-lang/unsafe-code-guidelines/issues/71> before doing
|
||||
// anything about that.
|
||||
let _val = mem::uninitialized::<i32>();
|
||||
let _val = mem::uninitialized::<*const ()>();
|
||||
if cfg!(strict) {
|
||||
test_panic_msg(
|
||||
|| mem::uninitialized::<i32>(),
|
||||
"attempted to leave type `i32` uninitialized, which is invalid"
|
||||
);
|
||||
|
||||
test_panic_msg(
|
||||
|| mem::uninitialized::<*const ()>(),
|
||||
"attempted to leave type `*const ()` uninitialized, which is invalid"
|
||||
);
|
||||
|
||||
test_panic_msg(
|
||||
|| mem::uninitialized::<[i32; 1]>(),
|
||||
"attempted to leave type `[i32; 1]` uninitialized, which is invalid"
|
||||
);
|
||||
|
||||
test_panic_msg(
|
||||
|| mem::zeroed::<NonNull<()>>(),
|
||||
"attempted to zero-initialize type `core::ptr::non_null::NonNull<()>`, which is invalid"
|
||||
);
|
||||
|
||||
test_panic_msg(
|
||||
|| mem::zeroed::<[NonNull<()>; 1]>(),
|
||||
"attempted to zero-initialize type `[core::ptr::non_null::NonNull<()>; 1]`, which is invalid"
|
||||
);
|
||||
} else {
|
||||
// These are UB because they have not been officially blessed, but we await the resolution
|
||||
// of <https://github.com/rust-lang/unsafe-code-guidelines/issues/71> before doing
|
||||
// anything about that.
|
||||
let _val = mem::uninitialized::<i32>();
|
||||
let _val = mem::uninitialized::<*const ()>();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user