libc++abi: pave the road for Windows threading model

[compnerd]

Summary

Create an abstraction layer for the threading model. The signatures closely
resemble pthreads for now, since it makes it more obvious to see that this is
purely an indirection layer. In the future, it may turn out to change the
signature, but should be possible to create Win32 equivalents using the same
signatures.

The change to the test is benign as it assumed the pthread threading model;
defining _POSIX_THREADS simply specifies it rather than just including
pthread.h.

[jroelofs]

src/__cxa_thread.h

28

I think it'd be better to keep these out of the __cxa_* namespace, as they're implementation details of libc++abi, and not part of its official public interface (as defined here: https://mentorembedded.github.io/cxx-abi/abi.html).

Something like __libcxxabi_* perhaps?

That should reduce hassles if anyone wants to standardize one of these names later on.

[compnerd]

src/__cxa_thread.h

28

That is a brilliant idea. Ill update the patch with an alternate prefix (__cxxabi_).

[compnerd]

Address naming suggestion from @jroelofs.

[EricWF]

Overall the patch seems like progress. However I know nothing about the Windows threading API's. Do you have experience with them? Are they even remotely similar to the POSIX ones? If not what to we gain by using POSIX API's under a different name?

include/__cxxabi_config.h

44

_value_ should be a reserved name.

44

__cxxabi_config.h is an installed header, but these macros are only used by the source files. They should probably live elsewhere.

src/__cxa_thread.h

32

This seems incorrect. Things like PTHREAD_ONCE_INIT are macros defined to be certain expressions. They don't think that's replacable using a named variable. For example on OS X the definition of 'PTHREAD_ONCE_INIT' is:

#define PTHREAD_ONCE_INIT {_PTHREAD_ONCE_SIG_init, {0}}

For that reason these new initializers will likely also have to be macros.

[compnerd]

The names are just that: names. I just used similar names to make the change more obvious. There are equivalent operations for each one of these even though they may not be written the same. If you have opinions on the naming, Im not hard set on the naming scheme.

include/__cxxabi_config.h

44

Recommendations for where? I don't have a problem with moving them.

src/__cxa_thread.h

32

Okay, I can change them to macros.

[EricWF]

src/__cxa_thread.h

32

Nevermind, I'm wrong about this. So long as we always have constexpr this should work OK.

[rmaprath]

CMakeLists.txt

277–278

Perhaps do something similar to what is done in libcxx at the moment? There we provide an explicit cmake option to select pthreads in case the default selection mechanism (in include/__config) cannot figure out the thread implementation based on the platform.

src/__cxa_thread.h

23

May be update to follow the convention used in libcxx? I.e. something like _LIBCXXABI_HAS_THREAD_API_PTHREAD.

38

Perhaps define a macro for this? Similar to _LIBCPP_ALWAYS_INLINE in libcxx.

[compnerd]

src/__cxa_thread.h

23

_POSIX_THREADS is a better solution IMO. Its defined by the specification itself. Furthermore, on platforms where POSIX threads are available, it allows you to select between them and an alternate threading mechanism. Consider Solaris, you can use pthreads or Solaris threads.

[rmaprath]

Btw, this review isn't posted on cfe-commits, you might need a new review to fix this. IIRC, adding cfe-commits later on does not work. Perhaps one of the reason @mclow.lists missed it!

[rmaprath]

@compnerd: Pinged you on IRC a couple of times, not sure if you got them. Can I steal this patch and implement my own changes on top of it? Basically, turn this into the mirror image of what we did with libcxx.

@mclow.lists has approved the libc++ patch (and it has landed), so I think we can ask @EricWF to review the rest of the patches (libunwind and libc++abi).

WDYT?

[compnerd]

Yeah, go ahead and commandeer the patch. I wouldn't be able to work on this outside of my spare time :)

I can review the unwind changes :)

[compnerd]

This is no longer needed with the external threading.

CMakeLists.txt

 
 if (NOT LIBCXXABI_ENABLE_THREADS)
   add_definitions(-DLIBCXXABI_HAS_NO_THREADS=1)
+else ()
+  # TODO: this should be defined for all targets when using the POSIX threading
+  # model by the driver as on NetBSD
+  add_definitions(-D_POSIX_THREADS)
 endif()
 
 # This is the _ONLY_ place where add_definitions is called.

