Skip to content

Commit

Permalink
Auto merge of #830 - RalfJung:check-place, r=RalfJung
Browse files Browse the repository at this point in the history
Fix validation and reborrowing of integer pointers

Depends on rust-lang/rust#62441
  • Loading branch information
bors committed Jul 11, 2019
2 parents 0b9434c + 7dd0dd3 commit fe9056e
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 38 deletions.
2 changes: 1 addition & 1 deletion rust-version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0b680cfce544ff9a59d720020e397c4bf3346650
97b1128589fdaa786a7cf65c5a6ff7ed37a1d2f3
3 changes: 1 addition & 2 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
ecx.machine.argc = Some(argc_place.ptr.to_ptr()?);
}

// FIXME: extract main source file path.
// Third argument (`argv`): created from `config.args`.
let dest = ecx.eval_place(&mir::Place::Base(mir::PlaceBase::Local(args.next().unwrap())))?;
// For Windows, construct a command string with all the aguments.
Expand Down Expand Up @@ -136,7 +135,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
let place = ecx.mplace_field(argvs_place, idx as u64)?;
ecx.write_scalar(Scalar::Ptr(arg), place.into())?;
}
ecx.memory_mut().mark_immutable(argvs_place.to_ptr()?.alloc_id)?;
ecx.memory_mut().mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
// Write a pointer to that place as the argument.
let argv = argvs_place.ptr;
ecx.write_scalar(argv, dest)?;
Expand Down
24 changes: 10 additions & 14 deletions src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,36 +116,32 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
.map(|(size, _)| size)
.unwrap_or_else(|| place.layout.size)
);
assert!(size.bytes() > 0);
// Store how far we proceeded into the place so far. Everything to the left of
// this offset has already been handled, in the sense that the frozen parts
// have had `action` called on them.
let mut end_ptr = place.ptr;
let mut end_ptr = place.ptr.assert_ptr();
// Called when we detected an `UnsafeCell` at the given offset and size.
// Calls `action` and advances `end_ptr`.
let mut unsafe_cell_action = |unsafe_cell_ptr: Scalar<Tag>, unsafe_cell_size: Size| {
if unsafe_cell_size != Size::ZERO {
debug_assert_eq!(unsafe_cell_ptr.to_ptr().unwrap().alloc_id,
end_ptr.to_ptr().unwrap().alloc_id);
debug_assert_eq!(unsafe_cell_ptr.to_ptr().unwrap().tag,
end_ptr.to_ptr().unwrap().tag);
}
let unsafe_cell_ptr = unsafe_cell_ptr.assert_ptr();
debug_assert_eq!(unsafe_cell_ptr.alloc_id, end_ptr.alloc_id);
debug_assert_eq!(unsafe_cell_ptr.tag, end_ptr.tag);
// We assume that we are given the fields in increasing offset order,
// and nothing else changes.
let unsafe_cell_offset = unsafe_cell_ptr.get_ptr_offset(this);
let end_offset = end_ptr.get_ptr_offset(this);
let unsafe_cell_offset = unsafe_cell_ptr.offset;
let end_offset = end_ptr.offset;
assert!(unsafe_cell_offset >= end_offset);
let frozen_size = unsafe_cell_offset - end_offset;
// Everything between the end_ptr and this `UnsafeCell` is frozen.
if frozen_size != Size::ZERO {
action(end_ptr.to_ptr()?, frozen_size, /*frozen*/true)?;
action(end_ptr, frozen_size, /*frozen*/true)?;
}
// This `UnsafeCell` is NOT frozen.
if unsafe_cell_size != Size::ZERO {
action(unsafe_cell_ptr.to_ptr()?, unsafe_cell_size, /*frozen*/false)?;
action(unsafe_cell_ptr, unsafe_cell_size, /*frozen*/false)?;
}
// Update end end_ptr.
end_ptr = unsafe_cell_ptr.ptr_wrapping_offset(unsafe_cell_size, this);
end_ptr = unsafe_cell_ptr.wrapping_offset(unsafe_cell_size, this);
// Done
Ok(())
};
Expand Down Expand Up @@ -234,7 +230,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
layout::FieldPlacement::Arbitrary { .. } => {
// Gather the subplaces and sort them before visiting.
let mut places = fields.collect::<InterpResult<'tcx, Vec<MPlaceTy<'tcx, Tag>>>>()?;
places.sort_by_key(|place| place.ptr.get_ptr_offset(self.ecx()));
places.sort_by_key(|place| place.ptr.assert_ptr().offset);
self.walk_aggregate(place, places.into_iter().map(Ok))
}
layout::FieldPlacement::Union { .. } => {
Expand Down
8 changes: 1 addition & 7 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,13 +842,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
},
"GetSystemInfo" => {
let system_info = this.deref_operand(args[0])?;
let (system_info_ptr, align) = system_info.to_scalar_ptr_align();
let system_info_ptr = this.memory()
.check_ptr_access(
system_info_ptr,
system_info.layout.size,
align,
)?
let system_info_ptr = this.check_mplace_access(system_info, None)?
.expect("cannot be a ZST");
// Initialize with `0`.
this.memory_mut().get_mut(system_info_ptr.alloc_id)?
Expand Down
22 changes: 13 additions & 9 deletions src/shims/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
let elem_size = elem_layout.size.bytes();
let count = this.read_scalar(args[2])?.to_usize(this)?;
let elem_align = elem_layout.align.abi;
// erase tags: this is a raw ptr operation

let size = Size::from_bytes(count * elem_size);
let src = this.read_scalar(args[0])?.not_undef()?;
let src = this.memory().check_ptr_access(src, size, elem_align)?;
let dest = this.read_scalar(args[1])?.not_undef()?;
this.memory_mut().copy(
src,
elem_align,
dest,
elem_align,
Size::from_bytes(count * elem_size),
intrinsic_name.ends_with("_nonoverlapping"),
)?;
let dest = this.memory().check_ptr_access(dest, size, elem_align)?;

if let (Some(src), Some(dest)) = (src, dest) {
this.memory_mut().copy(
src,
dest,
size,
intrinsic_name.ends_with("_nonoverlapping"),
)?;
}
}

