From 6181b29f5d9e81eb16826825da4a679e5da3f57a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 09:52:31 +0100 Subject: [PATCH 01/12] bump Rust --- rust-version | 2 +- src/fn_call.rs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/rust-version b/rust-version index dcd90feeda81..a3718d44da91 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -nightly-2018-11-20 +nightly-2018-11-21 diff --git a/src/fn_call.rs b/src/fn_call.rs index 509db0355e2b..5fc706c04d80 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -469,10 +469,9 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo instance, promoted: None, }; - let const_val = self.const_eval(cid)?; - let value = const_val.unwrap_bits( - self.tcx.tcx, - ty::ParamEnv::empty().and(self.tcx.types.i32)) as i32; + let const_val = self.const_eval_raw(cid)?; + let const_val = self.read_scalar(const_val.into())?; + let value = const_val.to_i32()?; if value == name { result = Some(path_value); break; From 0b7625a0794b9997cdd60dcc5957d8991212b86b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 10:19:00 +0100 Subject: [PATCH 02/12] make sure compile-fail tests would compile if we screw up --- .../stacked_borrows/box_exclusive_violation1.rs | 2 +- .../stacked_borrows/mut_exclusive_violation1.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/compile-fail-fullmir/stacked_borrows/box_exclusive_violation1.rs b/tests/compile-fail-fullmir/stacked_borrows/box_exclusive_violation1.rs index 73631173b932..bd0fec859d8f 100644 --- a/tests/compile-fail-fullmir/stacked_borrows/box_exclusive_violation1.rs +++ b/tests/compile-fail-fullmir/stacked_borrows/box_exclusive_violation1.rs @@ -25,5 +25,5 @@ fn unknown_code_2() { unsafe { } } fn main() { - assert_eq!(demo_mut_advanced_unique(Box::new(0)), 5); + demo_mut_advanced_unique(Box::new(0)); } diff --git a/tests/compile-fail-fullmir/stacked_borrows/mut_exclusive_violation1.rs b/tests/compile-fail-fullmir/stacked_borrows/mut_exclusive_violation1.rs index 255e35b14558..fec699e35bcf 100644 --- a/tests/compile-fail-fullmir/stacked_borrows/mut_exclusive_violation1.rs +++ b/tests/compile-fail-fullmir/stacked_borrows/mut_exclusive_violation1.rs @@ -25,5 +25,5 @@ fn unknown_code_2() { unsafe { } } fn main() { - assert_eq!(demo_mut_advanced_unique(&mut 0), 5); + demo_mut_advanced_unique(&mut 0); } From 984c3368a9d362eb1523d47c9efce20c793cd825 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 13:40:25 +0100 Subject: [PATCH 03/12] remove stabilized feature flag --- src/bin/miri-rustc-tests.rs | 2 +- src/bin/miri.rs | 2 +- src/lib.rs | 2 +- tests/run-pass-fullmir/foreign-fn-linkname.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bin/miri-rustc-tests.rs b/src/bin/miri-rustc-tests.rs index c9ec1011ddbb..6fa9b817ffee 100644 --- a/src/bin/miri-rustc-tests.rs +++ b/src/bin/miri-rustc-tests.rs @@ -1,4 +1,4 @@ -#![feature(rustc_private, extern_crate_item_prelude)] +#![feature(rustc_private)] extern crate miri; extern crate getopts; extern crate rustc; diff --git a/src/bin/miri.rs b/src/bin/miri.rs index bfe631b51f0f..1bbf3c8c4a4a 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -1,4 +1,4 @@ -#![feature(rustc_private, extern_crate_item_prelude)] +#![feature(rustc_private)] extern crate getopts; extern crate miri; diff --git a/src/lib.rs b/src/lib.rs index 418f8f60cecd..8eae6418fa7c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![feature(rustc_private, extern_crate_item_prelude)] +#![feature(rustc_private)] #![cfg_attr(feature = "cargo-clippy", allow(cast_lossless))] diff --git a/tests/run-pass-fullmir/foreign-fn-linkname.rs b/tests/run-pass-fullmir/foreign-fn-linkname.rs index b61cfa84ef75..3dd30fd676fd 100644 --- a/tests/run-pass-fullmir/foreign-fn-linkname.rs +++ b/tests/run-pass-fullmir/foreign-fn-linkname.rs @@ -10,7 +10,7 @@ //ignore-windows: Uses POSIX APIs -#![feature(libc, extern_crate_item_prelude)] +#![feature(libc)] #![allow(unused_extern_crates)] // rustc bug https://github.com/rust-lang/rust/issues/56098 extern crate libc; From ec8cc029c1904bcbbb6ede8fba0251c2aaaf70f8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 15:44:47 +0100 Subject: [PATCH 04/12] on a deref, check that we are not using a mutable ref with a frozen tag --- src/stacked_borrows.rs | 39 +++++-------------- .../static_memory_modification.rs | 3 -- 2 files changed, 9 insertions(+), 33 deletions(-) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 063a544baa65..16cd3e916434 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -151,6 +151,12 @@ impl<'tcx> Stack { /// Returns the index of the item we matched, `None` if it was the frozen one. /// `kind` indicates which kind of reference is being dereferenced. fn deref(&self, bor: Borrow, kind: RefKind) -> Result, String> { + // Exclude unique ref and frozen tag. + match (kind, bor) { + (RefKind::Unique, Borrow::Shr(Some(_))) => + return Err(format!("Encountered mutable reference with frozen tag")), + _ => {} + } // Checks related to freezing match bor { Borrow::Shr(Some(bor_t)) if kind == RefKind::Frozen => { @@ -490,36 +496,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { if let Some(mutability) = mutability { format!("{:?}", mutability) } else { format!("raw") }, place.ptr, place.layout.ty); let ptr = place.ptr.to_ptr()?; - // In principle we should not have to do anything here. However, with transmutes involved, - // it can happen that the tag of `ptr` does not actually match `mutability`, and we - // should adjust for that. - // Notably, the compiler can introduce such transmutes by optimizing away `&[mut]*`. - // That can transmute a raw ptr to a (shared/mut) ref, and a mut ref to a shared one. - match (mutability, ptr.tag) { - (None, _) => { - // No further validation on raw accesses. - return Ok(()); - } - (Some(MutMutable), Borrow::Uniq(_)) | - (Some(MutImmutable), Borrow::Shr(_)) => { - // Expected combinations. Nothing to do. - } - (Some(MutMutable), Borrow::Shr(None)) => { - // Raw transmuted to mut ref. This is something real unsafe code does. - // We cannot reborrow here because we do not want to mutate state on a deref. - } - (Some(MutImmutable), Borrow::Uniq(_)) => { - // A mut got transmuted to shr. Can happen even from compiler transformations: - // `&*x` gets optimized to `x` even when `x` is a `&mut`. - } - (Some(MutMutable), Borrow::Shr(Some(_))) => { - // This is just invalid: A shr got transmuted to a mut. - // If we ever allow this, we have to consider what we do when a turn a - // `Raw`-tagged `&mut` into a raw pointer pointing to a frozen location. - // We probably do not want to allow that, but we have to allow - // turning a `Raw`-tagged `&` into a raw ptr to a frozen location. - return err!(MachineError(format!("Encountered mutable reference with frozen tag {:?}", ptr.tag))) - } + if mutability.is_none() { + // No further checks on raw derefs -- only the access itself will be checked. + return Ok(()); } // Get the allocation diff --git a/tests/compile-fail-fullmir/stacked_borrows/static_memory_modification.rs b/tests/compile-fail-fullmir/stacked_borrows/static_memory_modification.rs index 5c605eff6784..c092cbfe5098 100644 --- a/tests/compile-fail-fullmir/stacked_borrows/static_memory_modification.rs +++ b/tests/compile-fail-fullmir/stacked_borrows/static_memory_modification.rs @@ -1,6 +1,3 @@ -// FIXME still considering whether we are okay with this not being an error -// ignore-test - static X: usize = 5; #[allow(mutable_transmutes)] From 41f89beb3f03cb7cdbfbd754ac2eee9e1f153f18 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 16:01:39 +0100 Subject: [PATCH 05/12] if let --- src/stacked_borrows.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 16cd3e916434..21addbfbf789 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -151,11 +151,9 @@ impl<'tcx> Stack { /// Returns the index of the item we matched, `None` if it was the frozen one. /// `kind` indicates which kind of reference is being dereferenced. fn deref(&self, bor: Borrow, kind: RefKind) -> Result, String> { - // Exclude unique ref and frozen tag. - match (kind, bor) { - (RefKind::Unique, Borrow::Shr(Some(_))) => - return Err(format!("Encountered mutable reference with frozen tag")), - _ => {} + // Exclude unique ref with frozen tag. + if let (RefKind::Unique, Borrow::Shr(Some(_))) = (kind, bor) { + return Err(format!("Encountered mutable reference with frozen tag")); } // Checks related to freezing match bor { From 694d2490f1f10cc46aff0452874ade547e4936b9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 16:02:38 +0100 Subject: [PATCH 06/12] slightly more verbose error msg --- src/stacked_borrows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index 21addbfbf789..f26ef40ea9f8 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -153,7 +153,7 @@ impl<'tcx> Stack { fn deref(&self, bor: Borrow, kind: RefKind) -> Result, String> { // Exclude unique ref with frozen tag. if let (RefKind::Unique, Borrow::Shr(Some(_))) = (kind, bor) { - return Err(format!("Encountered mutable reference with frozen tag")); + return Err(format!("Encountered mutable reference with frozen tag ({:?})", bor)); } // Checks related to freezing match bor { From 04794c4c2ac20e66043c235b45c19f6759f48dc9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 21 Nov 2018 16:08:46 +0100 Subject: [PATCH 07/12] test that we support partial invalidation of mutable references --- tests/run-pass/stacked-borrows.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/run-pass/stacked-borrows.rs b/tests/run-pass/stacked-borrows.rs index 72f26763be14..93bdf5ffbf32 100644 --- a/tests/run-pass/stacked-borrows.rs +++ b/tests/run-pass/stacked-borrows.rs @@ -7,6 +7,7 @@ fn main() { mut_shr_raw(); mut_raw_then_mut_shr(); mut_raw_mut(); + partially_invalidate_mut(); } // Deref a raw ptr to access a field of a large struct, where the field @@ -97,3 +98,12 @@ fn mut_raw_mut() { } assert_eq!(x, 4); } + +fn partially_invalidate_mut() { + let data = &mut (0u8, 0u8); + let reborrow = &mut *data as *mut (u8, u8); + let shard = unsafe { &mut (*reborrow).0 }; + data.1 += 1; // the deref overlaps with `shard`, but that is okay; the access does not overlap. + *shard += 1; // so we can still use `shard`. + assert_eq!(*data, (1, 1)); +} From 1703d31eacccc80beedc8bc4b393df0027531e60 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 22 Nov 2018 08:21:26 +0100 Subject: [PATCH 08/12] bump rust --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index a3718d44da91..23a0fa102b00 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -nightly-2018-11-21 +nightly-2018-11-22 From 68ba6cdbaa758484f56d8c7f5628ccfcb772756e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 23 Nov 2018 09:46:51 +0100 Subject: [PATCH 09/12] fix for new Align type --- src/fn_call.rs | 16 ++++++++-------- src/helpers.rs | 5 +++-- src/intrinsic.rs | 8 ++++---- src/lib.rs | 12 ++++++------ src/operator.rs | 8 ++++---- 5 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index 5fc706c04d80..e64d4f695487 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -124,7 +124,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo if size == 0 { self.write_null(dest)?; } else { - let align = self.tcx.data_layout.pointer_align; + let align = self.tcx.data_layout.pointer_align.abi; let ptr = self.memory_mut().allocate(Size::from_bytes(size), align, MiriMemoryKind::C.into())?; self.write_scalar(Scalar::Ptr(ptr.with_default_tag()), dest)?; } @@ -153,7 +153,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo let ptr = self.memory_mut() .allocate( Size::from_bytes(size), - Align::from_bytes(align, align).unwrap(), + Align::from_bytes(align).unwrap(), MiriMemoryKind::Rust.into() )? .with_default_tag(); @@ -171,7 +171,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo let ptr = self.memory_mut() .allocate( Size::from_bytes(size), - Align::from_bytes(align, align).unwrap(), + Align::from_bytes(align).unwrap(), MiriMemoryKind::Rust.into() )? .with_default_tag(); @@ -190,7 +190,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo } self.memory_mut().deallocate( ptr, - Some((Size::from_bytes(old_size), Align::from_bytes(align, align).unwrap())), + Some((Size::from_bytes(old_size), Align::from_bytes(align).unwrap())), MiriMemoryKind::Rust.into(), )?; } @@ -208,9 +208,9 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo let new_ptr = self.memory_mut().reallocate( ptr, Size::from_bytes(old_size), - Align::from_bytes(align, align).unwrap(), + Align::from_bytes(align).unwrap(), Size::from_bytes(new_size), - Align::from_bytes(align, align).unwrap(), + Align::from_bytes(align).unwrap(), MiriMemoryKind::Rust.into(), )?; self.write_scalar(Scalar::Ptr(new_ptr.with_default_tag()), dest)?; @@ -394,7 +394,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo // +1 for the null terminator let value_copy = self.memory_mut().allocate( Size::from_bytes((value.len() + 1) as u64), - Align::from_bytes(1, 1).unwrap(), + Align::from_bytes(1).unwrap(), MiriMemoryKind::Env.into(), )?.with_default_tag(); self.memory_mut().write_bytes(value_copy.into(), &value)?; @@ -513,7 +513,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo } self.memory_mut().write_scalar( key_ptr, - key_layout.align, + key_layout.align.abi, Scalar::from_uint(key, key_layout.size).into(), key_layout.size, )?; diff --git a/src/helpers.rs b/src/helpers.rs index 85329ddcf17f..2b1a28fe9e0d 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -130,9 +130,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super: unsafe_cell_action: |place| { trace!("unsafe_cell_action on {:?}", place.ptr); // We need a size to go on. - let (unsafe_cell_size, _) = self.size_and_align_of_mplace(place)? + let unsafe_cell_size = self.size_and_align_of_mplace(place)? + .map(|(size, _)| size) // for extern types, just cover what we can - .unwrap_or_else(|| place.layout.size_and_align()); + .unwrap_or_else(|| place.layout.size); // Now handle this `UnsafeCell`, unless it is empty. if unsafe_cell_size != Size::ZERO { unsafe_cell_action(place.ptr, unsafe_cell_size) diff --git a/src/intrinsic.rs b/src/intrinsic.rs index 6d5ac8d88bae..66dab00e0975 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -152,7 +152,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' let elem_layout = self.layout_of(elem_ty)?; let elem_size = elem_layout.size.bytes(); let count = self.read_scalar(args[2])?.to_usize(self)?; - let elem_align = elem_layout.align; + let elem_align = elem_layout.align.abi; // erase tags: this is a raw ptr operation let src = self.read_scalar(args[0])?.not_undef()?; let dest = self.read_scalar(args[1])?.not_undef()?; @@ -272,7 +272,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' "pref_align_of" => { let ty = substs.type_at(0); let layout = self.layout_of(ty)?; - let align = layout.align.pref(); + let align = layout.align.pref.bytes(); let ptr_size = self.pointer_size(); let align_val = Scalar::from_uint(align as u128, ptr_size); self.write_scalar(align_val, dest)?; @@ -364,7 +364,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' .expect("size_of_val called on extern type"); let ptr_size = self.pointer_size(); self.write_scalar( - Scalar::from_uint(align.abi(), ptr_size), + Scalar::from_uint(align.bytes(), ptr_size), dest, )?; } @@ -438,7 +438,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' let val_byte = self.read_scalar(args[1])?.to_u8()?; let ptr = self.read_scalar(args[0])?.not_undef()?; let count = self.read_scalar(args[2])?.to_usize(self)?; - self.memory().check_align(ptr, ty_layout.align)?; + self.memory().check_align(ptr, ty_layout.align.abi)?; self.memory_mut().write_repeat(ptr, val_byte, ty_layout.size * count)?; } diff --git a/src/lib.rs b/src/lib.rs index 8eae6418fa7c..10a1405b2a62 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -397,7 +397,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { // Second argument: align let arg = ecx.eval_place(&mir::Place::Local(args.next().unwrap()))?; - let align = layout.align.abi(); + let align = layout.align.abi.bytes(); ecx.write_scalar(Scalar::from_uint(align, arg.layout.size), arg)?; // No more arguments @@ -419,7 +419,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { "__cxa_thread_atexit_impl" => { // This should be all-zero, pointer-sized let data = vec![0; tcx.data_layout.pointer_size.bytes() as usize]; - Allocation::from_bytes(&data[..], tcx.data_layout.pointer_align) + Allocation::from_bytes(&data[..], tcx.data_layout.pointer_align.abi) } _ => return err!(Unimplemented( format!("can't access foreign static: {}", link_name), @@ -458,9 +458,9 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { place: MPlaceTy<'tcx, Borrow>, mutability: Option, ) -> EvalResult<'tcx, Scalar> { - let (size, _) = ecx.size_and_align_of_mplace(place)? + let size = ecx.size_and_align_of_mplace(place)?.map(|(size, _)| size) // for extern types, just cover what we can - .unwrap_or_else(|| place.layout.size_and_align()); + .unwrap_or_else(|| place.layout.size); if !ecx.tcx.sess.opts.debugging_opts.mir_emit_retag || !Self::enforce_validity(ecx) || size == Size::ZERO { @@ -498,9 +498,9 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> { // This is deliberately NOT `deref_operand` as we do not want `tag_dereference` // to be called! That would kill the original tag if we got a raw ptr. let place = ecx.ref_to_mplace(ecx.read_immediate(ptr)?)?; - let (size, _) = ecx.size_and_align_of_mplace(place)? + let size = ecx.size_and_align_of_mplace(place)?.map(|(size, _)| size) // for extern types, just cover what we can - .unwrap_or_else(|| place.layout.size_and_align()); + .unwrap_or_else(|| place.layout.size); if !ecx.tcx.sess.opts.debugging_opts.mir_emit_retag || !ecx.machine.validate || size == Size::ZERO { diff --git a/src/operator.rs b/src/operator.rs index be05c2259957..2f3a0de999ad 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -166,12 +166,12 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' let (alloc_size, alloc_align) = self.memory().get_size_and_align(ptr.alloc_id); // Case II: Alignment gives it away - if ptr.offset.bytes() % alloc_align.abi() == 0 { + if ptr.offset.bytes() % alloc_align.bytes() == 0 { // The offset maintains the allocation alignment, so we know `base+offset` // is aligned by `alloc_align`. // FIXME: We could be even more general, e.g. offset 2 into a 4-aligned // allocation cannot equal 3. - if bits % alloc_align.abi() != 0 { + if bits % alloc_align.bytes() != 0 { // The integer is *not* aligned. So they cannot be equal. return Ok(false); } @@ -226,7 +226,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' map_to_primval(left.overflowing_offset(Size::from_bytes(right as u64), self)), BitAnd if !signed => { - let ptr_base_align = self.memory().get(left.alloc_id)?.align.abi(); + let ptr_base_align = self.memory().get(left.alloc_id)?.align.bytes(); let base_mask = { // FIXME: Use interpret::truncate, once that takes a Size instead of a Layout let shift = 128 - self.memory().pointer_size().bits(); @@ -259,7 +259,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' Rem if !signed => { // Doing modulo a divisor of the alignment is allowed. // (Intuition: Modulo a divisor leaks less information.) - let ptr_base_align = self.memory().get(left.alloc_id)?.align.abi(); + let ptr_base_align = self.memory().get(left.alloc_id)?.align.bytes(); let right = right as u64; let ptr_size = self.memory().pointer_size().bytes() as u8; if right == 1 { From 32e93ed7762e5aa1a721636096848fc3c7bc7218 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Tue, 13 Nov 2018 17:19:42 +0100 Subject: [PATCH 10/12] Update to Memory -> Allocation method move --- src/fn_call.rs | 40 +++++++++++++++++++++++++--------------- src/intrinsic.rs | 21 +++++++++++++++++---- src/operator.rs | 18 +++++++++++------- src/stacked_borrows.rs | 10 +++++----- 4 files changed, 58 insertions(+), 31 deletions(-) diff --git a/src/fn_call.rs b/src/fn_call.rs index e64d4f695487..e9d3255a5b32 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -114,6 +114,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo None => self.tcx.item_name(def_id).as_str(), }; + let tcx = &{self.tcx.tcx}; + // All these functions take raw pointers, so if we access memory directly // (as opposed to through a place), we have to remember to erase any tag // that might still hang around! @@ -175,7 +177,9 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo MiriMemoryKind::Rust.into() )? .with_default_tag(); - self.memory_mut().write_repeat(ptr.into(), 0, Size::from_bytes(size))?; + self.memory_mut() + .get_mut(ptr.alloc_id)? + .write_repeat(tcx, ptr, 0, Size::from_bytes(size))?; self.write_scalar(Scalar::Ptr(ptr), dest)?; } "__rust_dealloc" => { @@ -239,7 +243,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo "dlsym" => { let _handle = self.read_scalar(args[0])?; let symbol = self.read_scalar(args[1])?.to_ptr()?; - let symbol_name = self.memory().read_c_str(symbol)?; + let symbol_name = self.memory().get(symbol.alloc_id)?.read_c_str(tcx, symbol)?; let err = format!("bad c unicode symbol: {:?}", symbol_name); let symbol_name = ::std::str::from_utf8(symbol_name).unwrap_or(&err); return err!(Unimplemented(format!( @@ -346,7 +350,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo "getenv" => { let result = { let name_ptr = self.read_scalar(args[0])?.to_ptr()?; - let name = self.memory().read_c_str(name_ptr)?; + let name = self.memory().get(name_ptr.alloc_id)?.read_c_str(tcx, name_ptr)?; match self.machine.env_vars.get(name) { Some(&var) => Scalar::Ptr(var), None => Scalar::ptr_null(&*self.tcx), @@ -360,8 +364,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo { let name_ptr = self.read_scalar(args[0])?.not_undef()?; if !name_ptr.is_null_ptr(self) { - let name = self.memory().read_c_str(name_ptr.to_ptr()? - )?.to_owned(); + let name_ptr = name_ptr.to_ptr()?; + let name = self.memory().get(name_ptr.alloc_id)?.read_c_str(tcx, name_ptr)?.to_owned(); if !name.is_empty() && !name.contains(&b'=') { success = Some(self.machine.env_vars.remove(&name)); } @@ -382,9 +386,10 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo { let name_ptr = self.read_scalar(args[0])?.not_undef()?; let value_ptr = self.read_scalar(args[1])?.to_ptr()?; - let value = self.memory().read_c_str(value_ptr)?; + let value = self.memory().get(value_ptr.alloc_id)?.read_c_str(tcx, value_ptr)?; if !name_ptr.is_null_ptr(self) { - let name = self.memory().read_c_str(name_ptr.to_ptr()?)?; + let name_ptr = name_ptr.to_ptr()?; + let name = self.memory().get(name_ptr.alloc_id)?.read_c_str(tcx, name_ptr)?; if !name.is_empty() && !name.contains(&b'=') { new = Some((name.to_owned(), value.to_owned())); } @@ -397,9 +402,12 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo Align::from_bytes(1).unwrap(), MiriMemoryKind::Env.into(), )?.with_default_tag(); - self.memory_mut().write_bytes(value_copy.into(), &value)?; - let trailing_zero_ptr = value_copy.offset(Size::from_bytes(value.len() as u64), self)?.into(); - self.memory_mut().write_bytes(trailing_zero_ptr, &[0])?; + { + let alloc = self.memory_mut().get_mut(value_copy.alloc_id)?; + alloc.write_bytes(tcx, value_copy, &value)?; + let trailing_zero_ptr = value_copy.offset(Size::from_bytes(value.len() as u64), tcx)?; + alloc.write_bytes(tcx, trailing_zero_ptr, &[0])?; + } if let Some(var) = self.machine.env_vars.insert( name.to_owned(), value_copy, @@ -444,7 +452,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo "strlen" => { let ptr = self.read_scalar(args[0])?.to_ptr()?; - let n = self.memory().read_c_str(ptr)?.len(); + let n = self.memory().get(ptr.alloc_id)?.read_c_str(tcx, ptr)?.len(); self.write_scalar(Scalar::from_uint(n as u64, dest.layout.size), dest)?; } @@ -507,13 +515,15 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo let key_layout = self.layout_of(key_type)?; // Create key and write it into the memory where key_ptr wants it - let key = self.machine.tls.create_tls_key(dtor, &*self.tcx) as u128; + let key = self.machine.tls.create_tls_key(dtor, tcx) as u128; if key_layout.size.bits() < 128 && key >= (1u128 << key_layout.size.bits() as u128) { return err!(OutOfTls); } - self.memory_mut().write_scalar( + + self.memory().check_align(key_ptr.into(), key_layout.align.abi)?; + self.memory_mut().get_mut(key_ptr.alloc_id)?.write_scalar( + tcx, key_ptr, - key_layout.align.abi, Scalar::from_uint(key, key_layout.size).into(), key_layout.size, )?; @@ -610,7 +620,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo // This just creates a key; Windows does not natively support TLS dtors. // Create key and return it - let key = self.machine.tls.create_tls_key(None, &*self.tcx) as u128; + let key = self.machine.tls.create_tls_key(None, tcx) as u128; // Figure out how large a TLS key actually is. This is c::DWORD. if dest.layout.size.bits() < 128 && key >= (1u128 << dest.layout.size.bits() as u128) { diff --git a/src/intrinsic.rs b/src/intrinsic.rs index 66dab00e0975..c9b16525e566 100644 --- a/src/intrinsic.rs +++ b/src/intrinsic.rs @@ -28,7 +28,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' if self.emulate_intrinsic(instance, args, dest)? { return Ok(()); } - + let tcx = &{self.tcx.tcx}; let substs = instance.substs; // All these intrinsics take raw pointers, so if we access memory directly @@ -248,6 +248,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // FIXME: We do not properly validate in case of ZSTs and when doing it in memory! // However, this only affects direct calls of the intrinsic; calls to the stable // functions wrapping them do get their validation. + // FIXME: should we check that the destination pointer is aligned even for ZSTs? if !dest.layout.is_zst() { // nothing to do for ZST match dest.layout.abi { layout::Abi::Scalar(ref s) => { @@ -263,7 +264,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // Do it in memory let mplace = self.force_allocation(dest)?; assert!(mplace.meta.is_none()); - self.memory_mut().write_repeat(mplace.ptr, 0, dest.layout.size)?; + // not a zst, must be valid pointer + let ptr = mplace.ptr.to_ptr()?; + self.memory_mut().get_mut(ptr.alloc_id)?.write_repeat(tcx, ptr, 0, dest.layout.size)?; } } } @@ -412,6 +415,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // FIXME: We do not properly validate in case of ZSTs and when doing it in memory! // However, this only affects direct calls of the intrinsic; calls to the stable // functions wrapping them do get their validation. + // FIXME: should we check alignment for ZSTs? if !dest.layout.is_zst() { // nothing to do for ZST match dest.layout.abi { layout::Abi::Scalar(..) => { @@ -426,7 +430,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // Do it in memory let mplace = self.force_allocation(dest)?; assert!(mplace.meta.is_none()); - self.memory_mut().mark_definedness(mplace.ptr.to_ptr()?, dest.layout.size, false)?; + let ptr = mplace.ptr.to_ptr()?; + self.memory_mut() + .get_mut(ptr.alloc_id)? + .mark_definedness(ptr, dest.layout.size, false)?; } } } @@ -439,7 +446,13 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' let ptr = self.read_scalar(args[0])?.not_undef()?; let count = self.read_scalar(args[2])?.to_usize(self)?; self.memory().check_align(ptr, ty_layout.align.abi)?; - self.memory_mut().write_repeat(ptr, val_byte, ty_layout.size * count)?; + let byte_count = ty_layout.size * count; + if byte_count.bytes() != 0 { + let ptr = ptr.to_ptr()?; + self.memory_mut() + .get_mut(ptr.alloc_id)? + .write_repeat(tcx, ptr, val_byte, byte_count)?; + } } name => return err!(Unimplemented(format!("unimplemented intrinsic: {}", name))), diff --git a/src/operator.rs b/src/operator.rs index 2f3a0de999ad..e1ccdf91995e 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -142,10 +142,12 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // allocations sit right next to each other. The C/C++ standards are // somewhat fuzzy about this case, so I think for now this check is // "good enough". - // We require liveness, as dead allocations can of course overlap. - self.memory().check_bounds_ptr(left, InboundsCheck::Live)?; - self.memory().check_bounds_ptr(right, InboundsCheck::Live)?; - // Two live in-bounds pointers, we can compare across allocations + // Dead allocations in miri cannot overlap with live allocations, but + // on read hardware this can easily happen. Thus for comparisons we require + // both pointers to be live. + self.memory().get(left.alloc_id)?.check_bounds_ptr(left)?; + self.memory().get(right.alloc_id)?.check_bounds_ptr(right)?; + // Two in-bounds pointers, we can compare across allocations left == right } } @@ -158,7 +160,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' // Case I: Comparing with NULL if bits == 0 { // Test if the ptr is in-bounds. Then it cannot be NULL. - if self.memory().check_bounds_ptr(ptr, InboundsCheck::MaybeDead).is_ok() { + // Even dangling pointers cannot be NULL. + if self.memory().check_bounds_ptr_maybe_dead(ptr).is_ok() { return Ok(false); } } @@ -298,9 +301,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, ' if let Scalar::Ptr(ptr) = ptr { // Both old and new pointer must be in-bounds of a *live* allocation. // (Of the same allocation, but that part is trivial with our representation.) - self.memory().check_bounds_ptr(ptr, InboundsCheck::Live)?; + let alloc = self.memory().get(ptr.alloc_id)?; + alloc.check_bounds_ptr(ptr)?; let ptr = ptr.signed_offset(offset, self)?; - self.memory().check_bounds_ptr(ptr, InboundsCheck::Live)?; + alloc.check_bounds_ptr(ptr)?; Ok(Scalar::Ptr(ptr)) } else { // An integer pointer. They can only be offset by 0, and we pretend there diff --git a/src/stacked_borrows.rs b/src/stacked_borrows.rs index f26ef40ea9f8..f292d083637a 100644 --- a/src/stacked_borrows.rs +++ b/src/stacked_borrows.rs @@ -5,7 +5,7 @@ use rustc::hir::{Mutability, MutMutable, MutImmutable}; use crate::{ EvalResult, EvalErrorKind, MiriEvalContext, HelpersEvalContextExt, Evaluator, MutValueVisitor, - MemoryKind, MiriMemoryKind, RangeMap, AllocId, Allocation, AllocationExtra, InboundsCheck, + MemoryKind, MiriMemoryKind, RangeMap, AllocId, Allocation, AllocationExtra, Pointer, MemPlace, Scalar, Immediate, ImmTy, PlaceTy, MPlaceTy, }; @@ -500,8 +500,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { } // Get the allocation - self.memory().check_bounds(ptr, size, InboundsCheck::Live)?; - let alloc = self.memory().get(ptr.alloc_id).expect("We checked that the ptr is fine!"); + let alloc = self.memory().get(ptr.alloc_id)?; + alloc.check_bounds(self, ptr, size)?; // If we got here, we do some checking, *but* we leave the tag unchanged. if let Borrow::Shr(Some(_)) = ptr.tag { assert_eq!(mutability, Some(MutImmutable)); @@ -543,8 +543,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for MiriEvalContext<'a, 'mir, 'tcx> { ptr, place.layout.ty, new_bor); // Get the allocation. It might not be mutable, so we cannot use `get_mut`. - self.memory().check_bounds(ptr, size, InboundsCheck::Live)?; - let alloc = self.memory().get(ptr.alloc_id).expect("We checked that the ptr is fine!"); + let alloc = self.memory().get(ptr.alloc_id)?; + alloc.check_bounds(self, ptr, size)?; // Update the stacks. if let Borrow::Shr(Some(_)) = new_bor { // Reference that cares about freezing. We need a frozen-sensitive reborrow. From 82d4146a6c3fec5d0f87e3df6423f123dec784f6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 24 Nov 2018 11:58:28 +0100 Subject: [PATCH 11/12] bump Rust --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 23a0fa102b00..5bdac314d169 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -nightly-2018-11-22 +nightly-2018-11-24 From ac9649b7c0fc0f106d4dc93ab3c4d9ff25228f32 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 26 Nov 2018 08:54:24 +0100 Subject: [PATCH 12/12] bump Rust version --- rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-version b/rust-version index 5bdac314d169..0268b07ac411 100644 --- a/rust-version +++ b/rust-version @@ -1 +1 @@ -nightly-2018-11-24 +nightly-2018-11-26