Refactor cxa_guard.cpp

[EricWF]

Summary

The implementation of __cxa_guard_acquire is a mess. This patch attempts to clean it up. No functionality change is intended.

This cleanup is motivated by upcoming changes. Specifically changes to allow libc++abi to tolerate when pthread_mutex_lock is being intercepted.

[EricWF]

• Fix layout incongruity for non-apple platforms.
• Various cleanups.

[EricWF]

• Misc cleanup.

[EricWF]

Abandoning...

I landed these changes as a series of smaller cleanups.

src/cxa_guard.cpp

 //===---------------------------- cxa_guard.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
 //
 //===----------------------------------------------------------------------===//
 
 #include "__cxxabi_config.h"
 
 #include "abort_message.h"
 #include <__threading_support>
 
 #include <stdint.h>
+#include <string.h>
+
+#ifdef __GNUC__
+#define MAY_ALIAS __attribute__((__may_alias__))
+#else
+#define MAY_ALIAS
+#endif
 
 /*
     This implementation must be careful to not call code external to this file
     which will turn around and try to call __cxa_guard_acquire reentrantly.
     For this reason, the headers of this file are as restricted as possible.
     Previous implementations of this code for __APPLE__ have used
     std::__libcpp_mutex_lock and the abort_message utility without problem. This
     implementation also uses std::__libcpp_condvar_wait which has tested
     to not be a problem.
 */
 
