Skip to content

Commit

Permalink
Rollup merge of #101061 - RalfJung:panic-on-uninit, r=oli-obk
Browse files Browse the repository at this point in the history
panic-on-uninit: adjust checks to 0x01-filling

Now that `mem::uninitiailized` actually fills memory with `0x01` (#99182), we can make it panic in a few less cases without risking a lot more UB -- which hopefully slightly improves compatibility with some old code, and which might increase the chance that we can check inside arrays in the future.

We detect almost all of these with our lint, so authors of such code should still be warned -- but if this happens deep inside a dependency, the panic can be quite interruptive, so it might be better not to do it when there is no risk of LLVM UB.  Therefore, adjust the `might_permit_raw_init` logic to care primarily about LLVM UB. To my knowledge, it actually covers all cases of LLVM UB now.

Fixes #66151

Cc ``@5225225``
  • Loading branch information
Dylan-DPC authored Oct 5, 2022
2 parents f8f5019 + a0131f0 commit ab88c19
Show file tree
Hide file tree
Showing 10 changed files with 393 additions and 240 deletions.
6 changes: 2 additions & 4 deletions compiler/rustc_const_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ extern crate rustc_middle;
pub mod const_eval;
mod errors;
pub mod interpret;
mod might_permit_raw_init;
pub mod transform;
pub mod util;

Expand Down Expand Up @@ -61,7 +60,6 @@ pub fn provide(providers: &mut Providers) {
const_eval::deref_mir_constant(tcx, param_env, value)
};
providers.permits_uninit_init =
|tcx, ty| might_permit_raw_init::might_permit_raw_init(tcx, ty, InitKind::Uninit);
providers.permits_zero_init =
|tcx, ty| might_permit_raw_init::might_permit_raw_init(tcx, ty, InitKind::Zero);
|tcx, ty| util::might_permit_raw_init(tcx, ty, InitKind::UninitMitigated0x01Fill);
providers.permits_zero_init = |tcx, ty| util::might_permit_raw_init(tcx, ty, InitKind::Zero);
}
44 changes: 0 additions & 44 deletions compiler/rustc_const_eval/src/might_permit_raw_init.rs

This file was deleted.

151 changes: 151 additions & 0 deletions compiler/rustc_const_eval/src/util/might_permit_raw_init.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
use rustc_middle::ty::layout::{LayoutCx, LayoutOf, TyAndLayout};
use rustc_middle::ty::{ParamEnv, TyCtxt};
use rustc_session::Limit;
use rustc_target::abi::{Abi, FieldsShape, InitKind, Scalar, Variants};

use crate::const_eval::CompileTimeInterpreter;
use crate::interpret::{InterpCx, MemoryKind, OpTy};

/// Determines if this type permits "raw" initialization by just transmuting some memory into an
/// instance of `T`.
///
/// `init_kind` indicates if the memory is zero-initialized or left uninitialized. We assume
/// uninitialized memory is mitigated by filling it with 0x01, which reduces the chance of causing
/// LLVM UB.
///
/// By default we check whether that operation would cause *LLVM UB*, i.e., whether the LLVM IR we
/// generate has UB or not. This is a mitigation strategy, which is why we are okay with accepting
/// Rust UB as long as there is no risk of miscompilations. The `strict_init_checks` can be set to
/// do a full check against Rust UB instead (in which case we will also ignore the 0x01-filling and
/// to the full uninit check).
pub fn might_permit_raw_init<'tcx>(
tcx: TyCtxt<'tcx>,
ty: TyAndLayout<'tcx>,
kind: InitKind,
) -> bool {
if tcx.sess.opts.unstable_opts.strict_init_checks {
might_permit_raw_init_strict(ty, tcx, kind)
} else {
let layout_cx = LayoutCx { tcx, param_env: ParamEnv::reveal_all() };
might_permit_raw_init_lax(ty, &layout_cx, kind)
}
}

/// Implements the 'strict' version of the `might_permit_raw_init` checks; see that function for
/// details.
fn might_permit_raw_init_strict<'tcx>(
ty: TyAndLayout<'tcx>,
tcx: TyCtxt<'tcx>,
kind: InitKind,
) -> bool {
let machine = CompileTimeInterpreter::new(
Limit::new(0),
/*can_access_statics:*/ false,
/*check_alignment:*/ true,
);

let mut cx = InterpCx::new(tcx, rustc_span::DUMMY_SP, ParamEnv::reveal_all(), machine);

let allocated = cx
.allocate(ty, MemoryKind::Machine(crate::const_eval::MemoryKind::Heap))
.expect("OOM: failed to allocate for uninit check");

if kind == InitKind::Zero {
cx.write_bytes_ptr(
allocated.ptr,
std::iter::repeat(0_u8).take(ty.layout.size().bytes_usize()),
)
.expect("failed to write bytes for zero valid check");
}

let ot: OpTy<'_, _> = allocated.into();

// Assume that if it failed, it's a validation failure.
// 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()
}