include/__cxxabi_config.h

  #endif
 #endif
 
+#define _LIBCXXABI_STRINGIFY_EXPANDED(_value_) #_value_
+#define _LIBCXXABI_STRINGIFY(_value_) _LIBCXXABI_STRINGIFY_EXPANDED(_value_)
+
+#if defined(__GNUC__) || defined(__clang__)
+#define _LIBCXXABI_PRAGMA(_parameter_) _Pragma(#_parameter_)
+#define _LIBCXXABI_WARNING(_message_) _LIBCXXABI_PRAGMA(GCC warning _message_)
+#elif defined(_MSC_VER)
+#define _LIBCXXABI_WARNING(_message_)                                          \
+  __pragma(message(__FILE__ "(" _LIBCXXABI_STRINGIFY(__LINE__) "): warning: " _message_))
+#else
+#define _LIBCXXABI_WARNING(_message_)
+#endif
+
 #endif // ____CXXABI_CONFIG_H

src/__cxa_thread.h

+//===---------------------------- cxa_thread.h ----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef ____CXA_THREAD_H
+#define ____CXA_THREAD_H
+
+#include "__cxxabi_config.h"
+
+#if defined(LIBCXXABI_HAS_NO_THREADS) && LIBCXXABI_HAS_NO_THREADS == 1
+
+#else
+
+#if defined(_WIN32)
+
+_LIBCXXABI_WARNING("unsupported threading model");
+
+#elif defined(_POSIX_THREADS)
+
+#include <pthread.h>
+
+typedef pthread_cond_t __cxxabi_condition_variable;
+typedef pthread_once_t __cxxabi_execute_once_control;
+typedef pthread_mutex_t __cxxabi_mutex;
+typedef pthread_key_t __cxxabi_thread_storage_key;
+
+constexpr __cxxabi_condition_variable __CXXABI_CONDITION_VARIABLE_INITIALIZER =
+    PTHREAD_COND_INITIALIZER;
+constexpr __cxxabi_execute_once_control __CXXABI_EXECUTE_ONCE_INITIALIZER =
+    PTHREAD_ONCE_INIT;
+constexpr __cxxabi_mutex __CXXABI_MUTEX_INITIALIZER = PTHREAD_MUTEX_INITIALIZER;
+
+inline int __attribute__((__always_inline__))
+__cxxabi_condition_variable_wait(__cxxabi_condition_variable *cv,
+                                 __cxxabi_mutex *mutex) {
+  return pthread_cond_wait(cv, mutex);
+}
+
+inline int __attribute__((__always_inline__))
+__cxxabi_condition_variable_wake_all(__cxxabi_condition_variable *cv) {
+  return pthread_cond_broadcast(cv);
+}
+
+inline int __attribute__((__always_inline__))
+__cxxabi_execute_once(__cxxabi_execute_once_control *flag,
+                      void (*init_routine)(void)) {
+  return pthread_once(flag, init_routine);
+}
+
+inline int __attribute__((__always_inline__))
+__cxxabi_mutex_lock(__cxxabi_mutex *mutex) {
+  return pthread_mutex_lock(mutex);
+}
+
+inline int __attribute__((__always_inline__))
+__cxxabi_mutex_unlock(__cxxabi_mutex *mutex) {
+  return pthread_mutex_unlock(mutex);
+}
+
+inline int __attribute__((__always_inline__))
+__cxxabi_thread_specific_key_init(__cxxabi_thread_storage_key *key,
+                                  void (*destructor)(void *)) {
+  return pthread_key_create(key, destructor);
+}
+
+inline void *__attribute__((__always_inline__))
+__cxxabi_get_thread_specific(__cxxabi_thread_storage_key key) {
+  return pthread_getspecific(key);
+}
+
+inline int __attribute__((__always_inline__))
+__cxxabi_set_thread_specific(__cxxabi_thread_storage_key key, void *value) {
+  return pthread_setspecific(key, value);
+}
+
+#else
+
+_LIBCXXABI_WARNING("unknown threading model");
+
+#endif
+
+#endif
+
+#endif
+

src/cxa_exception.cpp

 
 #include "config.h"
 #include "cxxabi.h"
