Skip to content

Commit

Permalink
Auto merge of #1904 - camelid:uninit-num, r=RalfJung
Browse files Browse the repository at this point in the history
Add flag to check for uninitialized numbers

Closes #1340.

Companion rustc PR that implements this in the Miri engine: rust-lang/rust#88670

r? `@RalfJung`
  • Loading branch information
bors committed Nov 10, 2021
2 parents 3f2c9ee + 6dd1082 commit a8b976e
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 7 deletions.
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,10 @@ up the sysroot. If you are using `miri` (the Miri driver) directly, see the
Miri adds its own set of `-Z` flags, which are usually set via the `MIRIFLAGS`
environment variable:

* `-Zmiri-check-number-validity` enables checking of integer and float validity
(e.g., they must be initialized and not carry pointer provenance) as part of
enforcing validity invariants. This has no effect when
`-Zmiri-disable-validation` is present.
* `-Zmiri-compare-exchange-weak-failure-rate=<rate>` changes the failure rate of
`compare_exchange_weak` operations. The default is `0.8` (so 4 out of 5 weak ops will fail).
You can change it to any value between `0.0` and `1.0`, where `1.0` means it
Expand Down
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
db062de72b0a064f45b6f86894cbdc7c0ec68844
68ca579406f2fa9ec62710e4a4d5d3e07a168d3c
3 changes: 3 additions & 0 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,9 @@ fn main() {
"-Zmiri-symbolic-alignment-check" => {
miri_config.check_alignment = miri::AlignmentCheck::Symbolic;
}
"-Zmiri-check-number-validity" => {
miri_config.check_number_validity = true;
}
"-Zmiri-disable-abi-check" => {
miri_config.check_abi = false;
}
Expand Down
3 changes: 3 additions & 0 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ pub struct MiriConfig {
pub stacked_borrows: bool,
/// Controls alignment checking.
pub check_alignment: AlignmentCheck,
/// Controls integer and float validity (e.g., initialization) checking.
pub check_number_validity: bool,
/// Controls function [ABI](Abi) checking.
pub check_abi: bool,
/// Action for an op requiring communication with the host.
Expand Down Expand Up @@ -104,6 +106,7 @@ impl Default for MiriConfig {
validate: true,
stacked_borrows: true,
check_alignment: AlignmentCheck::Int,
check_number_validity: false,
check_abi: true,
isolated_op: IsolatedOp::Reject(RejectOpWith::Abort),
ignore_leaks: false,
Expand Down
9 changes: 9 additions & 0 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,9 @@ pub struct Evaluator<'mir, 'tcx> {
/// Whether to enforce the validity invariant.
pub(crate) validate: bool,

/// Whether to enforce validity (e.g., initialization) of integers and floats.
pub(crate) enforce_number_validity: bool,

/// Whether to enforce [ABI](Abi) of function calls.
pub(crate) enforce_abi: bool,

Expand Down Expand Up @@ -356,6 +359,7 @@ impl<'mir, 'tcx> Evaluator<'mir, 'tcx> {
tls: TlsData::default(),
isolated_op: config.isolated_op,
validate: config.validate,
enforce_number_validity: config.check_number_validity,
enforce_abi: config.check_abi,
file_handler: Default::default(),
dir_handler: Default::default(),
Expand Down Expand Up @@ -426,6 +430,11 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'mir, 'tcx> {
ecx.machine.validate
}

#[inline(always)]
fn enforce_number_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
ecx.machine.enforce_number_validity
}

#[inline(always)]
fn enforce_abi(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool {
ecx.machine.enforce_abi
Expand Down
29 changes: 23 additions & 6 deletions src/shims/posix/sync.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use std::time::SystemTime;

use rustc_hir::LangItem;
use rustc_middle::ty::{layout::TyAndLayout, query::TyCtxtAt, subst::Subst, Ty};

use crate::*;
use thread::Time;

Expand Down Expand Up @@ -44,7 +47,7 @@ fn mutexattr_set_kind<'mir, 'tcx: 'mir>(
attr_op: &OpTy<'tcx, Tag>,
kind: impl Into<ScalarMaybeUninit<Tag>>,
) -> InterpResult<'tcx, ()> {
ecx.write_scalar_at_offset(attr_op, 0, kind, ecx.machine.layouts.i32)
ecx.write_scalar_at_offset(attr_op, 0, kind, layout_of_maybe_uninit(ecx.tcx, ecx.tcx.types.i32))
}

// pthread_mutex_t is between 24 and 48 bytes, depending on the platform.
Expand Down Expand Up @@ -79,7 +82,7 @@ fn mutex_set_kind<'mir, 'tcx: 'mir>(
mutex_op,
offset,
kind,
ecx.machine.layouts.i32,
layout_of_maybe_uninit(ecx.tcx, ecx.tcx.types.i32),
AtomicWriteOp::Relaxed,
)
}
Expand All @@ -100,7 +103,7 @@ fn mutex_set_id<'mir, 'tcx: 'mir>(
mutex_op,
4,
id,
ecx.machine.layouts.u32,
layout_of_maybe_uninit(ecx.tcx, ecx.tcx.types.u32),
AtomicWriteOp::Relaxed,
)
}
Expand Down Expand Up @@ -144,7 +147,7 @@ fn rwlock_set_id<'mir, 'tcx: 'mir>(
rwlock_op,
4,
id,
ecx.machine.layouts.u32,
layout_of_maybe_uninit(ecx.tcx, ecx.tcx.types.u32),
AtomicWriteOp::Relaxed,
)
}
Expand Down Expand Up @@ -211,7 +214,7 @@ fn cond_set_id<'mir, 'tcx: 'mir>(
cond_op,
4,
id,
ecx.machine.layouts.u32,
layout_of_maybe_uninit(ecx.tcx, ecx.tcx.types.u32),
AtomicWriteOp::Relaxed,
)
}
Expand Down Expand Up @@ -244,7 +247,12 @@ fn cond_set_clock_id<'mir, 'tcx: 'mir>(
cond_op: &OpTy<'tcx, Tag>,
clock_id: impl Into<ScalarMaybeUninit<Tag>>,
) -> InterpResult<'tcx, ()> {
ecx.write_scalar_at_offset(cond_op, 8, clock_id, ecx.machine.layouts.i32)
ecx.write_scalar_at_offset(
cond_op,
8,
clock_id,
layout_of_maybe_uninit(ecx.tcx, ecx.tcx.types.i32),
)
}

