[libcxxabi] Re-organized inheritance structure to remove CRTP in cxa_guard

[DanielMcIntosh-IBM]

Summary

Currently, the InitByte... classes inherit from GuardObject so they can
access the base_address, init_byte_address and thread_id_address. Then,
since GuardObject needs to call acquire/release/abort_init_byte, it uses
the curiously recurring template pattern (CRTP). This is rather messy.

Instead, we'll have GuardObject contain an instance of InitByte, and pass it
the addresses it needs in the constructor. GuardObject doesn't need the
addresses anyways, so it makes more sense for InitByte to keep them instead of
GuardObject. Then, GuardObject can call acquire/release/abort as one
of InitByte's member functions.

Organizing things this way not only gets rid of the use of the CRTP, but also
improves separation of concerns a bit since the InitByte classes are no longer
indirectly responsible for things because of their inheritance from
GuardObject. This means we no longer have strange things like calling
InitByteFutex.cxa_guard_acquire, instead we call
GuardObject<InitByteFutex>.cxa_guard_acquire.

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

Depends on D115367

[ldionne]

libcxxabi/src/cxa_guard_impl.h

247–250

The duplication of these comments feels weird to me. Instead, I believe we should have a comment like:

//===----------------------------------------------------------------------===//
//                    Initialization Byte Implementations
//===----------------------------------------------------------------------===//
//
// Each initialization byte implementation supports the following method:
//
//    InitByte(uint8_t* _init_byte_address, uint32_t*)
//        Here explain the purpose of these constructors.
//
//    bool acquire();
//        Here document what this method does and you can have the note about ownership documented in exactly one place, here.
//
//    etc...
//

Then, each implementation of an initialization byte can simply implement the required methods and document anything interesting that is specific to that implementation.

313

With this refactor, it now feels really weird to have this GetThreadID != nullptr here. Would it be possible to move the determination of whether the thread id is supported elsewhere (probably in the GuardObject constructor) so that InitByteGlobalMutex only has to check _thread_id_address != nullptr?

Same applies to Futex below.

433–436

Also, can you explain the /*_init_byte_address & ~0x3*/ comment?

[DanielMcIntosh-IBM]

Rebase, and address review comments

[DanielMcIntosh-IBM]

libcxxabi/src/cxa_guard_impl.h

313

It's possible, but without any further changes, InitByteGlobalMutex and InitByteFutex would still need the GetThreadID template parameter because of their current_thread_id member. So both the GuardObject and InitByte... classes would each need it as a template parameter, meaning we would have something like this:

GuardObject<InitByteGlobalMutex<LibcppMutex, LibcppCondVar, GlobalStatic<LibcppMutex>::instance,
                                               GlobalStatic<LibcppCondVar>::instance, PlatformThreadID>, PlatformThreadID>;

And for NoThreads we'd still have to pass PlatformThreadID as a template argument to GuardObject:

GuardObject<InitByteNoThreads, PlatformThreadID>;

That doesn't seem to me like much of an improvement to me, but you probably have a better idea of what is easiest to understand for someone who hasn't been looking at this for months.

I could try to avoid this by pulling out more than just has_thread_id_support, and maybe even going as far as to create a ThreadIdWord class or something. Unfortunately, while both GlobalMutex and Futex use LazyValue<uint32_t, GetThreadID> for fetching the current thread ID, after that they start to differ ever so slightly. Futex uses AtomicInt<uint32_t> thread_id while GlobalMutex reads and writes to it directly. There's also the issue of who should construct & who should own the ThreadIdWord instance - GuardObject is in charge of diving up the raw guard object, but InitByte... is the only one who interacts with it and (for GlobalMutex) all interaction with it needs to happen while we hold the lock.

All of which is to say, yes, it is possible, but I don't think it would make things better.

433–436

The goal here is not to compute the address of the raw guard object, but rather to compute the argument we need to pass to Wait() - a pointer to the word that includes _init_byte_address - i.e. _init_byte_address & ~0x3. However bitwise arithmetic is prohibited on pointers, so _init_byte_address & ~0x3 is an illegal expression.