+#include "__cxa_thread.h"
 
 #include <exception>        // for std::terminate
 #include <cstdlib>          // for malloc, free
 #include <cstring>          // for memset
-#if !LIBCXXABI_HAS_NO_THREADS
-#  include <pthread.h>      // for fallback_malloc.ipp's mutexes
-#endif
 #include "cxa_exception.hpp"
 #include "cxa_handlers.hpp"
 

src/cxa_exception_storage.cpp

 //===----------------------------------------------------------------------===//
 
 #include "cxa_exception.hpp"
+#include "__cxa_thread.h"
 
 #include "config.h"
 

 
 #else
 
-#include <pthread.h>
 #include <cstdlib>          // for calloc, free
 #include "abort_message.h"
 

 
 namespace __cxxabiv1 {
 namespace {
-    pthread_key_t  key_;
-    pthread_once_t flag_ = PTHREAD_ONCE_INIT;
+    __cxxabi_thread_storage_key key_;
+    __cxxabi_execute_once_control flag_ = __CXXABI_EXECUTE_ONCE_INITIALIZER;
 
     void destruct_ (void *p) {
         std::free ( p );
-        if ( 0 != ::pthread_setspecific ( key_, NULL ) ) 
+        if ( 0 != __cxxabi_set_thread_specific ( key_, NULL ) )
             abort_message("cannot zero out thread value for __cxa_get_globals()");
         }
 
     void construct_ () {
-        if ( 0 != pthread_key_create ( &key_, destruct_ ) )
-            abort_message("cannot create pthread key for __cxa_get_globals()");
+        if ( 0 != __cxxabi_thread_specific_key_init ( &key_, destruct_ ) )
+            abort_message("cannot create thread specific key for __cxa_get_globals()");
         }
 }   
 

                         (std::calloc (1, sizeof (__cxa_eh_globals)));
             if ( NULL == retVal )
                 abort_message("cannot allocate __cxa_eh_globals");
-            if ( 0 != pthread_setspecific ( key_, retVal ) )
-               abort_message("pthread_setspecific failure in __cxa_get_globals()");
+            if ( 0 != __cxxabi_set_thread_specific ( key_, retVal ) )
+               abort_message("__cxxabi_set_thread_specific failure in __cxa_get_globals()");
            }
         return retVal;
         }

     // libc++abi.
     __cxa_eh_globals * __cxa_get_globals_fast () {
     //  First time through, create the key.
-        if (0 != pthread_once(&flag_, construct_))
-            abort_message("pthread_once failure in __cxa_get_globals_fast()");
+        if (0 != __cxxabi_execute_once(&flag_, construct_))
+            abort_message("execute once failure in __cxa_get_globals_fast()");
 //        static int init = construct_();
-        return static_cast<__cxa_eh_globals*>(::pthread_getspecific(key_));
+        return static_cast<__cxa_eh_globals*>(__cxxabi_get_thread_specific(key_));
         }
     
 }

src/cxa_guard.cpp

 //===----------------------------------------------------------------------===//
 
 #include "__cxxabi_config.h"
+#include "__cxa_thread.h"
 
 #include "abort_message.h"
 #include "config.h"
 
-#if !LIBCXXABI_HAS_NO_THREADS
-#  include <pthread.h>
-#endif
 #include <stdint.h>
 
 /*
     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.
+    which will turn around and try to call __cxxabi_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
-    pthread_mutex_lock and the abort_message utility without problem.  This
-    implementation also uses pthread_cond_wait which has tested to not be a
-    problem.
+    __cxxabi_mutex_lock and the abort_message utility without problem.  This
+    implementation also uses __cxxabi_condition_variable_wait which has tested
+    to not be a problem.
 */
 
 namespace __cxxabiv1

 #endif
 
 #if !LIBCXXABI_HAS_NO_THREADS
