[libcxx testing] Fix UB in tests for std::lock_guard

[ikudrin]

Summary

If mutex::try_lock() is called in a thread that already owns the mutex, the behavior is undefined. The patch fixes the issue by creating another thread, where the call is allowed.

[davezarzycki]

I appreciate that you're trying to make this be defined behavior, but I honestly don't know why this is UB. I can't think of or find a platform where the underlying OS "trylock" routine would have UB when called on the same thread that owns the lock already. Can somebody with access to the C++ standard provide any hint to why the standard wants this to be UB?

[ikudrin]

The current code violates the C++ standard, which says (in different sections, depending on the version):

The expression m.try_lock() shall be well-formed and have the following semantics:
Requires: If m is of type mutex, timed_mutex, shared_mutex, or shared_timed_mutex, the calling thread does not own the mutex.

The particular UB depends on the implementation and is clearly triggered on our private platform, which is why we have noticed it.

[ldionne]

That seems correct to me: http://eel.is/c++draft/thread.mutex#requirements.mutex.general-14

Please fix the tests in C++03 mode as suggested, though.

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/adopt_lock.pass.cpp

34–36

With do_try_lock defined after the global mutex:

void do_try_lock() {
    assert(m.try_lock() == false);
}

This is annoying, but those tests need to run in C++03 mode.

[ikudrin]

• Updated as suggested. Thanks, @ldionne!

[ldionne]

LGTM if CI passes. I think you need to re-upload the diff for it to trigger, since the LLVM Monorepo tag was somehow removed.

[ikudrin]

Trying to force CI to run.

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/adopt_lock.pass.cpp

 // template <class Mutex> class lock_guard;
 
 // lock_guard(mutex_type& m, adopt_lock_t);
 
 #include <mutex>
 #include <cstdlib>
 #include <cassert>
 
+#include "make_test_thread.h"
 #include "test_macros.h"
 
 std::mutex m;
 
+void do_try_lock() {
+  assert(m.try_lock() == false);
+}
+
 int main(int, char**) {
   {
     m.lock();
     std::lock_guard<std::mutex> lg(m, std::adopt_lock);
-    assert(m.try_lock() == false);
+    std::thread t = support::make_test_thread(do_try_lock);
+    t.join();
   }
 
   m.lock();
   m.unlock();
 
   return 0;
 }

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.guard/mutex.pass.cpp

 
 // template<class _Mutex> lock_guard(lock_guard<_Mutex>)
 //     -> lock_guard<_Mutex>;  // C++17
 
 #include <mutex>
 #include <cstdlib>
 #include <cassert>
 
+#include "make_test_thread.h"
 #include "test_macros.h"
 
 std::mutex m;
 
+void do_try_lock() {
+  assert(m.try_lock() == false);
+}
+
 int main(int, char**) {
   {
     std::lock_guard<std::mutex> lg(m);
-    assert(m.try_lock() == false);
+    std::thread t = support::make_test_thread(do_try_lock);
+    t.join();
   }
 
   m.lock();
   m.unlock();
 
 #ifdef __cpp_deduction_guides
   std::lock_guard lg(m);
   static_assert((std::is_same<decltype(lg), std::lock_guard<decltype(m)>>::value), "" );
 #endif
 
   return 0;
 }