From 5da393970df87f13881fd1f8cebd9b2a42659759 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 23 Feb 2020 17:04:34 +0100 Subject: [PATCH 1/3] miri/machine: add canonical_alloc_id hook to replace find_foreign_static --- src/librustc_mir/const_eval/machine.rs | 10 +---- src/librustc_mir/interpret/machine.rs | 44 +++++++++++-------- src/librustc_mir/interpret/memory.rs | 56 +++++++++++++----------- src/librustc_mir/transform/const_prop.rs | 12 ----- 4 files changed, 57 insertions(+), 65 deletions(-) diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index e40436ccf0b4b..25727b75faf14 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -1,7 +1,6 @@ use rustc::mir; use rustc::ty::layout::HasTyCtxt; -use rustc::ty::{self, Ty, TyCtxt}; -use rustc_hir::def_id::DefId; +use rustc::ty::{self, Ty}; use std::borrow::{Borrow, Cow}; use std::collections::hash_map::Entry; use std::hash::Hash; @@ -320,13 +319,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, Err(ConstEvalErrKind::NeedsRfc("pointer arithmetic or comparison".to_string()).into()) } - fn find_foreign_static( - _tcx: TyCtxt<'tcx>, - _def_id: DefId, - ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { - throw_unsup!(ReadForeignStatic) - } - #[inline(always)] fn init_allocation_extra<'b>( _memory_extra: &MemoryExtra, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 5291000d10b3f..98a305ec2d92c 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -6,8 +6,7 @@ use std::borrow::{Borrow, Cow}; use std::hash::Hash; use rustc::mir; -use rustc::ty::{self, Ty, TyCtxt}; -use rustc_hir::def_id::DefId; +use rustc::ty::{self, Ty}; use rustc_span::Span; use super::{ @@ -123,10 +122,6 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Whether to enforce the validity invariant fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool; - /// Called before a basic block terminator is executed. - /// You can use this to detect endlessly running programs. - fn before_terminator(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx>; - /// Entry point to all function calls. /// /// Returns either the mir to use for the call, or `None` if execution should @@ -175,18 +170,6 @@ pub trait Machine<'mir, 'tcx>: Sized { unwind: Option, ) -> InterpResult<'tcx>; - /// Called for read access to a foreign static item. - /// - /// 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( - tcx: TyCtxt<'tcx>, - def_id: DefId, - ) -> InterpResult<'tcx, Cow<'tcx, Allocation>>; - /// Called for all binary operations where the LHS has pointer type. /// /// Returns a (value, overflowed) pair if the operation succeeded @@ -204,6 +187,7 @@ pub trait Machine<'mir, 'tcx>: Sized { ) -> InterpResult<'tcx>; /// Called to read the specified `local` from the `frame`. + #[inline] fn access_local( _ecx: &InterpCx<'mir, 'tcx, Self>, frame: &Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>, @@ -212,7 +196,15 @@ pub trait Machine<'mir, 'tcx>: Sized { frame.locals[local].access() } + /// Called before a basic block terminator is executed. + /// You can use this to detect endlessly running programs. + #[inline] + fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { + Ok(()) + } + /// Called before a `Static` value is accessed. + #[inline] fn before_access_static( _memory_extra: &Self::MemoryExtra, _allocation: &Allocation, @@ -220,6 +212,20 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(()) } + /// Called for *every* memory access to determine the real ID of the given allocation. + /// This provides a way for the machine to "redirect" certain allocations as it sees fit. + /// + /// This is used by Miri to redirect extern statics to real allocations. + /// + /// This function must be idempotent. + #[inline] + fn canonical_alloc_id( + _mem: &Memory<'mir, 'tcx, Self>, + id: AllocId, + ) -> AllocId { + id + } + /// 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. @@ -259,7 +265,7 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(()) } - /// Called immediately before a new stack frame got pushed + /// Called immediately before a new stack frame got pushed. fn stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx, Self::FrameExtra>; /// Called immediately after a stack frame gets popped diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 1df389d9c8bee..048c5d7b15956 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -421,6 +421,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// 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" form. + /// + /// Assumes `id` is already canonical. fn get_static_alloc( memory_extra: &M::MemoryExtra, tcx: TyCtxtAt<'tcx>, @@ -434,31 +436,30 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { 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(tcx.tcx, def_id)? - } 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 => err_inval!(ReferencedConstant), - ErrorHandled::TooGeneric => err_inval!(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); - - M::before_access_static(memory_extra, allocation)?; - Cow::Borrowed(allocation) + trace!("get_static_alloc: foreign item {:?}", def_id); + throw_unsup!(ReadForeignStatic) } + trace!("get_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 => err_inval!(ReferencedConstant), + ErrorHandled::TooGeneric => err_inval!(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); + + M::before_access_static(memory_extra, allocation)?; + Cow::Borrowed(allocation) } }; // We got tcx memory. Let the machine initialize its "extra" stuff. @@ -478,6 +479,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { &self, id: AllocId, ) -> InterpResult<'tcx, &Allocation> { + let id = M::canonical_alloc_id(self, id); // The error type of the inner closure here is somewhat funny. We have two // ways of "erroring": An actual error, or because we got a reference from // `get_static_alloc` that we can actually use directly without inserting anything anywhere. @@ -513,6 +515,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { &mut self, id: AllocId, ) -> InterpResult<'tcx, &mut Allocation> { + let id = M::canonical_alloc_id(self, id); let tcx = self.tcx; let memory_extra = &self.extra; let a = self.alloc_map.get_mut_or(id, || { @@ -550,6 +553,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { id: AllocId, liveness: AllocCheck, ) -> InterpResult<'static, (Size, Align)> { + let id = M::canonical_alloc_id(self, id); // # Regular allocations // Don't use `self.get_raw` here as that will // a) cause cycles in case `id` refers to a static @@ -602,6 +606,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } } + /// Assumes `id` is already canonical. fn get_fn_alloc(&self, id: AllocId) -> Option> { trace!("reading fn ptr: {}", id); if let Some(extra) = self.extra_fn_ptr_map.get(&id) { @@ -622,7 +627,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { if ptr.offset.bytes() != 0 { throw_unsup!(InvalidFunctionPointer) } - self.get_fn_alloc(ptr.alloc_id).ok_or_else(|| err_unsup!(ExecuteMemory).into()) + let id = M::canonical_alloc_id(self, ptr.alloc_id); + self.get_fn_alloc(id).ok_or_else(|| err_unsup!(ExecuteMemory).into()) } pub fn mark_immutable(&mut self, id: AllocId) -> InterpResult<'tcx> { diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 9e05133132e05..ccfe765f2bb3c 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -22,7 +22,6 @@ use rustc::ty::subst::{InternalSubsts, Subst}; use rustc::ty::{self, ConstKind, Instance, ParamEnv, Ty, TyCtxt, TypeFoldable}; use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::DefKind; -use rustc_hir::def_id::DefId; use rustc_hir::HirId; use rustc_index::vec::IndexVec; use rustc_infer::traits; @@ -222,13 +221,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { )); } - fn find_foreign_static( - _tcx: TyCtxt<'tcx>, - _def_id: DefId, - ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { - throw_unsup!(ReadForeignStatic) - } - #[inline(always)] fn init_allocation_extra<'b>( _memory_extra: &(), @@ -279,10 +271,6 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { Ok(()) } - fn before_terminator(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { - Ok(()) - } - #[inline(always)] fn stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> { Ok(()) From 01d932934748ef5c412c11a2ace18a504a7cb949 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 23 Feb 2020 19:50:34 +0100 Subject: [PATCH 2/3] canonicalize alloc ID before calling tag_static_base_pointer --- src/librustc_mir/interpret/machine.rs | 2 ++ src/librustc_mir/interpret/memory.rs | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 98a305ec2d92c..11d28ec582a88 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -253,6 +253,8 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Return the "base" tag for the given *static* allocation: the one that is used for direct /// accesses to this static/const/fn allocation. If `id` is not a static allocation, /// this will return an unusable tag (i.e., accesses will be UB)! + /// + /// Expects `id` to be already canonical, if needed. fn tag_static_base_pointer(memory_extra: &Self::MemoryExtra, id: AllocId) -> Self::PointerTag; /// Executes a retagging operation diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 048c5d7b15956..f60307e468bd2 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -150,7 +150,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// through a pointer that was created by the program. #[inline] pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { - ptr.with_tag(M::tag_static_base_pointer(&self.extra, ptr.alloc_id)) + let id = M::canonical_alloc_id(self, ptr.alloc_id); + ptr.with_tag(M::tag_static_base_pointer(&self.extra, id)) } pub fn create_fn_alloc( From 9b62d60db18415caf3460b894e4f43f51da4f645 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 23 Feb 2020 22:39:57 +0100 Subject: [PATCH 3/3] fmt --- src/librustc_mir/interpret/machine.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 11d28ec582a88..69c9664b35156 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -219,10 +219,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// /// This function must be idempotent. #[inline] - fn canonical_alloc_id( - _mem: &Memory<'mir, 'tcx, Self>, - id: AllocId, - ) -> AllocId { + fn canonical_alloc_id(_mem: &Memory<'mir, 'tcx, Self>, id: AllocId) -> AllocId { id }