Skip to content

Commit

Permalink
adjust Miri to Pointer type overhaul
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Jul 16, 2021
1 parent a4a9a36 commit a1233a7
Show file tree
Hide file tree
Showing 46 changed files with 750 additions and 659 deletions.
14 changes: 10 additions & 4 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ extern crate rustc_session;

use std::convert::TryFrom;
use std::env;
use std::num::NonZeroU64;
use std::path::PathBuf;
use std::rc::Rc;
use std::str::FromStr;
Expand Down Expand Up @@ -412,11 +413,16 @@ fn main() {
}
}
arg if arg.starts_with("-Zmiri-track-alloc-id=") => {
let id: u64 = match arg.strip_prefix("-Zmiri-track-alloc-id=").unwrap().parse()
let id = match arg
.strip_prefix("-Zmiri-track-alloc-id=")
.unwrap()
.parse()
.ok()
.and_then(NonZeroU64::new)
{
Ok(id) => id,
Err(err) =>
panic!("-Zmiri-track-alloc-id requires a valid `u64` argument: {}", err),
Some(id) => id,
None =>
panic!("-Zmiri-track-alloc-id requires a valid non-zero `u64` argument"),
};
miri_config.tracked_alloc_id = Some(miri::AllocId(id));
}
Expand Down
105 changes: 39 additions & 66 deletions src/data_race.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ use rustc_middle::{mir, ty::layout::TyAndLayout};
use rustc_target::abi::Size;

use crate::{
ImmTy, Immediate, InterpResult, MPlaceTy, MemPlaceMeta, MemoryKind, MiriEvalContext,
MiriEvalContextExt, MiriMemoryKind, OpTy, Pointer, RangeMap, Scalar, ScalarMaybeUninit, Tag,
ThreadId, VClock, VTimestamp, VectorIdx,
AllocId, AllocRange, ImmTy, Immediate, InterpResult, MPlaceTy, MemPlaceMeta, MemoryKind,
MiriEvalContext, MiriEvalContextExt, MiriMemoryKind, OpTy, Pointer, RangeMap, Scalar,
ScalarMaybeUninit, Tag, ThreadId, VClock, VTimestamp, VectorIdx,
};

pub type AllocExtra = VClockAlloc;
Expand Down Expand Up @@ -561,7 +561,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
if lt { &rhs } else { &old }
};

this.allow_data_races_mut(|this| this.write_immediate_to_mplace(**new_val, place))?;
this.allow_data_races_mut(|this| this.write_immediate(**new_val, &(*place).into()))?;

this.validate_atomic_rmw(&place, atomic)?;

Expand Down Expand Up @@ -713,18 +713,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
Ok(())
}
}

fn reset_vector_clocks(&mut self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
let this = self.eval_context_mut();
if let Some(data_race) = &mut this.memory.extra.data_race {
if data_race.multi_threaded.get() {
let alloc_meta =
this.memory.get_alloc_extra_mut(ptr.alloc_id)?.0.data_race.as_mut().unwrap();
alloc_meta.reset_clocks(ptr.offset, size);
}
}
Ok(())
}
}

/// Vector clock metadata for a logical memory allocation.
Expand Down Expand Up @@ -769,14 +757,6 @@ impl VClockAlloc {
}
}

fn reset_clocks(&mut self, offset: Size, len: Size) {
let alloc_ranges = self.alloc_ranges.get_mut();
for (_, range) in alloc_ranges.iter_mut(offset, len) {
// Reset the portion of the range
*range = MemoryCellClocks::new(0, VectorIdx::MAX_INDEX);
}
}

