[libcxx-commits] [libcxxabi] r357804 - Further refactor cxa_guard.cpp

Eric Fiselier via libcxx-commits libcxx-commits at lists.llvm.org
Fri Apr 5 12:58:15 PDT 2019


Author: ericwf
Date: Fri Apr  5 12:58:15 2019
New Revision: 357804

URL: http://llvm.org/viewvc/llvm-project?rev=357804&view=rev
Log:
Further refactor cxa_guard.cpp

This patch is a part of a series of patches to cleanup
our implementation of __cxa_acquire et al. No functionality
change was intended.

This patch does two primary things.

It introduces the GuardObject class to abstract the reading
and writing to the guard object. In future, it will be used
to ensure atomic accesses are used when needed.

It also introduces the GuardValue class used to represent
values of the guard object. It is an abstraction to access
and write to the various different bits of a guard.

Modified:
    libcxxabi/trunk/src/cxa_guard.cpp

Modified: libcxxabi/trunk/src/cxa_guard.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_guard.cpp?rev=357804&r1=357803&r2=357804&view=diff
==============================================================================
--- libcxxabi/trunk/src/cxa_guard.cpp (original)
+++ libcxxabi/trunk/src/cxa_guard.cpp Fri Apr  5 12:58:15 2019
@@ -12,6 +12,7 @@
 #include <__threading_support>
 
 #include <stdint.h>
