Skip to content

Add flag to check for uninitialized numbers #1904

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

Merged
merged 5 commits into from
Nov 10, 2021
Merged
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
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -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
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
@@ -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;
}
3 changes: 3 additions & 0 deletions src/eval.rs
Original file line number Diff line number Diff line change
@@ -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.
@@ -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,
9 changes: 9 additions & 0 deletions src/machine.rs
Original file line number Diff line number Diff line change
@@ -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,

@@ -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(),
@@ -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
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;

@@ -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.
@@ -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,
)
}
@@ -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,
)
}
@@ -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,
)
}
@@ -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,
)
}
@@ -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
@@ -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() };
}