From eb4c81d60e2a4decaffdbadccb6099db5e07e8ea Mon Sep 17 00:00:00 2001 From: Alastair Houghton Date: Wed, 4 May 2022 11:08:09 +0100 Subject: [PATCH] [Threading] Don't use TLS keys as template arguments. There's no guarantee that e.g. pthread_key_t is an integral type. It could be some kind of struct, or some other thing that isn't valid as a template argument. rdar://90776105 --- include/swift/Threading/Impl.h | 2 + include/swift/Threading/Impl/C11.h | 10 ++-- include/swift/Threading/Impl/Darwin.h | 51 ++++++++++++------- include/swift/Threading/Impl/Linux.h | 10 ++-- include/swift/Threading/Impl/Pthreads.h | 10 ++-- include/swift/Threading/Impl/Win32.h | 10 ++-- include/swift/Threading/TLSKeys.h | 29 +++++++++++ include/swift/Threading/ThreadLocalStorage.h | 37 ++++++++------ stdlib/public/Concurrency/Actor.cpp | 8 +-- stdlib/public/Concurrency/TaskLocal.cpp | 7 ++- stdlib/public/runtime/SwiftTLSContext.cpp | 8 +-- stdlib/public/stubs/ThreadLocalStorage.cpp | 8 +-- .../DynamicReplaceable.cpp | 2 +- 13 files changed, 122 insertions(+), 70 deletions(-) create mode 100644 include/swift/Threading/TLSKeys.h diff --git a/include/swift/Threading/Impl.h b/include/swift/Threading/Impl.h index 6d660969802..f25c6fb6330 100644 --- a/include/swift/Threading/Impl.h +++ b/include/swift/Threading/Impl.h @@ -18,6 +18,8 @@ #ifndef SWIFT_THREADING_IMPL_H #define SWIFT_THREADING_IMPL_H +#include "TLSKeys.h" + // Try to autodetect if we aren't told what to do #if !SWIFT_THREADING_NONE && !SWIFT_THREADING_DARWIN && \ !SWIFT_THREADING_LINUX && !SWIFT_THREADING_PTHREADS && \ diff --git a/include/swift/Threading/Impl/C11.h b/include/swift/Threading/Impl/C11.h index 16aeca2b9ef..ae080709eb2 100644 --- a/include/swift/Threading/Impl/C11.h +++ b/include/swift/Threading/Impl/C11.h @@ -164,16 +164,16 @@ inline void once_impl(once_t &predicate, void (*fn)(void *), void *context) { #define SWIFT_THREAD_LOCAL thread_local #endif -using tls_key = ::tss_t; -using tls_dtor = void (*)(void *); +using tls_key_t = ::tss_t; +using tls_dtor_t = void (*)(void *); -inline bool tls_alloc(tls_key &key, tls_dtor dtor) { +inline bool tls_alloc(tls_key_t &key, tls_dtor_t dtor) { return ::tss_create(&key, dtor) == thrd_success; } -inline void *tls_get(tls_key key) { return ::tss_get(key); } +inline void *tls_get(tls_key_t key) { return ::tss_get(key); } -inline void tls_set(tls_key key, void *ptr) { ::tss_set(key, ptr); } +inline void tls_set(tls_key_t key, void *ptr) { ::tss_set(key, ptr); } } // namespace threading_impl diff --git a/include/swift/Threading/Impl/Darwin.h b/include/swift/Threading/Impl/Darwin.h index df27dd42f62..4f68e973be8 100644 --- a/include/swift/Threading/Impl/Darwin.h +++ b/include/swift/Threading/Impl/Darwin.h @@ -128,45 +128,60 @@ inline void _pthread_setspecific_direct(pthread_key_t k, void *v) { } #endif -#define SWIFT_RUNTIME_TLS_KEY __PTK_FRAMEWORK_SWIFT_KEY0 -#define SWIFT_STDLIB_TLS_KEY __PTK_FRAMEWORK_SWIFT_KEY1 -#define SWIFT_COMPATIBILITY_50_TLS_KEY __PTK_FRAMEWORK_SWIFT_KEY2 -#define SWIFT_CONCURRENCY_TASK_KEY __PTK_FRAMEWORK_SWIFT_KEY3 -#define SWIFT_CONCURRENCY_EXECUTOR_TRACKING_INFO_KEY __PTK_FRAMEWORK_SWIFT_KEY4 -#define SWIFT_CONCURRENCY_FALLBACK_TASK_LOCAL_STORAGE_KEY \ - __PTK_FRAMEWORK_SWIFT_KEY5 -#define SWIFT_RESERVED_TLS_KEY_6 __PTK_FRAMEWORK_SWIFT_KEY6 -#define SWIFT_RESERVED_TLS_KEY_7 __PTK_FRAMEWORK_SWIFT_KEY7 -#define SWIFT_RESERVED_TLS_KEY_8 __PTK_FRAMEWORK_SWIFT_KEY8 -#define SWIFT_RESERVED_TLS_KEY_9 __PTK_FRAMEWORK_SWIFT_KEY9 - #define SWIFT_TLS_DECLARE_DTOR(name) void name(void *) -using tls_key = pthread_key_t; -using tls_dtor = void (*)(void *); +using tls_key_t = pthread_key_t; +using tls_dtor_t = void (*)(void *); -inline bool tls_init(tls_key key, tls_dtor dtor) { +inline tls_key_t tls_get_key(tls_key k) { + switch (k) { + case tls_key::runtime: + return __PTK_FRAMEWORK_SWIFT_KEY0; + case tls_key::stdlib: + return __PTK_FRAMEWORK_SWIFT_KEY1; + case tls_key::compatibility50: + return __PTK_FRAMEWORK_SWIFT_KEY2; + case tls_key::concurrency_task: + return __PTK_FRAMEWORK_SWIFT_KEY3; + case tls_key::concurrency_executor_tracking_info: + return __PTK_FRAMEWORK_SWIFT_KEY4; + case tls_key::concurrency_fallback: + return __PTK_FRAMEWORK_SWIFT_KEY5; + } +} + +inline bool tls_init(tls_key_t key, tls_dtor_t dtor) { return pthread_key_init_np(key, dtor) == 0; } -inline bool tls_alloc(tls_key &key, tls_dtor dtor) { +inline bool tls_init(tls_key key, tls_dtor_t dtor) { + return tls_init(tls_get_key(key), dtor); +} + +inline bool tls_alloc(tls_key_t &key, tls_dtor_t dtor) { return pthread_key_create(&key, dtor) == 0; } -inline void *tls_get(tls_key key) { +inline void *tls_get(tls_key_t key) { if (_pthread_has_direct_tsd()) return _pthread_getspecific_direct(key); else return pthread_getspecific(key); } -inline void tls_set(tls_key key, void *value) { +inline void *tls_get(tls_key key) { return tls_get(tls_get_key(key)); } + +inline void tls_set(tls_key_t key, void *value) { if (_pthread_has_direct_tsd()) _pthread_setspecific_direct(key, value); else pthread_setspecific(key, value); } +inline void tls_set(tls_key key, void *value) { + tls_set(tls_get_key(key), value); +} + } // namespace threading_impl } // namespace swift diff --git a/include/swift/Threading/Impl/Linux.h b/include/swift/Threading/Impl/Linux.h index c447bcb80e8..000a8bf48b3 100644 --- a/include/swift/Threading/Impl/Linux.h +++ b/include/swift/Threading/Impl/Linux.h @@ -149,16 +149,16 @@ inline void once_impl(once_t &predicate, void (*fn)(void *), void *context) { #define SWIFT_THREAD_LOCAL thread_local #endif -using tls_key = pthread_key_t; -using tls_dtor = void (*)(void *); +using tls_key_t = pthread_key_t; +using tls_dtor_t = void (*)(void *); -inline bool tls_alloc(tls_key &key, tls_dtor dtor) { +inline bool tls_alloc(tls_key_t &key, tls_dtor_t dtor) { return pthread_key_create(&key, dtor) == 0; } -inline void *tls_get(tls_key key) { return pthread_getspecific(key); } +inline void *tls_get(tls_key_t key) { return pthread_getspecific(key); } -inline void tls_set(tls_key key, void *value) { +inline void tls_set(tls_key_t key, void *value) { pthread_setspecific(key, value); } diff --git a/include/swift/Threading/Impl/Pthreads.h b/include/swift/Threading/Impl/Pthreads.h index 83139ab07f4..f07dd47b084 100644 --- a/include/swift/Threading/Impl/Pthreads.h +++ b/include/swift/Threading/Impl/Pthreads.h @@ -145,16 +145,16 @@ inline void once_impl(once_t &predicate, void (*fn)(void *), void *context) { #define SWIFT_THREAD_LOCAL thread_local #endif -using tls_key = pthread_key_t; -using tls_dtor = void (*)(void *); +using tls_key_t = pthread_key_t; +using tls_dtor_t = void (*)(void *); -inline bool tls_alloc(tls_key &key, tls_dtor dtor) { +inline bool tls_alloc(tls_key_t &key, tls_dtor_t dtor) { return pthread_key_create(&key, dtor) == 0; } -inline void *tls_get(tls_key key) { return pthread_getspecific(key); } +inline void *tls_get(tls_key_t key) { return pthread_getspecific(key); } -inline void tls_set(tls_key key, void *value) { +inline void tls_set(tls_key_t key, void *value) { pthread_setspecific(key, value); } diff --git a/include/swift/Threading/Impl/Win32.h b/include/swift/Threading/Impl/Win32.h index 3f0d7258af9..ae2ed58a5eb 100644 --- a/include/swift/Threading/Impl/Win32.h +++ b/include/swift/Threading/Impl/Win32.h @@ -109,17 +109,17 @@ inline void once_impl(once_t &predicate, void (*fn)(void *), void *context) { #define SWIFT_TLS_DECLARE_DTOR(name) void NTAPI name(void *) -using tls_key = ::DWORD; -using tls_dtor = ::PFLS_CALLBACK_FUNCTION; +using tls_key_t = ::DWORD; +using tls_dtor_t = ::PFLS_CALLBACK_FUNCTION; -inline bool tls_alloc(tls_key &key, tls_dtor dtor) { +inline bool tls_alloc(tls_key_t &key, tls_dtor_t dtor) { key = ::FlsAlloc(dtor); return key != FLS_OUT_OF_INDEXES; } -inline void *tls_get(tls_key key) { return ::FlsGetValue(key); } +inline void *tls_get(tls_key_t key) { return ::FlsGetValue(key); } -inline void tls_set(tls_key key, void *value) { ::FlsSetValue(key, value); } +inline void tls_set(tls_key_t key, void *value) { ::FlsSetValue(key, value); } } // namespace threading_impl diff --git a/include/swift/Threading/TLSKeys.h b/include/swift/Threading/TLSKeys.h new file mode 100644 index 00000000000..a9feaae389a --- /dev/null +++ b/include/swift/Threading/TLSKeys.h @@ -0,0 +1,29 @@ +//===--- TLSKeys.h - Reserved TLS keys ------------------------ -*- C++ -*-===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +#ifndef SWIFT_THREADING_TLSKEYS_H +#define SWIFT_THREADING_TLSKEYS_H + +namespace swift { + +enum class tls_key { + runtime, + stdlib, + compatibility50, + concurrency_task, + concurrency_executor_tracking_info, + concurrency_fallback +}; + +} // namespace swift + +#endif // SWIFT_THREADING_TLSKEYS_H diff --git a/include/swift/Threading/ThreadLocalStorage.h b/include/swift/Threading/ThreadLocalStorage.h index e681fc1a18e..0bbf0fae55b 100644 --- a/include/swift/Threading/ThreadLocalStorage.h +++ b/include/swift/Threading/ThreadLocalStorage.h @@ -1,5 +1,4 @@ -//===--- ThreadLocalStorage.h - Thread-local storage interface. --*- C++ -//-*-===// +//===--- ThreadLocalStorage.h - Thread-local storage interface. -*- C++ -*-===// // // This source file is part of the Swift.org open source project // @@ -19,23 +18,25 @@ #include "Errors.h" #include "Impl.h" #include "Once.h" +#include "TLSKeys.h" namespace swift { // -- Low-level TLS functions ------------------------------------------------- #if !SWIFT_THREADING_NONE -using tls_key = threading_impl::tls_key; -using tls_dtor = threading_impl::tls_dtor; +using tls_key_t = threading_impl::tls_key_t; +using tls_dtor_t = threading_impl::tls_dtor_t; #if SWIFT_THREADING_USE_RESERVED_TLS_KEYS +using threading_impl::tls_get_key; using threading_impl::tls_init; /// tls_init_once() - Initialize TLS, once only -inline void tls_init_once(once_t &token, tls_key key, tls_dtor dtor) { +inline void tls_init_once(once_t &token, tls_key_t key, tls_dtor_t dtor) { const struct tls_init_info { - tls_key &k; - tls_dtor d; + tls_key_t &k; + tls_dtor_t d; } info = {key, dtor}; once( token, @@ -47,6 +48,10 @@ inline void tls_init_once(once_t &token, tls_key key, tls_dtor dtor) { }, (void *)&info); } + +inline void tls_init_once(once_t &token, tls_key key, tls_dtor_t dtor) { + tls_init_once(token, tls_get_key(key), dtor); +} #endif // SWIFT_THREADING_USE_RESERVED_TLS_KEYS using threading_impl::tls_alloc; @@ -54,10 +59,10 @@ using threading_impl::tls_get; using threading_impl::tls_set; /// tls_alloc_once() - Allocate TLS key, once only -inline void tls_alloc_once(once_t &token, tls_key &key, tls_dtor dtor) { +inline void tls_alloc_once(once_t &token, tls_key_t &key, tls_dtor_t dtor) { const struct tls_init_info { - tls_key &k; - tls_dtor d; + tls_key_t &k; + tls_dtor_t d; } info = {key, dtor}; once( token, @@ -117,14 +122,14 @@ class ThreadLocalKey { // We rely on the zero-initialization of objects with static storage // duration. once_t onceFlag; - tls_key key; + tls_key_t key; public: - threading_impl::tls_key getKey() { + threading_impl::tls_key_t getKey() { once( onceFlag, [](void *ctx) { - tls_key *pkey = reinterpret_cast(ctx); + tls_key_t *pkey = reinterpret_cast(ctx); tls_alloc(*pkey, nullptr); }, &key); @@ -132,13 +137,15 @@ public: } }; +#if SWIFT_THREADING_USE_RESERVED_TLS_KEYS // A type representing a constant TLS key, for use on platforms // that provide reserved keys. template class ConstantThreadLocalKey { public: - tls_key getKey() { return constantKey; } + tls_key_t getKey() { return tls_get_key(constantKey); } }; +#endif template class ThreadLocal { @@ -172,7 +179,7 @@ public: /// /// For example /// -/// static SWIFT_THREAD_LOCAL_TYPE(int, SWIFT_RESERVED_TLS_KEY_9) frobble; +/// static SWIFT_THREAD_LOCAL_TYPE(int, SWIFT_RESERVED_TLS_KEY_T_9) frobble; /// /// Because of the fallback path, the default-initialization of the /// type must be equivalent to a bitwise zero-initialization, and the diff --git a/stdlib/public/Concurrency/Actor.cpp b/stdlib/public/Concurrency/Actor.cpp index b81e4b71c32..ddeef857879 100644 --- a/stdlib/public/Concurrency/Actor.cpp +++ b/stdlib/public/Concurrency/Actor.cpp @@ -108,7 +108,7 @@ class ExecutorTrackingInfo { /// separate thread-local variable (or whatever is most efficient /// on the target platform). static SWIFT_THREAD_LOCAL_TYPE(Pointer, - SWIFT_CONCURRENCY_EXECUTOR_TRACKING_INFO_KEY) + tls_key::concurrency_executor_tracking_info) ActiveInfoInThread; /// The active executor. @@ -176,7 +176,7 @@ class ActiveTask { /// A thread-local variable pointing to the active tracking /// information about the current thread, if any. static SWIFT_THREAD_LOCAL_TYPE(Pointer, - SWIFT_CONCURRENCY_TASK_KEY) Value; + tls_key::concurrency_task) Value; public: static void set(AsyncTask *task) { Value.set(task); } @@ -184,11 +184,11 @@ public: }; /// Define the thread-locals. -SWIFT_THREAD_LOCAL_TYPE(Pointer, SWIFT_CONCURRENCY_TASK_KEY) +SWIFT_THREAD_LOCAL_TYPE(Pointer, tls_key::concurrency_task) ActiveTask::Value; SWIFT_THREAD_LOCAL_TYPE(Pointer, - SWIFT_CONCURRENCY_EXECUTOR_TRACKING_INFO_KEY) + tls_key::concurrency_executor_tracking_info) ExecutorTrackingInfo::ActiveInfoInThread; } // end anonymous namespace diff --git a/stdlib/public/Concurrency/TaskLocal.cpp b/stdlib/public/Concurrency/TaskLocal.cpp index ea48ee1cadd..c6799ee1f06 100644 --- a/stdlib/public/Concurrency/TaskLocal.cpp +++ b/stdlib/public/Concurrency/TaskLocal.cpp @@ -54,9 +54,8 @@ template struct Pointer { /// THIS IS RUNTIME INTERNAL AND NOT ABI. class FallbackTaskLocalStorage { - static SWIFT_THREAD_LOCAL_TYPE( - Pointer, - SWIFT_CONCURRENCY_FALLBACK_TASK_LOCAL_STORAGE_KEY) Value; + static SWIFT_THREAD_LOCAL_TYPE(Pointer, + tls_key::concurrency_fallback) Value; public: static void set(TaskLocal::Storage *task) { Value.set(task); } @@ -65,7 +64,7 @@ public: /// Define the thread-locals. SWIFT_THREAD_LOCAL_TYPE(Pointer, - SWIFT_CONCURRENCY_FALLBACK_TASK_LOCAL_STORAGE_KEY) + tls_key::concurrency_fallback) FallbackTaskLocalStorage::Value; // ==== ABI -------------------------------------------------------------------- diff --git a/stdlib/public/runtime/SwiftTLSContext.cpp b/stdlib/public/runtime/SwiftTLSContext.cpp index ff5b248e0b4..8a627566cb5 100644 --- a/stdlib/public/runtime/SwiftTLSContext.cpp +++ b/stdlib/public/runtime/SwiftTLSContext.cpp @@ -24,17 +24,17 @@ SwiftTLSContext &SwiftTLSContext::get() { // If we have reserved keys, use those SwiftTLSContext *ctx = - static_cast(swift::tls_get(SWIFT_RUNTIME_TLS_KEY)); + static_cast(swift::tls_get(swift::tls_key::runtime)); if (ctx) return *ctx; static swift::once_t token; - swift::tls_init_once(token, SWIFT_RUNTIME_TLS_KEY, [](void *pointer) { + swift::tls_init_once(token, swift::tls_key::runtime, [](void *pointer) { delete static_cast(pointer); }); ctx = new SwiftTLSContext(); - swift::tls_set(SWIFT_RUNTIME_TLS_KEY, ctx); + swift::tls_set(swift::tls_key::runtime, ctx); return *ctx; #elif defined(SWIFT_THREAD_LOCAL) @@ -47,7 +47,7 @@ SwiftTLSContext &SwiftTLSContext::get() { #else // Otherwise, allocate ourselves a key and use that - static swift::tls_key runtimeKey; + static swift::tls_key_t runtimeKey; static swift::once_t token; swift::tls_alloc_once(token, runtimeKey, [](void *pointer) { diff --git a/stdlib/public/stubs/ThreadLocalStorage.cpp b/stdlib/public/stubs/ThreadLocalStorage.cpp index f347bd2c2bc..d357263fc9d 100644 --- a/stdlib/public/stubs/ThreadLocalStorage.cpp +++ b/stdlib/public/stubs/ThreadLocalStorage.cpp @@ -35,22 +35,22 @@ _swift_stdlib_threadLocalStorageGet(void) { #elif SWIFT_THREADING_USE_RESERVED_TLS_KEYS // If we have reserved keys, use those - void *value = swift::tls_get(SWIFT_STDLIB_TLS_KEY); + void *value = swift::tls_get(swift::tls_key::stdlib); if (value) return value; static swift::once_t token; - swift::tls_init_once(token, SWIFT_STDLIB_TLS_KEY, + swift::tls_init_once(token, swift::tls_key::stdlib, [](void *pointer) { _stdlib_destroyTLS(pointer); }); value = _stdlib_createTLS(); - swift::tls_set(SWIFT_STDLIB_TLS_KEY, value); + swift::tls_set(swift::tls_key::stdlib, value); return value; #else // Threaded, but not using reserved keys // Register a key and use it - static swift::tls_key key; + static swift::tls_key_t key; static swift::once_t token; swift::tls_alloc_once(token, key, diff --git a/stdlib/toolchain/CompatibilityDynamicReplacements/DynamicReplaceable.cpp b/stdlib/toolchain/CompatibilityDynamicReplacements/DynamicReplaceable.cpp index a9b2668432f..63785be88af 100644 --- a/stdlib/toolchain/CompatibilityDynamicReplacements/DynamicReplaceable.cpp +++ b/stdlib/toolchain/CompatibilityDynamicReplacements/DynamicReplaceable.cpp @@ -29,7 +29,7 @@ namespace { // unavoidable in the general case, so turn off the warning just for this line. #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wglobal-constructors" -SWIFT_THREAD_LOCAL_TYPE(uintptr_t, SWIFT_COMPATIBILITY_50_TLS_KEY) compat50Key; +SWIFT_THREAD_LOCAL_TYPE(uintptr_t, tls_key::compatibility50) compat50Key; #pragma clang diagnostic pop } // namespace