-pthread_mutex_t guard_mut = PTHREAD_MUTEX_INITIALIZER;
-pthread_cond_t  guard_cv  = PTHREAD_COND_INITIALIZER;
+__cxxabi_mutex guard_mut = __CXXABI_MUTEX_INITIALIZER;
+__cxxabi_condition_variable guard_cv = __CXXABI_CONDITION_VARIABLE_INITIALIZER;
 #endif
 
 #if defined(__APPLE__) && !defined(__arm__)

 
 _LIBCXXABI_FUNC_VIS int __cxa_guard_acquire(guard_type *guard_object) {
     char* initialized = (char*)guard_object;
-    if (pthread_mutex_lock(&guard_mut))
+    if (__cxxabi_mutex_lock(&guard_mut))
         abort_message("__cxa_guard_acquire failed to acquire mutex");
     int result = *initialized == 0;
     if (result)

                 abort_message("__cxa_guard_acquire detected deadlock");
             do
             {
-                if (pthread_cond_wait(&guard_cv, &guard_mut))
+                if (__cxxabi_condition_variable_wait(&guard_cv, &guard_mut))
                     abort_message("__cxa_guard_acquire condition variable wait failed");
                 lock = get_lock(*guard_object);
             } while (lock);

             set_lock(*guard_object, id);
 #else  // !__APPLE__ || __arm__
         while (get_lock(*guard_object))
-            if (pthread_cond_wait(&guard_cv, &guard_mut))
+            if (__cxxabi_condition_variable_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__
     }
-    if (pthread_mutex_unlock(&guard_mut))
+    if (__cxxabi_mutex_unlock(&guard_mut))
         abort_message("__cxa_guard_acquire failed to release mutex");
     return result;
 }
 
 _LIBCXXABI_FUNC_VIS void __cxa_guard_release(guard_type *guard_object) {
-    if (pthread_mutex_lock(&guard_mut))
+    if (__cxxabi_mutex_lock(&guard_mut))
         abort_message("__cxa_guard_release failed to acquire mutex");
     *guard_object = 0;
     set_initialized(guard_object);
-    if (pthread_mutex_unlock(&guard_mut))
+    if (__cxxabi_mutex_unlock(&guard_mut))
         abort_message("__cxa_guard_release failed to release mutex");
-    if (pthread_cond_broadcast(&guard_cv))
+    if (__cxxabi_condition_variable_wake_all(&guard_cv))
         abort_message("__cxa_guard_release failed to broadcast condition variable");
 }
 
 _LIBCXXABI_FUNC_VIS void __cxa_guard_abort(guard_type *guard_object) {
-    if (pthread_mutex_lock(&guard_mut))
+    if (__cxxabi_mutex_lock(&guard_mut))
         abort_message("__cxa_guard_abort failed to acquire mutex");
     *guard_object = 0;
-    if (pthread_mutex_unlock(&guard_mut))
+    if (__cxxabi_mutex_unlock(&guard_mut))
         abort_message("__cxa_guard_abort failed to release mutex");
-    if (pthread_cond_broadcast(&guard_cv))
+    if (__cxxabi_condition_variable_wake_all(&guard_cv))
         abort_message("__cxa_guard_abort failed to broadcast condition variable");
 }
 

src/fallback_malloc.ipp

 #if LIBCXXABI_HAS_NO_THREADS
 static void * heap_mutex = 0;
 #else
-static pthread_mutex_t heap_mutex = PTHREAD_MUTEX_INITIALIZER;
+static __cxxabi_mutex heap_mutex = __CXXABI_MUTEX_INITIALIZER;
 #endif
 
 class mutexor {

     mutexor ( void * ) {}
     ~mutexor () {}
 #else
-    mutexor ( pthread_mutex_t *m ) : mtx_(m) { pthread_mutex_lock ( mtx_ ); }
-    ~mutexor () { pthread_mutex_unlock ( mtx_ ); }
+    mutexor ( __cxxabi_mutex *m ) : mtx_(m) { __cxxabi_mutex_lock ( mtx_ ); }
+    ~mutexor () { __cxxabi_mutex_unlock ( mtx_ ); }
 #endif
 private:
     mutexor ( const mutexor &rhs );
     mutexor & operator = ( const mutexor &rhs );
 #if !LIBCXXABI_HAS_NO_THREADS
-    pthread_mutex_t *mtx_;
+    __cxxabi_mutex *mtx_;
 #endif
     };
 

test/test_fallback_malloc.pass.cpp

 #include <iostream>
 #include <deque>
 
-#include <pthread.h>
+#define _POSIX_THREADS
+#include "../src/__cxa_thread.h"
 
 typedef std::deque<void *> container;