From 1d9efbbd8f618b1618840c2ab677feed52eac138 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 26 Jul 2020 11:11:17 +0200 Subject: [PATCH 1/5] Miri: replace canonical_alloc_id mechanism by extern_static_alloc_id which is called only when a pointer is 'imported' into the machine --- src/librustc_middle/mir/interpret/error.rs | 8 +-- src/librustc_mir/interpret/eval_context.rs | 15 +++-- src/librustc_mir/interpret/machine.rs | 69 +++++++--------------- src/librustc_mir/interpret/memory.rs | 59 ++++++++++-------- src/librustc_mir/interpret/operand.rs | 18 +++--- src/librustc_mir/interpret/place.rs | 2 +- src/librustc_mir/interpret/step.rs | 4 +- 7 files changed, 78 insertions(+), 97 deletions(-) diff --git a/src/librustc_middle/mir/interpret/error.rs b/src/librustc_middle/mir/interpret/error.rs index abbbbf7fbd6a4..ff6284e75db81 100644 --- a/src/librustc_middle/mir/interpret/error.rs +++ b/src/librustc_middle/mir/interpret/error.rs @@ -502,8 +502,6 @@ impl fmt::Display for UndefinedBehaviorInfo<'_> { pub enum UnsupportedOpInfo { /// Free-form case. Only for errors that are never caught! Unsupported(String), - /// Accessing an unsupported foreign static. - ReadForeignStatic(DefId), /// Could not find MIR for a function. NoMirFor(DefId), /// Encountered a pointer where we needed raw bytes. @@ -515,6 +513,8 @@ pub enum UnsupportedOpInfo { ReadBytesAsPointer, /// Accessing thread local statics ThreadLocalStatic(DefId), + /// Accessing an unsupported extern static. + ReadExternStatic(DefId), } impl fmt::Display for UnsupportedOpInfo { @@ -522,8 +522,8 @@ impl fmt::Display for UnsupportedOpInfo { use UnsupportedOpInfo::*; match self { Unsupported(ref msg) => write!(f, "{}", msg), - ReadForeignStatic(did) => { - write!(f, "cannot read from foreign (extern) static ({:?})", did) + ReadExternStatic(did) => { + write!(f, "cannot read from extern static ({:?})", did) } NoMirFor(did) => write!(f, "no MIR body is available for {:?}", did), ReadPointerAsBytes => write!(f, "unable to turn pointer into raw bytes",), diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index ba462ec35eacf..0f580b308142f 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -323,14 +323,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } /// Call this to turn untagged "global" pointers (obtained via `tcx`) into - /// the *canonical* machine pointer to the allocation. Must never be used - /// for any other pointers! + /// the machine pointer to the allocation. Must never be used + /// for any other pointers, nor for TLS statics. /// - /// This represents a *direct* access to that memory, as opposed to access - /// through a pointer that was created by the program. + /// Using the resulting pointer represents a *direct* access to that memory + /// (e.g. by directly using a `static`), + /// as opposed to access through a pointer that was created by the program. + /// + /// This function can fail only if `ptr` points to an `extern static`. #[inline(always)] - pub fn tag_global_base_pointer(&self, ptr: Pointer) -> Pointer { - self.memory.tag_global_base_pointer(ptr) + pub fn global_base_pointer(&self, ptr: Pointer) -> InterpResult<'tcx, Pointer> { + self.memory.global_base_pointer(ptr) } #[inline(always)] diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index ec1c93c81657e..634f3c96e6c9f 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -238,45 +238,30 @@ 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 + /// Return the `AllocId` for the given thread-local static in the current thread. + fn thread_local_static_alloc_id( + _ecx: &mut InterpCx<'mir, 'tcx, Self>, + def_id: DefId, + ) -> InterpResult<'tcx, AllocId> { + throw_unsup!(ThreadLocalStatic(def_id)) } - /// Called when converting a `ty::Const` to an operand (in - /// `eval_const_to_op`). - /// - /// Miri uses this callback for creating per thread allocations for thread - /// locals. In Rust, one way of creating a thread local is by marking a - /// static with `#[thread_local]`. On supported platforms this gets - /// translated to a LLVM thread local for which LLVM automatically ensures - /// that each thread gets its own copy. Since LLVM automatically handles - /// thread locals, the Rust compiler just treats thread local statics as - /// regular statics even though accessing a thread local static should be an - /// effectful computation that depends on the current thread. The long term - /// plan is to change MIR to make accesses to thread locals explicit - /// (https://github.com/rust-lang/rust/issues/70685). While the issue 70685 - /// is not fixed, our current workaround in Miri is to use this function to - /// make per-thread copies of thread locals. Please note that we cannot make - /// these copies in `canonical_alloc_id` because that is too late: for - /// example, if one created a pointer in thread `t1` to a thread local and - /// sent it to another thread `t2`, resolving the access in - /// `canonical_alloc_id` would result in pointer pointing to `t2`'s thread - /// local and not `t1` as it should. - #[inline] - fn adjust_global_const( - _ecx: &InterpCx<'mir, 'tcx, Self>, - val: mir::interpret::ConstValue<'tcx>, - ) -> InterpResult<'tcx, mir::interpret::ConstValue<'tcx>> { - Ok(val) + /// Return the `AllocId` backing the given `extern static`. + fn extern_static_alloc_id( + mem: &Memory<'mir, 'tcx, Self>, + def_id: DefId, + ) -> InterpResult<'tcx, AllocId> { + // Use the `AllocId` associated with the `DefId`. Any actual *access* will fail. + Ok(mem.tcx.create_static_alloc(def_id)) } + /// Return the "base" tag for the given *global* allocation: the one that is used for direct + /// accesses to this static/const/fn allocation. If `id` is not a global allocation, + /// this will return an unusable tag (i.e., accesses will be UB)! + /// + /// Called on the id returned by `thread_local_static_alloc_id` and `extern_static_alloc_id`, if needed. + fn tag_global_base_pointer(memory_extra: &Self::MemoryExtra, id: AllocId) -> Self::PointerTag; + /// 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. @@ -309,13 +294,6 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(()) } - /// Return the "base" tag for the given *global* allocation: the one that is used for direct - /// accesses to this static/const/fn allocation. If `id` is not a global allocation, - /// this will return an unusable tag (i.e., accesses will be UB)! - /// - /// Expects `id` to be already canonical, if needed. - fn tag_global_base_pointer(memory_extra: &Self::MemoryExtra, id: AllocId) -> Self::PointerTag; - /// Executes a retagging operation #[inline] fn retag( @@ -375,13 +353,6 @@ pub trait Machine<'mir, 'tcx>: Sized { _mem: &Memory<'mir, 'tcx, Self>, _ptr: Pointer, ) -> InterpResult<'tcx, u64>; - - fn thread_local_alloc_id( - _ecx: &mut InterpCx<'mir, 'tcx, Self>, - did: DefId, - ) -> InterpResult<'tcx, AllocId> { - throw_unsup!(ThreadLocalStatic(did)) - } } // A lot of the flexibility above is just needed for `Miri`, but all "compile-time" machines diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 8af1a8ac608ac..fce33eed66fe6 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -137,15 +137,33 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } /// Call this to turn untagged "global" pointers (obtained via `tcx`) into - /// the *canonical* machine pointer to the allocation. Must never be used - /// for any other pointers! + /// the machine pointer to the allocation. Must never be used + /// for any other pointers, nor for TLS statics. /// - /// This represents a *direct* access to that memory, as opposed to access - /// through a pointer that was created by the program. + /// Using the resulting pointer represents a *direct* access to that memory + /// (e.g. by directly using a `static`), + /// as opposed to access through a pointer that was created by the program. + /// + /// This function can fail only if `ptr` points to an `extern static`. #[inline] - pub fn tag_global_base_pointer(&self, ptr: Pointer) -> Pointer { - let id = M::canonical_alloc_id(self, ptr.alloc_id); - ptr.with_tag(M::tag_global_base_pointer(&self.extra, id)) + pub fn global_base_pointer(&self, mut ptr: Pointer) -> InterpResult<'tcx, Pointer> { + // We need to handle `extern static`. + let ptr = match self.tcx.get_global_alloc(ptr.alloc_id) { + Some(GlobalAlloc::Static(def_id)) if self.tcx.is_thread_local_static(def_id) => { + bug!("global memory cannot point to thread-local static") + } + Some(GlobalAlloc::Static(def_id)) if self.tcx.is_foreign_item(def_id) => { + ptr.alloc_id = M::extern_static_alloc_id(self, def_id)?; + ptr + } + _ => { + // No need to change the `AllocId`. + ptr + } + }; + // And we need to get the tag. + let tag = M::tag_global_base_pointer(&self.extra, ptr.alloc_id); + Ok(ptr.with_tag(tag)) } pub fn create_fn_alloc( @@ -162,7 +180,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { id } }; - self.tag_global_base_pointer(Pointer::from(id)) + // Functions are global allocations, so make sure we get the right base pointer. + // We know this is not an `extern static` so this cannmot fail. + self.global_base_pointer(Pointer::from(id)).unwrap() } pub fn allocate( @@ -195,6 +215,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { M::GLOBAL_KIND.map(MemoryKind::Machine), "dynamically allocating global memory" ); + // This is a new allocation, not a new global one, so no `global_base_ptr`. let (alloc, tag) = M::init_allocation_extra(&self.extra, id, Cow::Owned(alloc), Some(kind)); self.alloc_map.insert(id, (kind, alloc.into_owned())); Pointer::from(id).with_tag(tag) @@ -437,6 +458,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { Some(GlobalAlloc::Function(..)) => throw_ub!(DerefFunctionPointer(id)), None => throw_ub!(PointerUseAfterFree(id)), Some(GlobalAlloc::Static(def_id)) => { + assert!(tcx.is_static(def_id)); assert!(!tcx.is_thread_local_static(def_id)); // 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, @@ -448,24 +470,15 @@ 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. if tcx.is_foreign_item(def_id) { - trace!("get_global_alloc: foreign item {:?}", def_id); - throw_unsup!(ReadForeignStatic(def_id)) + throw_unsup!(ReadExternStatic(def_id)); } trace!("get_global_alloc: Need to compute {:?}", def_id); let instance = Instance::mono(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)); - err - })?; + let raw_const = tcx.const_eval_raw(ty::ParamEnv::reveal_all().and(gid))?; // Make sure we use the ID of the resolved memory, not the lazy one! let id = raw_const.alloc_id; let allocation = tcx.global_alloc(id).unwrap_memory(); @@ -482,6 +495,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { alloc, M::GLOBAL_KIND.map(MemoryKind::Machine), ); + // Sanity check that this is the same pointer we would have gotten via `global_base_pointer`. debug_assert_eq!(tag, M::tag_global_base_pointer(memory_extra, id)); Ok(alloc) } @@ -492,7 +506,6 @@ 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_global_alloc` that we can actually use directly without inserting anything anywhere. @@ -529,7 +542,6 @@ 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, || { @@ -568,7 +580,6 @@ 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 @@ -621,7 +632,6 @@ 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) { @@ -642,8 +652,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { if ptr.offset.bytes() != 0 { throw_ub!(InvalidFunctionPointer(ptr.erase_tag())) } - let id = M::canonical_alloc_id(self, ptr.alloc_id); - self.get_fn_alloc(id).ok_or_else(|| err_ub!(InvalidFunctionPointer(ptr.erase_tag())).into()) + self.get_fn_alloc(ptr.alloc_id).ok_or_else(|| err_ub!(InvalidFunctionPointer(ptr.erase_tag())).into()) } pub fn mark_immutable(&mut self, id: AllocId) -> InterpResult<'tcx> { diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index face72d70cea0..bb058476823e8 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -541,9 +541,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { val: &ty::Const<'tcx>, layout: Option>, ) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> { - let tag_scalar = |scalar| match scalar { - Scalar::Ptr(ptr) => Scalar::Ptr(self.tag_global_base_pointer(ptr)), - Scalar::Raw { data, size } => Scalar::Raw { data, size }, + let tag_scalar = |scalar| -> InterpResult<'tcx, _> { + Ok(match scalar { + Scalar::Ptr(ptr) => Scalar::Ptr(self.global_base_pointer(ptr)?), + Scalar::Raw { data, size } => Scalar::Raw { data, size }, + }) }; // Early-return cases. let val_val = match val.val { @@ -570,10 +572,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } ty::ConstKind::Value(val_val) => val_val, }; - // This call allows the machine to create fresh allocation ids for - // thread-local statics (see the `adjust_global_const` function - // documentation). - let val_val = M::adjust_global_const(self, val_val)?; // Other cases need layout. let layout = from_known_layout(self.tcx, self.param_env, layout, || self.layout_of(val.ty))?; @@ -582,10 +580,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let id = self.tcx.create_memory_alloc(alloc); // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen. - let ptr = self.tag_global_base_pointer(Pointer::new(id, offset)); + let ptr = self.global_base_pointer(Pointer::new(id, offset))?; Operand::Indirect(MemPlace::from_ptr(ptr, layout.align.abi)) } - ConstValue::Scalar(x) => Operand::Immediate(tag_scalar(x).into()), + ConstValue::Scalar(x) => Operand::Immediate(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. @@ -594,7 +592,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Size::from_bytes(start), // offset: `start` ); Operand::Immediate(Immediate::new_slice( - self.tag_global_base_pointer(ptr).into(), + self.global_base_pointer(ptr)?.into(), u64::try_from(end.checked_sub(start).unwrap()).unwrap(), // len: `end - start` self, )) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 270be98606454..76f0e82db96f3 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -1126,7 +1126,7 @@ where ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { // This must be an allocation in `tcx` let _ = self.tcx.global_alloc(raw.alloc_id); - let ptr = self.tag_global_base_pointer(Pointer::from(raw.alloc_id)); + let ptr = self.global_base_pointer(Pointer::from(raw.alloc_id))?; let layout = self.layout_of(raw.ty)?; Ok(MPlaceTy::from_aligned_ptr(ptr, layout)) } diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 18f9bbd2e3150..3d1e3eccc6147 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -141,8 +141,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { use rustc_middle::mir::Rvalue::*; match *rvalue { ThreadLocalRef(did) => { - let id = M::thread_local_alloc_id(self, did)?; - let val = Scalar::Ptr(self.tag_global_base_pointer(id.into())); + let id = M::thread_local_static_alloc_id(self, did)?; + let val = self.global_base_pointer(id.into())?; self.write_scalar(val, dest)?; } From debe597a9a4339f7ea7dc58fbfe8196aadacc593 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 26 Jul 2020 11:12:22 +0200 Subject: [PATCH 2/5] check that even referencing a TLS static during CTFE fails --- src/test/ui/consts/miri_unleashed/tls.rs | 7 +++++++ src/test/ui/consts/miri_unleashed/tls.stderr | 13 ++++++++++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/test/ui/consts/miri_unleashed/tls.rs b/src/test/ui/consts/miri_unleashed/tls.rs index ba86a554bbb68..7d4f8962a192c 100644 --- a/src/test/ui/consts/miri_unleashed/tls.rs +++ b/src/test/ui/consts/miri_unleashed/tls.rs @@ -14,4 +14,11 @@ static TEST_BAD: () = { //~| NOTE cannot access thread local static }; +// Make sure we catch taking a reference to thread-local storage. +static TEST_BAD_REF: () = { + unsafe { let _val = &A; } + //~^ ERROR could not evaluate static initializer + //~| NOTE cannot access thread local static +}; + fn main() {} diff --git a/src/test/ui/consts/miri_unleashed/tls.stderr b/src/test/ui/consts/miri_unleashed/tls.stderr index d3e87f319acde..27d2b2df4d8ae 100644 --- a/src/test/ui/consts/miri_unleashed/tls.stderr +++ b/src/test/ui/consts/miri_unleashed/tls.stderr @@ -4,6 +4,12 @@ error[E0080]: could not evaluate static initializer LL | unsafe { let _val = A; } | ^ cannot access thread local static (DefId(0:4 ~ tls[317d]::A[0])) +error[E0080]: could not evaluate static initializer + --> $DIR/tls.rs:19:26 + | +LL | unsafe { let _val = &A; } + | ^ cannot access thread local static (DefId(0:4 ~ tls[317d]::A[0])) + warning: skipping const checks | help: skipping check that does not even have a feature gate @@ -11,7 +17,12 @@ help: skipping check that does not even have a feature gate | LL | unsafe { let _val = A; } | ^ +help: skipping check that does not even have a feature gate + --> $DIR/tls.rs:19:26 + | +LL | unsafe { let _val = &A; } + | ^ -error: aborting due to previous error; 1 warning emitted +error: aborting due to 2 previous errors; 1 warning emitted For more information about this error, try `rustc --explain E0080`. From 069e84a1181d81665f355f15e9bd417a78e6e377 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 26 Jul 2020 12:02:03 +0200 Subject: [PATCH 3/5] move getting the initial value of a static into helper function --- src/librustc_mir/interpret/memory.rs | 29 +++++++++++++++++----------- src/librustc_mir/interpret/mod.rs | 2 +- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index fce33eed66fe6..4dbaafe4a21d5 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -13,6 +13,7 @@ use std::fmt; use std::ptr; use rustc_ast::ast::Mutability; +use rustc_hir::def_id::DefId; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_middle::ty::{self, Instance, ParamEnv, TyCtxt}; use rustc_target::abi::{Align, HasDataLayout, Size, TargetDataLayout}; @@ -118,6 +119,21 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { pub tcx: TyCtxt<'tcx>, } + +/// Return the `tcx` allocation containing the initial value of the given static +pub fn get_static( + tcx: TyCtxt<'tcx>, + def_id: DefId, +) -> InterpResult<'tcx, &'tcx Allocation> { + trace!("get_static: Need to compute {:?}", def_id); + let instance = Instance::mono(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))?; + Ok(tcx.global_alloc(raw_const.alloc_id).unwrap_memory()) +} + impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M> { #[inline] fn data_layout(&self) -> &TargetDataLayout { @@ -473,17 +489,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { if tcx.is_foreign_item(def_id) { throw_unsup!(ReadExternStatic(def_id)); } - trace!("get_global_alloc: Need to compute {:?}", def_id); - let instance = Instance::mono(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))?; - // Make sure we use the ID of the resolved memory, not the lazy one! - let id = raw_const.alloc_id; - let allocation = tcx.global_alloc(id).unwrap_memory(); - - (allocation, Some(def_id)) + + (get_static(tcx, def_id)?, Some(def_id)) } }; M::before_access_global(memory_extra, id, alloc, def_id, is_write)?; diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index d46010d98a5aa..04a2c3baef4bf 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -20,7 +20,7 @@ pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in pub use self::eval_context::{Frame, InterpCx, LocalState, LocalValue, StackPopCleanup}; pub use self::intern::{intern_const_alloc_recursive, InternKind}; pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackPopJump}; -pub use self::memory::{AllocCheck, FnVal, Memory, MemoryKind}; +pub use self::memory::{AllocCheck, FnVal, Memory, MemoryKind, get_static}; pub use self::operand::{ImmTy, Immediate, OpTy, Operand}; pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy}; pub use self::validity::RefTracking; From 1df9f44db7e0be3122335d961c3aa3542c923c7d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 26 Jul 2020 15:45:09 +0200 Subject: [PATCH 4/5] typos + fmt --- src/librustc_middle/mir/interpret/error.rs | 4 +--- src/librustc_mir/interpret/memory.rs | 18 +++++++++--------- src/librustc_mir/interpret/mod.rs | 2 +- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/librustc_middle/mir/interpret/error.rs b/src/librustc_middle/mir/interpret/error.rs index ff6284e75db81..1d76bb525ebf4 100644 --- a/src/librustc_middle/mir/interpret/error.rs +++ b/src/librustc_middle/mir/interpret/error.rs @@ -522,9 +522,7 @@ impl fmt::Display for UnsupportedOpInfo { use UnsupportedOpInfo::*; match self { Unsupported(ref msg) => write!(f, "{}", msg), - ReadExternStatic(did) => { - write!(f, "cannot read from extern static ({:?})", did) - } + ReadExternStatic(did) => write!(f, "cannot read from extern static ({:?})", did), NoMirFor(did) => write!(f, "no MIR body is available for {:?}", did), ReadPointerAsBytes => write!(f, "unable to turn pointer into raw bytes",), ReadBytesAsPointer => write!(f, "unable to turn bytes into a pointer"), diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 4dbaafe4a21d5..6867a299d81bf 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -13,8 +13,8 @@ use std::fmt; use std::ptr; use rustc_ast::ast::Mutability; -use rustc_hir::def_id::DefId; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_hir::def_id::DefId; use rustc_middle::ty::{self, Instance, ParamEnv, TyCtxt}; use rustc_target::abi::{Align, HasDataLayout, Size, TargetDataLayout}; @@ -119,12 +119,8 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { pub tcx: TyCtxt<'tcx>, } - /// Return the `tcx` allocation containing the initial value of the given static -pub fn get_static( - tcx: TyCtxt<'tcx>, - def_id: DefId, -) -> InterpResult<'tcx, &'tcx Allocation> { +pub fn get_static(tcx: TyCtxt<'tcx>, def_id: DefId) -> InterpResult<'tcx, &'tcx Allocation> { trace!("get_static: Need to compute {:?}", def_id); let instance = Instance::mono(tcx, def_id); let gid = GlobalId { instance, promoted: None }; @@ -162,7 +158,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// /// This function can fail only if `ptr` points to an `extern static`. #[inline] - pub fn global_base_pointer(&self, mut ptr: Pointer) -> InterpResult<'tcx, Pointer> { + pub fn global_base_pointer( + &self, + mut ptr: Pointer, + ) -> InterpResult<'tcx, Pointer> { // We need to handle `extern static`. let ptr = match self.tcx.get_global_alloc(ptr.alloc_id) { Some(GlobalAlloc::Static(def_id)) if self.tcx.is_thread_local_static(def_id) => { @@ -197,7 +196,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } }; // Functions are global allocations, so make sure we get the right base pointer. - // We know this is not an `extern static` so this cannmot fail. + // We know this is not an `extern static` so this cannot fail. self.global_base_pointer(Pointer::from(id)).unwrap() } @@ -659,7 +658,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { if ptr.offset.bytes() != 0 { throw_ub!(InvalidFunctionPointer(ptr.erase_tag())) } - self.get_fn_alloc(ptr.alloc_id).ok_or_else(|| err_ub!(InvalidFunctionPointer(ptr.erase_tag())).into()) + self.get_fn_alloc(ptr.alloc_id) + .ok_or_else(|| err_ub!(InvalidFunctionPointer(ptr.erase_tag())).into()) } pub fn mark_immutable(&mut self, id: AllocId) -> InterpResult<'tcx> { diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 04a2c3baef4bf..5de9b502a37c4 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -20,7 +20,7 @@ pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in pub use self::eval_context::{Frame, InterpCx, LocalState, LocalValue, StackPopCleanup}; pub use self::intern::{intern_const_alloc_recursive, InternKind}; pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackPopJump}; -pub use self::memory::{AllocCheck, FnVal, Memory, MemoryKind, get_static}; +pub use self::memory::{get_static, AllocCheck, FnVal, Memory, MemoryKind}; pub use self::operand::{ImmTy, Immediate, OpTy, Operand}; pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy}; pub use self::validity::RefTracking; From b8fd0f6a13e4b898a12f09b3d2d483c5f8e25cff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 27 Jul 2020 13:01:01 +0200 Subject: [PATCH 5/5] rename eval_const_to_op -> const_to_op --- src/librustc_mir/const_eval/mod.rs | 2 +- src/librustc_mir/interpret/eval_context.rs | 6 +++--- src/librustc_mir/interpret/operand.rs | 10 +++------- src/librustc_mir/transform/const_prop.rs | 2 +- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/librustc_mir/const_eval/mod.rs b/src/librustc_mir/const_eval/mod.rs index ed992a5983954..e7eeb4b4de496 100644 --- a/src/librustc_mir/const_eval/mod.rs +++ b/src/librustc_mir/const_eval/mod.rs @@ -41,7 +41,7 @@ pub(crate) fn destructure_const<'tcx>( ) -> mir::DestructuredConst<'tcx> { trace!("destructure_const: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, false); - let op = ecx.eval_const_to_op(val, None).unwrap(); + let op = ecx.const_to_op(val, None).unwrap(); // We go to `usize` as we cannot allocate anything bigger anyway. let (field_count, variant, down) = match val.ty.kind { diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 0f580b308142f..3d3d756cffe5a 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -848,12 +848,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; let val = self.tcx.const_eval_global_id(param_env, gid, Some(self.tcx.span))?; - // Even though `ecx.const_eval` is called from `eval_const_to_op` we can never have a + // Even though `ecx.const_eval` is called from `const_to_op` we can never have a // recursion deeper than one level, because the `tcx.const_eval` above is guaranteed to not - // return `ConstValue::Unevaluated`, which is the only way that `eval_const_to_op` will call + // return `ConstValue::Unevaluated`, which is the only way that `const_to_op` will call // `ecx.const_eval`. let const_ = ty::Const { val: ty::ConstKind::Value(val), ty }; - self.eval_const_to_op(&const_, None) + self.const_to_op(&const_, None) } pub fn const_eval_raw( diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index bb058476823e8..f9f72464b277b 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -517,7 +517,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Constant(ref constant) => { let val = self.subst_from_current_frame_and_normalize_erasing_regions(constant.literal); - self.eval_const_to_op(val, layout)? + self.const_to_op(val, layout)? } }; trace!("{:?}: {:?}", mir_op, *op); @@ -536,7 +536,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // in patterns via the `const_eval` module /// The `val` and `layout` are assumed to already be in our interpreter /// "universe" (param_env). - crate fn eval_const_to_op( + crate fn const_to_op( &self, val: &ty::Const<'tcx>, layout: Option>, @@ -559,16 +559,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // potentially requiring the current static to be evaluated again. This is not a // problem here, because we are building an operand which means an actual read is // happening. - // - // The machine callback `adjust_global_const` below is guaranteed to - // be called for all constants because `const_eval` calls - // `eval_const_to_op` recursively. return Ok(self.const_eval(GlobalId { instance, promoted }, val.ty)?); } ty::ConstKind::Infer(..) | ty::ConstKind::Bound(..) | ty::ConstKind::Placeholder(..) => { - span_bug!(self.cur_span(), "eval_const_to_op: Unexpected ConstKind {:?}", val) + span_bug!(self.cur_span(), "const_to_op: Unexpected ConstKind {:?}", val) } ty::ConstKind::Value(val_val) => val_val, }; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 3073bf53afd78..59003ce9e76be 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -436,7 +436,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { return None; } - match self.ecx.eval_const_to_op(c.literal, None) { + match self.ecx.const_to_op(c.literal, None) { Ok(op) => Some(op), Err(error) => { let tcx = self.ecx.tcx.at(c.span);