From b67b5a8d0149096712e75336f6aa32daffcaa42d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 10 Dec 2015 12:21:55 -0800 Subject: [PATCH 1/3] rustc: Add feature-gated cfg(target_thread_local) Currently the standard library has some pretty complicated logic to detect whether #[thread_local] should be used or whether it's supported. This is also unfortunately not quite true for OSX where not all versions support the #[thread_local] attribute (only 10.7+ does). Compiling code for OSX 10.6 is typically requested via the MACOSX_DEPLOYMENT_TARGET environment variable (e.g. the linker recognizes this), but the standard library unfortunately does not respect this. This commit updates the compiler to add a `target_thread_local` cfg annotation if the platform being targeted supports the `#[thread_local]` attribute. This is feature gated for now, and it is only true on non-aarch64 Linux and 10.7+ OSX (e.g. what the module already does today). Logic has also been added to parse the deployment target environment variable. --- src/librustc/session/config.rs | 3 +++ .../target/aarch64_unknown_linux_gnu.rs | 3 ++- src/librustc_back/target/apple_base.rs | 23 ++++++++++++++++++- src/librustc_back/target/apple_ios_base.rs | 1 + .../target/arm_linux_androideabi.rs | 1 + src/librustc_back/target/linux_base.rs | 1 + src/librustc_back/target/mod.rs | 5 ++++ src/libsyntax/feature_gate.rs | 8 +++++++ 8 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index e33fe9570c0..b275480a6fc 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -660,6 +660,9 @@ pub fn default_configuration(sess: &Session) -> ast::CrateConfig { "windows" | "unix" => ret.push(attr::mk_word_item(fam)), _ => (), } + if sess.target.target.options.has_elf_tls { + ret.push(attr::mk_word_item(InternedString::new("target_thread_local"))); + } if sess.opts.debug_assertions { ret.push(attr::mk_word_item(InternedString::new("debug_assertions"))); } diff --git a/src/librustc_back/target/aarch64_unknown_linux_gnu.rs b/src/librustc_back/target/aarch64_unknown_linux_gnu.rs index 51abab6609a..21bfd87e412 100644 --- a/src/librustc_back/target/aarch64_unknown_linux_gnu.rs +++ b/src/librustc_back/target/aarch64_unknown_linux_gnu.rs @@ -11,7 +11,8 @@ use target::Target; pub fn target() -> Target { - let base = super::linux_base::opts(); + let mut base = super::linux_base::opts(); + base.has_elf_tls = false; Target { llvm_target: "aarch64-unknown-linux-gnu".to_string(), target_endian: "little".to_string(), diff --git a/src/librustc_back/target/apple_base.rs b/src/librustc_back/target/apple_base.rs index 8b75bb39414..ffcb6f971ae 100644 --- a/src/librustc_back/target/apple_base.rs +++ b/src/librustc_back/target/apple_base.rs @@ -8,10 +8,30 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::env; + use target::TargetOptions; -use std::default::Default; pub fn opts() -> TargetOptions { + // ELF TLS is only available in OSX 10.7+. If you try to compile for 10.6 + // either the linker will complain if it is used or the binary will end up + // segfaulting at runtime when run on 10.6. Rust by default supports OSX + // 10.7+, but there is a standard environment variable, + // MACOSX_DEPLOYMENT_TARGET, which is used to signal targeting older + // versions of OSX. For example compiling on 10.10 with + // MACOSX_DEPLOYMENT_TARGET set to 10.6 will cause the linker to generate + // warnings about the usage of ELF TLS. + // + // Here we detect what version is being requested, defaulting to 10.7. ELF + // TLS is flagged as enabled if it looks to be supported. + let deployment_target = env::var("MACOSX_DEPLOYMENT_TARGET").ok(); + let version = deployment_target.as_ref().and_then(|s| { + let mut i = s.splitn(2, "."); + i.next().and_then(|a| i.next().map(|b| (a, b))) + }).and_then(|(a, b)| { + a.parse::().and_then(|a| b.parse::().map(|b| (a, b))).ok() + }).unwrap_or((10, 7)); + TargetOptions { // OSX has -dead_strip, which doesn't rely on ffunction_sections function_sections: false, @@ -25,6 +45,7 @@ pub fn opts() -> TargetOptions { archive_format: "bsd".to_string(), pre_link_args: Vec::new(), exe_allocation_crate: super::maybe_jemalloc(), + has_elf_tls: version >= (10, 7), .. Default::default() } } diff --git a/src/librustc_back/target/apple_ios_base.rs b/src/librustc_back/target/apple_ios_base.rs index 77030f5d768..d182fd96056 100644 --- a/src/librustc_back/target/apple_ios_base.rs +++ b/src/librustc_back/target/apple_ios_base.rs @@ -88,6 +88,7 @@ pub fn opts(arch: Arch) -> TargetOptions { dynamic_linking: false, executables: true, pre_link_args: pre_link_args(arch), + has_elf_tls: false, .. super::apple_base::opts() } } diff --git a/src/librustc_back/target/arm_linux_androideabi.rs b/src/librustc_back/target/arm_linux_androideabi.rs index 732f1a353a8..7776aaacd00 100644 --- a/src/librustc_back/target/arm_linux_androideabi.rs +++ b/src/librustc_back/target/arm_linux_androideabi.rs @@ -13,6 +13,7 @@ use target::Target; pub fn target() -> Target { let mut base = super::android_base::opts(); base.features = "+v7".to_string(); + base.has_elf_tls = false; Target { llvm_target: "arm-linux-androideabi".to_string(), diff --git a/src/librustc_back/target/linux_base.rs b/src/librustc_back/target/linux_base.rs index 6492fa50159..0efcf73ee86 100644 --- a/src/librustc_back/target/linux_base.rs +++ b/src/librustc_back/target/linux_base.rs @@ -30,6 +30,7 @@ pub fn opts() -> TargetOptions { position_independent_executables: true, archive_format: "gnu".to_string(), exe_allocation_crate: super::maybe_jemalloc(), + has_elf_tls: true, .. Default::default() } } diff --git a/src/librustc_back/target/mod.rs b/src/librustc_back/target/mod.rs index f259698a220..666903b4eed 100644 --- a/src/librustc_back/target/mod.rs +++ b/src/librustc_back/target/mod.rs @@ -195,6 +195,10 @@ pub struct TargetOptions { /// Default crate for allocation symbols to link against pub lib_allocation_crate: String, pub exe_allocation_crate: String, + + /// Flag indicating whether ELF TLS (e.g. #[thread_local]) is available for + /// this target. + pub has_elf_tls: bool, } impl Default for TargetOptions { @@ -240,6 +244,7 @@ impl Default for TargetOptions { lib_allocation_crate: "alloc_system".to_string(), exe_allocation_crate: "alloc_system".to_string(), allow_asm: true, + has_elf_tls: false, } } } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index b2989c42a9e..390e8253dcf 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -236,6 +236,9 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Option, Status // allow using type ascription in expressions ("type_ascription", "1.6.0", Some(23416), Active), + + // Allows cfg(target_thread_local) + ("cfg_target_thread_local", "1.7.0", Some(26581), Active), ]; // (changing above list without updating src/doc/reference.md makes @cmr sad) @@ -414,6 +417,8 @@ const GATED_CFGS: &'static [(&'static str, &'static str, fn(&Features) -> bool)] // (name in cfg, feature, function to check if the feature is enabled) ("target_feature", "cfg_target_feature", cfg_fn!(|x| x.cfg_target_feature)), ("target_vendor", "cfg_target_vendor", cfg_fn!(|x| x.cfg_target_vendor)), + ("target_thread_local", "cfg_target_thread_local", + cfg_fn!(|x| x.cfg_target_thread_local)), ]; #[derive(Debug, Eq, PartialEq)] @@ -541,6 +546,7 @@ pub struct Features { pub type_macros: bool, pub cfg_target_feature: bool, pub cfg_target_vendor: bool, + pub cfg_target_thread_local: bool, pub augmented_assignments: bool, pub braced_empty_structs: bool, pub staged_api: bool, @@ -575,6 +581,7 @@ impl Features { type_macros: false, cfg_target_feature: false, cfg_target_vendor: false, + cfg_target_thread_local: false, augmented_assignments: false, braced_empty_structs: false, staged_api: false, @@ -1157,6 +1164,7 @@ fn check_crate_inner(cm: &CodeMap, span_handler: &Handler, type_macros: cx.has_feature("type_macros"), cfg_target_feature: cx.has_feature("cfg_target_feature"), cfg_target_vendor: cx.has_feature("cfg_target_vendor"), + cfg_target_thread_local: cx.has_feature("cfg_target_thread_local"), augmented_assignments: cx.has_feature("augmented_assignments"), braced_empty_structs: cx.has_feature("braced_empty_structs"), staged_api: cx.has_feature("staged_api"), From 617a7af70400c7a3f82fafcb50daf01f01db95a0 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 16 Dec 2015 09:24:08 -0800 Subject: [PATCH 2/3] syntax: Respect allow_internal_unstable in macros This change modifies the feature gating of special `#[cfg]` attributes to not require a `#![feature]` directive in the crate-of-use if the source of the macro was declared with `#[allow_internal_unstable]`. This enables the standard library's macro for `thread_local!` to make use of the `#[cfg(target_thread_local)]` attribute despite it being feature gated (e.g. it's a hidden implementation detail). --- src/librustc_driver/driver.rs | 2 +- src/libsyntax/feature_gate.rs | 14 ++++++++++---- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 5f2548a5504..89299c01199 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -606,7 +606,7 @@ pub fn phase_2_configure_and_expand(sess: &Session, feature_gated_cfgs.sort(); feature_gated_cfgs.dedup(); for cfg in &feature_gated_cfgs { - cfg.check_and_emit(sess.diagnostic(), &features); + cfg.check_and_emit(sess.diagnostic(), &features, sess.codemap()); } }); diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 390e8253dcf..caad7c6e7f5 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -454,10 +454,13 @@ impl PartialOrd for GatedCfgAttr { } impl GatedCfgAttr { - pub fn check_and_emit(&self, diagnostic: &Handler, features: &Features) { + pub fn check_and_emit(&self, + diagnostic: &Handler, + features: &Features, + codemap: &CodeMap) { match *self { GatedCfgAttr::GatedCfg(ref cfg) => { - cfg.check_and_emit(diagnostic, features); + cfg.check_and_emit(diagnostic, features, codemap); } GatedCfgAttr::GatedAttr(span) => { if !features.stmt_expr_attributes { @@ -484,9 +487,12 @@ impl GatedCfg { } }) } - fn check_and_emit(&self, diagnostic: &Handler, features: &Features) { + fn check_and_emit(&self, + diagnostic: &Handler, + features: &Features, + codemap: &CodeMap) { let (cfg, feature, has_feature) = GATED_CFGS[self.index]; - if !has_feature(features) { + if !has_feature(features) && !codemap.span_allows_unstable(self.span) { let explain = format!("`cfg({})` is experimental and subject to change", cfg); emit_feature_err(diagnostic, feature, self.span, GateIssue::Language, &explain); } From cd74364e5ddd3e81fa27ea149194966a3a172d9b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 11 Dec 2015 12:42:29 -0800 Subject: [PATCH 3/3] std: Use cfg(target_thread_local) in thread_local! This transitions the standard library's `thread_local!` macro to use the freshly-added and gated `#[cfg(target_thread_local)]` attribute. This greatly simplifies the `#[cfg]` logic in play here, but requires that the standard library expose both the OS and ELF TLS implementation modules as unstable implementation details. The implementation details were shuffled around a bit but end up generally compiling to the same thing. Closes #26581 (this supersedes the need for the option) Closes #27057 (this also starts ignoring the option) --- src/libstd/lib.rs | 1 + src/libstd/thread/local.rs | 120 +++++++++++++--------------------- src/libstd/thread/mod.rs | 5 +- src/libsyntax/feature_gate.rs | 2 +- 4 files changed, 50 insertions(+), 78 deletions(-) diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index eba0c799cd2..01bf5147557 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -227,6 +227,7 @@ #![feature(borrow_state)] #![feature(box_syntax)] #![feature(cfg_target_vendor)] +#![feature(cfg_target_thread_local)] #![feature(char_internals)] #![feature(clone_from_slice)] #![feature(collections)] diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs index 870247f7e82..ca0f1031562 100644 --- a/src/libstd/thread/local.rs +++ b/src/libstd/thread/local.rs @@ -15,10 +15,6 @@ use cell::UnsafeCell; use mem; -// Sure wish we had macro hygiene, no? -#[doc(hidden)] -pub use self::imp::Key as __KeyInner; - /// A thread local storage key which owns its contents. /// /// This key uses the fastest possible implementation available to it for the @@ -61,41 +57,27 @@ pub use self::imp::Key as __KeyInner; /// }); /// ``` #[stable(feature = "rust1", since = "1.0.0")] -pub struct LocalKey { - // The key itself may be tagged with #[thread_local], and this `Key` is - // stored as a `static`, and it's not valid for a static to reference the - // address of another thread_local static. For this reason we kinda wonkily - // work around this by generating a shim function which will give us the - // address of the inner TLS key at runtime. +pub struct LocalKey { + // This outer `LocalKey` type is what's going to be stored in statics, + // but actual data inside will sometimes be tagged with #[thread_local]. + // It's not valid for a true static to reference a #[thread_local] static, + // so we get around that by exposing an accessor through a layer of function + // indirection (this thunk). // - // This is trivially devirtualizable by LLVM because we never store anything - // to this field and rustc can declare the `static` as constant as well. - inner: fn() -> &'static __KeyInner, + // Note that the thunk is itself unsafe because the returned lifetime of the + // slot where data lives, `'static`, is not actually valid. The lifetime + // here is actually `'thread`! + // + // Although this is an extra layer of indirection, it should in theory be + // trivially devirtualizable by LLVM because the value of `inner` never + // changes and the constant should be readonly within a crate. This mainly + // only runs into problems when TLS statics are exported across crates. + inner: unsafe fn() -> Option<&'static UnsafeCell>>, // initialization routine to invoke to create a value init: fn() -> T, } -// Macro pain #4586: -// -// When cross compiling, rustc will load plugins and macros from the *host* -// platform before search for macros from the target platform. This is primarily -// done to detect, for example, plugins. Ideally the macro below would be -// defined once per module below, but unfortunately this means we have the -// following situation: -// -// 1. We compile libstd for x86_64-unknown-linux-gnu, this thread_local!() macro -// will inject #[thread_local] statics. -// 2. We then try to compile a program for arm-linux-androideabi -// 3. The compiler has a host of linux and a target of android, so it loads -// macros from the *linux* libstd. -// 4. The macro generates a #[thread_local] field, but the android libstd does -// not use #[thread_local] -// 5. Compile error about structs with wrong fields. -// -// To get around this, we're forced to inject the #[cfg] logic into the macro -// itself. Woohoo. - /// Declare a new thread local storage key of type `std::thread::LocalKey`. /// /// See [LocalKey documentation](thread/struct.LocalKey.html) for more @@ -103,36 +85,14 @@ pub struct LocalKey { #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] #[allow_internal_unstable] -#[cfg(not(no_elf_tls))] macro_rules! thread_local { (static $name:ident: $t:ty = $init:expr) => ( static $name: $crate::thread::LocalKey<$t> = - __thread_local_inner!($t, $init, - #[cfg_attr(all(any(target_os = "macos", target_os = "linux"), - not(target_arch = "aarch64")), - thread_local)]); + __thread_local_inner!($t, $init); ); (pub static $name:ident: $t:ty = $init:expr) => ( pub static $name: $crate::thread::LocalKey<$t> = - __thread_local_inner!($t, $init, - #[cfg_attr(all(any(target_os = "macos", target_os = "linux"), - not(target_arch = "aarch64")), - thread_local)]); - ); -} - -#[macro_export] -#[stable(feature = "rust1", since = "1.0.0")] -#[allow_internal_unstable] -#[cfg(no_elf_tls)] -macro_rules! thread_local { - (static $name:ident: $t:ty = $init:expr) => ( - static $name: $crate::thread::LocalKey<$t> = - __thread_local_inner!($t, $init, #[]); - ); - (pub static $name:ident: $t:ty = $init:expr) => ( - pub static $name: $crate::thread::LocalKey<$t> = - __thread_local_inner!($t, $init, #[]); + __thread_local_inner!($t, $init); ); } @@ -143,12 +103,25 @@ macro_rules! thread_local { #[macro_export] #[allow_internal_unstable] macro_rules! __thread_local_inner { - ($t:ty, $init:expr, #[$($attr:meta),*]) => {{ - $(#[$attr])* - static __KEY: $crate::thread::__LocalKeyInner<$t> = - $crate::thread::__LocalKeyInner::new(); + ($t:ty, $init:expr) => {{ fn __init() -> $t { $init } - fn __getit() -> &'static $crate::thread::__LocalKeyInner<$t> { &__KEY } + + unsafe fn __getit() -> $crate::option::Option< + &'static $crate::cell::UnsafeCell< + $crate::option::Option<$t>>> + { + #[thread_local] + #[cfg(target_thread_local)] + static __KEY: $crate::thread::__ElfLocalKeyInner<$t> = + $crate::thread::__ElfLocalKeyInner::new(); + + #[cfg(not(target_thread_local))] + static __KEY: $crate::thread::__OsLocalKeyInner<$t> = + $crate::thread::__OsLocalKeyInner::new(); + + __KEY.get() + } + $crate::thread::LocalKey::new(__getit, __init) }} } @@ -190,11 +163,11 @@ impl LocalKey { #[unstable(feature = "thread_local_internals", reason = "recently added to create a key", issue = "0")] - pub const fn new(inner: fn() -> &'static __KeyInner, + pub const fn new(inner: unsafe fn() -> Option<&'static UnsafeCell>>, init: fn() -> T) -> LocalKey { LocalKey { inner: inner, - init: init + init: init, } } @@ -211,10 +184,10 @@ impl LocalKey { #[stable(feature = "rust1", since = "1.0.0")] pub fn with(&'static self, f: F) -> R where F: FnOnce(&T) -> R { - let slot = (self.inner)(); unsafe { - let slot = slot.get().expect("cannot access a TLS value during or \ - after it is destroyed"); + let slot = (self.inner)(); + let slot = slot.expect("cannot access a TLS value during or \ + after it is destroyed"); f(match *slot.get() { Some(ref inner) => inner, None => self.init(slot), @@ -270,7 +243,7 @@ impl LocalKey { issue = "27716")] pub fn state(&'static self) -> LocalKeyState { unsafe { - match (self.inner)().get() { + match (self.inner)() { Some(cell) => { match *cell.get() { Some(..) => LocalKeyState::Valid, @@ -283,11 +256,9 @@ impl LocalKey { } } -#[cfg(all(any(target_os = "macos", target_os = "linux"), - not(target_arch = "aarch64"), - not(no_elf_tls)))] +#[cfg(target_thread_local)] #[doc(hidden)] -mod imp { +pub mod elf { use cell::{Cell, UnsafeCell}; use intrinsics; use ptr; @@ -431,11 +402,8 @@ mod imp { } } -#[cfg(any(not(any(target_os = "macos", target_os = "linux")), - target_arch = "aarch64", - no_elf_tls))] #[doc(hidden)] -mod imp { +pub mod os { use prelude::v1::*; use cell::{Cell, UnsafeCell}; diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index d4f04d5d94f..0e525f39421 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -191,7 +191,10 @@ pub use self::local::{LocalKey, LocalKeyState}; pub use self::scoped_tls::ScopedKey; #[unstable(feature = "libstd_thread_internals", issue = "0")] -#[doc(hidden)] pub use self::local::__KeyInner as __LocalKeyInner; +#[cfg(target_thread_local)] +#[doc(hidden)] pub use self::local::elf::Key as __ElfLocalKeyInner; +#[unstable(feature = "libstd_thread_internals", issue = "0")] +#[doc(hidden)] pub use self::local::os::Key as __OsLocalKeyInner; #[unstable(feature = "libstd_thread_internals", issue = "0")] #[doc(hidden)] pub use self::scoped_tls::__KeyInner as __ScopedKeyInner; diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index caad7c6e7f5..4ea0fd76fea 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -238,7 +238,7 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Option, Status ("type_ascription", "1.6.0", Some(23416), Active), // Allows cfg(target_thread_local) - ("cfg_target_thread_local", "1.7.0", Some(26581), Active), + ("cfg_target_thread_local", "1.7.0", Some(29594), Active), ]; // (changing above list without updating src/doc/reference.md makes @cmr sad)