"discriminant_value" => {
Expand Down
1 change: 1 addition & 0 deletions src/stacked_borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
// Nothing to do for ZSTs.
return Ok(*val);
}
let place = this.force_mplace_ptr(place)?;

// Compute new borrow.
let new_tag = match kind {
Expand Down
3 changes: 1 addition & 2 deletions test-cargo-miri/run-test.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ def test_cargo_miri_run():
)

def test_cargo_miri_test():
# FIXME: enable validation again, once that no longer conflicts with intptrcast
test("cargo miri test",
cargo_miri("test") + ["--", "-Zmiri-seed=feed", "-Zmiri-disable-validation"],
cargo_miri("test") + ["--", "-Zmiri-seed=feed"],
"test.stdout.ref", "test.stderr.ref"
)
test("cargo miri test (with filter)",
Expand Down
5 changes: 5 additions & 0 deletions tests/run-pass/unique-send.rs → tests/run-pass/mpsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,9 @@ pub fn main() {
tx.send(box 100).unwrap();
let v = rx.recv().unwrap();
assert_eq!(v, box 100);

tx.send(box 101).unwrap();
tx.send(box 102).unwrap();
assert_eq!(rx.recv().unwrap(), box 101);
assert_eq!(rx.recv().unwrap(), box 102);
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// FIXME move this to run-pass, it should work with intptrcast.
use std::mem;
use std::ptr;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// FIXME move this to run-pass, it should work with intptrcast.

fn f() -> i32 { 42 }

fn main() {
Expand Down

0 comments on commit fe9056e

Please sign in to comment.