We could either cast to an arithmetic type such as uintptr_t and back again, or go the more direct route and use what we know about the layout of the raw guard object (and perform the exact inverse of what we do in GuardObject to compute _init_byte_address). I believe I went the direct route because using casts would have technically been UB, even if it is all but guaranteed to be safe (though I'm not too sure anymore since it has been a long time since I made the decision). Unfortunately, during the internal review downstream, the direct approach led to some confusion, so I added a short comment to hopefully clarify the intentions a bit.

[DanielMcIntosh-IBM]

libcxxabi/src/cxa_guard_impl.h

247–250

I left very small summary still, just for tools which recognize documentation comments to pick up.

[DanielMcIntosh-IBM]

Ping

[ldionne]

Please don't mark this as being a NFC in the commit message. This is a refactoring, and we don't expect it to change functionality, but we generally tag things like "remove unused include" or "fix comment" as NFCs. If there happens to be a bug in there, it would be bad to skip it from the commit list when searching for potential regressions because this was marked as a NFC.

Apart from that, thanks for your patience going through the review and for improving the documentation like this. LGTM with a few comments.

libcxxabi/src/cxa_guard_impl.h

215

217

433–436

Got it, thanks for the explanation. Please consider reformatting as I suggested, though.

[DanielMcIntosh-IBM]

Address review comments

libcxxabi/src/cxa_guard_impl.h

  * The second "init byte" does the rest of the bookkeeping. It tracks if
  * initialization is complete or pending, and if there are waiting threads.
  *
  * If the guard variable is 64-bits and the platforms supplies a 32-bit thread
  * identifier, it is used to detect recursive initialization. The thread ID of
  * the thread currently performing initialization is stored in the second word.
  *
  *  Guard Object Layout:
- * -------------------------------------------------------------------------
- * |a: guard byte | a+1: init byte | a+2 : unused ... | a+4: thread-id ... |
- * -------------------------------------------------------------------------
+ * ---------------------------------------------------------------------------
+ * | a+0: guard byte | a+1: init byte | a+2: unused ... | a+4: thread-id ... |
+ * ---------------------------------------------------------------------------
+ *
+ * Note that we don't do what the ABI docs suggest (put a mutex in the guard
+ * object which we acquire in cxa_guard_acquire and release in
+ * cxa_guard_release). Instead we use the init byte to imitate that behaviour,
+ * but without actually holding anything mutex related between aquire and
+ * release/abort.
  *
  *  Access Protocol:
  *    For each implementation the guard byte is checked and set before accessing
  *    the init byte.
  *
  *  Overall Design:
  *    The implementation was designed to allow each implementation to be tested
  *    independent of the C++ runtime or platform support.
struct GuardByte {
   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
   /// 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() {
     // if guard_byte is non-zero, we have already completed initialization
     // (i.e. release has been called)
     return guard_byte.load(std::_AO_Acquire) != UNSET;
   }
 
   /// 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.store(COMPLETE_BIT, std::_AO_Release); }
 
   /// The guard byte portion of cxa_guard_abort.
   void abort() {} // Nothing to do
 
 private:
   AtomicInt<uint8_t> guard_byte;
 };
 
 //===----------------------------------------------------------------------===//
-//                          GuardBase
+//                       InitByte Implementations
 //===----------------------------------------------------------------------===//
