Skip to content

Commit 25da527

Browse files
committed
adjust Miri to Pointer type overhaul
1 parent a4a9a36 commit 25da527

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+709
-634
lines changed

src/bin/miri.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use std::env;
1313
use std::path::PathBuf;
1414
use std::rc::Rc;
1515
use std::str::FromStr;
16+
use std::num::NonZeroU64;
1617

1718
use hex::FromHexError;
1819
use log::debug;
@@ -412,11 +413,11 @@ fn main() {
412413
}
413414
}
414415
arg if arg.starts_with("-Zmiri-track-alloc-id=") => {
415-
let id: u64 = match arg.strip_prefix("-Zmiri-track-alloc-id=").unwrap().parse()
416+
let id = match arg.strip_prefix("-Zmiri-track-alloc-id=").unwrap().parse().ok().and_then(NonZeroU64::new)
416417
{
417-
Ok(id) => id,
418-
Err(err) =>
419-
panic!("-Zmiri-track-alloc-id requires a valid `u64` argument: {}", err),
418+
Some(id) => id,
419+
None =>
420+
panic!("-Zmiri-track-alloc-id requires a valid non-zero `u64` argument"),
420421
};
421422
miri_config.tracked_alloc_id = Some(miri::AllocId(id));
422423
}

src/data_race.rs

+31-64
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ use rustc_target::abi::Size;
7575
use crate::{
7676
ImmTy, Immediate, InterpResult, MPlaceTy, MemPlaceMeta, MemoryKind, MiriEvalContext,
7777
MiriEvalContextExt, MiriMemoryKind, OpTy, Pointer, RangeMap, Scalar, ScalarMaybeUninit, Tag,
78-
ThreadId, VClock, VTimestamp, VectorIdx,
78+
ThreadId, VClock, VTimestamp, VectorIdx, AllocId, AllocRange,
7979
};
8080

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

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

566566
this.validate_atomic_rmw(&place, atomic)?;
567567

@@ -713,18 +713,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
713713
Ok(())
714714
}
715715
}
716-
717-
fn reset_vector_clocks(&mut self, ptr: Pointer<Tag>, size: Size) -> InterpResult<'tcx> {
718-
let this = self.eval_context_mut();
719-
if let Some(data_race) = &mut this.memory.extra.data_race {
720-
if data_race.multi_threaded.get() {
721-
let alloc_meta =
722-
this.memory.get_alloc_extra_mut(ptr.alloc_id)?.0.data_race.as_mut().unwrap();
723-
alloc_meta.reset_clocks(ptr.offset, size);
724-
}
725-
}
726-
Ok(())
727-
}
728716
}
729717

730718
/// Vector clock metadata for a logical memory allocation.
@@ -769,14 +757,6 @@ impl VClockAlloc {
769757
}
770758
}
771759

