From 2726a91ccacb6d3e2936ba3f6d12a85fb1f24e4f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 25 May 2019 13:48:56 +0200 Subject: [PATCH 1/7] cache tag for vtable; fn_alloc's don't have a stack nor tag --- src/librustc_mir/interpret/cast.rs | 4 ++-- src/librustc_mir/interpret/eval_context.rs | 7 +++++-- src/librustc_mir/interpret/memory.rs | 5 +++-- src/librustc_mir/interpret/traits.rs | 8 ++++---- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index 76b11ac2fe646..f1e23655741fc 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -86,7 +86,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> def_id, substs, ).ok_or_else(|| InterpError::TooGeneric.into()); - let fn_ptr = self.memory.create_fn_alloc(instance?).with_default_tag(); + let fn_ptr = self.memory.create_fn_alloc(instance?); self.write_scalar(Scalar::Ptr(fn_ptr.into()), dest)?; } _ => bug!("reify fn pointer on {:?}", src.layout.ty), @@ -115,7 +115,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> substs, ty::ClosureKind::FnOnce, ); - let fn_ptr = self.memory.create_fn_alloc(instance).with_default_tag(); + let fn_ptr = self.memory.create_fn_alloc(instance); let val = Immediate::Scalar(Scalar::Ptr(fn_ptr.into()).into()); self.write_immediate(val, dest)?; } diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index d3cbd2bcc03a5..1ed04c013eee3 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -15,7 +15,7 @@ use rustc::ty::query::TyCtxtAt; use rustc_data_structures::indexed_vec::IndexVec; use rustc::mir::interpret::{ ErrorHandled, - GlobalId, Scalar, FrameInfo, AllocId, + GlobalId, Scalar, Pointer, FrameInfo, AllocId, EvalResult, InterpError, truncate, sign_extend, }; @@ -43,7 +43,10 @@ pub struct InterpretCx<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'a, 'mir, 'tcx>> { pub(crate) stack: Vec>, /// A cache for deduplicating vtables - pub(super) vtables: FxHashMap<(Ty<'tcx>, Option>), AllocId>, + pub(super) vtables: FxHashMap< + (Ty<'tcx>, Option>), + Pointer + >, } /// A stack frame. diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index e089b9a148fee..3e145975774bc 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -108,8 +108,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } - pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { - Pointer::from(self.tcx.alloc_map.lock().create_fn_alloc(instance)) + pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { + // Default tag is okay because anyway you cannot access memory with this. + Pointer::from(self.tcx.alloc_map.lock().create_fn_alloc(instance)).with_default_tag() } pub fn allocate_static_bytes(&mut self, bytes: &[u8]) -> Pointer { diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 4eb79cf56fcf2..208bba60bf2f2 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -25,7 +25,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> // always use the same vtable for the same (Type, Trait) combination. // That's not what happens in rustc, but emulating per-crate deduplication // does not sound like it actually makes anything any better. - return Ok(Pointer::from(vtable).with_default_tag()); + return Ok(vtable); } let methods = if let Some(poly_trait_ref) = poly_trait_ref { @@ -56,7 +56,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> let tcx = &*self.tcx; let drop = crate::monomorphize::resolve_drop_in_place(*tcx, ty); - let drop = self.memory.create_fn_alloc(drop).with_default_tag(); + let drop = self.memory.create_fn_alloc(drop); // no need to do any alignment checks on the memory accesses below, because we know the // allocation is correctly aligned as we created it above. Also we're only offsetting by // multiples of `ptr_align`, which means that it will stay aligned to `ptr_align`. @@ -83,7 +83,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> def_id, substs, ).ok_or_else(|| InterpError::TooGeneric)?; - let fn_ptr = self.memory.create_fn_alloc(instance).with_default_tag(); + let fn_ptr = self.memory.create_fn_alloc(instance); let method_ptr = vtable.offset(ptr_size * (3 + i as u64), self)?; self.memory .get_mut(method_ptr.alloc_id)? @@ -92,7 +92,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> } self.memory.mark_immutable(vtable.alloc_id)?; - assert!(self.vtables.insert((ty, poly_trait_ref), vtable.alloc_id).is_none()); + assert!(self.vtables.insert((ty, poly_trait_ref), vtable).is_none()); Ok(vtable) } From 77be06b7ed147c80b4b8e035a877cea45ece9f3f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 May 2019 10:44:46 +0200 Subject: [PATCH 2/7] no longer assume that there is a default tag: give the machine the chance to tag all allocations --- src/librustc/mir/interpret/allocation.rs | 15 +- src/librustc/mir/interpret/pointer.rs | 7 - src/librustc/mir/interpret/value.rs | 27 +--- src/librustc/ty/context.rs | 2 +- src/librustc_mir/const_eval.rs | 28 ++-- src/librustc_mir/hair/constant.rs | 4 +- src/librustc_mir/interpret/eval_context.rs | 10 +- .../interpret/intrinsics/type_name.rs | 2 +- src/librustc_mir/interpret/machine.rs | 64 +++++---- src/librustc_mir/interpret/memory.rs | 132 +++++++++++------- src/librustc_mir/interpret/operand.rs | 96 +++++-------- src/librustc_mir/interpret/place.rs | 36 ++--- 12 files changed, 196 insertions(+), 227 deletions(-) diff --git a/src/librustc/mir/interpret/allocation.rs b/src/librustc/mir/interpret/allocation.rs index 856b47c7e1b6f..215d4295e44e1 100644 --- a/src/librustc/mir/interpret/allocation.rs +++ b/src/librustc/mir/interpret/allocation.rs @@ -111,9 +111,10 @@ pub trait AllocationExtra: ::std::fmt::Debug + Clone { // For Tag=() and no extra state, we have is a trivial implementation. impl AllocationExtra<()> for () { } -impl Allocation { +// The constructors are all without extra; the extra gets added by a machine hook later. +impl Allocation { /// Creates a read-only allocation initialized by the given bytes - pub fn from_bytes<'a>(slice: impl Into>, align: Align, extra: Extra) -> Self { + pub fn from_bytes<'a>(slice: impl Into>, align: Align) -> Self { let bytes = slice.into().into_owned(); let undef_mask = UndefMask::new(Size::from_bytes(bytes.len() as u64), true); Self { @@ -122,15 +123,15 @@ impl Allocation { undef_mask, align, mutability: Mutability::Immutable, - extra, + extra: (), } } - pub fn from_byte_aligned_bytes<'a>(slice: impl Into>, extra: Extra) -> Self { - Allocation::from_bytes(slice, Align::from_bytes(1).unwrap(), extra) + pub fn from_byte_aligned_bytes<'a>(slice: impl Into>) -> Self { + Allocation::from_bytes(slice, Align::from_bytes(1).unwrap()) } - pub fn undef(size: Size, align: Align, extra: Extra) -> Self { + pub fn undef(size: Size, align: Align) -> Self { assert_eq!(size.bytes() as usize as u64, size.bytes()); Allocation { bytes: vec![0; size.bytes() as usize], @@ -138,7 +139,7 @@ impl Allocation { undef_mask: UndefMask::new(size, false), align, mutability: Mutability::Mutable, - extra, + extra: (), } } } diff --git a/src/librustc/mir/interpret/pointer.rs b/src/librustc/mir/interpret/pointer.rs index 9e71399d4fdf7..4aa83a79d52b8 100644 --- a/src/librustc/mir/interpret/pointer.rs +++ b/src/librustc/mir/interpret/pointer.rs @@ -116,13 +116,6 @@ impl<'tcx> Pointer<()> { { Pointer::new_with_tag(self.alloc_id, self.offset, tag) } - - #[inline(always)] - pub fn with_default_tag(self) -> Pointer - where Tag: Default - { - self.with_tag(Tag::default()) - } } impl<'tcx, Tag> Pointer { diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 21792b847db77..1c24f5c116cce 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -140,23 +140,18 @@ impl<'tcx> Scalar<()> { #[inline] pub fn with_tag(self, new_tag: Tag) -> Scalar { + // Used by `MemPlace::replace_tag` match self { Scalar::Ptr(ptr) => Scalar::Ptr(ptr.with_tag(new_tag)), Scalar::Raw { data, size } => Scalar::Raw { data, size }, } } - - #[inline(always)] - pub fn with_default_tag(self) -> Scalar - where Tag: Default - { - self.with_tag(Tag::default()) - } } impl<'tcx, Tag> Scalar { #[inline] pub fn erase_tag(self) -> Scalar { + // Used by error reporting code to avoid having the error type depend on `Tag` match self { Scalar::Ptr(ptr) => Scalar::Ptr(ptr.erase_tag()), Scalar::Raw { data, size } => Scalar::Raw { data, size }, @@ -476,27 +471,11 @@ impl fmt::Display for ScalarMaybeUndef { } } -impl<'tcx> ScalarMaybeUndef<()> { - #[inline] - pub fn with_tag(self, new_tag: Tag) -> ScalarMaybeUndef { - match self { - ScalarMaybeUndef::Scalar(s) => ScalarMaybeUndef::Scalar(s.with_tag(new_tag)), - ScalarMaybeUndef::Undef => ScalarMaybeUndef::Undef, - } - } - - #[inline(always)] - pub fn with_default_tag(self) -> ScalarMaybeUndef - where Tag: Default - { - self.with_tag(Tag::default()) - } -} - impl<'tcx, Tag> ScalarMaybeUndef { #[inline] pub fn erase_tag(self) -> ScalarMaybeUndef { + // Used by error reporting code to avoid having the error type depend on `Tag` match self { ScalarMaybeUndef::Scalar(s) => ScalarMaybeUndef::Scalar(s.erase_tag()), ScalarMaybeUndef::Undef => ScalarMaybeUndef::Undef, diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index b63527bc37f7e..0cd19ca165aad 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1154,7 +1154,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { /// Allocates a byte or string literal for `mir::interpret`, read-only pub fn allocate_bytes(self, bytes: &[u8]) -> interpret::AllocId { // create an allocation that just contains these bytes - let alloc = interpret::Allocation::from_byte_aligned_bytes(bytes, ()); + let alloc = interpret::Allocation::from_byte_aligned_bytes(bytes); let alloc = self.intern_const_alloc(alloc); self.alloc_map.lock().create_memory_alloc(alloc) } diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 2c68ed58d50d4..dc7aa0f17e88c 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -12,7 +12,7 @@ use rustc::hir::def_id::DefId; use rustc::mir::interpret::{ConstEvalErr, ErrorHandled}; use rustc::mir; use rustc::ty::{self, TyCtxt, query::TyCtxtAt}; -use rustc::ty::layout::{self, LayoutOf, VariantIdx, Size}; +use rustc::ty::layout::{self, LayoutOf, VariantIdx}; use rustc::ty::subst::Subst; use rustc::traits::Reveal; use rustc::util::common::ErrorReported; @@ -117,7 +117,7 @@ fn op_to_const<'tcx>( ), Scalar::Raw { .. } => ( ecx.tcx.intern_const_alloc(Allocation::from_byte_aligned_bytes( - b"" as &[u8], (), + b"" as &[u8], )), 0, ), @@ -397,27 +397,27 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx> fn find_foreign_static( _def_id: DefId, _tcx: TyCtxtAt<'a, 'tcx, 'tcx>, - _memory_extra: &(), ) -> EvalResult<'tcx, Cow<'tcx, Allocation>> { err!(ReadForeignStatic) } #[inline(always)] - fn adjust_static_allocation<'b>( - alloc: &'b Allocation, + fn tag_allocation<'b>( + _id: AllocId, + alloc: Cow<'b, Allocation>, + _kind: Option>, _memory_extra: &(), - ) -> Cow<'b, Allocation> { - // We do not use a tag so we can just cheaply forward the reference - Cow::Borrowed(alloc) + ) -> (Cow<'b, Allocation>, Self::PointerTag) { + // We do not use a tag so we can just cheaply forward the allocation + (alloc, ()) } #[inline(always)] - fn new_allocation( - _size: Size, - _extra: &Self::MemoryExtra, - _kind: MemoryKind, - ) -> (Self::AllocExtra, Self::PointerTag) { - ((), ()) + fn tag_static_base_pointer( + _id: AllocId, + _memory_extra: &(), + ) -> Self::PointerTag { + () } fn box_alloc( diff --git a/src/librustc_mir/hair/constant.rs b/src/librustc_mir/hair/constant.rs index 69df36348a69e..a5be55d16d488 100644 --- a/src/librustc_mir/hair/constant.rs +++ b/src/librustc_mir/hair/constant.rs @@ -30,13 +30,13 @@ crate fn lit_to_const<'a, 'gcx, 'tcx>( let lit = match *lit { LitKind::Str(ref s, _) => { let s = s.as_str(); - let allocation = Allocation::from_byte_aligned_bytes(s.as_bytes(), ()); + let allocation = Allocation::from_byte_aligned_bytes(s.as_bytes()); let allocation = tcx.intern_const_alloc(allocation); ConstValue::Slice { data: allocation, start: 0, end: s.len() } }, LitKind::Err(ref s) => { let s = s.as_str(); - let allocation = Allocation::from_byte_aligned_bytes(s.as_bytes(), ()); + let allocation = Allocation::from_byte_aligned_bytes(s.as_bytes()); let allocation = tcx.intern_const_alloc(allocation); return Ok(tcx.mk_const(ty::Const { val: ConstValue::Slice{ data: allocation, start: 0, end: s.len() }, diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 1ed04c013eee3..594802a03410c 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -225,6 +225,11 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc &mut self.memory } + #[inline(always)] + pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { + self.memory.tag_static_base_pointer(ptr) + } + #[inline(always)] pub fn stack(&self) -> &[Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>] { &self.stack @@ -363,11 +368,6 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tc } } - pub fn str_to_immediate(&mut self, s: &str) -> EvalResult<'tcx, Immediate> { - let ptr = self.memory.allocate_static_bytes(s.as_bytes()).with_default_tag(); - Ok(Immediate::new_slice(Scalar::Ptr(ptr), s.len() as u64, self)) - } - /// Returns the actual dynamic size and alignment of the place at the given type. /// Only the "meta" (metadata) part of the place matters. /// This can fail to provide an answer for extern types. diff --git a/src/librustc_mir/interpret/intrinsics/type_name.rs b/src/librustc_mir/interpret/intrinsics/type_name.rs index 48b02d8e11b60..456fc70fc0d1d 100644 --- a/src/librustc_mir/interpret/intrinsics/type_name.rs +++ b/src/librustc_mir/interpret/intrinsics/type_name.rs @@ -215,7 +215,7 @@ impl Write for AbsolutePathPrinter<'_, '_> { pub fn type_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, ty: Ty<'tcx>) -> &'tcx ty::Const<'tcx> { let path = AbsolutePathPrinter { tcx, path: String::new() }.print_type(ty).unwrap().path; let len = path.len(); - let alloc = Allocation::from_byte_aligned_bytes(path.into_bytes(), ()); + let alloc = Allocation::from_byte_aligned_bytes(path.into_bytes()); let alloc = tcx.intern_const_alloc(alloc); tcx.mk_const(ty::Const { val: ConstValue::Slice { diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 873c2d2ec6088..7bf92711766a0 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -5,13 +5,13 @@ use std::borrow::{Borrow, Cow}; use std::hash::Hash; -use rustc::hir::{self, def_id::DefId}; +use rustc::hir::def_id::DefId; use rustc::mir; -use rustc::ty::{self, query::TyCtxtAt, layout::Size}; +use rustc::ty::{self, query::TyCtxtAt}; use super::{ Allocation, AllocId, EvalResult, Scalar, AllocationExtra, - InterpretCx, PlaceTy, MPlaceTy, OpTy, ImmTy, MemoryKind, + InterpretCx, PlaceTy, OpTy, ImmTy, MemoryKind, }; /// Whether this kind of memory is allowed to leak @@ -65,7 +65,7 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { /// Tag tracked alongside every pointer. This is used to implement "Stacked Borrows" /// . /// The `default()` is used for pointers to consts, statics, vtables and functions. - type PointerTag: ::std::fmt::Debug + Default + Copy + Eq + Hash + 'static; + type PointerTag: ::std::fmt::Debug + Copy + Eq + Hash + 'static; /// Extra data stored in every call frame. type FrameExtra; @@ -90,7 +90,7 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { /// The memory kind to use for copied statics -- or None if statics should not be mutated /// and thus any such attempt will cause a `ModifiedStatic` error to be raised. /// Statics are copied under two circumstances: When they are mutated, and when - /// `static_with_default_tag` or `find_foreign_static` (see below) returns an owned allocation + /// `tag_allocation` or `find_foreign_static` (see below) returns an owned allocation /// that is added to the memory so that the work is not done twice. const STATIC_KIND: Option; @@ -133,11 +133,12 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { /// This will only be called once per static and machine; the result is cached in /// the machine memory. (This relies on `AllocMap::get_or` being able to add the /// owned allocation to the map even when the map is shared.) + /// + /// This allocation will then be fed to `tag_allocation` to initialize the "extra" state. fn find_foreign_static( def_id: DefId, tcx: TyCtxtAt<'a, 'tcx, 'tcx>, - memory_extra: &Self::MemoryExtra, - ) -> EvalResult<'tcx, Cow<'tcx, Allocation>>; + ) -> EvalResult<'tcx, Cow<'tcx, Allocation>>; /// Called for all binary operations on integer(-like) types when one operand is a pointer /// value, and for the `Offset` operation that is inherently about pointers. @@ -156,36 +157,37 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { dest: PlaceTy<'tcx, Self::PointerTag>, ) -> EvalResult<'tcx>; - /// Called to turn an allocation obtained from the `tcx` into one that has - /// the right type for this machine. + /// Called to initialize the "extra" state of an allocation and make the pointers + /// it contains (in relocations) tagged. The way we construct allocations is + /// to always first construct it without extra and then add the extra. + /// This keeps uniform code paths for handling both allocations created by CTFE + /// for statics, and allocations ceated by Miri during evaluation. + /// + /// `kind` is the kind of the allocation being tagged; it can be `None` when + /// it's a static and `STATIC_KIND` is `None`. /// /// This should avoid copying if no work has to be done! If this returns an owned /// allocation (because a copy had to be done to add tags or metadata), machine memory will /// cache the result. (This relies on `AllocMap::get_or` being able to add the /// owned allocation to the map even when the map is shared.) - fn adjust_static_allocation<'b>( - alloc: &'b Allocation, + /// + /// The tag returned must be the same as the one returned by `tag_base_pointer`. + fn tag_allocation<'b>( + id: AllocId, + alloc: Cow<'b, Allocation>, + kind: Option>, memory_extra: &Self::MemoryExtra, - ) -> Cow<'b, Allocation>; - - /// Computes the extra state and the tag for a new allocation. - fn new_allocation( - size: Size, - extra: &Self::MemoryExtra, - kind: MemoryKind, - ) -> (Self::AllocExtra, Self::PointerTag); - - /// Executed when evaluating the `*` operator: Following a reference. - /// This has the chance to adjust the tag. It should not change anything else! - /// `mutability` can be `None` in case a raw ptr is being dereferenced. - #[inline] - fn tag_dereference( - _ecx: &InterpretCx<'a, 'mir, 'tcx, Self>, - place: MPlaceTy<'tcx, Self::PointerTag>, - _mutability: Option, - ) -> EvalResult<'tcx, Scalar> { - Ok(place.ptr) - } + ) -> (Cow<'b, Allocation>, Self::PointerTag); + + /// Return the "base" tag for the given static allocation: the one that is used for direct + /// accesses to this static/const/fn allocation. + /// + /// Be aware that requesting the `Allocation` for that `id` will lead to cycles + /// for cyclic statics! + fn tag_static_base_pointer( + id: AllocId, + memory_extra: &Self::MemoryExtra, + ) -> Self::PointerTag; /// Executes a retagging operation #[inline] diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 3e145975774bc..2538a82f5c5c4 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -108,33 +108,44 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { } } + #[inline] + pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { + ptr.with_tag(M::tag_static_base_pointer(ptr.alloc_id, &self.extra)) + } + pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { - // Default tag is okay because anyway you cannot access memory with this. - Pointer::from(self.tcx.alloc_map.lock().create_fn_alloc(instance)).with_default_tag() + let id = self.tcx.alloc_map.lock().create_fn_alloc(instance); + self.tag_static_base_pointer(Pointer::from(id)) } - pub fn allocate_static_bytes(&mut self, bytes: &[u8]) -> Pointer { - Pointer::from(self.tcx.allocate_bytes(bytes)) + pub fn allocate( + &mut self, + size: Size, + align: Align, + kind: MemoryKind, + ) -> Pointer { + let alloc = Allocation::undef(size, align); + self.allocate_with(alloc, kind) } - pub fn allocate_with( + pub fn allocate_static_bytes( &mut self, - alloc: Allocation, + bytes: &[u8], kind: MemoryKind, - ) -> AllocId { - let id = self.tcx.alloc_map.lock().reserve(); - self.alloc_map.insert(id, (kind, alloc)); - id + ) -> Pointer { + let alloc = Allocation::from_byte_aligned_bytes(bytes); + self.allocate_with(alloc, kind) } - pub fn allocate( + pub fn allocate_with( &mut self, - size: Size, - align: Align, + alloc: Allocation, kind: MemoryKind, ) -> Pointer { - let (extra, tag) = M::new_allocation(size, &self.extra, kind); - Pointer::from(self.allocate_with(Allocation::undef(size, align, extra), kind)).with_tag(tag) + let id = self.tcx.alloc_map.lock().reserve(); + let (alloc, tag) = M::tag_allocation(id, Cow::Owned(alloc), Some(kind), &self.extra); + self.alloc_map.insert(id, (kind, alloc.into_owned())); + Pointer::from(id).with_tag(tag) } pub fn reallocate( @@ -306,53 +317,68 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// This attempts to return a reference to an existing allocation if /// one can be found in `tcx`. That, however, is only possible if `tcx` and /// this machine use the same pointer tag, so it is indirected through - /// `M::static_with_default_tag`. + /// `M::tag_allocation`. + /// + /// Notice that every static has two `AllocId` that will resolve to the same + /// thing here: one maps to `GlobalAlloc::Static`, this is the "lazy" ID, + /// and the other one is maps to `GlobalAlloc::Memory`, this is returned by + /// `const_eval_raw` and it is the "resolved" ID. + /// The resolved ID is never used by the interpreted progrma, it is hidden. + /// The `GlobalAlloc::Memory` branch here is still reachable though; when a static + /// contains a reference to memory that was created during its evaluation (i.e., not to + /// another static), those inner references only exist in "resolved" from. fn get_static_alloc( id: AllocId, tcx: TyCtxtAt<'a, 'tcx, 'tcx>, memory_extra: &M::MemoryExtra, ) -> EvalResult<'tcx, Cow<'tcx, Allocation>> { let alloc = tcx.alloc_map.lock().get(id); - let def_id = match alloc { - Some(GlobalAlloc::Memory(mem)) => { - // We got tcx memory. Let the machine figure out whether and how to - // turn that into memory with the right pointer tag. - return Ok(M::adjust_static_allocation(mem, memory_extra)) - } - Some(GlobalAlloc::Function(..)) => { - return err!(DerefFunctionPointer) - } - Some(GlobalAlloc::Static(did)) => { - did - } + let alloc = match alloc { + Some(GlobalAlloc::Memory(mem)) => + Cow::Borrowed(mem), + Some(GlobalAlloc::Function(..)) => + return err!(DerefFunctionPointer), None => return err!(DanglingPointerDeref), - }; - // We got a "lazy" static that has not been computed yet, do some work - trace!("static_alloc: Need to compute {:?}", def_id); - if tcx.is_foreign_item(def_id) { - return M::find_foreign_static(def_id, tcx, memory_extra); - } - let instance = Instance::mono(tcx.tcx, def_id); - let gid = GlobalId { - instance, - promoted: None, - }; - // use the raw query here to break validation cycles. Later uses of the static will call the - // full query anyway - tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid)).map_err(|err| { - // no need to report anything, the const_eval call takes care of that for statics - assert!(tcx.is_static(def_id)); - match err { - ErrorHandled::Reported => InterpError::ReferencedConstant.into(), - ErrorHandled::TooGeneric => InterpError::TooGeneric.into(), + Some(GlobalAlloc::Static(def_id)) => { + // We got a "lazy" static that has not been computed yet. + if tcx.is_foreign_item(def_id) { + trace!("static_alloc: foreign item {:?}", def_id); + M::find_foreign_static(def_id, tcx)? + } else { + trace!("static_alloc: Need to compute {:?}", def_id); + let instance = Instance::mono(tcx.tcx, def_id); + let gid = GlobalId { + instance, + promoted: None, + }; + // use the raw query here to break validation cycles. Later uses of the static + // will call the full query anyway + let raw_const = tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid)) + .map_err(|err| { + // no need to report anything, the const_eval call takes care of that + // for statics + assert!(tcx.is_static(def_id)); + match err { + ErrorHandled::Reported => InterpError::ReferencedConstant, + ErrorHandled::TooGeneric => InterpError::TooGeneric, + } + })?; + // Make sure we use the ID of the resolved memory, not the lazy one! + let id = raw_const.alloc_id; + let allocation = tcx.alloc_map.lock().unwrap_memory(id); + Cow::Borrowed(allocation) + } } - }).map(|raw_const| { - let allocation = tcx.alloc_map.lock().unwrap_memory(raw_const.alloc_id); - // We got tcx memory. Let the machine figure out whether and how to - // turn that into memory with the right pointer tag. - M::adjust_static_allocation(allocation, memory_extra) - }) + }; + // We got tcx memory. Let the machine figure out whether and how to + // turn that into memory with the right pointer tag. + Ok(M::tag_allocation( + id, // always use the ID we got as input, not the "hidden" one. + alloc, + M::STATIC_KIND.map(MemoryKind::Machine), + memory_extra + ).0) } pub fn get(&self, id: AllocId) -> EvalResult<'tcx, &Allocation> { diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index f8352e8a95d50..130343d0b2fb6 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -37,16 +37,6 @@ impl<'tcx, Tag> Immediate { Immediate::Scalar(ScalarMaybeUndef::Scalar(val)) } - #[inline] - pub fn erase_tag(self) -> Immediate - { - match self { - Immediate::Scalar(x) => Immediate::Scalar(x.erase_tag()), - Immediate::ScalarPair(x, y) => - Immediate::ScalarPair(x.erase_tag(), y.erase_tag()), - } - } - pub fn new_slice( val: Scalar, len: u64, @@ -130,15 +120,6 @@ pub enum Operand { } impl Operand { - #[inline] - pub fn erase_tag(self) -> Operand - { - match self { - Operand::Immediate(x) => Operand::Immediate(x.erase_tag()), - Operand::Indirect(x) => Operand::Indirect(x.erase_tag()), - } - } - #[inline] pub fn to_mem_place(self) -> MemPlace where Tag: ::std::fmt::Debug @@ -209,18 +190,6 @@ impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag> } } -impl<'tcx, Tag> OpTy<'tcx, Tag> -{ - #[inline] - pub fn erase_tag(self) -> OpTy<'tcx> - { - OpTy { - op: self.op.erase_tag(), - layout: self.layout, - } - } -} - // Use the existing layout if given (but sanity check in debug mode), // or compute the layout. #[inline(always)] @@ -537,44 +506,53 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> val: &'tcx ty::Const<'tcx>, layout: Option>, ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { - let op = match val.val { + let tag_scalar = |scalar| match scalar { + Scalar::Ptr(ptr) => Scalar::Ptr(self.tag_static_base_pointer(ptr)), + Scalar::Raw { data, size } => Scalar::Raw { data, size }, + }; + // Early-return cases. + match val.val { ConstValue::Param(_) => return err!(TooGeneric), - ConstValue::Infer(_) | ConstValue::Placeholder(_) => bug!(), - ConstValue::ByRef(ptr, alloc) => { - // We rely on mutability being set correctly in that allocation to prevent writes - // where none should happen -- and for `static mut`, we copy on demand anyway. - Operand::Indirect( - MemPlace::from_ptr(ptr.with_default_tag(), alloc.align) - ) - }, - ConstValue::Slice { data, start, end } => - Operand::Immediate(Immediate::ScalarPair( - Scalar::from(Pointer::new( - self.tcx.alloc_map.lock().create_memory_alloc(data), - Size::from_bytes(start as u64), - )).with_default_tag().into(), - Scalar::from_uint( - (end - start) as u64, - self.tcx.data_layout.pointer_size, - ).with_default_tag().into(), - )), - ConstValue::Scalar(x) => - Operand::Immediate(Immediate::Scalar(x.with_default_tag().into())), ConstValue::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; return Ok(OpTy::from(self.const_eval_raw(GlobalId { instance, promoted: None, })?)); - }, - }; + } + _ => {} + } + // Other cases need layout. let layout = from_known_layout(layout, || { self.layout_of(self.monomorphize(val.ty)?) })?; - Ok(OpTy { - op, - layout, - }) + let op = match val.val { + ConstValue::ByRef(ptr, _alloc) => { + // We rely on mutability being set correctly in that allocation to prevent writes + // where none should happen -- and for `static mut`, we copy on demand anyway. + let ptr = self.tag_static_base_pointer(ptr); + Operand::Indirect(MemPlace::from_ptr(ptr, layout.align.abi)) + }, + ConstValue::Scalar(x) => + Operand::Immediate(Immediate::Scalar(tag_scalar(x).into())), + ConstValue::Slice { data, start, end } => { + let ptr = Pointer::new( + self.tcx.alloc_map.lock().create_memory_alloc(data), + Size::from_bytes(start as u64), // offset: `start` + ); + Operand::Immediate(Immediate::new_slice( + self.tag_static_base_pointer(ptr).into(), + (end - start) as u64, // len: `end - start` + self, + )) + } + ConstValue::Param(..) | + ConstValue::Infer(..) | + ConstValue::Placeholder(..) | + ConstValue::Unevaluated(..) => + bug!("eval_const_to_op: Unexpected ConstValue {:?}", val), + }; + Ok(OpTy { op, layout }) } /// Read discriminant, return the runtime value as well as the variant index. diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 6381d3888c293..fac9665d968e2 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -5,7 +5,6 @@ use std::convert::TryFrom; use std::hash::Hash; -use rustc::hir; use rustc::mir; use rustc::mir::interpret::truncate; use rustc::ty::{self, Ty}; @@ -294,7 +293,7 @@ impl<'tcx, Tag: ::std::fmt::Debug> PlaceTy<'tcx, Tag> { impl<'a, 'mir, 'tcx, Tag, M> InterpretCx<'a, 'mir, 'tcx, M> where // FIXME: Working around https://github.com/rust-lang/rust/issues/54385 - Tag: ::std::fmt::Debug+Default+Copy+Eq+Hash+'static, + Tag: ::std::fmt::Debug + Copy + Eq + Hash + 'static, M: Machine<'a, 'mir, 'tcx, PointerTag=Tag>, // FIXME: Working around https://github.com/rust-lang/rust/issues/24159 M::MemoryMap: AllocMap, Allocation)>, @@ -325,25 +324,13 @@ where // Take an operand, representing a pointer, and dereference it to a place -- that // will always be a MemPlace. Lives in `place.rs` because it creates a place. - // This calls the "deref" machine hook, and counts as a deref as far as - // Stacked Borrows is concerned. pub fn deref_operand( &self, src: OpTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { let val = self.read_immediate(src)?; trace!("deref to {} on {:?}", val.layout.ty, *val); - let mut place = self.ref_to_mplace(val)?; - // Pointer tag tracking might want to adjust the tag. - let mutbl = match val.layout.ty.sty { - // `builtin_deref` considers boxes immutable, that's useless for our purposes - ty::Ref(_, _, mutbl) => Some(mutbl), - ty::Adt(def, _) if def.is_box() => Some(hir::MutMutable), - ty::RawPtr(_) => None, - _ => bug!("Unexpected pointer type {}", val.layout.ty), - }; - place.mplace.ptr = M::tag_dereference(self, place, mutbl)?; - Ok(place) + self.ref_to_mplace(val) } /// Offset a pointer to project to a field. Unlike `place_field`, this is always @@ -587,18 +574,23 @@ where promoted: None }; // Just create a lazy reference, so we can support recursive statics. - // tcx takes are of assigning every static one and only one unique AllocId. + // tcx takes care of assigning every static one and only one unique AllocId. // When the data here is ever actually used, memory will notice, // and it knows how to deal with alloc_id that are present in the // global table but not in its local memory: It calls back into tcx through // a query, triggering the CTFE machinery to actually turn this lazy reference // into a bunch of bytes. IOW, statics are evaluated with CTFE even when // this InterpretCx uses another Machine (e.g., in miri). This is what we - // want! This way, computing statics works concistently between codegen + // want! This way, computing statics works consistently between codegen // and miri: They use the same query to eventually obtain a `ty::Const` // and use that for further computation. - let alloc = self.tcx.alloc_map.lock().create_static_alloc(cid.instance.def_id()); - MPlaceTy::from_aligned_ptr(Pointer::from(alloc).with_default_tag(), layout) + // + // Notice that statics have *two* AllocIds: the lazy one, and the resolved + // one. Here we make sure that the interpreted program never sees the + // resolved ID. Also see the doc comment of `Memory::get_static_alloc`. + let alloc_id = self.tcx.alloc_map.lock().create_static_alloc(cid.instance.def_id()); + let ptr = self.tag_static_base_pointer(Pointer::from(alloc_id)); + MPlaceTy::from_aligned_ptr(ptr, layout) } }) } @@ -1032,11 +1024,9 @@ where ) -> EvalResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { // This must be an allocation in `tcx` assert!(self.tcx.alloc_map.lock().get(raw.alloc_id).is_some()); + let ptr = self.tag_static_base_pointer(Pointer::from(raw.alloc_id)); let layout = self.layout_of(raw.ty)?; - Ok(MPlaceTy::from_aligned_ptr( - Pointer::new(raw.alloc_id, Size::ZERO).with_default_tag(), - layout, - )) + Ok(MPlaceTy::from_aligned_ptr(ptr, layout)) } /// Turn a place with a `dyn Trait` type into a place with the actual dynamic type. From 6dd9389008e50040dcd97165b3503a0e1a10c987 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 28 May 2019 19:08:14 +0200 Subject: [PATCH 3/7] fix comments --- src/librustc_mir/interpret/machine.rs | 3 ++- src/librustc_mir/interpret/memory.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 7bf92711766a0..fa0750fa82aa9 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -171,7 +171,8 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized { /// cache the result. (This relies on `AllocMap::get_or` being able to add the /// owned allocation to the map even when the map is shared.) /// - /// The tag returned must be the same as the one returned by `tag_base_pointer`. + /// For static allocations, the tag returned must be the same as the one returned by + /// `tag_static_base_pointer`. fn tag_allocation<'b>( id: AllocId, alloc: Cow<'b, Allocation>, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 2538a82f5c5c4..7db963a89aaef 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -326,7 +326,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> { /// The resolved ID is never used by the interpreted progrma, it is hidden. /// The `GlobalAlloc::Memory` branch here is still reachable though; when a static /// contains a reference to memory that was created during its evaluation (i.e., not to - /// another static), those inner references only exist in "resolved" from. + /// another static), those inner references only exist in "resolved" form. fn get_static_alloc( id: AllocId, tcx: TyCtxtAt<'a, 'tcx, 'tcx>, From 0f96dd51c584bf7b93155a41ae31dee7777f1508 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 29 May 2019 11:51:43 +0200 Subject: [PATCH 4/7] turn comments into doc-comments --- src/librustc/mir/interpret/value.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 1c24f5c116cce..b8d6c12244631 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -138,9 +138,11 @@ impl<'tcx> Scalar<()> { "Scalar value {:#x} exceeds size of {} bytes", data, size); } + /// Tag this scalar with `new_tag` if it is a pointer, leave it unchanged otherwise. + /// + /// Used by `MemPlace::replace_tag`. #[inline] pub fn with_tag(self, new_tag: Tag) -> Scalar { - // Used by `MemPlace::replace_tag` match self { Scalar::Ptr(ptr) => Scalar::Ptr(ptr.with_tag(new_tag)), Scalar::Raw { data, size } => Scalar::Raw { data, size }, @@ -149,9 +151,11 @@ impl<'tcx> Scalar<()> { } impl<'tcx, Tag> Scalar { + /// Erase the tag from the scalar, if any. + /// + /// Used by error reporting code to avoid having the error type depend on `Tag`. #[inline] pub fn erase_tag(self) -> Scalar { - // Used by error reporting code to avoid having the error type depend on `Tag` match self { Scalar::Ptr(ptr) => Scalar::Ptr(ptr.erase_tag()), Scalar::Raw { data, size } => Scalar::Raw { data, size }, @@ -472,10 +476,12 @@ impl fmt::Display for ScalarMaybeUndef { } impl<'tcx, Tag> ScalarMaybeUndef { + /// Erase the tag from the scalar, if any. + /// + /// Used by error reporting code to avoid having the error type depend on `Tag`. #[inline] pub fn erase_tag(self) -> ScalarMaybeUndef { - // Used by error reporting code to avoid having the error type depend on `Tag` match self { ScalarMaybeUndef::Scalar(s) => ScalarMaybeUndef::Scalar(s.erase_tag()), ScalarMaybeUndef::Undef => ScalarMaybeUndef::Undef, From 222ed9f57118a2542a67d39e6df07152bac0b962 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 29 May 2019 11:52:14 +0200 Subject: [PATCH 5/7] fix indentation --- src/librustc_mir/interpret/eval_context.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 594802a03410c..a6153bf055d60 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -44,9 +44,9 @@ pub struct InterpretCx<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'a, 'mir, 'tcx>> { /// A cache for deduplicating vtables pub(super) vtables: FxHashMap< - (Ty<'tcx>, Option>), - Pointer - >, + (Ty<'tcx>, Option>), + Pointer + >, } /// A stack frame. From 2ba7e776b611b797b00e5a2cd02584c5993514d5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 29 May 2019 14:49:02 +0200 Subject: [PATCH 6/7] FIXME --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 130343d0b2fb6..2fc7d65d655df 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -512,7 +512,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> }; // Early-return cases. match val.val { - ConstValue::Param(_) => return err!(TooGeneric), + ConstValue::Param(_) => return err!(TooGeneric), // FIXME(oli-obk): try to monomorphize ConstValue::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; return Ok(OpTy::from(self.const_eval_raw(GlobalId { From 823ffaa834bde2fb0daabd010c369aae561fb765 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 29 May 2019 15:16:18 +0200 Subject: [PATCH 7/7] update comments --- src/librustc_mir/interpret/operand.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 2fc7d65d655df..899cc40dc3206 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -529,13 +529,15 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> InterpretCx<'a, 'mir, 'tcx, M> let op = match val.val { ConstValue::ByRef(ptr, _alloc) => { // We rely on mutability being set correctly in that allocation to prevent writes - // where none should happen -- and for `static mut`, we copy on demand anyway. + // where none should happen. let ptr = self.tag_static_base_pointer(ptr); Operand::Indirect(MemPlace::from_ptr(ptr, layout.align.abi)) }, ConstValue::Scalar(x) => Operand::Immediate(Immediate::Scalar(tag_scalar(x).into())), ConstValue::Slice { data, start, end } => { + // We rely on mutability being set correctly in `data` to prevent writes + // where none should happen. let ptr = Pointer::new( self.tcx.alloc_map.lock().create_memory_alloc(data), Size::from_bytes(start as u64), // offset: `start`