Merge "Revert "Replace usage of ICU4C in bionic with ICU4X"" into main
diff --git a/libc/Android.bp b/libc/Android.bp
index b5ff680..18f1ad7 100644
--- a/libc/Android.bp
+++ b/libc/Android.bp
@@ -223,12 +223,6 @@
         "bionic/getauxval.cpp",
     ],
     arch: {
-        arm64: {
-            srcs: ["arch-arm64/bionic/__set_tls.c"],
-        },
-        riscv64: {
-            srcs: ["arch-riscv64/bionic/__set_tls.c"],
-        },
         x86: {
             srcs: [
                 "arch-x86/bionic/__libc_init_sysinfo.cpp",
@@ -236,9 +230,6 @@
                 "arch-x86/bionic/__set_tls.cpp",
             ],
         },
-        x86_64: {
-            srcs: ["arch-x86_64/bionic/__set_tls.c"],
-        },
     },
 
     defaults: ["libc_defaults"],
diff --git a/libc/arch-arm64/bionic/__set_tls.c b/libc/arch-arm64/bionic/__set_tls.c
deleted file mode 100644
index 0d88d11..0000000
--- a/libc/arch-arm64/bionic/__set_tls.c
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Copyright (C) 2013 The Android Open Source Project
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *  * Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- *  * Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in
- *    the documentation and/or other materials provided with the
- *    distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
- * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <sys/cdefs.h>
-
-__LIBC_HIDDEN__ void __set_tls(void* tls) {
-  asm("msr tpidr_el0, %0" : : "r" (tls));
-}
diff --git a/libc/arch-riscv64/bionic/__set_tls.c b/libc/arch-riscv64/bionic/__set_tls.c
deleted file mode 100644
index 57383ab..0000000
--- a/libc/arch-riscv64/bionic/__set_tls.c
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Copyright (C) 2022 The Android Open Source Project
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *  * Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- *  * Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in
- *    the documentation and/or other materials provided with the
- *    distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
- * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <sys/cdefs.h>
-
-__LIBC_HIDDEN__ void __set_tls(void* tls) {
-  asm("mv tp, %0" : : "r"(tls));
-}
diff --git a/libc/arch-x86_64/bionic/__set_tls.c b/libc/arch-x86_64/bionic/__set_tls.c
deleted file mode 100644
index 9460a03..0000000
--- a/libc/arch-x86_64/bionic/__set_tls.c
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Copyright (C) 2013 The Android Open Source Project
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *  * Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- *  * Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in
- *    the documentation and/or other materials provided with the
- *    distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
- * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include <sys/cdefs.h>
-
-// ARCH_SET_FS is not exposed via <sys/prctl.h> or <linux/prctl.h>.
-#include <asm/prctl.h>
-
-extern int arch_prctl(int, unsigned long);
-
-__LIBC_HIDDEN__ int __set_tls(void* ptr) {
-  return arch_prctl(ARCH_SET_FS, (unsigned long) ptr);
-}
diff --git a/libc/bionic/ndk_cruft.cpp b/libc/bionic/ndk_cruft.cpp
index b15a317..a69b77f 100644
--- a/libc/bionic/ndk_cruft.cpp
+++ b/libc/bionic/ndk_cruft.cpp
@@ -28,6 +28,9 @@
 
 // This file perpetuates the mistakes of the past.
 
+// LP64 doesn't need to support any legacy cruft.
+#if !defined(__LP64__)
+
 #include <ctype.h>
 #include <dirent.h>
 #include <errno.h>
@@ -47,10 +50,19 @@
 
 #include "platform/bionic/macros.h"
 
-extern "C" {
+#define __futex_wake __real_futex_wake
+#define __futex_wait __real_futex_wait
+#include "private/bionic_futex.h"
+#undef __futex_wake
+#undef __futex_wait
 
-// LP64 doesn't need to support any legacy cruft.
-#if !defined(__LP64__)
+#define __get_thread __real_get_thread
+#define __get_tls __real_get_tls
+#include "pthread_internal.h"
+#undef __get_thread
+#undef __get_tls
+
+extern "C" {
 
 // By the time any NDK-built code is running, there are plenty of threads.
 int __isthreaded = 1;
@@ -73,8 +85,7 @@
 
 // TODO: does anything still need this?
 void** __get_tls() {
-#include "platform/bionic/tls.h"
-  return __get_tls();
+  return __real_get_tls();
 }
 
 // This non-standard function was in our <string.h> for some reason.
@@ -213,12 +224,6 @@
   return vdprintf(fd, fmt, ap);
 }
 
-#define __futex_wake __real_futex_wake
-#define __futex_wait __real_futex_wait
-#include "private/bionic_futex.h"
-#undef __futex_wake
-#undef __futex_wait
-
 // This used to be in <sys/atomics.h>.
 int __futex_wake(volatile void* ftx, int count) {
   return __real_futex_wake(ftx, count);
@@ -356,14 +361,6 @@
   return malloc(size);
 }
 
-} // extern "C"
-
-#define __get_thread __real_get_thread
-#include "pthread_internal.h"
-#undef __get_thread
-
-extern "C" {
-
 // Various third-party apps contain a backport of our pthread_rwlock implementation that uses this.
 pthread_internal_t* __get_thread() {
   return __real_get_thread();
@@ -388,6 +385,6 @@
     return fwrite(&value, sizeof(value), 1, fp) == 1 ? 0 : EOF;
 }
 
-#endif // !defined (__LP64__)
-
 } // extern "C"
+
+#endif // !defined (__LP64__)
diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h
index cbaa9a6..ae9a791 100644
--- a/libc/bionic/pthread_internal.h
+++ b/libc/bionic/pthread_internal.h
@@ -240,8 +240,6 @@
   tcb->tls_slot(TLS_SLOT_DTV) = &val->generation;
 }
 
