[libcxxabi] Make InitByteGlobalMutex check GetThreadID instead of PlatformThreadID

[DanielMcIntosh-IBM]

Summary

By relying on PlatformSupportsThreadID, InitByteGlobalMutex disregards
the GetThreadID template argument, rendering it useless.

This is the 2nd of 5 changes to overhaul cxa_guard.
See D108343 for what the final result will be.

Depends on D109539

[ldionne]

Can you quickly explain why this is going to simplify your life in the other patches? Without knowing more about your motivations, I would say that PlatformSupportsThreadID() provides more information to the reader than testing PlatfromThreadID != nullptr, so I'd be tempted to think it's a useful abstraction to keep. Why are you removing it?

[DanielMcIntosh-IBM]

In D110088#3010529, @ldionne wrote:

Can you quickly explain why this is going to simplify your life in the other patches? Without knowing more about your motivations, I would say that PlatformSupportsThreadID() provides more information to the reader than testing PlatfromThreadID != nullptr, so I'd be tempted to think it's a useful abstraction to keep. Why are you removing it?

Removing PlatformSupportsThreadID() was not the purpose of this change. The purpose was to fix InitByteGlobalMutex. It just so happens that after fixing InitByteGlobalMutex, PlatformSupportsThreadID() is no longer used outside the test, at which point I figured it's dead code and should be removed. I could leave it there too, and it wouldn't cause any problems, but since all the Guards (except InitByteNoThreads) have a function pointer template parameter, they should be checking their template argument, not PlatfromThreadID, rendering PlatformSupportsThreadID useless anyways.

As for simplifying my life in the other patches, I suppose it doesn't really, and could probably go in as a separate patch independent of the rest of the cxa_guard re-write.

[DanielMcIntosh-IBM]

No change, just need to trigger the build again

[ldionne]

Ahhh I see, thanks. I hadn't read the commit description carefully, thanks a lot. LGTM (and good find)!

[DanielMcIntosh-IBM]

(I'm waiting for the other 5 related PRs to get approved before landing to reduce the number of times we have to resolve merge conflicts downstream)

[DanielMcIntosh-IBM]

No change, just re-trigger CI so later dependent changes update their base revision

libcxxabi/src/cxa_guard_impl.h

 uint32_t PlatformThreadID() {
   static_assert(sizeof(pid_t) == sizeof(uint32_t), "");
   return static_cast<uint32_t>(syscall(SYS_gettid));
 }
 #else
 constexpr uint32_t (*PlatformThreadID)() = nullptr;
 #endif
 
-constexpr bool PlatformSupportsThreadID() { return +PlatformThreadID != nullptr; }
-
 //===----------------------------------------------------------------------===//
 //                          GuardBase
 //===----------------------------------------------------------------------===//
 
 enum class AcquireResult {
   INIT_IS_DONE,
   INIT_IS_PENDING,
 };

 template <class Mutex, class CondVar, Mutex& global_mutex, CondVar& global_cond,
           uint32_t (*GetThreadID)() = PlatformThreadID>
 struct InitByteGlobalMutex : GuardObject<InitByteGlobalMutex<Mutex, CondVar, global_mutex, global_cond, GetThreadID>> {
 
   using BaseT = typename InitByteGlobalMutex::GuardObject;
   using BaseT::BaseT;
 
   explicit InitByteGlobalMutex(uint32_t* g) : BaseT(g), has_thread_id_support(false) {}
-  explicit InitByteGlobalMutex(uint64_t* g) : BaseT(g), has_thread_id_support(PlatformSupportsThreadID()) {}
+  explicit InitByteGlobalMutex(uint64_t* g) : BaseT(g), has_thread_id_support(GetThreadID != nullptr) {}
 
 public:
   AcquireResult acquire_init_byte() {
     LockGuard g("__cxa_guard_acquire");
     // Check for possible recursive initialization.
     if (has_thread_id_support && (*init_byte_address & PENDING_BIT)) {
       if (*thread_id_address == current_thread_id.get())
         ABORT_WITH_MESSAGE("__cxa_guard_acquire detected recursive initialization");

 /// for waiting and waking.
 template <void (*Wait)(int*, int) = PlatformFutexWait, void (*Wake)(int*) = PlatformFutexWake,
           uint32_t (*GetThreadIDArg)() = PlatformThreadID>
 struct InitByteFutex : GuardObject<InitByteFutex<Wait, Wake, GetThreadIDArg>> {
   using BaseT = typename InitByteFutex::GuardObject;
 
   /// ARM Constructor
   explicit InitByteFutex(uint32_t* g)
-      : BaseT(g), init_byte(this->init_byte_address), has_thread_id_support(this->thread_id_address && GetThreadIDArg),
+      : BaseT(g), init_byte(this->init_byte_address),
+        has_thread_id_support(this->thread_id_address && GetThreadIDArg != nullptr),
         thread_id(this->thread_id_address) {}
 
   /// Itanium Constructor
   explicit InitByteFutex(uint64_t* g)
-      : BaseT(g), init_byte(this->init_byte_address), has_thread_id_support(this->thread_id_address && GetThreadIDArg),
+      : BaseT(g), init_byte(this->init_byte_address),
+        has_thread_id_support(this->thread_id_address && GetThreadIDArg != nullptr),
         thread_id(this->thread_id_address) {}
 
 public:
   AcquireResult acquire_init_byte() {
     while (true) {
       uint8_t last_val = UNSET;
       if (init_byte.compare_exchange(&last_val, PENDING_BIT, std::_AO_Acq_Rel, std::_AO_Acquire)) {
         if (has_thread_id_support) {

libcxxabi/test/guard_test_basic.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // UNSUPPORTED: c++03
 
 #define TESTING_CXA_GUARD
 #include "../src/cxa_guard_impl.h"
 #include <cassert>
 
-// Disable GCC warning about tautological comparison of a function's address
-#if defined(__GNUC__) && !defined(__clang__)
-# pragma GCC diagnostic ignored "-Waddress"
+#if defined(__clang__)
+#  pragma clang diagnostic ignored "-Wtautological-pointer-compare"
+#elif defined(__GNUC__)
+#  pragma GCC diagnostic ignored "-Waddress"
 #endif
 
 using namespace __cxxabiv1;
 
 template <class GuardType, class Impl>
 struct Tests {
 private:
   Tests() : g{}, impl(&g) {}
static_assert(
                                 GlobalStatic<LibcppCondVar>::instance>>::value,
         "");
 #endif
   }
   {
 #if (defined(__APPLE__) || defined(__linux__))  && !defined(_LIBCXXABI_HAS_NO_THREADS)
     assert(PlatformThreadID);
 #endif
-    if (PlatformSupportsThreadID()) {
+    if (PlatformThreadID != nullptr) {
       assert(PlatformThreadID() != 0);
       assert(PlatformThreadID() == PlatformThreadID());
     }
   }
   {
     Tests<uint32_t, InitByteNoThreads>::test();
     Tests<uint64_t, InitByteNoThreads>::test();
   }