address review comments

Author: max-heller

src/concurrency/thread.rs

@@ -1051,7 +1051,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                         // See if this thread can do something else.
                         match this.run_on_stack_empty()? {
                             Poll::Pending => {} // keep going
-                            Poll::Ready(()) => this.terminate_active_thread(false)?,
+                            Poll::Ready(()) =>
+                                this.terminate_active_thread(TerminatedThreadTls::Deallocate)?,
                         }
                     }
                 }
@@ -1066,12 +1067,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
     }
 
     /// Handles thread termination of the active thread: wakes up threads joining on this one,
-    /// and deallocates thread-local statics (unless `leak_tls` is set, in which case the thread's
-    /// TLS are marked as static roots for leakage analysis).
+    /// and deals with the thread's thread-local statics according to `tls`.
     ///
     /// This is called by the eval loop when a thread's on_stack_empty returns `Ready`.
     #[inline]
-    fn terminate_active_thread(&mut self, leak_tls: bool) -> InterpResult<'tcx> {
+    fn terminate_active_thread(&mut self, tls: TerminatedThreadTls) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
         let thread = this.active_thread_mut();
         assert!(thread.stack.is_empty(), "only threads with an empty stack can be terminated");
@@ -1080,20 +1080,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
         let current_span = this.machine.current_span();
         let thread_local_allocations =
             this.machine.threads.thread_terminated(this.machine.data_race.as_mut(), current_span);
-        if leak_tls {
-            // Add thread-local statics to static roots and skip deallocating backing memory
-            for ptr in thread_local_allocations {
-                if let Some(alloc) = ptr.provenance.get_alloc_id() {
-                    trace!("Thread-local static leaked and stored as static root: {:?}", alloc);
-                    this.machine.static_roots.push(alloc);
-                }
-            }
-        } else {
-            // Deallocate backing memory of thread-local statics
-            for ptr in thread_local_allocations {
-                this.deallocate_ptr(ptr.into(), None, MiriMemoryKind::Tls.into())?;
+        for ptr in thread_local_allocations {
+            match tls {
+                TerminatedThreadTls::Deallocate =>
+                    this.deallocate_ptr(ptr.into(), None, MiriMemoryKind::Tls.into())?,
+                TerminatedThreadTls::Leak =>
+                    if let Some(alloc) = ptr.provenance.get_alloc_id() {
+                        trace!("Thread-local static leaked and stored as static root: {:?}", alloc);
+                        this.machine.static_roots.push(alloc);
+                    },
             }
         }
         Ok(())
     }
 }
+
+/// What to do with TLS allocations from terminated threads
+pub enum TerminatedThreadTls {
+    /// Deallocate backing memory of thread-local statics as usual
+    Deallocate,
+    /// Skip deallocating backing memory of thread-local statics and consider all memory reachable
+    /// from them as not leaked (like global `static`s).
+    Leak,
+}

src/eval.rs

@@ -11,6 +11,7 @@ use log::info;
 use rustc_middle::ty::Ty;
 
 use crate::borrow_tracker::RetagFields;
+use crate::concurrency::thread::TerminatedThreadTls;
 use crate::diagnostics::report_leaks;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def::Namespace;
@@ -246,7 +247,7 @@ impl MainThreadState {
                 let exit_code = this.read_target_isize(&ret_place)?;
                 // Deal with our thread-local memory. We do *not* want to actually free it, instead we consider TLS
                 // to be like a global `static`, so that all memory reached by it is considered to "not leak".
-                this.terminate_active_thread(true)?;
+                this.terminate_active_thread(TerminatedThreadTls::Leak)?;
                 // Stop interpreter loop.
                 throw_machine_stop!(TerminationInfo::Exit { code: exit_code, leak_check: true });
             }