-
-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;
-
-template <class Derived>
-struct GuardObject {
-  GuardObject() = delete;
-  GuardObject(GuardObject const&) = delete;
-  GuardObject& operator=(GuardObject const&) = delete;
-
-private:
-  GuardByte guard_byte;
-
-public:
-  /// ARM Constructor
-  explicit GuardObject(uint32_t* g)
-      : guard_byte(reinterpret_cast<uint8_t*>(g)), base_address(g),
-        init_byte_address(reinterpret_cast<uint8_t*>(g) + 1), thread_id_address(nullptr) {}
-
-  /// Itanium Constructor
-  explicit GuardObject(uint64_t* g)
-      : guard_byte(reinterpret_cast<uint8_t*>(g)), base_address(g),
-        init_byte_address(reinterpret_cast<uint8_t*>(g) + 1), thread_id_address(reinterpret_cast<uint32_t*>(g) + 1) {}
-
-  /// Implements __cxa_guard_acquire.
-  AcquireResult cxa_guard_acquire() {
-    if (guard_byte.acquire())
-      return INIT_IS_DONE;
-    return derived()->acquire_init_byte();
-  }
-
-  /// Implements __cxa_guard_release.
-  void cxa_guard_release() {
-    // Update guard byte first, so if somebody is woken up by release_init_byte
-    // and comes all the way back around to __cxa_guard_acquire again, they see
-    // it as having completed initialization.
-    guard_byte.release();
-    derived()->release_init_byte();
-  }
-
-  /// Implements __cxa_guard_abort.
-  void cxa_guard_abort() {
-    guard_byte.abort();
-    derived()->abort_init_byte();
-  }
-
-public:
-  /// base_address - the address of the original guard object.
-  void* const base_address;
-  /// The address of the byte used by the implementation during initialization.
-  uint8_t* const init_byte_address;
-  /// An optional address storing an identifier for the thread performing initialization.
-  /// It's used to detect recursive initialization.
-  uint32_t* const thread_id_address;
-
-private:
-  Derived* derived() { return static_cast<Derived*>(this); }
-};
+//
+// Each initialization byte implementation supports the following methods:
+//
+//  InitByte(uint8_t* _init_byte_address, uint32_t* _thread_id_address)
+//    Construct the InitByte object, initializing our member variables
+//
+//  bool acquire()
+//    Called before we start the initialization. Check if someone else has already started, and if
+//    not to signal our intent to start it ourselves. We determine the current status from the init
+//    byte, which is one of 4 possible values:
+//      COMPLETE:           Initialization was finished by somebody else. Return true.
+//      PENDING:            Somebody has started the initialization already, set the WAITING bit,
+//                          then wait for the init byte to get updated with a new value.
+//      (PENDING|WAITING):  Somebody has started the initialization already, and we're not the
+//                          first one waiting. Wait for the init byte to get updated.
+//      UNSET:              Initialization hasn't successfully completed, and nobody is currently
+//                          performing the initialization. Set the PENDING bit to indicate our
+//                          intention to start the initialization, and return false.
+//    The return value indicates whether initialization has already been completed.
+//
+//  void release()
+//    Called after successfully completing the initialization. Update the init byte to reflect
+//    that, then if anybody else is waiting, wake them up.
+//
+//  void abort()
+//    Called after an error is thrown during the initialization. Reset the init byte to UNSET to
+//    indicate that we're no longer performing the initialization, then if anybody is waiting, wake
+//    them up so they can try performing the initialization.
+//
 
 //===----------------------------------------------------------------------===//
 //                    Single Threaded Implementation
 //===----------------------------------------------------------------------===//
 
-struct InitByteNoThreads : GuardObject<InitByteNoThreads> {
-  using GuardObject::GuardObject;
+/// 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;
 
-  AcquireResult acquire_init_byte() {
+  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
+  /// initialization has already been completed.
+  bool acquire() {
     if (*init_byte_address == COMPLETE_BIT)
-      return INIT_IS_DONE;
+      return true;
     if (*init_byte_address & PENDING_BIT)
       ABORT_WITH_MESSAGE("__cxa_guard_acquire detected recursive initialization");
     *init_byte_address = PENDING_BIT;
-    return INIT_IS_PENDING;
+    return false;
   }
 
-  void release_init_byte() { *init_byte_address = COMPLETE_BIT; }
-  void abort_init_byte() { *init_byte_address = UNSET; }
+  /// The init byte portion of cxa_guard_release.
+  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
 //===----------------------------------------------------------------------===//
 
 struct LibcppMutex;
 struct LibcppCondVar;

 private:
   std::__libcpp_condvar_t cond = _LIBCPP_CONDVAR_INITIALIZER;
 };
 #else
 struct LibcppMutex {};
 struct LibcppCondVar {};
 #endif // !defined(_LIBCXXABI_HAS_NO_THREADS)
 
+/// InitByteGlobalMutex - Uses a global mutex and condition variable (common to
+/// all static local variables) to manage reads and writes to the init byte.
 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;
+struct InitByteGlobalMutex {
 
-  explicit InitByteGlobalMutex(uint32_t* g) : BaseT(g), has_thread_id_support(false) {}
-  explicit InitByteGlobalMutex(uint64_t* g) : BaseT(g), has_thread_id_support(GetThreadID != nullptr) {}
+  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:
-  AcquireResult acquire_init_byte() {
+  /// The init byte portion of cxa_guard_acquire. Returns true if
+  /// initialization has already been completed.
+  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 INIT_IS_DONE;
+      return true;
 
     if (has_thread_id_support)
       *thread_id_address = current_thread_id.get();
 
     *init_byte_address = PENDING_BIT;
-    return INIT_IS_PENDING;
+    return false;
   }
 
-  void release_init_byte() {
+  /// The init byte portion of cxa_guard_release.
+  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");
       }
     }
   }
 
