Properly deallocate thread locals

[vakaras]

Note: this is currently blocked on rust-lang/rust#70685

Currently Miri has only partial support for deallocation of thread-locals. It runs destructors of the library thread locals (src/shims/tls.rs), but it does not deallocate the thread-local statics #[thread_local]. As a result, Miri misses undefined behaviour in the following program:

#![feature(thread_local)]

use std::thread;

#[thread_local]
static mut A: u8 = 0;

struct Sender(*const u8);

unsafe impl Send for Sender {}

fn main() {
    let handle = thread::spawn(|| {
        let ptr = unsafe { &mut A as *mut u8 };
        assert_eq!(unsafe { *ptr }, 0);
        unsafe { *ptr = 5; }
        Sender(ptr)
    });
    let ptr = handle.join().unwrap().0;
    let x = unsafe { *ptr };  // This should be UB.
    let y = x + 1;
}

Fixing this issue before rust-lang/rust#70685 is very hard because currently we allocate thread local statics in tcx.alloc_map, which allows adding elements, but does not allow removing them.

[RalfJung]

Currently Miri has only partial support for deallocation of thread-locals. It runs destructors of the library thread locals (src/shims/tls.rs),

To be sure, there is no memory to deallocate here, right? Library thread-locals are not addressable, they just provide word-sized storage.

[RalfJung]

rust-random/rand#968 is related in the sense that it is also about deallocation of thread-locals. I wonder what it would take for Miri to be able to notice the problem in rand. @josephlr your input would be appreciated. :)

You wrote

This was my initial reaction as well, and I don't think it is. If a thread has multiple thread-local objects, on thread termination you could either (a) run the destructor for every object then free all of the thread-local memory at once or (b) run the destructor for one object, free its memory, then destory+free the next object, and so on.

Most platforms do (a), but this bug is easier to spot with (b). Either is fine, as Rust just says that the destuctor runs then memory gets freed. With either (a) or (b) we could still hit this bug, as the thread-local RNG could get destroyed while other thread-local variables still have a handle to it. The std::thread_local implementation includes checks in LocalKey::with to handle this scenario.

However, given that "thread-local" can refer to #[thread_local] (which cannot drop, i.e., it does not have a destructor) or to "TLS keys" (which are not addressable directly, i.e., there is no memory), I am a bit confused by these statements. You make it sound like there are TLS objects that have memory and a destructor ("run the destructor for one object, free its memory"), what would those be? There are also __cxa_thread_atexit_impl and similar mechanism to register a callback to be run when a thread dies, but again that is not associated with memory that would be freed.

Rust's thread_local! are AFAIK implemented with a combination of both -- __cxa_thread_atexit_impl (or as a fallback for platforms without such a "fast thread local" mechanism, a TLS key) is used to trigger running drop, but #[thread_local] is used to do the actual handling of memory. As long as TLS key dtors and __cxa_thread_atexit_impl run before #[thread_local] are deallocated, pointers should never dangle (and that is something Rust relies on for soundness I think, so all platforms better do it this way).

[RalfJung]

Hm, maybe you were talking about the Rust-level thread_local!, not the underlying OS mechanisms that implement that. I guess this is relevant for rand because, unlike the safe API via with, rand "leaks" references to the thread-local in a way that they can be accessed outside a with, and thus sidesteps all the protections that thread_local! has been setting up?

Indeed on that level I don't think thread_local! can meaningfully guarantee anything about pointers-to-TLS once dtor's start running. Still, would be interesting to have a test for this in Miri. So far all we got is this, checking that drop runs; we should have some compile-fail tests that actually dereference some dangling pointers to TLS (but probably we can only do that once #[thread_local]s actually get deallocated).

[vakaras]

Currently Miri has only partial support for deallocation of thread-locals. It runs destructors of the library thread locals (src/shims/tls.rs),

To be sure, there is no memory to deallocate here, right? Library thread-locals are not addressable, they just provide word-sized storage.

I think you are right. (I am definitely not an expert in this area.)