[libcxxabi] Fix build of cxa_guard.cpp on configurations with _LIBCXXABI_HAS_NO_THREADS

[JamesNagurne]

Summary

Fix build of cxa_guard.cpp on configurations with _LIBCXXABI_HAS_NO_THREADS

cxa_guard_impl.h currently contains a forward declaration of two classes,
LibcppMutex and LibcppCondVar, whose definitions are guarded by
!defined(_LIBCXXABI_HAS_NO_THREADS). When this configuration is set, the
definitions are not seen, and the compiler emits an error when the
incomplete types are used.

I've refined the locations of the guards around these definitions to only the
spots where not having thread support matters, and have stubbed those
definitions with the expected return code.

[JamesNagurne]

This is an example of a solution, but I'm not married to the mechanisms by which the results are achieved:

• If you note the definition of CurrentImplementation on line 555, there's no path by which GlobalLock is used when _LIBCXXABI_HAS_NO_THREADS is set. Thus, we could alternatively guard the entirety of the implementation of InitByteGlobalMutex and insert a stub for the class.
• The code could use helpers that either call __libcpp_mutex_*/_libcpp_cond_* or return 0, so that the implementation of LibcppCondVar and LibcppMutex are clear of preprocessor directives.

[EricWF]

Hi James,

It's embarrassing we don't have test coverage for this. I've filed llvm.org/PR41596 to add buildbots for this configuration.
Sorry for the breakage and thanks for the patch.

libcxxabi/src/cxa_guard_impl.h

241

We just need to make the types complete. They don't need a mock interface. (In fact they shouldn't have one).

How about this:

1. Remove the forward declarations.
2. #ifdef out the entire body of LibcppMutex and LibcppCondVar when threads are disabled.

Ex:

struct LibcppMutex {
#ifndef _LIBCXXABI_HAS_NO_THREADS
  ...
#endif
};

[JamesNagurne]

libcxxabi/src/cxa_guard_impl.h

241

It looks like someone already made a commit. to fix this. Do you still want this modification to my patch?
I apologize for the delayed response. The responses came during my after-hours.

[sbc100]

Looks like someone already landed a fix: rL359175

[JamesNagurne]

I saw that too. Not really too thrilled with the fix itself, since it further fragments the source with directives, but it works.
I do have another question for @EricWF regarding the inclusion of unistd.h, but I should probably take that to a separate forum (though I've commented on the original review that added the inclusion).

Shall I cancel this review? This is my first so I'm just starting to learn the ropes :)

[JamesNagurne]

Closing this, as a fix has already been committed and validated on our end.

libcxxabi/src/cxa_guard_impl.h

 
 //===----------------------------------------------------------------------===//
 //                     Global Mutex Implementation
 //===----------------------------------------------------------------------===//
 
 struct LibcppMutex;
 struct LibcppCondVar;
 
-#ifndef _LIBCXXABI_HAS_NO_THREADS
 struct LibcppMutex {
   LibcppMutex() = default;
   LibcppMutex(LibcppMutex const&) = delete;
   LibcppMutex& operator=(LibcppMutex const&) = delete;
 
+#ifndef _LIBCXXABI_HAS_NO_THREADS
   bool lock() { return std::__libcpp_mutex_lock(&mutex); }
   bool unlock() { return std::__libcpp_mutex_unlock(&mutex); }
+#else
+  bool lock() { return 0; }
+  bool unlock() { return 0; }
+#endif
 
 private:
   friend struct LibcppCondVar;
+#ifndef _LIBCXXABI_HAS_NO_THREADS
   std::__libcpp_mutex_t mutex = _LIBCPP_MUTEX_INITIALIZER;
+#endif
 };
 
 struct LibcppCondVar {
   LibcppCondVar() = default;
   LibcppCondVar(LibcppCondVar const&) = delete;
   LibcppCondVar& operator=(LibcppCondVar const&) = delete;
 
+#ifndef _LIBCXXABI_HAS_NO_THREADS
   bool wait(LibcppMutex& mut) {
     return std::__libcpp_condvar_wait(&cond, &mut.mutex);
   }
   bool broadcast() { return std::__libcpp_condvar_broadcast(&cond); }
+#else
+  bool wait(LibcppMutex& mut) {
+    return 0;
+  }
+  bool broadcast() { return 0; }
+#endif
 
 private:
+#ifndef _LIBCXXABI_HAS_NO_THREADS
   std::__libcpp_condvar_t cond = _LIBCPP_CONDVAR_INITIALIZER;
+#endif
 };
-#endif // !defined(_LIBCXXABI_HAS_NO_THREADS)
 
 
 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>> {