Skip to content

Commit

Permalink
change might_permit_raw_init to fully detect LLVM UB, but not more th…
Browse files Browse the repository at this point in the history
…an that
  • Loading branch information
RalfJung committed Oct 5, 2022
1 parent 02cd79a commit a0131f0
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 a0131f0

Please sign in to comment.