-namespace __cxxabiv1
-{
+namespace __cxxabiv1 {
 
-namespace
-{
+namespace {
 
-#ifdef __arm__
-// A 32-bit, 4-byte-aligned static data value. The least significant 2 bits must
-// be statically initialized to 0.
-typedef uint32_t guard_type;
+constexpr int INIT_COMPLETE = 0;
+constexpr int INIT_NOT_COMPLETE = 1;
 
-inline void set_initialized(guard_type* guard_object) {
-    *guard_object |= 1;
-}
-#else
-typedef uint64_t guard_type;
+class GlobalMutexGuard {
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+  static std::__libcpp_mutex_t guard_mut;
+  static std::__libcpp_condvar_t guard_cv;
+#endif
 
-void set_initialized(guard_type* guard_object) {
-    char* initialized = (char*)guard_object;
-    *initialized = 1;
+public:
+  GlobalMutexGuard() : has_lock(false) { lock(); }
+  ~GlobalMutexGuard() {
+    if (has_lock)
+      unlock();
   }
+  void lock() {
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+    if (std::__libcpp_mutex_lock(&guard_mut))
+      abort_message("failed to acquire mutex");
+    has_lock = true;
 #endif
-
-#if defined(_LIBCXXABI_HAS_NO_THREADS) || (defined(__APPLE__) && !defined(__arm__))
-#ifdef __arm__
-
-// Test the lowest bit.
-inline bool is_initialized(guard_type* guard_object) {
-    return (*guard_object) & 1;
   }
-
-#else
-
-bool is_initialized(guard_type* guard_object) {
-    char* initialized = (char*)guard_object;
-    return *initialized;
+  void unlock() {
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+    if (std::__libcpp_mutex_unlock(&guard_mut))
+      abort_message("__cxa_guard_release failed to acquire mutex");
+    has_lock = false;
+#endif
   }
 
+  void notify_all() {
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+    if (std::__libcpp_condvar_broadcast(&guard_cv))
+      abort_message("__cxa_guard failed to broadcast condition variable");
 #endif
+  }
+
+  void wait_for_notification() {
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+    if (std::__libcpp_condvar_wait(&guard_cv, &guard_mut))
+      abort_message("__cxa_guard_acquire condition variable wait failed");
 #endif
+  }
+
+private:
+  bool has_lock = false;
+};
 
 #ifndef _LIBCXXABI_HAS_NO_THREADS
-std::__libcpp_mutex_t guard_mut = _LIBCPP_MUTEX_INITIALIZER;
-std::__libcpp_condvar_t guard_cv = _LIBCPP_CONDVAR_INITIALIZER;
+std::__libcpp_mutex_t GlobalMutexGuard::guard_mut = _LIBCPP_MUTEX_INITIALIZER;
+std::__libcpp_condvar_t GlobalMutexGuard::guard_cv =
+    _LIBCPP_CONDVAR_INITIALIZER;
 #endif
 
 #if defined(__APPLE__) && !defined(__arm__)
+typedef uint32_t MAY_ALIAS lock_type;
+#else
+typedef bool MAY_ALIAS lock_type;
+#endif
 
-typedef uint32_t lock_type;
-
-#if __LITTLE_ENDIAN__
+#ifdef __arm__
+// A 32-bit, 4-byte-aligned static data value. The least significant 2 bits must
+// be statically initialized to 0.
+typedef uint32_t MAY_ALIAS guard_type;
+#else
+typedef uint64_t MAY_ALIAS guard_type;
+#endif
 
-inline
-lock_type
-get_lock(uint64_t x)
-{
-    return static_cast<lock_type>(x >> 32);
-}
+#if defined(__APPLE__) && !defined(__arm__) &&                                 \
+    !defined(_LIBCXXABI_HAS_NO_THREADS)
+// This is a special-case pthread dependency for Mac. We can't pull this
+// out into libcxx's threading API (__threading_support) because not all
+// supported Mac environments provide this function (in pthread.h). To
+// make it possible to build/use libcxx in those environments, we have to
+// keep this pthread dependency local to libcxxabi. If there is some
+// convenient way to detect precisely when pthread_mach_thread_np is
+// available in a given Mac environment, it might still be possible to
+// bury this dependency in __threading_support.
+#ifndef _LIBCPP_HAS_THREAD_API_PTHREAD
+#error "How do I pthread_mach_thread_np()?"
+#endif
+#else
+#define _LIBCXXABI_HAS_NO_DEADLOCK_DETECTION
+#endif
 
-inline
-void
-set_lock(uint64_t& x, lock_type y)
-{
-    x = static_cast<uint64_t>(y) << 32;
-}
+typedef unsigned char MAY_ALIAS byte_type;
 
-#else  // __LITTLE_ENDIAN__
+/// LockLayout - A type representing the location of the initialization "lock"
+/// within the guard object.
+struct MAY_ALIAS alignas(guard_type) LockLayout {
+#if defined(__APPLE__) && !defined(__arm__)
+#if __LITTLE_ENDIAN__
+  byte_type unused[sizeof(guard_type) - sizeof(lock_type)];
+  alignas(lock_type) byte_type lock[sizeof(lock_type)];
+#else
+  static_assert(alignof(guard_type) >= alignof(lock_type), "");
+  byte_type lock[sizeof(lock_type)];
+  byte_type unused[sizeof(guard_type) - sizeof(lock_type)];
+#endif
+#else
+  static_assert(sizeof(lock_type) == 1,
+                "layout requires single byte initialization lock");
+  byte_type unused1;
+  byte_type lock;
+  byte_type unused2[sizeof(guard_type) - 2];
+#endif
+};
 
-inline
-lock_type
-get_lock(uint64_t x)
-{
-    return static_cast<lock_type>(x);
-}
+static_assert(sizeof(LockLayout) == sizeof(guard_type), "");
+static_assert(alignof(LockLayout) == alignof(guard_type), "");
 
-inline
-void
-set_lock(uint64_t& x, lock_type y)
-{
-    x = y;
-}
+/// GuardObjectLayout - An abstraction for accessing the various fields of the
+/// guard object (using type-punning).
+union MAY_ALIAS GuardObjectLayout {
+private:
+  // Alternatives
+  guard_type m_guard;
+  LockLayout m_lock;
+  byte_type m_buff[sizeof(m_guard)];
 
-#endif  // __LITTLE_ENDIAN__
+public:
+  guard_type* guard() { return &m_guard; }
+  lock_type* lock() { return reinterpret_cast<lock_type*>(&m_lock.lock); }
+  byte_type* guard_byte(int n) { return &m_buff[n]; }
+};
 
-#else  // !__APPLE__ || __arm__
+struct GuardObject {
+  explicit GuardObject(guard_type* g)
+      : guard_object(reinterpret_cast<GuardObjectLayout*>(g)) {}
 
-typedef bool lock_type;
+  void clear() { memset(guard_object->guard(), 0, sizeof(guard_type)); }
 
-#if !defined(__arm__)
-static_assert(std::is_same<guard_type, uint64_t>::value, "");
+  bool is_object_initialized() const;
+  void set_object_initialized();
 
-inline lock_type get_lock(uint64_t x)
-{
-    union
-    {
-        uint64_t guard;
-        uint8_t lock[2];
-    } f = {x};
-    return f.lock[1] != 0;
+  void take_initialization_lock() {
+    *guard_object->lock() = get_lock_id_for_thread();
   }
 
-inline void set_lock(uint64_t& x, lock_type y)
-{
-    union
-    {
-        uint64_t guard;
-        uint8_t lock[2];
-    } f = {0};
-    f.lock[1] = y;
-    x = f.guard;
+  int wait_for_pending_initialization() {
+    while (is_locked_for_initialization()) {
+      global_lock_guard.wait_for_notification();
     }
-#else // defined(__arm__)
-static_assert(std::is_same<guard_type, uint32_t>::value, "");
-
-inline lock_type get_lock(uint32_t x)
-{
-    union
-    {
-        uint32_t guard;
-        uint8_t lock[2];
-    } f = {x};
-    return f.lock[1] != 0;
+    return is_object_initialized() ? INIT_COMPLETE : INIT_NOT_COMPLETE;
   }
 
-inline void set_lock(uint32_t& x, lock_type y)
-{
-    union
-    {
-        uint32_t guard;
-        uint8_t lock[2];
-    } f = {0};
-    f.lock[1] = y;
-    x = f.guard;
+  void release_global_lock_and_notify_all() {
+    global_lock_guard.unlock();
+    global_lock_guard.notify_all();
   }
 
-#endif // !defined(__arm__)
+private:
+  GuardObject() = delete;
+  GuardObject(GuardObject const&) = delete;
+  GuardObject& operator=(GuardObject const&) = delete;
 
-#endif  // __APPLE__ && !__arm__
+  lock_type get_initialization_lock_value() const {
+    lock_type result;
+    memcpy(&result, guard_object->lock(), sizeof(lock_type));
+    return result;
+  }
 
-}  // unnamed namespace
+  void set_initialization_lock(lock_type val) {
+    memcpy(guard_object->lock(), &val, sizeof(lock_type));
+  }
 
-extern "C"
-{
+  bool is_locked_for_initialization() {
+    lock_type lock_val = *guard_object->lock();
+#ifndef _LIBCXXABI_HAS_NO_DEADLOCK_DETECTION
+    if (lock_val == get_lock_id_for_thread())
+      abort_message(
+          "__cxa_guard_acquire deadlock detected during initialization");
+#endif
+    return lock_val != 0;
+  }
 
-#ifndef _LIBCXXABI_HAS_NO_THREADS
-_LIBCXXABI_FUNC_VIS int __cxa_guard_acquire(guard_type *guard_object) {
-    char* initialized = (char*)guard_object;
-    if (std::__libcpp_mutex_lock(&guard_mut))
-        abort_message("__cxa_guard_acquire failed to acquire mutex");
-    int result = *initialized == 0;
-    if (result)
-    {
-#if defined(__APPLE__) && !defined(__arm__)
-        // This is a special-case pthread dependency for Mac. We can't pull this
-        // out into libcxx's threading API (__threading_support) because not all
-        // supported Mac environments provide this function (in pthread.h). To
-        // make it possible to build/use libcxx in those environments, we have to
-        // keep this pthread dependency local to libcxxabi. If there is some
-        // convenient way to detect precisely when pthread_mach_thread_np is
-        // available in a given Mac environment, it might still be possible to
-        // bury this dependency in __threading_support.
-        #ifdef _LIBCPP_HAS_THREAD_API_PTHREAD
-           const lock_type id = pthread_mach_thread_np(std::__libcpp_thread_get_current_id());
-        #else
-           #error "How do I pthread_mach_thread_np()?"
-        #endif
-        lock_type lock = get_lock(*guard_object);
-        if (lock)
-        {
-            // if this thread set lock for this same guard_object, abort
-            if (lock == id)
-                abort_message("__cxa_guard_acquire detected deadlock");
-            do
-            {
-                if (std::__libcpp_condvar_wait(&guard_cv, &guard_mut))
-                    abort_message("__cxa_guard_acquire condition variable wait failed");
-                lock = get_lock(*guard_object);
-            } while (lock);
-            result = !is_initialized(guard_object);
-            if (result)
-                set_lock(*guard_object, id);
-        }
-        else
-            set_lock(*guard_object, id);
-#else  // !__APPLE__ || __arm__
-        while (get_lock(*guard_object))
-            if (std::__libcpp_condvar_wait(&guard_cv, &guard_mut))
-                abort_message("__cxa_guard_acquire condition variable wait failed");
-        result = *initialized == 0;
-        if (result)
-            set_lock(*guard_object, true);
-#endif  // !__APPLE__ || __arm__
+  lock_type get_lock_id_for_thread() {
+#if defined(_LIBCXXABI_HAS_NO_DEADLOCK_DETECTION)
+    return true;
+    if (!has_lock_id_cache) {
+      has_lock_id_cache = true;
+      cached_lock_id =
+          pthread_mach_thread_np(std::__libcpp_thread_get_current_id());
     }
-    if (std::__libcpp_mutex_unlock(&guard_mut))
-        abort_message("__cxa_guard_acquire failed to release mutex");
-    return result;
+    return cached_lock_id;
+#endif
   }
 
-_LIBCXXABI_FUNC_VIS void __cxa_guard_release(guard_type *guard_object) {
-    if (std::__libcpp_mutex_lock(&guard_mut))
-        abort_message("__cxa_guard_release failed to acquire mutex");
-    *guard_object = 0;
-    set_initialized(guard_object);
-    if (std::__libcpp_mutex_unlock(&guard_mut))
-        abort_message("__cxa_guard_release failed to release mutex");
-    if (std::__libcpp_condvar_broadcast(&guard_cv))
-        abort_message("__cxa_guard_release failed to broadcast condition variable");
+private:
+  GuardObjectLayout* guard_object;
+  GlobalMutexGuard global_lock_guard;
+
+#ifndef _LIBCXXABI_HAS_NO_DEADLOCK_DETECTION
+  bool has_lock_id_cache = false;
+  lock_type cached_lock_id;
+#endif
+};
+
+bool GuardObject::is_object_initialized() const {
+#ifdef __arm__
+  return (*guard_object->guard()) & 1;
+#else
+  return *guard_object->guard_byte(0);
+#endif
 }
 
-_LIBCXXABI_FUNC_VIS void __cxa_guard_abort(guard_type *guard_object) {
-    if (std::__libcpp_mutex_lock(&guard_mut))
-        abort_message("__cxa_guard_abort failed to acquire mutex");
-    *guard_object = 0;
-    if (std::__libcpp_mutex_unlock(&guard_mut))
-        abort_message("__cxa_guard_abort failed to release mutex");
-    if (std::__libcpp_condvar_broadcast(&guard_cv))
-        abort_message("__cxa_guard_abort failed to broadcast condition variable");
+void GuardObject::set_object_initialized() {
+#if defined(__arm__)
+  (*guard_object->guard()) |= 1;
+#else
+  *guard_object->guard_byte(0) = 1;
+#endif
 }
 
-#else // _LIBCXXABI_HAS_NO_THREADS
+} // unnamed namespace
+
+extern "C" {
 
-_LIBCXXABI_FUNC_VIS int __cxa_guard_acquire(guard_type *guard_object) {
-    return !is_initialized(guard_object);
+_LIBCXXABI_FUNC_VIS int __cxa_guard_acquire(guard_type* guard_object) {
+  GuardObject guard(guard_object);
+
+  if (guard.wait_for_pending_initialization() == INIT_COMPLETE)
+    return INIT_COMPLETE;
+
+  guard.take_initialization_lock();
+  return INIT_NOT_COMPLETE;
 }
 
-_LIBCXXABI_FUNC_VIS void __cxa_guard_release(guard_type *guard_object) {
-    *guard_object = 0;
-    set_initialized(guard_object);
+_LIBCXXABI_FUNC_VIS void __cxa_guard_release(guard_type* guard_object) {
+  GuardObject guard(guard_object);
+  guard.clear();
+  guard.set_object_initialized();
+  guard.release_global_lock_and_notify_all();
 }
 
-_LIBCXXABI_FUNC_VIS void __cxa_guard_abort(guard_type *guard_object) {
-    *guard_object = 0;
+_LIBCXXABI_FUNC_VIS void __cxa_guard_abort(guard_type* guard_object) {
+  GuardObject guard(guard_object);
+  guard.clear();
+  guard.release_global_lock_and_notify_all();
 }
 
-#endif // !_LIBCXXABI_HAS_NO_THREADS
-
-}  // extern "C"
+} // extern "C"
 
-}  // __cxxabiv1
+} // namespace __cxxabiv1