search

rust-lang / miri

An interpreter for Rust's mid-level intermediate representation
Apache License 2.0
4.13k stars 318 forks source link

disable tls with Box::leak test on solaris/illumos. #3675

Open devnexen opened 1 week ago

devnexen commented 1 week ago

on these platforms, thread_local is disabled as there is no support for cleanup callback, the test is leaking here.

saethlin commented 1 week ago

Instead of ignoring the test on the target, can you add -Zmiri-ignore-leaks using the revisions system such as done in this test: https://github.com/rust-lang/miri/blob/60a720040d6c60656ab9cac0980e587d19d9c07a/tests/pass/tree_borrows/tree-borrows.rs#L1-L3

saethlin commented 1 week ago

I think you need multiple revisions as in the example. Does the test now fail on Linux if you add a leak to it?

RalfJung commented 1 week ago

I don't see why this test should be ignored on Solarish. It shows that we are not properly recognizing main thread TLS variables as being allowed-to-leak -- that should be fixed, not ignored. There is no cleanup callback called here on any platform, since Cell<Option<&'static i32>> has no destructor -- that's the entire point.

RalfJung commented 1 week ago

Also, other tests like tls_macro_drop and tls_static work fine. "on these platforms, thread_local is disabled" does not seem to be correct. The cleanup callback is called when it is needed. So I don't follow your PR description either.