-extern "C" __LIBC_HIDDEN__ int __set_tls(void* ptr);
-
 __LIBC_HIDDEN__ void pthread_key_clean_all(void);
 
 // Address space is precious on LP32, so use the minimum unit: one page.
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/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/libc/platform/bionic/tls.h b/libc/platform/bionic/tls.h
index e01eccd..e77e91f 100644
--- a/libc/platform/bionic/tls.h
+++ b/libc/platform/bionic/tls.h
@@ -28,16 +28,78 @@
 
 #pragma once
 
+#include <sys/cdefs.h>
+
 #if defined(__aarch64__)
-# define __get_tls() ({ void** __val; __asm__("mrs %0, tpidr_el0" : "=r"(__val)); __val; })
+
+static inline void** __get_tls() {
+  void** result;
+  __asm__("mrs %0, tpidr_el0" : "=r"(result));
+  return result;
+}
+
+static inline void __set_tls(void* tls) {
+  __asm__("msr tpidr_el0, %0" : : "r" (tls));
+}
+
 #elif defined(__arm__)
-# define __get_tls() ({ void** __val; __asm__("mrc p15, 0, %0, c13, c0, 3" : "=r"(__val)); __val; })
+
+static inline void** __get_tls() {
+  void** result;
+  __asm__("mrc p15, 0, %0, c13, c0, 3" : "=r"(result));
+  return result;
+}
+
+// arm32 requires a syscall to set the thread pointer.
+// By historical accident it's public API, but not in any header except this one.
+__BEGIN_DECLS
+int __set_tls(void* tls);
+__END_DECLS
+
 #elif defined(__i386__)
-# define __get_tls() ({ void** __val; __asm__("movl %%gs:0, %0" : "=r"(__val)); __val; })
+
+static inline void** __get_tls() {
+  void** result;
+  __asm__("movl %%gs:0, %0" : "=r"(result));
+  return result;
+}
+
+// x86 is really hairy, so we keep that out of line.
+__BEGIN_DECLS
+int __set_tls(void* tls);
+__END_DECLS
+
 #elif defined(__riscv)
-# define __get_tls() ({ void** __val; __asm__("mv %0, tp" : "=r"(__val)); __val; })
+
+static inline void** __get_tls() {
+  void** result;
+  __asm__("mv %0, tp" : "=r"(result));
+  return result;
+}
+
+static inline void __set_tls(void* tls) {
+  __asm__("mv tp, %0" : : "r"(tls));
+}
+
 #elif defined(__x86_64__)
-# define __get_tls() ({ void** __val; __asm__("mov %%fs:0, %0" : "=r"(__val)); __val; })
+
+static inline void** __get_tls() {
+  void** result;
+  __asm__("mov %%fs:0, %0" : "=r"(result));
+  return result;
+}
+
+// ARCH_SET_FS is not exposed via <sys/prctl.h> or <linux/prctl.h>.
+#include <asm/prctl.h>
+// This syscall stub is generated but it's not declared in any header.
+__BEGIN_DECLS
+int arch_prctl(int, unsigned long);
+__END_DECLS
+
+static inline int __set_tls(void* tls) {
+  return arch_prctl(ARCH_SET_FS, reinterpret_cast<unsigned long>(tls));
+}
+
 #else
 #error unsupported architecture
 #endif
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..a5e5a57 100644
--- a/tests/pty_test.cpp
+++ b/tests/pty_test.cpp
@@ -139,9 +139,10 @@
   arg.fd = tty;
   arg.data_count = TEST_DATA_COUNT;
   arg.matched = true;
-  ASSERT_EQ(0, pthread_create(&thread, nullptr,
-                              reinterpret_cast<void*(*)(void*)>(PtyReader_28979140),
-                              &arg));
+  ASSERT_EQ(0, pthread_create(&thread, nullptr, [](void* arg)->void* {
+    PtyReader_28979140(static_cast<PtyReader_28979140_Arg*>(arg));
+    return nullptr;
+  }, &arg));
 
   CPU_ZERO(&cpus);
   CPU_SET(arg.main_cpu_id, &cpus);
diff --git a/tests/stdio_ext_test.cpp b/tests/stdio_ext_test.cpp
index dce1a66..dc0e9ef 100644
--- a/tests/stdio_ext_test.cpp
+++ b/tests/stdio_ext_test.cpp
@@ -29,6 +29,8 @@
 #include <wchar.h>
 #include <locale.h>
 
+#include <thread>
+
 #include <android-base/file.h>
 
 #include "utils.h"
@@ -235,23 +237,20 @@
   fclose(fp);
 }
 
-static void LockingByCallerHelper(std::atomic<pid_t>* pid) {
-  *pid = gettid();
-  flockfile(stdout);
-  funlockfile(stdout);
-}
-
 TEST(stdio_ext, __fsetlocking_BYCALLER) {
   // Check if users can use flockfile/funlockfile to protect stdio operations.
   int old_state = __fsetlocking(stdout, FSETLOCKING_BYCALLER);
   flockfile(stdout);
-  pthread_t thread;
+
   std::atomic<pid_t> pid(0);
-  ASSERT_EQ(0, pthread_create(&thread, nullptr,
-                              reinterpret_cast<void* (*)(void*)>(LockingByCallerHelper), &pid));
+  std::thread thread([&]() {
+    pid = gettid();
+    flockfile(stdout);
+    funlockfile(stdout);
+  });
   WaitUntilThreadSleep(pid);
   funlockfile(stdout);
 
-  ASSERT_EQ(0, pthread_join(thread, nullptr));
+  thread.join();
   __fsetlocking(stdout, old_state);
 }