From e41cce03a00d29d56f0bb74fb0d5d3e69ee7dfc8 Mon Sep 17 00:00:00 2001 From: Andrei Damian Date: Sun, 2 Mar 2025 21:51:23 +0200 Subject: [PATCH] fix pthread-based tls on apple targets --- library/std/src/sys/thread_local/mod.rs | 1 + library/std/tests/thread_local/lib.rs | 2 + library/std/tests/thread_local/tests.rs | 21 ++++++++++- tests/run-make/apple-slow-tls/rmake.rs | 37 +++++++++++++++++++ .../apple-slow-tls/tls_test/Cargo.toml | 6 +++ .../apple-slow-tls/tls_test/src/main.rs | 10 +++++ 6 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 tests/run-make/apple-slow-tls/rmake.rs create mode 100644 tests/run-make/apple-slow-tls/tls_test/Cargo.toml create mode 100644 tests/run-make/apple-slow-tls/tls_test/src/main.rs diff --git a/library/std/src/sys/thread_local/mod.rs b/library/std/src/sys/thread_local/mod.rs index 1ff13154b7b3c..b7ce6fcdc05a7 100644 --- a/library/std/src/sys/thread_local/mod.rs +++ b/library/std/src/sys/thread_local/mod.rs @@ -138,6 +138,7 @@ pub(crate) mod key { not(target_family = "wasm"), target_family = "unix", ), + all(not(target_thread_local), target_vendor = "apple"), target_os = "teeos", all(target_os = "wasi", target_env = "p1", target_feature = "atomics"), ))] { diff --git a/library/std/tests/thread_local/lib.rs b/library/std/tests/thread_local/lib.rs index c52914354253c..26af5f1eb0a9d 100644 --- a/library/std/tests/thread_local/lib.rs +++ b/library/std/tests/thread_local/lib.rs @@ -1,3 +1,5 @@ +#![feature(cfg_target_thread_local)] + #[cfg(not(any(target_os = "emscripten", target_os = "wasi")))] mod tests; diff --git a/library/std/tests/thread_local/tests.rs b/library/std/tests/thread_local/tests.rs index aa020c2559cc5..e8278361d9337 100644 --- a/library/std/tests/thread_local/tests.rs +++ b/library/std/tests/thread_local/tests.rs @@ -1,7 +1,7 @@ use std::cell::{Cell, UnsafeCell}; use std::sync::atomic::{AtomicU8, Ordering}; use std::sync::{Arc, Condvar, Mutex}; -use std::thread::{self, Builder, LocalKey}; +use std::thread::{self, LocalKey}; use std::thread_local; #[derive(Clone, Default)] @@ -345,8 +345,27 @@ fn join_orders_after_tls_destructors() { } // Test that thread::current is still available in TLS destructors. +// +// The test won't currently work without target_thread_local, aka with slow tls. +// The runtime tries very hard to drop last the TLS variable that keeps the information about the +// current thread, by using several tricks like deffering the drop to a later round of TLS destruction. +// However, this only seems to work with fast tls. +// +// With slow TLS, it seems that multiple libc implementations will just set the value to null the first +// time they encounter it, regardless of it having a destructor or not. This means that trying to +// retrieve it later in a drop impl of another TLS variable will not work. +// +// ** Apple libc: https://github.com/apple-oss-distributions/libpthread/blob/c032e0b076700a0a47db75528a282b8d3a06531a/src/pthread_tsd.c#L293 +// Sets the variable to null if it has a destructor and the value is not null. However, all variables +// created with pthread_key_create are marked as having a destructor, even if the fn ptr called with +// it is null. +// ** glibc: https://github.com/bminor/glibc/blob/e5893e6349541d871e8a25120bca014551d13ff5/nptl/nptl_deallocate_tsd.c#L59 +// ** musl: https://github.com/kraj/musl/blob/1880359b54ff7dd9f5016002bfdae4b136007dde/src/thread/pthread_key_create.c#L87 +#[cfg(target_thread_local)] #[test] fn thread_current_in_dtor() { + use std::thread::Builder; + // Go through one round of TLS destruction first. struct Defer; impl Drop for Defer { diff --git a/tests/run-make/apple-slow-tls/rmake.rs b/tests/run-make/apple-slow-tls/rmake.rs new file mode 100644 index 0000000000000..231e0b1668e99 --- /dev/null +++ b/tests/run-make/apple-slow-tls/rmake.rs @@ -0,0 +1,37 @@ +//! Test if compilation with has-thread-local=false works, and if the output +//! has indeed no fast TLS variables. + +//@ only-apple + +use run_make_support::serde_json::{self, Value}; +use run_make_support::{cargo, llvm_nm, rfs, rustc}; + +fn main() { + let output = + rustc().print("target-spec-json").args(["-Z", "unstable-options"]).run().stdout_utf8(); + + let mut target_json: Value = serde_json::from_str(&output).unwrap(); + let has_thread_local = &mut target_json["has-thread-local"]; + assert!(matches!(has_thread_local, Value::Bool(true)), "{:?}", has_thread_local); + *has_thread_local = Value::Bool(false); + + let out_path = "t.json"; + rfs::write(out_path, serde_json::to_string(&target_json).unwrap()); + + cargo() + .args([ + "b", + "--manifest-path", + "tls_test/Cargo.toml", + "--target", + "t.json", + "-Zbuild-std=std,core,panic_abort", + ]) + .run(); + + // If a binary has any fast TLS variables, it should also contain the symbols + // __tlv_bootstrap and __tlv_atexit. We don't want them. + let output = llvm_nm().arg("tls_test/target/t/debug/tls_test").run().stdout_utf8(); + assert!(!output.contains("_tlv_bootstrap")); + assert!(!output.contains("_tlv_atexit")); +} diff --git a/tests/run-make/apple-slow-tls/tls_test/Cargo.toml b/tests/run-make/apple-slow-tls/tls_test/Cargo.toml new file mode 100644 index 0000000000000..c8e5a228eef79 --- /dev/null +++ b/tests/run-make/apple-slow-tls/tls_test/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "tls_test" +version = "0.1.0" +edition = "2024" + +[dependencies] diff --git a/tests/run-make/apple-slow-tls/tls_test/src/main.rs b/tests/run-make/apple-slow-tls/tls_test/src/main.rs new file mode 100644 index 0000000000000..d48d22bb3d17b --- /dev/null +++ b/tests/run-make/apple-slow-tls/tls_test/src/main.rs @@ -0,0 +1,10 @@ +use std::cell::RefCell; + +fn main() { + thread_local! { + static S: RefCell = RefCell::default(); + } + + S.with(|x| *x.borrow_mut() = "pika pika".to_string()); + S.with(|x| println!("{}", x.borrow())); +}