/// Implements the 'lax' (default) version of the `might_permit_raw_init` checks; see that function for
/// details.
fn might_permit_raw_init_lax<'tcx>(
this: TyAndLayout<'tcx>,
cx: &LayoutCx<'tcx, TyCtxt<'tcx>>,
init_kind: InitKind,
) -> bool {
let scalar_allows_raw_init = move |s: Scalar| -> bool {
match init_kind {
InitKind::Zero => {
// The range must contain 0.
s.valid_range(cx).contains(0)
}
InitKind::UninitMitigated0x01Fill => {
// The range must include an 0x01-filled buffer.
let mut val: u128 = 0x01;
for _ in 1..s.size(cx).bytes() {
// For sizes >1, repeat the 0x01.
val = (val << 8) | 0x01;
}
s.valid_range(cx).contains(val)
}
}
};

// Check the ABI.
let valid = match this.abi {
Abi::Uninhabited => false, // definitely UB
Abi::Scalar(s) => scalar_allows_raw_init(s),
Abi::ScalarPair(s1, s2) => scalar_allows_raw_init(s1) && scalar_allows_raw_init(s2),
Abi::Vector { element: s, count } => count == 0 || scalar_allows_raw_init(s),
Abi::Aggregate { .. } => true, // Fields are checked below.
};
if !valid {
// This is definitely not okay.
return 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");
// 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;
}
if pointee.size.bytes() > 0 {
// A 'fake' integer pointer is not sufficiently dereferenceable.
return false;
}
}

// If we have not found an error yet, we need to recursively descend into fields.
match &this.fields {
FieldsShape::Primitive | FieldsShape::Union { .. } => {}
FieldsShape::Array { .. } => {
// Arrays never have scalar layout in LLVM, so if the array is not actually
// accessed, there is no LLVM UB -- therefore we can skip this.
}
FieldsShape::Arbitrary { offsets, .. } => {
for idx in 0..offsets.len() {
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;
}
}
}
}

match &this.variants {
Variants::Single { .. } => {
// All fields of this single variant have already been checked above, there is nothing
// else to do.
}
Variants::Multiple { .. } => {
// We cannot tell LLVM anything about the details of this multi-variant layout, so
// invalid values "hidden" inside the variant cannot cause LLVM trouble.
}
}

true
}
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ mod alignment;
mod call_kind;
pub mod collect_writes;
mod find_self_call;
mod might_permit_raw_init;

pub use self::aggregate::expand_aggregate;
pub use self::alignment::is_disaligned;
pub use self::call_kind::{call_kind, CallDesugaringKind, CallKind};
pub use self::find_self_call::find_self_call;
pub use self::might_permit_raw_init::might_permit_raw_init;
70 changes: 1 addition & 69 deletions compiler/rustc_target/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,7 @@ pub struct PointeeInfo {
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum InitKind {
Zero,
Uninit,
UninitMitigated0x01Fill,
}

/// Trait that needs to be implemented by the higher-level type representation
Expand Down Expand Up @@ -1498,72 +1498,4 @@ impl<'a, Ty> TyAndLayout<'a, Ty> {
Abi::Aggregate { sized } => sized && self.size.bytes() == 0,
}
}

