Merge "Move the trivial __set_tls() implementations to "bionic_tls.h"." into main
diff --git a/libc/bionic/pthread_key.cpp b/libc/bionic/pthread_key.cpp
index 53f0f11..f8c765d 100644
--- a/libc/bionic/pthread_key.cpp
+++ b/libc/bionic/pthread_key.cpp
@@ -83,11 +83,18 @@
size_t called_destructor_count = 0;
for (size_t i = 0; i < BIONIC_PTHREAD_KEY_COUNT; ++i) {
uintptr_t seq = atomic_load_explicit(&key_map[i].seq, memory_order_relaxed);
- if (SeqOfKeyInUse(seq) && seq == key_data[i].seq && key_data[i].data != nullptr) {
- // Other threads may be calling pthread_key_delete/pthread_key_create while current thread
- // is exiting. So we need to ensure we read the right key_destructor.
+ if (SeqOfKeyInUse(seq) && seq == key_data[i].seq) {
+ // POSIX explicitly says that the destructor is only called if the
+ // thread has a non-null value for the key.
+ if (key_data[i].data == nullptr) {
+ continue;
+ }
+
+ // Other threads can call pthread_key_delete()/pthread_key_create()
+ // while this thread is exiting, so we need to ensure we read the right
+ // key_destructor.
// We can rely on a user-established happens-before relationship between the creation and
- // use of pthread key to ensure that we're not getting an earlier key_destructor.
+ // use of a pthread key to ensure that we're not getting an earlier key_destructor.
// To avoid using the key_destructor of the newly created key in the same slot, we need to
// recheck the sequence number after reading key_destructor. As a result, we either see the
// right key_destructor, or the sequence number must have changed when we reread it below.
@@ -107,7 +114,6 @@
// function is responsible for manually releasing the corresponding data.
void* data = key_data[i].data;
key_data[i].data = nullptr;
-
(*key_destructor)(data);
++called_destructor_count;
}
@@ -163,13 +169,13 @@
key &= ~KEY_VALID_FLAG;
uintptr_t seq = atomic_load_explicit(&key_map[key].seq, memory_order_relaxed);
pthread_key_data_t* data = &get_thread_key_data()[key];
- // It is user's responsibility to synchornize between the creation and use of pthread keys,
+ // It is the user's responsibility to synchronize between the creation and use of pthread keys,
// so we use memory_order_relaxed when checking the sequence number.
if (__predict_true(SeqOfKeyInUse(seq) && data->seq == seq)) {
return data->data;
}
- // We arrive here when current thread holds the seq of an deleted pthread key. So the
- // data is for the deleted pthread key, and should be cleared.
+ // We arrive here when the current thread holds the seq of a deleted pthread key.
+ // The data is for the deleted pthread key, and should be cleared.
data->data = nullptr;
return nullptr;
}
diff --git a/libc/include/bits/fortify/string.h b/libc/include/bits/fortify/string.h
index 6f0ee4a..0b4b70b 100644
--- a/libc/include/bits/fortify/string.h
+++ b/libc/include/bits/fortify/string.h
@@ -42,7 +42,6 @@
size_t __strlcat_chk(char* _Nonnull, const char* _Nonnull, size_t, size_t);
#if defined(__BIONIC_FORTIFY)
-void* _Nullable __memrchr_real(const void* _Nonnull, int, size_t) __RENAME(memrchr);
#if __BIONIC_FORTIFY_RUNTIME_CHECKS_ENABLED
/* No diag -- clang diagnoses misuses of this on its own. */
@@ -153,6 +152,8 @@
return __memchr_chk(s, c, n, bos);
}
+void* _Nullable __memrchr_real(const void* _Nonnull, int, size_t) __RENAME(memrchr);
+
__BIONIC_FORTIFY_INLINE
void* _Nullable __memrchr_fortify(const void* _Nonnull const __pass_object_size s, int c, size_t n) __overloadable {
size_t bos = __bos(s);
diff --git a/libc/include/pthread.h b/libc/include/pthread.h
index f3b7068..5a3376a 100644
--- a/libc/include/pthread.h
+++ b/libc/include/pthread.h
@@ -187,8 +187,7 @@
* different language, you should consider similar implementation choices and
* avoid a direct one-to-one mapping from thread locals to pthread keys.
*
- * On Android, the destructor function is called even for null values,
- * even though POSIX specifies that it is only called for non-null values.
+ * The destructor function is only called for non-null values.
*
* Returns 0 on success and returns an error number on failure.
*/
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index 5ce7d4d..680ef6e 100644
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -59,6 +59,39 @@
ASSERT_EQ(EINVAL, pthread_key_delete(key));
}
+static std::vector<void*> example_key_destructor_data;
+static pthread_key_t example_key;
+static void example_key_destructor(void *data) {
+ // By the time the destructor function is running,
+ // this thread's value for the key should have been zeroed.
+ ASSERT_EQ(NULL, pthread_getspecific(example_key));
+
+ // Store the value so we can check we got the expected result.
+ example_key_destructor_data.push_back(data);
+}
+
+TEST(pthread, pthread_key_destructors) {
+ ASSERT_EQ(0, pthread_key_create(&example_key, example_key_destructor));
+
+ // Check that the destructor isn't called for a default null value.
+ std::thread([]() {}).join();
+ ASSERT_TRUE(example_key_destructor_data.empty());
+
+ // Check that the destructor isn't called for an explicit null value.
+ std::thread([]() {
+ ASSERT_EQ(0, pthread_setspecific(example_key, (void*) 1234));
+ ASSERT_EQ(0, pthread_setspecific(example_key, nullptr));
+ }).join();
+ ASSERT_TRUE(example_key_destructor_data.empty());
+
+ // Check that the destructor is called for a non-null value.
+ std::thread([]() { ASSERT_EQ(0, pthread_setspecific(example_key, (void*) 1234)); }).join();
+ ASSERT_EQ(1u, example_key_destructor_data.size());
+ ASSERT_EQ((void*) 1234, example_key_destructor_data[0]);
+
+ ASSERT_EQ(0, pthread_key_delete(example_key));
+}
+
TEST(pthread, pthread_keys_max) {
// POSIX says PTHREAD_KEYS_MAX should be at least _POSIX_THREAD_KEYS_MAX.
ASSERT_GE(PTHREAD_KEYS_MAX, _POSIX_THREAD_KEYS_MAX);
diff --git a/tests/pty_test.cpp b/tests/pty_test.cpp
index d5d8994..6e3d2c7 100644
--- a/tests/pty_test.cpp
+++ b/tests/pty_test.cpp
@@ -103,6 +103,8 @@
arg->finished = true;
}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wcast-function-type-mismatch"
TEST(pty, bug_28979140) {
// This test is to test a kernel bug, which uses a lock free ring-buffer to
// pass data through a raw pty, but missing necessary memory barriers.
@@ -164,3 +166,4 @@
ASSERT_TRUE(arg.matched);
close(pty);
}
+#pragma clang diagnostic pop
\ No newline at end of file
diff --git a/tests/stdio_ext_test.cpp b/tests/stdio_ext_test.cpp
index dce1a66..56483f5 100644
--- a/tests/stdio_ext_test.cpp
+++ b/tests/stdio_ext_test.cpp
@@ -241,6 +241,8 @@
funlockfile(stdout);
}
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wcast-function-type-mismatch"
TEST(stdio_ext, __fsetlocking_BYCALLER) {
// Check if users can use flockfile/funlockfile to protect stdio operations.
int old_state = __fsetlocking(stdout, FSETLOCKING_BYCALLER);
@@ -255,3 +257,4 @@
ASSERT_EQ(0, pthread_join(thread, nullptr));
__fsetlocking(stdout, old_state);
}
+#pragma clang diagnostic pop