-  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;
     }
     if (has_waiting) {
       if (global_cond.broadcast()) {
         ABORT_WITH_MESSAGE("%s failed to broadcast", "__cxa_guard_abort");
       }
     }
   }
 
 private:
-  using BaseT::init_byte_address;
-  using BaseT::thread_id_address;
+  /// The address of the byte used during initialization.
+  uint8_t* const init_byte_address;
+  /// An optional address storing an identifier for the thread performing initialization.
+  /// It's used to detect recursive initialization.
+  uint32_t* const thread_id_address;
+
   const bool has_thread_id_support;
   LazyValue<uint32_t, GetThreadID> current_thread_id;
 
 private:
   struct LockGuard {
     LockGuard() = delete;
     LockGuard(LockGuard const&) = delete;
     LockGuard& operator=(LockGuard const&) = delete;

 }
 #else
 constexpr void (*PlatformFutexWait)(int*, int) = nullptr;
 constexpr void (*PlatformFutexWake)(int*) = nullptr;
 #endif
 
 constexpr bool PlatformSupportsFutex() { return +PlatformFutexWait != nullptr; }
 
-/// InitByteFutex - Manages initialization using atomics and the futex syscall
-/// for waiting and waking.
+/// InitByteFutex - Uses a futex to manage reads and writes to the init byte.
 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 != nullptr),
-        thread_id(this->thread_id_address) {}
+struct InitByteFutex {
 
-  /// Itanium Constructor
-  explicit InitByteFutex(uint64_t* g)
-      : BaseT(g), init_byte(this->init_byte_address),
-        has_thread_id_support(this->thread_id_address && GetThreadIDArg != nullptr),
-        thread_id(this->thread_id_address) {}
+  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:
-  AcquireResult acquire_init_byte() {
+  /// The init byte portion of cxa_guard_acquire. Returns true if
+  /// initialization has already been completed.
+  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 INIT_IS_PENDING;
+        return false;
       }
 
       if (last_val == COMPLETE_BIT)
-        return INIT_IS_DONE;
+        return true;
 
       if (last_val & PENDING_BIT) {
 
         // Check for recursive initialization
         if (has_thread_id_support && thread_id.load(std::_AO_Relaxed) == current_thread_id.get()) {
           ABORT_WITH_MESSAGE("__cxa_guard_acquire detected recursive initialization");
         }
 
         if ((last_val & WAITING_BIT) == 0) {
           // This compare exchange can fail for several reasons
           // (1) another thread finished the whole thing before we got here
           // (2) another thread set the waiting bit we were trying to thread
           // (3) another thread had an exception and failed to finish
           if (!init_byte.compare_exchange(&last_val, PENDING_BIT | WAITING_BIT, std::_AO_Acq_Rel, std::_AO_Release)) {
             // (1) success, via someone else's work!
             if (last_val == COMPLETE_BIT)
-              return INIT_IS_DONE;
+              return true;
 
             // (3) someone else, bailed on doing the work, retry from the start!
             if (last_val == UNSET)
               continue;
 
             // (2) the waiting bit got set, so we are happy to keep waiting
           }
         }
         wait_on_initialization();
       }
     }
   }
 