/// Determines if this type permits "raw" initialization by just transmuting some
/// memory into an instance of `T`.
///
/// `init_kind` indicates if the memory is zero-initialized or left uninitialized.
///
/// This code is intentionally conservative, and will not detect
/// * zero init of an enum whose 0 variant does not allow zero initialization
/// * making uninitialized types who have a full valid range (ints, floats, raw pointers)
/// * Any form of invalid value being made inside an array (unless the value is uninhabited)
///
/// A strict form of these checks that uses const evaluation exists in
/// `rustc_const_eval::might_permit_raw_init`, and a tracking issue for making these checks
/// stricter is <https://github.com/rust-lang/rust/issues/66151>.
///
/// FIXME: Once all the conservatism is removed from here, and the checks are ran by default,
/// we can use the const evaluation checks always instead.
pub fn might_permit_raw_init<C>(self, cx: &C, init_kind: InitKind) -> bool
where
Self: Copy,
Ty: TyAbiInterface<'a, C>,
C: HasDataLayout,
{
let scalar_allows_raw_init = move |s: Scalar| -> bool {
match init_kind {
InitKind::Zero => {
// The range must contain 0.
s.valid_range(cx).contains(0)
}
InitKind::Uninit => {
// The range must include all values.
s.is_always_valid(cx)
}
}
};

// Check the ABI.
let valid = match self.abi {
Abi::Uninhabited => false, // definitely UB
Abi::Scalar(s) => scalar_allows_raw_init(s),
Abi::ScalarPair(s1, s2) => scalar_allows_raw_init(s1) && scalar_allows_raw_init(s2),
Abi::Vector { element: s, count } => count == 0 || scalar_allows_raw_init(s),
Abi::Aggregate { .. } => true, // Fields are checked below.
};
if !valid {
// This is definitely not okay.
return false;
}

// 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 by default.
}
FieldsShape::Arbitrary { offsets, .. } => {
for idx in 0..offsets.len() {
if !self.field(cx, idx).might_permit_raw_init(cx, init_kind) {
// We found a field that is unhappy with this kind of initialization.
return false;
}
}
}
}

// FIXME(#66151): For now, we are conservative and do not check `self.variants`.
true
}
}
9 changes: 6 additions & 3 deletions src/test/ui/consts/assert-type-intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ fn main() {
use std::mem::MaybeUninit;

const _BAD1: () = unsafe {
MaybeUninit::<!>::uninit().assume_init();
intrinsics::assert_inhabited::<!>(); //~ERROR: any use of this value will cause an error
//~^WARN: previously accepted
};
const _BAD2: () = {
intrinsics::assert_uninit_valid::<bool>();
intrinsics::assert_uninit_valid::<!>(); //~ERROR: any use of this value will cause an error
//~^WARN: previously accepted
};
const _BAD3: () = {
intrinsics::assert_zero_valid::<&'static i32>();
intrinsics::assert_zero_valid::<&'static i32>(); //~ERROR: any use of this value will cause an error
//~^WARN: previously accepted
};
}
24 changes: 12 additions & 12 deletions src/test/ui/consts/assert-type-intrinsics.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@ error: any use of this value will cause an error
|
LL | const _BAD1: () = unsafe {
| ---------------
LL | MaybeUninit::<!>::uninit().assume_init();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ aborted execution: attempted to instantiate uninhabited type `!`
LL | intrinsics::assert_inhabited::<!>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ aborted execution: attempted to instantiate uninhabited type `!`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>
= note: `#[deny(const_err)]` on by default

error: any use of this value will cause an error
--> $DIR/assert-type-intrinsics.rs:17:9
--> $DIR/assert-type-intrinsics.rs:18:9
|
LL | const _BAD2: () = {
| ---------------
LL | intrinsics::assert_uninit_valid::<bool>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ aborted execution: attempted to leave type `bool` uninitialized, which is invalid
LL | intrinsics::assert_uninit_valid::<!>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ aborted execution: attempted to instantiate uninhabited type `!`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>

error: any use of this value will cause an error
--> $DIR/assert-type-intrinsics.rs:20:9
--> $DIR/assert-type-intrinsics.rs:22:9
|
LL | const _BAD3: () = {
| ---------------
Expand All @@ -40,29 +40,29 @@ error: any use of this value will cause an error
|
LL | const _BAD1: () = unsafe {
| ---------------
LL | MaybeUninit::<!>::uninit().assume_init();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ aborted execution: attempted to instantiate uninhabited type `!`
LL | intrinsics::assert_inhabited::<!>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ aborted execution: attempted to instantiate uninhabited type `!`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>
= note: `#[deny(const_err)]` on by default

Future breakage diagnostic:
error: any use of this value will cause an error
--> $DIR/assert-type-intrinsics.rs:17:9
--> $DIR/assert-type-intrinsics.rs:18:9
|
LL | const _BAD2: () = {
| ---------------
LL | intrinsics::assert_uninit_valid::<bool>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ aborted execution: attempted to leave type `bool` uninitialized, which is invalid
LL | intrinsics::assert_uninit_valid::<!>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ aborted execution: attempted to instantiate uninhabited type `!`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #71800 <https://github.com/rust-lang/rust/issues/71800>
= note: `#[deny(const_err)]` on by default

Future breakage diagnostic:
error: any use of this value will cause an error
--> $DIR/assert-type-intrinsics.rs:20:9
--> $DIR/assert-type-intrinsics.rs:22:9
|
LL | const _BAD3: () = {
| ---------------
Expand Down
Loading

0 comments on commit ab88c19

Please sign in to comment.