From 738ebcfb6ab1aa6f960f62ff78d7029e554ee51f Mon Sep 17 00:00:00 2001 From: Vytautas Astrauskas Date: Wed, 15 Apr 2020 14:05:14 -0700 Subject: [PATCH] Remove now unnecessary resolve_maybe_global_alloc. --- src/librustc_mir/interpret/machine.rs | 36 ++++++++------------------- src/librustc_mir/interpret/memory.rs | 24 +++++------------- 2 files changed, 17 insertions(+), 43 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 4daadb9726b..376ce3b239f 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -6,7 +6,7 @@ use std::borrow::{Borrow, Cow}; use std::hash::Hash; use rustc_middle::mir; -use rustc_middle::ty::{self, query::TyCtxtAt, Ty}; +use rustc_middle::ty::{self, Ty}; use rustc_span::def_id::DefId; use super::{ @@ -230,10 +230,11 @@ pub trait Machine<'mir, 'tcx>: Sized { id } - /// Called when converting a `ty::Const` to an operand. + /// Called when converting a `ty::Const` to an operand (in + /// `eval_const_to_op`). /// - /// Miri uses this callback for creating unique allocation ids for thread - /// locals. In Rust, one way for creating a thread local is by marking a + /// 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 @@ -243,11 +244,12 @@ pub trait Machine<'mir, 'tcx>: Sized { /// 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 - /// reserve fresh allocation ids for each thread. Please note that here we - /// only **reserve** the allocation ids; the actual allocation for the - /// thread local statics is done in `Memory::get_global_alloc`, which uses - /// `resolve_maybe_global_alloc` to retrieve information about the - /// allocation id we generated. + /// 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 eval_maybe_thread_local_static_const( _ecx: &InterpCx<'mir, 'tcx, Self>, @@ -256,22 +258,6 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(val) } - /// Called to obtain the `GlobalAlloc` associated with the allocation id. - /// - /// Miri uses this callback to resolve the information about the original - /// thread local static for which `canonical_alloc_id` reserved a fresh - /// allocation id. Since `canonical_alloc_id` does not create the actual - /// allocation and the reserved allocation id has no reference to its - /// parent, we need to ask Miri to retrieve information for us. - #[inline(always)] - fn resolve_maybe_global_alloc( - tcx: TyCtxtAt<'tcx>, - _memory_extra: &Self::MemoryExtra, - id: AllocId, - ) -> Option> { - tcx.alloc_map.lock().get(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. diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 27c8bf45906..bcad7855c37 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -429,16 +429,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { id: AllocId, is_write: bool, ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { - // The call to `resolve_maybe_global_alloc` is needed to enable Miri to - // support thread local statics. In - // `M::eval_maybe_thread_local_static_const`, for a thread local static, - // Miri reserves a fresh allocation id, but the actual allocation is - // left to the code that handles statics which calls this function - // (`get_global_alloc`). Since the allocation id is fresh, it has no - // information about the original static. The call to - // `resolve_maybe_global_alloc` allows Miri to retrieve this information - // for us. - let (alloc, def_id) = match M::resolve_maybe_global_alloc(tcx, memory_extra, id) { + let alloc = tcx.alloc_map.lock().get(id); + let (alloc, def_id) = match alloc { Some(GlobalAlloc::Memory(mem)) => { // Memory of a constant or promoted or anonymous memory referenced by a static. (mem, None) @@ -598,14 +590,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } // # Statics - // The call to `resolve_maybe_global_alloc` is needed here because Miri - // via the callback to `eval_maybe_thread_local_static_const` in - // `eval_const_to_op` reserves fresh allocation ids for thread local - // statics. However, the actual allocation is done not in - // `resolve_maybe_global_alloc`, but in `get_raw` and `get_raw_mut`. - // Since this function may get called before `get_raw`, we need to allow - // Miri to retrieve the information about the static for us. - match M::resolve_maybe_global_alloc(self.tcx, &self.extra, id) { + // Can't do this in the match argument, we may get cycle errors since the lock would + // be held throughout the match. + let alloc = self.tcx.alloc_map.lock().get(id); + match alloc { Some(GlobalAlloc::Static(did)) => { // Use size and align of the type. let ty = self.tcx.type_of(did);