772-
fn reset_clocks(&mut self, offset: Size, len: Size) {
773-
let alloc_ranges = self.alloc_ranges.get_mut();
774-
for (_, range) in alloc_ranges.iter_mut(offset, len) {
775-
// Reset the portion of the range
776-
*range = MemoryCellClocks::new(0, VectorIdx::MAX_INDEX);
777-
}
778-
}
779-
780760
// Find an index, if one exists where the value
781761
// in `l` is greater than the value in `r`.
782762
fn find_gt_index(l: &VClock, r: &VClock) -> Option<VectorIdx> {
@@ -820,8 +800,7 @@ impl VClockAlloc {
820800
range: &MemoryCellClocks,
821801
action: &str,
822802
is_atomic: bool,
823-
pointer: Pointer<Tag>,
824-
len: Size,
803+
ptr_dbg: Pointer<AllocId>,
825804
) -> InterpResult<'tcx> {
826805
let (current_index, current_clocks) = global.current_thread_state();
827806
let write_clock;
@@ -863,15 +842,12 @@ impl VClockAlloc {
863842

864843
// Throw the data-race detection.
865844
throw_ub_format!(
866-
"Data race detected between {} on {} and {} on {}, memory({:?},offset={},size={})\
867-
\n(current vector clock = {:?}, conflicting timestamp = {:?})",
845+
"Data race detected between {} on {} and {} on {} at {:?} (current vector clock = {:?}, conflicting timestamp = {:?})",
868846
action,
869847
current_thread_info,
870848
other_action,
871849
other_thread_info,
872-
pointer.alloc_id,
873-
pointer.offset.bytes(),
874-
len.bytes(),
850+
ptr_dbg,
875851
current_clocks.clock,
876852
other_clock
877853
)
@@ -884,17 +860,17 @@ impl VClockAlloc {
884860
/// atomic read operations.
885861
pub fn read<'tcx>(
886862
&self,
887-
pointer: Pointer<Tag>,
888-
len: Size,
863+
alloc_id: AllocId,
864+
range: AllocRange,
889865
global: &GlobalState,
890866
) -> InterpResult<'tcx> {
891867
if global.multi_threaded.get() {
892868
let (index, clocks) = global.current_thread_state();
893869
let mut alloc_ranges = self.alloc_ranges.borrow_mut();
894-
for (_, range) in alloc_ranges.iter_mut(pointer.offset, len) {
870+
for (offset, range) in alloc_ranges.iter_mut(range.start, range.size) {
895871
if let Err(DataRace) = range.read_race_detect(&*clocks, index) {
896872
// Report data-race.
897-
return Self::report_data_race(global, range, "Read", false, pointer, len);
873+
return Self::report_data_race(global, range, "Read", false, Pointer::new(alloc_id, offset));
898874
}
899875
}
900876
Ok(())
@@ -906,23 +882,22 @@ impl VClockAlloc {
906882
// Shared code for detecting data-races on unique access to a section of memory
907883
fn unique_access<'tcx>(
908884
&mut self,
909-
pointer: Pointer<Tag>,
910-
len: Size,
885+
alloc_id: AllocId,
886+
range: AllocRange,
911887
write_type: WriteType,
912888
global: &mut GlobalState,
913889
) -> InterpResult<'tcx> {
914890
if global.multi_threaded.get() {
915891
let (index, clocks) = global.current_thread_state();
916-
for (_, range) in self.alloc_ranges.get_mut().iter_mut(pointer.offset, len) {
892+
for (offset, range) in self.alloc_ranges.get_mut().iter_mut(range.start, range.size) {
917893
if let Err(DataRace) = range.write_race_detect(&*clocks, index, write_type) {
918894
// Report data-race
919895
return Self::report_data_race(
920896
global,
921897
range,
922898
write_type.get_descriptor(),
923899
false,
924-
pointer,
925-
len,
900+
Pointer::new(alloc_id, offset),
926901
);
927902
}
928903
}
@@ -938,11 +913,11 @@ impl VClockAlloc {
938913
/// operation
939914
pub fn write<'tcx>(
940915
&mut self,
941-
pointer: Pointer<Tag>,
942-
len: Size,
916+
alloc_id: AllocId,
917+
range: AllocRange,
943918
global: &mut GlobalState,
944919
) -> InterpResult<'tcx> {
945-
self.unique_access(pointer, len, WriteType::Write, global)
920+
self.unique_access(alloc_id, range, WriteType::Write, global)
946921
}
947922

948923
/// Detect data-races for an unsynchronized deallocate operation, will not perform
@@ -951,11 +926,11 @@ impl VClockAlloc {
951926
/// operation
952927
pub fn deallocate<'tcx>(
953928
&mut self,
954-
pointer: Pointer<Tag>,
955-
len: Size,
929+
alloc_id: AllocId,
930+
range: AllocRange,
956931
global: &mut GlobalState,
957932
) -> InterpResult<'tcx> {
958-
self.unique_access(pointer, len, WriteType::Deallocate, global)
933+
self.unique_access(alloc_id, range, WriteType::Deallocate, global)
959934
}
960935
}
961936

@@ -1002,12 +977,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
1002977
result
1003978
}
1004979

1005-
/// Generic atomic operation implementation,
1006-
/// this accesses memory via get_raw instead of
1007-
/// get_raw_mut, due to issues calling get_raw_mut
1008-
/// for atomic loads from read-only memory.
1009-
/// FIXME: is this valid, or should get_raw_mut be used for
1010-
/// atomic-stores/atomic-rmw?
980+
/// Generic atomic operation implementation
1011981
fn validate_atomic_op<A: Debug + Copy>(
1012982
&self,
1013983
place: &MPlaceTy<'tcx, Tag>,
@@ -1023,25 +993,24 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
1023993
let this = self.eval_context_ref();
1024994
if let Some(data_race) = &this.memory.extra.data_race {
1025995
if data_race.multi_threaded.get() {
996+
let size = place.layout.size;
997+
let (alloc_id, base_offset, ptr) = this.memory.ptr_get_alloc(place.ptr)?;
1026998
// Load and log the atomic operation.
1027999
// Note that atomic loads are possible even from read-only allocations, so `get_alloc_extra_mut` is not an option.
1028-
let place_ptr = place.ptr.assert_ptr();
1029-
let size = place.layout.size;
10301000
let alloc_meta =
1031-
&this.memory.get_alloc_extra(place_ptr.alloc_id)?.data_race.as_ref().unwrap();
1001+
&this.memory.get_alloc_extra(alloc_id)?.data_race.as_ref().unwrap();
10321002
log::trace!(
1033-
"Atomic op({}) with ordering {:?} on memory({:?}, offset={}, size={})",
1003+
"Atomic op({}) with ordering {:?} on {:?} (size={})",
10341004
description,
10351005
&atomic,
1036-
place_ptr.alloc_id,
1037-
place_ptr.offset.bytes(),
1006+
ptr,
10381007
size.bytes()
10391008
);
10401009

10411010
// Perform the atomic operation.
10421011
data_race.maybe_perform_sync_operation(|index, mut clocks| {
1043-
for (_, range) in
1044-
alloc_meta.alloc_ranges.borrow_mut().iter_mut(place_ptr.offset, size)
1012+
for (offset, range) in
1013+
alloc_meta.alloc_ranges.borrow_mut().iter_mut(base_offset, size)
10451014
{
10461015
if let Err(DataRace) = op(range, &mut *clocks, index, atomic) {
10471016
mem::drop(clocks);
@@ -1050,8 +1019,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
10501019
range,
10511020
description,
10521021
true,
1053-
place_ptr,
1054-
size,
1022+
Pointer::new(alloc_id, offset),
10551023
)
10561024
.map(|_| true);
10571025
}
@@ -1063,12 +1031,11 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: MiriEvalContextExt<'mir, 'tcx> {
10631031

10641032
// Log changes to atomic memory.
10651033
if log::log_enabled!(log::Level::Trace) {
1066-
for (_, range) in alloc_meta.alloc_ranges.borrow().iter(place_ptr.offset, size)
1034+
for (_offset, range) in alloc_meta.alloc_ranges.borrow().iter(base_offset, size)
10671035
{
10681036
log::trace!(
1069-
"Updated atomic memory({:?}, offset={}, size={}) to {:#?}",
1070-
place.ptr.assert_ptr().alloc_id,
1071-
place_ptr.offset.bytes(),
1037+
"Updated atomic memory({:?}, size={}) to {:#?}",
1038+
ptr,
10721039
size.bytes(),
10731040
range.atomic_ops
10741041
);

src/diagnostics.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ pub fn report_error<'tcx, 'mir>(
134134
let helps = match e.kind() {
135135
Unsupported(UnsupportedOpInfo::NoMirFor(..)) =>
136136
vec![(None, format!("make sure to use a Miri sysroot, which you can prepare with `cargo miri setup`"))],
137-
Unsupported(UnsupportedOpInfo::ReadBytesAsPointer | UnsupportedOpInfo::ThreadLocalStatic(_) | UnsupportedOpInfo::ReadExternStatic(_)) =>
137+
Unsupported(UnsupportedOpInfo::ThreadLocalStatic(_) | UnsupportedOpInfo::ReadExternStatic(_)) =>
138138
panic!("Error should never be raised by Miri: {:?}", e.kind()),
139139
Unsupported(_) =>
140140
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"))],

src/eval.rs

+15-11
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,16 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
164164
// Third argument (`argv`): created from `config.args`.
165165
let argv = {
166166
// Put each argument in memory, collect pointers.
167-
let mut argvs = Vec::<Scalar<Tag>>::new();
167+
let mut argvs = Vec::<Immediate<Tag>>::new();
168168
for arg in config.args.iter() {
169169
// Make space for `0` terminator.
170170
let size = u64::try_from(arg.len()).unwrap().checked_add(1).unwrap();
171171
let arg_type = tcx.mk_array(tcx.types.u8, size);
172172
let arg_place =
173173
ecx.allocate(ecx.layout_of(arg_type)?, MiriMemoryKind::Machine.into())?;
174174
ecx.write_os_str_to_c_str(OsStr::new(arg), arg_place.ptr, size)?;
175-
argvs.push(arg_place.ptr);
175+
ecx.mark_immutable(&*arg_place);
176+
argvs.push(arg_place.to_ref(&ecx));
176177
}
177178
// Make an array with all these pointers, in the Miri memory.
178179
let argvs_layout = ecx.layout_of(
@@ -181,24 +182,26 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
181182
let argvs_place = ecx.allocate(argvs_layout, MiriMemoryKind::Machine.into())?;
182183
for (idx, arg) in argvs.into_iter().enumerate() {
183184
let place = ecx.mplace_field(&argvs_place, idx)?;
184-
ecx.write_scalar(arg, &place.into())?;
185+
ecx.write_immediate(arg, &place.into())?;
185186
}
186-
ecx.memory.mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
187+
ecx.mark_immutable(&*argvs_place);
187188
// A pointer to that place is the 3rd argument for main.
188-
let argv = argvs_place.ptr;
189+
let argv = argvs_place.to_ref(&ecx);
189190
// Store `argc` and `argv` for macOS `_NSGetArg{c,v}`.
190191
{
191192
let argc_place =
192193
ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
193194
ecx.write_scalar(argc, &argc_place.into())?;
194-
ecx.machine.argc = Some(argc_place.ptr);
195+
ecx.mark_immutable(&*argc_place);
196+
ecx.machine.argc = Some(*argc_place);
195197

196198
let argv_place = ecx.allocate(
197199
ecx.layout_of(tcx.mk_imm_ptr(tcx.types.unit))?,
198200
MiriMemoryKind::Machine.into(),
199201
)?;
200-
ecx.write_scalar(argv, &argv_place.into())?;
201-
ecx.machine.argv = Some(argv_place.ptr);
202+
ecx.write_immediate(argv, &argv_place.into())?;
203+
ecx.mark_immutable(&*argv_place);
204+
ecx.machine.argv = Some(*argv_place);
202205
}
203206
// Store command line as UTF-16 for Windows `GetCommandLineW`.
204207
{
@@ -217,12 +220,13 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
217220
let cmd_type = tcx.mk_array(tcx.types.u16, u64::try_from(cmd_utf16.len()).unwrap());
218221
let cmd_place =
219222
ecx.allocate(ecx.layout_of(cmd_type)?, MiriMemoryKind::Machine.into())?;
220-
ecx.machine.cmd_line = Some(cmd_place.ptr);
223+
ecx.machine.cmd_line = Some(*cmd_place);
221224
// Store the UTF-16 string. We just allocated so we know the bounds are fine.
222225
for (idx, &c) in cmd_utf16.iter().enumerate() {
223226
let place = ecx.mplace_field(&cmd_place, idx)?;
224227
ecx.write_scalar(Scalar::from_u16(c), &place.into())?;
225228
}
229+
ecx.mark_immutable(&*cmd_place);
226230
}
227231
argv
228232
};
@@ -233,7 +237,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
233237
ecx.call_function(
234238
start_instance,
235239
Abi::Rust,
236-
&[main_ptr.into(), argc.into(), argv.into()],
240+
&[Scalar::from_pointer(main_ptr, &ecx).into(), argc.into(), argv],
237241
Some(&ret_place.into()),
238242
StackPopCleanup::None { cleanup: true },
239243
)?;
@@ -286,7 +290,7 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
286290
ecx.process_diagnostics(info);
287291
}
288292
let return_code =
289-
ecx.read_scalar(&ret_place.into())?.check_init()?.to_machine_isize(&ecx)?;
293+
ecx.read_scalar(&ret_place.into())?.to_machine_isize(&ecx)?;
290294
Ok(return_code)
291295
})();
292296

0 commit comments

Comments
 (0)