+#include <string.h>
 
 /*
     This implementation must be careful to not call code external to this file
@@ -29,32 +30,38 @@ namespace __cxxabiv1
 namespace
 {
 
+enum InitializationResult {
+  INIT_COMPLETE,
+  INIT_NOT_COMPLETE,
+};
+
 #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;
-
-inline void set_initialized(guard_type* guard_object) {
-    *guard_object |= 1;
-}
-
-// Test the lowest bit.
-inline bool is_initialized(guard_type* guard_object) {
-    return (*guard_object) & 1;
-}
-
 #else
 typedef uint64_t guard_type;
+#endif
 
-void set_initialized(guard_type* guard_object) {
-    char* initialized = (char*)guard_object;
-    *initialized = 1;
-}
-
-bool is_initialized(guard_type* guard_object) {
-    char* initialized = (char*)guard_object;
-    return *initialized;
-}
+#if !defined(_LIBCXXABI_HAS_NO_THREADS) && 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.
+#ifndef _LIBCPP_HAS_THREAD_API_PTHREAD
+#error "How do I pthread_mach_thread_np()?"
+#endif
+#define LIBCXXABI_HAS_DEADLOCK_DETECTION
+#define LOCK_ID_FOR_THREAD() pthread_mach_thread_np(std::__libcpp_thread_get_current_id())
+typedef uint32_t lock_type;
+#else
+#define LOCK_ID_FOR_THREAD() true
+typedef bool lock_type;
 #endif
 
 enum class OnRelease : char { UNLOCK, UNLOCK_AND_BROADCAST };
@@ -106,164 +113,189 @@ std::__libcpp_condvar_t GlobalMutexGuard
     _LIBCPP_CONDVAR_INITIALIZER;
 #endif
 
-#if defined(__APPLE__) && !defined(__arm__)
+struct GuardObject;
 
-typedef uint32_t lock_type;
+/// GuardValue - An abstraction for accessing the various fields and bits of
+///   the guard object.
+struct GuardValue {
+private:
+  explicit GuardValue(guard_type v) : value(v) {}
+  friend struct GuardObject;
 
-#if __LITTLE_ENDIAN__
+public:
+  /// Functions returning the values used to represent the uninitialized,
+  /// initialized, and initialization pending states.
+  static GuardValue ZERO();
+  static GuardValue INIT_COMPLETE();
+  static GuardValue INIT_PENDING();
+
+  /// Returns true if the guard value represents that the initialization is
+  /// complete.
+  bool is_initialization_complete() const;
+
+  /// Returns true if the guard value represents that the initialization is
+  /// currently pending.
+  bool is_initialization_pending() const;
 
-inline
-lock_type
-get_lock(uint64_t x)
-{
-    return static_cast<lock_type>(x >> 32);
-}
+  /// Returns the lock value for the current guard value.
+  lock_type get_lock_value() const;
 
-inline
-void
-set_lock(uint64_t& x, lock_type y)
-{
-    x = static_cast<uint64_t>(y) << 32;
-}
+private:
+  // Returns a guard object corresponding to the specified lock value.
+  static guard_type guard_value_from_lock(lock_type l);
 
-#else  // __LITTLE_ENDIAN__
+  // Returns the lock value represented by the specified guard object.
+  static lock_type lock_value_from_guard(guard_type g);
 
-inline
-lock_type
-get_lock(uint64_t x)
-{
-    return static_cast<lock_type>(x);
-}
+private:
+  guard_type value;
+};
 
-inline
-void
-set_lock(uint64_t& x, lock_type y)
+/// GuardObject - Manages correctly reading and writing to the guard object.
+struct GuardObject {
+  explicit GuardObject(guard_type *g) : guard(g) {}
+
+  // Read the current value of the guard object.
+  // TODO: Make this read atomic.
+  GuardValue read() const;
+
+  // Write the specified value to the guard object.
+  // TODO: Make this atomic
+  void write(GuardValue new_val);
+
+private:
+  GuardObject(const GuardObject&) = delete;
+  GuardObject& operator=(const GuardObject&) = delete;
+
+  guard_type *guard;
+};
+
+}  // unnamed namespace
+
+extern "C"
 {
-    x = y;
-}
 
-#endif  // __LITTLE_ENDIAN__
+_LIBCXXABI_FUNC_VIS int __cxa_guard_acquire(guard_type* raw_guard_object) {
+  GlobalMutexGuard gmutex("__cxa_guard_acquire", OnRelease::UNLOCK);
+  GuardObject guard(raw_guard_object);
+  GuardValue current_value = guard.read();
 
-#else  // !__APPLE__ || __arm__
+  if (current_value.is_initialization_complete())
+    return INIT_COMPLETE;
 
-typedef bool lock_type;
+  const GuardValue LOCK_ID = GuardValue::INIT_PENDING();
+#ifdef LIBCXXABI_HAS_DEADLOCK_DETECTION
+   if (current_value.is_initialization_pending() &&
+       current_value.get_lock_value() == LOCK_ID.get_lock_value()) {
+    abort_message("__cxa_guard_acquire detected deadlock");
+  }
+#endif
+  while (current_value.is_initialization_pending()) {
+      gmutex.wait_for_signal();
+      current_value = guard.read();
+  }
+  if (current_value.is_initialization_complete())
+    return INIT_COMPLETE;
 
-#if !defined(__arm__)
-static_assert(std::is_same<guard_type, uint64_t>::value, "");
+  guard.write(LOCK_ID);
+  return INIT_NOT_COMPLETE;
+}
 
-inline lock_type get_lock(uint64_t x)
-{
-    union
-    {
-        uint64_t guard;
-        uint8_t lock[2];
-    } f = {x};
-    return f.lock[1] != 0;
+_LIBCXXABI_FUNC_VIS void __cxa_guard_release(guard_type *raw_guard_object) {
+  GlobalMutexGuard gmutex("__cxa_guard_release",
+                          OnRelease::UNLOCK_AND_BROADCAST);
+  GuardObject guard(raw_guard_object);
+  guard.write(GuardValue::ZERO());
+  guard.write(GuardValue::INIT_COMPLETE());
 }
 
-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;
+_LIBCXXABI_FUNC_VIS void __cxa_guard_abort(guard_type *raw_guard_object) {
+  GlobalMutexGuard gmutex("__cxa_guard_abort", OnRelease::UNLOCK);
+  GuardObject guard(raw_guard_object);
+  guard.write(GuardValue::ZERO());
 }
-#else // defined(__arm__)
-static_assert(std::is_same<guard_type, uint32_t>::value, "");
+}  // extern "C"
 
-inline lock_type get_lock(uint32_t x)
-{
-    union
-    {
-        uint32_t guard;
-        uint8_t lock[2];
-    } f = {x};
-    return f.lock[1] != 0;
+//===----------------------------------------------------------------------===//
+//                        GuardObject Definitions
+//===----------------------------------------------------------------------===//
+
+GuardValue GuardObject::read() const {
+  // FIXME: Make this atomic
+  guard_type val = *guard;
+  return GuardValue(val);
 }
 
-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 GuardObject::write(GuardValue new_val) {
+  // FIXME: make this atomic
+  *guard = new_val.value;
 }
 
-#endif // !defined(__arm__)
+//===----------------------------------------------------------------------===//
+//                        GuardValue Definitions
+//===----------------------------------------------------------------------===//
 
-#endif  // __APPLE__ && !__arm__
+GuardValue GuardValue::ZERO() { return GuardValue(0); }
 
-}  // unnamed namespace
+GuardValue GuardValue::INIT_COMPLETE() {
+  guard_type value = {0};
+#ifdef __arm__
+  value |= 1;
+#else
+  char* init_bit = (char*)&value;
+  *init_bit = 1;
+#endif
+  return GuardValue(value);
+}
 
-extern "C"
-{
+GuardValue GuardValue::INIT_PENDING() {
+  return GuardValue(guard_value_from_lock(LOCK_ID_FOR_THREAD()));
+}
 
-_LIBCXXABI_FUNC_VIS int __cxa_guard_acquire(guard_type *guard_object) {
-  GlobalMutexGuard gmutex("__cxa_guard_acquire", OnRelease::UNLOCK);
-  int result = !is_initialized(guard_object);
-  if (result) {
-#if defined(_LIBCXXABI_HAS_NO_THREADS)
-    // nothing to do
-#elif 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());
+bool GuardValue::is_initialization_complete() const {
+#ifdef __arm__
+  return value & 1;
 #else
-#error "How do I pthread_mach_thread_np()?"
+  const char* init_bit = (const char*)&value;
+  return *init_bit;
 #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 {
-        gmutex.wait_for_signal();
-        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)) {
-      gmutex.wait_for_signal();
-    }
-    result = !is_initialized(guard_object);
-    if (result)
-      set_lock(*guard_object, true);
-#endif  // !__APPLE__ || __arm__
-  }
-  return result;
 }
 
-_LIBCXXABI_FUNC_VIS void __cxa_guard_release(guard_type *guard_object) {
-  GlobalMutexGuard gmutex("__cxa_guard_release",
-                          OnRelease::UNLOCK_AND_BROADCAST);
-  *guard_object = 0;
-  set_initialized(guard_object);
+bool GuardValue::is_initialization_pending() const {
+  return lock_value_from_guard(value) != 0;
 }
 
-_LIBCXXABI_FUNC_VIS void __cxa_guard_abort(guard_type *guard_object) {
-  GlobalMutexGuard gmutex("__cxa_guard_abort", OnRelease::UNLOCK);
-  *guard_object = 0;
+lock_type GuardValue::get_lock_value() const {
+  return lock_value_from_guard(value);
 }
 
+// Create a guard object with the lock set to the specified value.
+guard_type GuardValue::guard_value_from_lock(lock_type l) {
+#if defined(__APPLE__) && !defined(__arm__)
+#if __LITTLE_ENDIAN__
+  return static_cast<guard_type>(l) << 32;
+#else
+  return static_cast<guard_type>(l);
+#endif
+#else  // defined(__APPLE__) && !defined(__arm__)
+  guard_type f = {0};
+  memcpy(static_cast<char*>(static_cast<void*>(&f)) + 1, &l, sizeof(lock_type));
+  return f;
+#endif // defined(__APPLE__) && !defined(__arm__)
+}
 
-}  // extern "C"
+lock_type GuardValue::lock_value_from_guard(guard_type g) {
+#if defined(__APPLE__) && !defined(__arm__)
+#if __LITTLE_ENDIAN__
+  return static_cast<lock_type>(g >> 32);
+#else
+  return static_cast<lock_type>(g);
+#endif
+#else  // defined(__APPLE__) && !defined(__arm__)
+  uint8_t guard_bytes[sizeof(guard_type)];
+  memcpy(&guard_bytes, &g, sizeof(guard_type));
+  return guard_bytes[1] != 0;
+#endif // defined(__APPLE__) && !defined(__arm__)
+}
 
 }  // __cxxabiv1




More information about the libcxx-commits mailing list