[NFC][libcxxabi] In cxa_guard, switch away from an inheritance based design

[DanielMcIntosh-IBM]

Summary

This gets rid of the need for multiple inheritance, and better
reflects the design - a Guard isn't an instance of GuardByte or
InitByte, it merely uses them.

This is the 6th of 6 changes to overhaul cxa_guard.
See D108343 for what the final result will be and more details on why
cxa_guard is getting overhauled.

Depends on D110101

[DanielMcIntosh-IBM]

Ping

[DanielMcIntosh-IBM]

Ping

[DanielMcIntosh-IBM]

Ping

[DanielMcIntosh-IBM]

Abandoning, since I no longer think this is the best way of grouping the changes. See D115367, D115368 and D115369 for a newer version that groups things a little better, but with the same end result.

libcxxabi/src/cxa_guard_impl.h

 struct GuardByte {
   GuardByte() = delete;
   GuardByte(GuardByte const&) = delete;
   GuardByte& operator=(GuardByte const&) = delete;
 
   explicit GuardByte(uint8_t* const guard_byte_address) : guard_byte(guard_byte_address) {}
 
 public:
-  /// The guard_byte portion of cxa_guard_acquire. Returns true if
+  /// The guard byte portion of cxa_guard_acquire. Returns true if
   /// initialization has already been completed.
   /// Note: On completion, we haven't 'acquired' ownership of anything mutex
   /// related. The name is simply referring to __cxa_guard_acquire.
-  bool acquire_guard_byte() {
+  bool acquire() {
     // if guard_byte is non-zero, we have already completed initialization
-    // (i.e. release_guard_byte has been called)
+    // (i.e. release has been called)
     return guard_byte.load(std::_AO_Acquire) != UNSET;
   }
 
-  /// The guard_byte portion of cxa_guard_release.
+  /// The guard byte portion of cxa_guard_release.
   /// Note: On completion, we haven't 'released' ownership of anything mutex
   /// related. The name is simply referring to __cxa_guard_release.
-  void release_guard_byte() { guard_byte.store(COMPLETE_BIT, std::_AO_Release); }
+  void release() { guard_byte.store(COMPLETE_BIT, std::_AO_Release); }
 
-  /// The guard_byte portion of cxa_guard_abort.
-  void abort_guard_byte() {} // Nothing to do
+  /// The guard byte portion of cxa_guard_abort.
+  void abort() {} // Nothing to do
 
 private:
   AtomicInt<uint8_t> guard_byte;
 };
 
 //===----------------------------------------------------------------------===//
 //                    Single Threaded Implementation
 //===----------------------------------------------------------------------===//
 
 /// InitByteNoThreads - Doesn't use any inter-thread synchronization when
 /// managing reads and writes to the init byte.
 struct InitByteNoThreads {
   InitByteNoThreads() = delete;
   InitByteNoThreads(InitByteNoThreads const&) = delete;
   InitByteNoThreads& operator=(InitByteNoThreads const&) = delete;
 
   explicit InitByteNoThreads(uint8_t* _init_byte_address, uint32_t*) : init_byte_address(_init_byte_address) {}
 
-  /// The init_byte portion of cxa_guard_acquire. Returns true if
+  /// The init byte portion of cxa_guard_acquire. Returns true if
   /// initialization has already been completed.
   /// Note: On completion, we haven't 'acquired' ownership of anything mutex
   /// related. The name is simply referring to __cxa_guard_acquire.
-  bool acquire_init_byte() {
+  bool acquire() {
     if (*init_byte_address == COMPLETE_BIT)
       return true;
     if (*init_byte_address & PENDING_BIT)
       ABORT_WITH_MESSAGE("__cxa_guard_acquire detected recursive initialization");
     *init_byte_address = PENDING_BIT;
     return false;
   }
 
-  /// The init_byte portion of cxa_guard_release.
+  /// The init byte portion of cxa_guard_release.
   /// Note: On completion, we haven't 'released' ownership of anything mutex
   /// related. The name is simply referring to __cxa_guard_release.
-  void release_init_byte() { *init_byte_address = COMPLETE_BIT; }
-  /// The init_byte portion of cxa_guard_abort.
-  void abort_init_byte() { *init_byte_address = UNSET; }
+  void release() { *init_byte_address = COMPLETE_BIT; }
+  /// The init byte portion of cxa_guard_abort.
+  void abort() { *init_byte_address = UNSET; }
 
 private:
   /// The address of the byte used during initialization.
   uint8_t* const init_byte_address;
 };
 
 //===----------------------------------------------------------------------===//
 //                     Global Mutex Implementation
template <class Mutex, class CondVar, Mutex& global_mutex, CondVar& global_cond,
           uint32_t (*GetThreadID)() = PlatformThreadID>
 struct InitByteGlobalMutex {
 
   explicit InitByteGlobalMutex(uint8_t* _init_byte_address, uint32_t* _thread_id_address)
       : init_byte_address(_init_byte_address), thread_id_address(_thread_id_address),
         has_thread_id_support(_thread_id_address != nullptr && GetThreadID != nullptr) {}
 
 public:
-  /// The init_byte portion of cxa_guard_acquire. Returns true if
+  /// The init byte portion of cxa_guard_acquire. Returns true if
   /// initialization has already been completed.
   /// Note: On completion, we haven't 'acquired' ownership of anything mutex
   /// related. The name is simply referring to __cxa_guard_acquire.
-  bool acquire_init_byte() {
+  bool acquire() {
     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");
     }
 
     // Wait until the pending bit is not set.
     while (*init_byte_address & PENDING_BIT) {
       *init_byte_address |= WAITING_BIT;
       global_cond.wait(global_mutex);
     }
 
     if (*init_byte_address == COMPLETE_BIT)
       return true;
 
     if (has_thread_id_support)
       *thread_id_address = current_thread_id.get();
 
     *init_byte_address = PENDING_BIT;
     return false;
   }
 
-  /// The init_byte portion of cxa_guard_release.
+  /// The init byte portion of cxa_guard_release.
   /// Note: On completion, we haven't 'released' ownership of anything mutex
   /// related. The name is simply referring to __cxa_guard_release.
-  void release_init_byte() {
+  void release() {
     bool has_waiting;
     {
       LockGuard g("__cxa_guard_release");
       has_waiting = *init_byte_address & WAITING_BIT;
       *init_byte_address = COMPLETE_BIT;
     }
     if (has_waiting) {
       if (global_cond.broadcast()) {
         ABORT_WITH_MESSAGE("%s failed to broadcast", "__cxa_guard_release");
       }
     }
   }
 
-  /// The init_byte portion of cxa_guard_abort.
-  void abort_init_byte() {
+  /// The init byte portion of cxa_guard_abort.
+  void abort() {
     bool has_waiting;
     {
       LockGuard g("__cxa_guard_abort");
       if (has_thread_id_support)
         *thread_id_address = 0;
       has_waiting = *init_byte_address & WAITING_BIT;
       *init_byte_address = UNSET;
     }
struct InitByteFutex {
 
   explicit InitByteFutex(uint8_t* _init_byte_address, uint32_t* _thread_id_address)
       : init_byte(_init_byte_address),
         has_thread_id_support(_thread_id_address != nullptr && GetThreadIDArg != nullptr),
         thread_id(_thread_id_address), base_address(reinterpret_cast<int*>(
                                            /*_init_byte_address & ~0x3*/ _init_byte_address - 1)) {}
 
 public:
-  /// The init_byte portion of cxa_guard_acquire. Returns true if
+  /// The init byte portion of cxa_guard_acquire. Returns true if
   /// initialization has already been completed.
   /// Note: On completion, we haven't 'acquired' ownership of anything mutex
   /// related. The name is simply referring to __cxa_guard_acquire.
-  bool acquire_init_byte() {
+  bool acquire() {
     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) {
           thread_id.store(current_thread_id.get(), std::_AO_Relaxed);
         }
         return false;
       }
while (true) {
             // (2) the waiting bit got set, so we are happy to keep waiting
           }
         }
         wait_on_initialization();
       }
     }
   }
 
-  /// The init_byte portion of cxa_guard_release.
+  /// The init byte portion of cxa_guard_release.
   /// Note: On completion, we haven't 'released' ownership of anything mutex
   /// related. The name is simply referring to __cxa_guard_release.
-  void release_init_byte() {
+  void release() {
     uint8_t old = init_byte.exchange(COMPLETE_BIT, std::_AO_Acq_Rel);
     if (old & WAITING_BIT)
       wake_all();
   }
 
-  /// The init_byte portion of cxa_guard_abort.
-  void abort_init_byte() {
+  /// The init byte portion of cxa_guard_abort.
+  void abort() {
     if (has_thread_id_support)
       thread_id.store(0, std::_AO_Relaxed);
 
     uint8_t old = init_byte.exchange(UNSET, std::_AO_Acq_Rel);
     if (old & WAITING_BIT)
       wake_all();
   }
 
enum class AcquireResult {
   INIT_IS_DONE,
   INIT_IS_PENDING,
 };
 constexpr AcquireResult INIT_IS_DONE = AcquireResult::INIT_IS_DONE;
 constexpr AcquireResult INIT_IS_PENDING = AcquireResult::INIT_IS_PENDING;
 
 /// Co-ordinates between GuardByte and InitByte.
 template <class InitByteT>
-struct GuardBase : GuardByte, InitByteT {
+struct GuardBase {
   GuardBase() = delete;
   GuardBase(GuardBase const&) = delete;
   GuardBase& operator=(GuardBase const&) = delete;
 
+private:
+  GuardByte guard_byte;
+  InitByteT init_byte;
+
+public:
   /// ARM Constructor
   explicit GuardBase(uint32_t* raw_guard_object)
-      : GuardByte(reinterpret_cast<uint8_t*>(raw_guard_object)),
-        InitByteT(reinterpret_cast<uint8_t*>(raw_guard_object) + 1, nullptr) {}
+      : guard_byte(reinterpret_cast<uint8_t*>(raw_guard_object)),
+        init_byte(reinterpret_cast<uint8_t*>(raw_guard_object) + 1, nullptr) {}
 
   /// Itanium Constructor
   explicit GuardBase(uint64_t* raw_guard_object)
-      : GuardByte(reinterpret_cast<uint8_t*>(raw_guard_object)),
-        InitByteT(reinterpret_cast<uint8_t*>(raw_guard_object) + 1, reinterpret_cast<uint32_t*>(raw_guard_object) + 1) {
+      : guard_byte(reinterpret_cast<uint8_t*>(raw_guard_object)),
+        init_byte(reinterpret_cast<uint8_t*>(raw_guard_object) + 1, reinterpret_cast<uint32_t*>(raw_guard_object) + 1) {
   }
 
   /// Implements __cxa_guard_acquire.
   AcquireResult cxa_guard_acquire() {
-    // use short-circuit evalutation to avoid calling acquire_init_byte when
-    // acquire_guard_byte returns true. (i.e. don't call it when we know from
+    // use short-circuit evalutation to avoid calling init_byte.acquire when
+    // guard_byte.acquire returns true. (i.e. don't call it when we know from
     // the guard byte that initialization has already been completed)
-    if (acquire_guard_byte() || InitByteT::acquire_init_byte())
+    if (guard_byte.acquire() || init_byte.acquire())
       return INIT_IS_DONE;
     return INIT_IS_PENDING;
   }
 
   /// Implements __cxa_guard_release.
   void cxa_guard_release() {
-    // Update guard byte first, so if somebody is woken up by release_init_byte
+    // Update guard byte first, so if somebody is woken up by init_byte.release
     // and comes all the way back around to __cxa_guard_acquire again, they see
     // it as having completed initialization.
-    release_guard_byte();
-    InitByteT::release_init_byte();
+    guard_byte.release();
+    init_byte.release();
   }
 
   /// Implements __cxa_guard_abort.
   void cxa_guard_abort() {
-    abort_guard_byte();
-    InitByteT::abort_init_byte();
+    guard_byte.abort();
+    init_byte.abort();
   }
 };
 
 /// NoThreadsGuard - Manages initialization without performing any inter-thread
 /// synchronization.
 struct NoThreadsGuard : GuardBase<InitByteNoThreads> {
   using BaseT = typename NoThreadsGuard::GuardBase;
   using BaseT::BaseT;