-  void release_init_byte() {
+  /// The init byte portion of cxa_guard_release.
+  void release() {
     uint8_t old = init_byte.exchange(COMPLETE_BIT, std::_AO_Acq_Rel);
     if (old & WAITING_BIT)
       wake_all();
   }
 
-  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();
   }
 
 private:
   /// Use the futex to wait on the current guard variable. Futex expects a
-  /// 32-bit 4-byte aligned address as the first argument, so we have to use use
-  /// the base address of the guard variable (not the init byte).
-  void wait_on_initialization() {
-    Wait(static_cast<int*>(this->base_address), expected_value_for_futex(PENDING_BIT | WAITING_BIT));
-  }
-  void wake_all() { Wake(static_cast<int*>(this->base_address)); }
+  /// 32-bit 4-byte aligned address as the first argument, so we use the 4-byte
+  /// aligned address that encompasses the init byte (i.e. the address of the
+  /// raw guard object that was passed to __cxa_guard_acquire/release/abort).
+  void wait_on_initialization() { Wait(base_address, expected_value_for_futex(PENDING_BIT | WAITING_BIT)); }
+  void wake_all() { Wake(base_address); }
 
 private:
   AtomicInt<uint8_t> init_byte;
 
   const bool has_thread_id_support;
   // Unsafe to use unless has_thread_id_support
   AtomicInt<uint32_t> thread_id;
   LazyValue<uint32_t, GetThreadIDArg> current_thread_id;
 
+  /// the 4-byte-aligned address that encompasses the init byte (i.e. the
+  /// address of the raw guard object).
+  int* const base_address;
+
   /// Create the expected integer value for futex `wait(int* addr, int expected)`.
   /// We pass the base address as the first argument, So this function creates
   /// an zero-initialized integer  with `b` copied at the correct offset.
   static int expected_value_for_futex(uint8_t b) {
     int dest_val = 0;
     std::memcpy(reinterpret_cast<char*>(&dest_val) + 1, &b, 1);
     return dest_val;
   }
 
   static_assert(Wait != nullptr && Wake != nullptr, "");
 };
 
 //===----------------------------------------------------------------------===//
+//                          GuardObject
+//===----------------------------------------------------------------------===//
+
+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 GuardObject {
+  GuardObject() = delete;
+  GuardObject(GuardObject const&) = delete;
+  GuardObject& operator=(GuardObject const&) = delete;
+
+private:
+  GuardByte guard_byte;
+  InitByteT init_byte;
+
+public:
+  /// ARM Constructor
+  explicit GuardObject(uint32_t* raw_guard_object)
+      : guard_byte(reinterpret_cast<uint8_t*>(raw_guard_object)),
+        init_byte(reinterpret_cast<uint8_t*>(raw_guard_object) + 1, nullptr) {}
+
+  /// Itanium Constructor
+  explicit GuardObject(uint64_t* raw_guard_object)
+      : 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 evaluation 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 (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 init_byte.release
+    // and comes all the way back around to __cxa_guard_acquire again, they see
+    // it as having completed initialization.
+    guard_byte.release();
+    init_byte.release();
+  }
+
+  /// Implements __cxa_guard_abort.
+  void cxa_guard_abort() {
+    guard_byte.abort();
+    init_byte.abort();
+  }
+};
+
+//===----------------------------------------------------------------------===//
 //
 //===----------------------------------------------------------------------===//
 
 template <class T>
 struct GlobalStatic {
   static T instance;
 };
 template <class T>
 _LIBCPP_SAFE_STATIC T GlobalStatic<T>::instance = {};
 
 enum class Implementation { NoThreads, GlobalMutex, Futex };
 
 template <Implementation Impl>
 struct SelectImplementation;
 
+/// Manage initialization without performing any inter-thread synchronization.
 template <>
 struct SelectImplementation<Implementation::NoThreads> {
-  using type = InitByteNoThreads;
+  using type = GuardObject<InitByteNoThreads>;
 };
 
