Add function eval_maybe_thread_local_static_const that allows handling thread locals without touching debug info; address other PR comments.

Author: Vytautas Astrauskas

src/librustc_mir/interpret/eval_context.rs

@@ -28,6 +28,8 @@ use crate::util::storage::AlwaysLiveLocals;
 
 pub struct InterpCx<'mir, 'tcx, M: Machine<'mir, 'tcx>> {
     /// Stores the `Machine` instance.
+    ///
+    /// Note: the stack is provided by the machine.
     pub machine: M,
 
     /// The results of the type checker, from rustc.
@@ -343,17 +345,19 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     }
 
     #[inline(always)]
-    pub fn stack(&self) -> &[Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>] {
+    pub(crate) fn stack(&self) -> &[Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>] {
         M::stack(self)
     }
 
     #[inline(always)]
-    pub fn stack_mut(&mut self) -> &mut Vec<Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>> {
+    pub(crate) fn stack_mut(
+        &mut self,
+    ) -> &mut Vec<Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>> {
         M::stack_mut(self)
     }
 
     #[inline(always)]
-    pub fn cur_frame(&self) -> usize {
+    pub fn frame_idx(&self) -> usize {
         let stack = self.stack();
         assert!(!stack.is_empty());
         stack.len() - 1
@@ -598,7 +602,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         return_to_block: StackPopCleanup,
     ) -> InterpResult<'tcx> {
         if !self.stack().is_empty() {
-            info!("PAUSING({}) {}", self.cur_frame(), self.frame().instance);
+            info!("PAUSING({}) {}", self.frame_idx(), self.frame().instance);
         }
         ::log_settings::settings().indentation += 1;
 
@@ -649,7 +653,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         }
 
         M::after_stack_push(self)?;
-        info!("ENTERING({}) {}", self.cur_frame(), self.frame().instance);
+        info!("ENTERING({}) {}", self.frame_idx(), self.frame().instance);
 
         if self.stack().len() > *self.tcx.sess.recursion_limit.get() {
             throw_exhaust!(StackFrameLimitReached)
@@ -706,7 +710,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     pub(super) fn pop_stack_frame(&mut self, unwinding: bool) -> InterpResult<'tcx> {
         info!(
             "LEAVING({}) {} (unwinding = {})",
-            self.cur_frame(),
+            self.frame_idx(),
             self.frame().instance,
             unwinding
         );
@@ -789,7 +793,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         if !self.stack().is_empty() {
             info!(
                 "CONTINUING({}) {} (unwinding = {})",
-                self.cur_frame(),
+                self.frame_idx(),
                 self.frame().instance,
                 unwinding
             );
@@ -897,8 +901,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             Place::Local { frame, local } => {
                 let mut allocs = Vec::new();
                 let mut msg = format!("{:?}", local);
-                if frame != self.cur_frame() {
-                    write!(msg, " ({} frames up)", self.cur_frame() - frame).unwrap();
+                if frame != self.frame_idx() {
+                    write!(msg, " ({} frames up)", self.frame_idx() - frame).unwrap();
                 }
                 write!(msg, ":").unwrap();
 

src/librustc_mir/interpret/machine.rs

@@ -120,16 +120,6 @@ pub trait Machine<'mir, 'tcx>: Sized {
     /// Whether memory accesses should be alignment-checked.
     fn enforce_alignment(memory_extra: &Self::MemoryExtra) -> bool;
 
-    /// Borrow the current thread's stack.
-    fn stack(
-        ecx: &'a InterpCx<'mir, 'tcx, Self>,
-    ) -> &'a [Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>];
-
-    /// Mutably borrow the current thread's stack.
-    fn stack_mut(
-        ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
-    ) -> &'a mut Vec<Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>>;
-
     /// Whether to enforce the validity invariant
     fn enforce_validity(ecx: &InterpCx<'mir, 'tcx, Self>) -> bool;
 
@@ -229,32 +219,43 @@ 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.
+    /// 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 for two purposes:
-    /// 1.  Redirecting extern statics to real allocations.
-    /// 2.  Creating unique allocation ids for thread locals.
-    ///
-    ///     In Rust, one way for creating a thread local is by marking a static
-    ///     with `#[thread_local]`. On supported platforms this gets translated
-    ///     to a LLVM thread local. The problem with supporting these thread
-    ///     locals in Miri is that in the internals of the compiler they look as
-    ///     normal statics, except that they have the `thread_local` attribute.
-    ///     However, in Miri we want to have a property that each allocation has
-    ///     a unique id. Therefore, for these thread locals in
-    ///     `canonical_alloc_id` we reserve fresh allocation ids for each
-    ///     thread. Please note that `canonical_alloc_id` only reserves the
-    ///     allocation ids, the actual allocation for the thread local statics
-    ///     is done in the same way as for regular statics.
+    /// 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 when converting a `ty::Const` to an operand.
+    ///
+    /// 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
+    /// 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
+    /// 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.
+    #[inline]
+    fn eval_maybe_thread_local_static_const(
+        _ecx: &InterpCx<'mir, 'tcx, Self>,
+        val: mir::interpret::ConstValue<'tcx>,
+    ) -> InterpResult<'tcx, mir::interpret::ConstValue<'tcx>> {
+        Ok(val)
+    }
+
     /// Called to obtain the `GlobalAlloc` associated with the allocation id.
     ///
     /// Miri uses this callback to resolve the information about the original
@@ -326,6 +327,16 @@ pub trait Machine<'mir, 'tcx>: Sized {
         frame: Frame<'mir, 'tcx, Self::PointerTag>,
     ) -> InterpResult<'tcx, Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>>;
 
+    /// Borrow the current thread's stack.
+    fn stack(
+        ecx: &'a InterpCx<'mir, 'tcx, Self>,
+    ) -> &'a [Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>];
+
+    /// Mutably borrow the current thread's stack.
+    fn stack_mut(
+        ecx: &'a mut InterpCx<'mir, 'tcx, Self>,
+    ) -> &'a mut Vec<Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>>;
+
     /// Called immediately after a stack frame got pushed and its locals got initialized.
     fn after_stack_push(_ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
         Ok(())

src/librustc_mir/interpret/memory.rs

@@ -430,11 +430,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
         is_write: bool,
     ) -> InterpResult<'tcx, Cow<'tcx, Allocation<M::PointerTag, M::AllocExtra>>> {
         // The call to `resolve_maybe_global_alloc` is needed to enable Miri to
-        // support thread local statics. In `M::canonical_alloc_id`, 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
+        // 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) {
@@ -598,9 +599,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 call to `canonical_alloc_id` above reserves fresh allocation
-        // ids for thread local statics. However, the actual allocation is done
-        // not in `canonical_alloc_id`, but in `get_raw` and `get_raw_mut`.
+        // 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) {

src/librustc_mir/interpret/operand.rs

@@ -537,6 +537,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
             ty::ConstKind::Value(val_val) => val_val,
         };
+        let val_val = M::eval_maybe_thread_local_static_const(self, val_val)?;
         // Other cases need layout.
         let layout = from_known_layout(self.tcx, layout, || self.layout_of(val.ty))?;
         let op = match val_val {

src/librustc_mir/interpret/place.rs

@@ -662,7 +662,7 @@ where
             }
             local => PlaceTy {
                 // This works even for dead/uninitialized locals; we check further when writing
-                place: Place::Local { frame: self.cur_frame(), local },
+                place: Place::Local { frame: self.frame_idx(), local },
                 layout: self.layout_of_local(self.frame(), local, None)?,
             },
         };

src/librustc_mir/interpret/step.rs

@@ -60,18 +60,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let body = self.body();
         let basic_block = &body.basic_blocks()[block];
 
-        let old_frames = self.cur_frame();
+        let old_frames = self.frame_idx();
 
         if let Some(stmt) = basic_block.statements.get(stmt_id) {
-            assert_eq!(old_frames, self.cur_frame());
+            assert_eq!(old_frames, self.frame_idx());
             self.statement(stmt)?;
             return Ok(true);
         }
 
         M::before_terminator(self)?;
 
         let terminator = basic_block.terminator();
-        assert_eq!(old_frames, self.cur_frame());
+        assert_eq!(old_frames, self.frame_idx());
         self.terminator(terminator)?;
         Ok(true)
     }
@@ -84,7 +84,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
         // Some statements (e.g., box) push new stack frames.
         // We have to record the stack frame number *before* executing the statement.
-        let frame_idx = self.cur_frame();
+        let frame_idx = self.frame_idx();
 
         match &stmt.kind {
             Assign(box (place, rvalue)) => self.eval_rvalue_into_place(rvalue, *place)?,

src/librustc_mir/interpret/terminator.rs

@@ -52,7 +52,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
 
             Call { ref func, ref args, destination, ref cleanup, .. } => {
-                let old_stack = self.cur_frame();
+                let old_stack = self.frame_idx();
                 let old_bb = self.frame().block;
                 let func = self.eval_operand(func, None)?;
                 let (fn_val, abi) = match func.layout.ty.kind {
@@ -80,7 +80,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 self.eval_fn_call(fn_val, abi, &args[..], ret, *cleanup)?;
                 // Sanity-check that `eval_fn_call` either pushed a new frame or
                 // did a jump to another block.
-                if self.cur_frame() == old_stack && self.frame().block == old_bb {
+                if self.frame_idx() == old_stack && self.frame().block == old_bb {
                     span_bug!(terminator.source_info.span, "evaluating this call made no progress");
                 }
             }