/// Try to reacquire the mutex associated with the condition variable after we
Expand Down Expand Up @@ -788,3 +796,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
Ok(0)
}
}

fn layout_of_maybe_uninit<'tcx>(tcx: TyCtxtAt<'tcx>, param: Ty<'tcx>) -> TyAndLayout<'tcx> {
let def_id = tcx.require_lang_item(LangItem::MaybeUninit, None);
let def_ty = tcx.type_of(def_id);
let ty = def_ty.subst(*tcx, &[param.into()]);

let param_env = tcx.param_env(def_id);
tcx.layout_of(param_env.and(ty)).unwrap()
}
8 changes: 8 additions & 0 deletions tests/compile-fail/uninit_float.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// compile-flags: -Zmiri-check-number-validity

// This test is adapted from https://github.com/rust-lang/miri/issues/1340#issue-600900312.

fn main() {
let _val = unsafe { std::mem::MaybeUninit::<f32>::uninit().assume_init() };
//~^ ERROR type validation failed at .value: encountered uninitialized bytes, but expected initialized plain (non-pointer) bytes
}
8 changes: 8 additions & 0 deletions tests/compile-fail/uninit_integer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// compile-flags: -Zmiri-check-number-validity

// This test is from https://github.com/rust-lang/miri/issues/1340#issue-600900312.

fn main() {
let _val = unsafe { std::mem::MaybeUninit::<usize>::uninit().assume_init() };
//~^ ERROR type validation failed at .value: encountered uninitialized bytes, but expected initialized plain (non-pointer) bytes
}
8 changes: 8 additions & 0 deletions tests/compile-fail/uninit_integer_signed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// compile-flags: -Zmiri-check-number-validity

// This test is adapted from https://github.com/rust-lang/miri/issues/1340#issue-600900312.

fn main() {
let _val = unsafe { std::mem::MaybeUninit::<i32>::uninit().assume_init() };
//~^ ERROR type validation failed at .value: encountered uninitialized bytes, but expected initialized plain (non-pointer) bytes
}
4 changes: 4 additions & 0 deletions tests/compile-fail/uninit_raw_ptr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
fn main() {
let _val = unsafe { std::mem::MaybeUninit::<*const u8>::uninit().assume_init() };
//~^ ERROR type validation failed at .value: encountered uninitialized raw pointer
}
8 changes: 8 additions & 0 deletions tests/run-pass/uninit_number_ignored.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// This test is adapted from https://github.com/rust-lang/miri/issues/1340#issue-600900312.
// This test passes because -Zmiri-check-number-validity is not passed.

fn main() {
let _val1 = unsafe { std::mem::MaybeUninit::<usize>::uninit().assume_init() };
let _val2 = unsafe { std::mem::MaybeUninit::<i32>::uninit().assume_init() };
let _val3 = unsafe { std::mem::MaybeUninit::<f32>::uninit().assume_init() };
}

0 comments on commit a8b976e

Please sign in to comment.