+/// Manage initialization using a global mutex and condition variable.
 template <>
 struct SelectImplementation<Implementation::GlobalMutex> {
-  using type = InitByteGlobalMutex<LibcppMutex, LibcppCondVar, GlobalStatic<LibcppMutex>::instance,
-                                   GlobalStatic<LibcppCondVar>::instance, PlatformThreadID>;
+  using type = GuardObject<InitByteGlobalMutex<LibcppMutex, LibcppCondVar, GlobalStatic<LibcppMutex>::instance,
+                                               GlobalStatic<LibcppCondVar>::instance, PlatformThreadID>>;
 };
 
+/// Manage initialization using atomics and the futex syscall for waiting and waking.
 template <>
 struct SelectImplementation<Implementation::Futex> {
-  using type = InitByteFutex<PlatformFutexWait, PlatformFutexWake, PlatformThreadID>;
+  using type = GuardObject<InitByteFutex<PlatformFutexWait, PlatformFutexWake, PlatformThreadID>>;
 };
 
 // TODO(EricWF): We should prefer the futex implementation when available. But
 // it should be done in a separate step from adding the implementation.
 constexpr Implementation CurrentImplementation =
 #if defined(_LIBCXXABI_HAS_NO_THREADS)
     Implementation::NoThreads;
 #elif defined(_LIBCXXABI_USE_FUTEX)

libcxxabi/test/guard_test_basic.pass.cpp

 void NopFutexWait(int*, int) { assert(false); }
 void NopFutexWake(int*) { assert(false); }
 uint32_t MockGetThreadID() { return 0; }
 
 int main(int, char**) {
   {
 #if defined(_LIBCXXABI_HAS_NO_THREADS)
     static_assert(CurrentImplementation == Implementation::NoThreads, "");
-    static_assert(
-        std::is_same<SelectedImplementation, InitByteNoThreads>::value, "");
+    static_assert(std::is_same<SelectedImplementation, GuardObject<InitByteNoThreads>>::value, "");
 #else
     static_assert(CurrentImplementation == Implementation::GlobalMutex, "");
     static_assert(
-        std::is_same<
-            SelectedImplementation,
-            InitByteGlobalMutex<LibcppMutex, LibcppCondVar,
-                                GlobalStatic<LibcppMutex>::instance,
-                                GlobalStatic<LibcppCondVar>::instance>>::value,
+        std::is_same<SelectedImplementation,
+                     GuardObject<InitByteGlobalMutex<LibcppMutex, LibcppCondVar, GlobalStatic<LibcppMutex>::instance,
+                                                     GlobalStatic<LibcppCondVar>::instance>>>::value,
         "");
 #endif
   }
   {
 #if (defined(__APPLE__) || defined(__linux__))  && !defined(_LIBCXXABI_HAS_NO_THREADS)
     assert(PlatformThreadID);
 #endif
     if (PlatformThreadID != nullptr) {
       assert(PlatformThreadID() != 0);
       assert(PlatformThreadID() == PlatformThreadID());
     }
   }
   {
-    Tests<uint32_t, InitByteNoThreads>::test();
-    Tests<uint64_t, InitByteNoThreads>::test();
+    Tests<uint32_t, GuardObject<InitByteNoThreads>>::test();
+    Tests<uint64_t, GuardObject<InitByteNoThreads>>::test();
   }
   {
     using MutexImpl =
-        InitByteGlobalMutex<NopMutex, NopCondVar, global_nop_mutex,
-                            global_nop_cond, MockGetThreadID>;
+        GuardObject<InitByteGlobalMutex<NopMutex, NopCondVar, global_nop_mutex, global_nop_cond, MockGetThreadID>>;
     Tests<uint32_t, MutexImpl>::test();
     Tests<uint64_t, MutexImpl>::test();
   }
   {
-    using FutexImpl =
-        InitByteFutex<&NopFutexWait, &NopFutexWake, &MockGetThreadID>;
+    using FutexImpl = GuardObject<InitByteFutex<&NopFutexWait, &NopFutexWake, &MockGetThreadID>>;
     Tests<uint32_t, FutexImpl>::test();
     Tests<uint64_t, FutexImpl>::test();
   }
 
   return 0;
 }