// Find an index, if one exists where the value
// in `l` is greater than the value in `r`.
fn find_gt_index(l: &VClock, r: &VClock) -> Option<VectorIdx> {
Expand Down Expand Up @@ -820,8 +800,7 @@ impl VClockAlloc {
range: &MemoryCellClocks,
action: &str,
is_atomic: bool,
pointer: Pointer<Tag>,
len: Size,
ptr_dbg: Pointer<AllocId>,
) -> InterpResult<'tcx> {
let (current_index, current_clocks) = global.current_thread_state();
let write_clock;
Expand Down Expand Up @@ -863,15 +842,12 @@ impl VClockAlloc {

// Throw the data-race detection.
throw_ub_format!(
"Data race detected between {} on {} and {} on {}, memory({:?},offset={},size={})\
\n(current vector clock = {:?}, conflicting timestamp = {:?})",
"Data race detected between {} on {} and {} on {} at {:?} (current vector clock = {:?}, conflicting timestamp = {:?})",
action,
current_thread_info,
other_action,
other_thread_info,
pointer.alloc_id,
pointer.offset.bytes(),
len.bytes(),
ptr_dbg,
current_clocks.clock,
other_clock
)
Expand All @@ -884,17 +860,23 @@ impl VClockAlloc {
/// atomic read operations.
pub fn read<'tcx>(
&self,
pointer: Pointer<Tag>,
len: Size,
alloc_id: AllocId,
range: AllocRange,
global: &GlobalState,
) -> InterpResult<'tcx> {
if global.multi_threaded.get() {
let (index, clocks) = global.current_thread_state();
let mut alloc_ranges = self.alloc_ranges.borrow_mut();
for (_, range) in alloc_ranges.iter_mut(pointer.offset, len) {
for (offset, range) in alloc_ranges.iter_mut(range.start, range.size) {
if let Err(DataRace) = range.read_race_detect(&*clocks, index) {
// Report data-race.
return Self::report_data_race(global, range, "Read", false, pointer, len);
return Self::report_data_race(
global,
range,
"Read",
false,
Pointer::new(alloc_id, offset),
);
}
}
Ok(())
Expand All @@ -906,23 +888,22 @@ impl VClockAlloc {
// Shared code for detecting data-races on unique access to a section of memory
fn unique_access<'tcx>(
&mut self,
pointer: Pointer<Tag>,
len: Size,
alloc_id: AllocId,
range: AllocRange,
write_type: WriteType,
global: &mut GlobalState,
) -> InterpResult<'tcx> {
if global.multi_threaded.get() {
let (index, clocks) = global.current_thread_state();
for (_, range) in self.alloc_ranges.get_mut().iter_mut(pointer.offset, len) {
for (offset, range) in self.alloc_ranges.get_mut().iter_mut(range.start, range.size) {
if let Err(DataRace) = range.write_race_detect(&*clocks, index, write_type) {
// Report data-race
return Self::report_data_race(
global,
range,
write_type.get_descriptor(),
false,
pointer,
len,
Pointer::new(alloc_id, offset),
);
}
}
Expand All @@ -938,11 +919,11 @@ impl VClockAlloc {
/// operation
pub fn write<'tcx>(
&mut self,
pointer: Pointer<Tag>,
len: Size,
alloc_id: AllocId,
range: AllocRange,
global: &mut GlobalState,
) -> InterpResult<'tcx> {
self.unique_access(pointer, len, WriteType::Write, global)
self.unique_access(alloc_id, range, WriteType::Write, global)
}

/// Detect data-races for an unsynchronized deallocate operation, will not perform
Expand All @@ -951,11 +932,11 @@ impl VClockAlloc {
/// operation
pub fn deallocate<'tcx>(
&mut self,
pointer: Pointer<Tag>,
len: Size,
alloc_id: AllocId,
range: AllocRange,
global: &mut GlobalState,
) -> InterpResult<'tcx> {
self.unique_access(pointer, len, WriteType::Deallocate, global)
self.unique_access(alloc_id, range, WriteType::Deallocate, global)
}
}

Expand Down Expand Up @@ -1002,12 +983,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
result
}

/// Generic atomic operation implementation,
/// this accesses memory via get_raw instead of
/// get_raw_mut, due to issues calling get_raw_mut
/// for atomic loads from read-only memory.
/// FIXME: is this valid, or should get_raw_mut be used for
/// atomic-stores/atomic-rmw?
/// Generic atomic operation implementation
fn validate_atomic_op<A: Debug + Copy>(
&self,
place: &MPlaceTy<'tcx, Tag>,
Expand All @@ -1023,25 +999,24 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
let this = self.eval_context_ref();
if let Some(data_race) = &this.memory.extra.data_race {
if data_race.multi_threaded.get() {
let size = place.layout.size;
let (alloc_id, base_offset, ptr) = this.memory.ptr_get_alloc(place.ptr)?;
// Load and log the atomic operation.
// Note that atomic loads are possible even from read-only allocations, so `get_alloc_extra_mut` is not an option.
let place_ptr = place.ptr.assert_ptr();
let size = place.layout.size;
let alloc_meta =
&this.memory.get_alloc_extra(place_ptr.alloc_id)?.data_race.as_ref().unwrap();
&this.memory.get_alloc_extra(alloc_id)?.data_race.as_ref().unwrap();
log::trace!(
"Atomic op({}) with ordering {:?} on memory({:?}, offset={}, size={})",
"Atomic op({}) with ordering {:?} on {:?} (size={})",
description,
&atomic,
place_ptr.alloc_id,
place_ptr.offset.bytes(),
ptr,
size.bytes()
);

// Perform the atomic operation.
data_race.maybe_perform_sync_operation(|index, mut clocks| {
for (_, range) in
alloc_meta.alloc_ranges.borrow_mut().iter_mut(place_ptr.offset, size)
for (offset, range) in
alloc_meta.alloc_ranges.borrow_mut().iter_mut(base_offset, size)
{
if let Err(DataRace) = op(range, &mut *clocks, index, atomic) {
mem::drop(clocks);
Expand All @@ -1050,8 +1025,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
range,
description,
true,
place_ptr,
size,
Pointer::new(alloc_id, offset),
)
.map(|_| true);
}
Expand All @@ -1063,12 +1037,11 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {

// Log changes to atomic memory.
if log::log_enabled!(log::Level::Trace) {
for (_, range) in alloc_meta.alloc_ranges.borrow().iter(place_ptr.offset, size)
for (_offset, range) in alloc_meta.alloc_ranges.borrow().iter(base_offset, size)
{
log::trace!(
"Updated atomic memory({:?}, offset={}, size={}) to {:#?}",
place.ptr.assert_ptr().alloc_id,
place_ptr.offset.bytes(),
"Updated atomic memory({:?}, size={}) to {:#?}",
ptr,
size.bytes(),
range.atomic_ops
);
Expand Down
2 changes: 1 addition & 1 deletion src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ pub fn report_error<'tcx, 'mir>(
let helps = match e.kind() {
Unsupported(UnsupportedOpInfo::NoMirFor(..)) =>
vec![(None, format!("make sure to use a Miri sysroot, which you can prepare with `cargo miri setup`"))],
Unsupported(UnsupportedOpInfo::ReadBytesAsPointer | UnsupportedOpInfo::ThreadLocalStatic(_) | UnsupportedOpInfo::ReadExternStatic(_)) =>
Unsupported(UnsupportedOpInfo::ThreadLocalStatic(_) | UnsupportedOpInfo::ReadExternStatic(_)) =>
panic!("Error should never be raised by Miri: {:?}", e.kind()),
Unsupported(_) =>
vec![(None, format!("this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support"))],
Expand Down
27 changes: 15 additions & 12 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,16 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
// Third argument (`argv`): created from `config.args`.
let argv = {
// Put each argument in memory, collect pointers.
let mut argvs = Vec::<Scalar<Tag>>::new();
let mut argvs = Vec::<Immediate<Tag>>::new();
for arg in config.args.iter() {
// Make space for `0` terminator.
let size = u64::try_from(arg.len()).unwrap().checked_add(1).unwrap();
let arg_type = tcx.mk_array(tcx.types.u8, size);
let arg_place =
ecx.allocate(ecx.layout_of(arg_type)?, MiriMemoryKind::Machine.into())?;
ecx.write_os_str_to_c_str(OsStr::new(arg), arg_place.ptr, size)?;
argvs.push(arg_place.ptr);
ecx.mark_immutable(&*arg_place);
argvs.push(arg_place.to_ref(&ecx));
}
// Make an array with all these pointers, in the Miri memory.
let argvs_layout = ecx.layout_of(
Expand All @@ -181,24 +182,26 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
let argvs_place = ecx.allocate(argvs_layout, MiriMemoryKind::Machine.into())?;
for (idx, arg) in argvs.into_iter().enumerate() {
let place = ecx.mplace_field(&argvs_place, idx)?;
ecx.write_scalar(arg, &place.into())?;
ecx.write_immediate(arg, &place.into())?;
}
ecx.memory.mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
ecx.mark_immutable(&*argvs_place);
// A pointer to that place is the 3rd argument for main.
let argv = argvs_place.ptr;
let argv = argvs_place.to_ref(&ecx);
// Store `argc` and `argv` for macOS `_NSGetArg{c,v}`.
{
let argc_place =
ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
ecx.write_scalar(argc, &argc_place.into())?;
ecx.machine.argc = Some(argc_place.ptr);
ecx.mark_immutable(&*argc_place);
ecx.machine.argc = Some(*argc_place);

let argv_place = ecx.allocate(
ecx.layout_of(tcx.mk_imm_ptr(tcx.types.unit))?,
MiriMemoryKind::Machine.into(),
)?;
ecx.write_scalar(argv, &argv_place.into())?;
ecx.machine.argv = Some(argv_place.ptr);
ecx.write_immediate(argv, &argv_place.into())?;
ecx.mark_immutable(&*argv_place);
ecx.machine.argv = Some(*argv_place);
}
// Store command line as UTF-16 for Windows `GetCommandLineW`.
{
Expand All @@ -217,12 +220,13 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
let cmd_type = tcx.mk_array(tcx.types.u16, u64::try_from(cmd_utf16.len()).unwrap());
let cmd_place =
ecx.allocate(ecx.layout_of(cmd_type)?, MiriMemoryKind::Machine.into())?;
ecx.machine.cmd_line = Some(cmd_place.ptr);
ecx.machine.cmd_line = Some(*cmd_place);
// Store the UTF-16 string. We just allocated so we know the bounds are fine.
for (idx, &c) in cmd_utf16.iter().enumerate() {
let place = ecx.mplace_field(&cmd_place, idx)?;
ecx.write_scalar(Scalar::from_u16(c), &place.into())?;
}
ecx.mark_immutable(&*cmd_place);
}
argv
};
Expand All @@ -233,7 +237,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
ecx.call_function(
start_instance,
Abi::Rust,
&[main_ptr.into(), argc.into(), argv.into()],
&[Scalar::from_pointer(main_ptr, &ecx).into(), argc.into(), argv],
Some(&ret_place.into()),
StackPopCleanup::None { cleanup: true },
)?;
Expand Down Expand Up @@ -285,8 +289,7 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
}
ecx.process_diagnostics(info);
}
let return_code =
ecx.read_scalar(&ret_place.into())?.check_init()?.to_machine_isize(&ecx)?;
let return_code = ecx.read_scalar(&ret_place.into())?.to_machine_isize(&ecx)?;
Ok(return_code)
})();

Expand Down
Loading

0 comments on commit a1233a7

Please sign in to comment.