add main thread TLS data to static roots

Author: max-heller

src/shims/tls.rs

@@ -105,12 +105,21 @@ impl<'tcx> TlsData<'tcx> {
         thread_id: ThreadId,
         new_data: Scalar<Provenance>,
         cx: &impl HasDataLayout,
+        static_roots: &mut Vec<AllocId>,
     ) -> InterpResult<'tcx> {
         match self.keys.get_mut(&key) {
             Some(TlsEntry { data, .. }) => {
                 if new_data.to_target_usize(cx)? != 0 {
                     trace!("TLS key {} for thread {:?} stored: {:?}", key, thread_id, new_data);
                     data.insert(thread_id, new_data);
+                    if thread_id.to_u32() == 0 {
+                        if let Some(alloc) = (new_data.to_pointer(cx).ok())
+                            .and_then(|ptr| ptr.provenance.and_then(|prov| prov.get_alloc_id()))
+                        {
+                            trace!("TLS key {} for main thread stored as static root", key);
+                            static_roots.push(alloc);
+                        }
+                    }
                 } else {
                     trace!("TLS key {} for thread {:?} removed", key, thread_id);
                     data.remove(&thread_id);

src/shims/unix/foreign_items.rs

@@ -305,7 +305,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let key = this.read_scalar(key)?.to_bits(key.layout.size)?;
                 let active_thread = this.get_active_thread();
                 let new_data = this.read_scalar(new_ptr)?;
-                this.machine.tls.store_tls(key, active_thread, new_data, &*this.tcx)?;
+                this.machine.tls.store_tls(key, active_thread, new_data, &*this.tcx, &mut this.machine.static_roots)?;
 
                 // Return success (`0`).
                 this.write_null(dest)?;

src/shims/windows/foreign_items.rs

@@ -260,7 +260,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> {
                 let key = u128::from(this.read_scalar(key)?.to_u32()?);
                 let active_thread = this.get_active_thread();
                 let new_data = this.read_scalar(new_ptr)?;
-                this.machine.tls.store_tls(key, active_thread, new_data, &*this.tcx)?;
+                this.machine.tls.store_tls(
+                    key,
+                    active_thread,
+                    new_data,
+                    &*this.tcx,
+                    &mut this.machine.static_roots,
+                )?;
 
                 // Return success (`1`).
                 this.write_scalar(Scalar::from_i32(1), dest)?;

tests/fail/leak_in_tls.rs

@@ -0,0 +1,24 @@
+//@error-in-other-file: memory leaked
+//@normalize-stderr-test: ".*│.*" -> "$$stripped$$"
+
+use std::cell::Cell;
+
+pub fn main() {
+    thread_local! {
+        static REF: Cell<Option<&'static i32>> = Cell::new(None);
+    }
+
+    std::thread::spawn(|| {
+        REF.with(|cell| {
+            let a = 123;
+            let b = Box::new(a);
+            let r = Box::leak(b);
+            cell.set(Some(r));
+        })
+    })
+    .join()
+    .unwrap();
+
+    // Imagine the program running for a long time while the thread is gone
+    // and this memory still sits around, unused -- leaked.
+}

tests/fail/leak_in_tls.stderr

@@ -0,0 +1,35 @@
+error: memory leaked: ALLOC (Rust heap, size: 4, align: 4), allocated here:
+  --> RUSTLIB/alloc/src/alloc.rs:LL:CC
+   |
+LL |         __rust_alloc(layout.size(), layout.align())
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: inside `std::alloc::alloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `std::alloc::Global::alloc_impl` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `<std::alloc::Global as std::alloc::Allocator>::allocate` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `alloc::alloc::exchange_malloc` at RUSTLIB/alloc/src/alloc.rs:LL:CC
+   = note: inside `std::boxed::Box::<i32>::new` at RUSTLIB/alloc/src/boxed.rs:LL:CC
+note: inside closure
+  --> $DIR/leak_in_tls.rs:LL:CC
+   |
+LL |             let b = Box::new(a);
+   |                     ^^^^^^^^^^^
+   = note: inside `std::thread::LocalKey::<std::cell::Cell<std::option::Option<&i32>>>::try_with::<[closure@$DIR/leak_in_tls.rs:LL:CC], ()>` at RUSTLIB/std/src/thread/local.rs:LL:CC
+   = note: inside `std::thread::LocalKey::<std::cell::Cell<std::option::Option<&i32>>>::with::<[closure@$DIR/leak_in_tls.rs:LL:CC], ()>` at RUSTLIB/std/src/thread/local.rs:LL:CC
+note: inside closure
+  --> $DIR/leak_in_tls.rs:LL:CC
+   |
+LL | /         REF.with(|cell| {
+LL | |             let a = 123;
+LL | |             let b = Box::new(a);
+LL | |             let r = Box::leak(b);
+LL | |             cell.set(Some(r));
+LL | |         })
+   | |__________^
+
+note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
+
+note: the evaluated program leaked memory, pass `-Zmiri-ignore-leaks` to disable this check
+
+error: aborting due to previous error
+

tests/pass/issues/issue-miri-2881.rs

@@ -0,0 +1,15 @@
+use std::cell::Cell;
+
+pub fn main() {
+    let a = 123;
+    let b = Box::new(a);
+    let r = Box::leak(b);
+
+    thread_local! {
+        static REF: Cell<Option<&'static i32>> = Cell::new(None);
+    }
+
+    REF.with(|cell| {
+        cell.set(Some(r));
+    })
+}