[PATCH] D49893: [asan/win] Use SRW locks to fix a race in BlockingMutex

[Reid Kleckner via Phabricator via llvm-commits]

This revision was not accepted when it landed; it landed in state "Needs Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338331: [asan/win] Use SRW locks to fix a race in BlockingMutex (authored by rnk, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D49893?vs=157616&id=158130#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49893

Files:
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h
  compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc


Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
===================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
@@ -767,43 +767,22 @@
 void internal_join_thread(void *th) { }
 
 // ---------------------- BlockingMutex ---------------- {{{1
-const uptr LOCK_UNINITIALIZED = 0;
-const uptr LOCK_READY = (uptr)-1;
-
-BlockingMutex::BlockingMutex(LinkerInitialized li) {
-  // FIXME: see comments in BlockingMutex::Lock() for the details.
-  CHECK(li == LINKER_INITIALIZED || owner_ == LOCK_UNINITIALIZED);
-
-  CHECK(sizeof(CRITICAL_SECTION) <= sizeof(opaque_storage_));
-  InitializeCriticalSection((LPCRITICAL_SECTION)opaque_storage_);
-  owner_ = LOCK_READY;
-}
 
 BlockingMutex::BlockingMutex() {
-  CHECK(sizeof(CRITICAL_SECTION) <= sizeof(opaque_storage_));
-  InitializeCriticalSection((LPCRITICAL_SECTION)opaque_storage_);
-  owner_ = LOCK_READY;
+  CHECK(sizeof(SRWLOCK) <= sizeof(opaque_storage_));
+  internal_memset(this, 0, sizeof(*this));
 }
 
 void BlockingMutex::Lock() {
-  if (owner_ == LOCK_UNINITIALIZED) {
-    // FIXME: hm, global BlockingMutex objects are not initialized?!?
-    // This might be a side effect of the clang+cl+link Frankenbuild...
-    new(this) BlockingMutex((LinkerInitialized)(LINKER_INITIALIZED + 1));
-
-    // FIXME: If it turns out the linker doesn't invoke our
-    // constructors, we should probably manually Lock/Unlock all the global
-    // locks while we're starting in one thread to avoid double-init races.
-  }
-  EnterCriticalSection((LPCRITICAL_SECTION)opaque_storage_);
-  CHECK_EQ(owner_, LOCK_READY);
+  AcquireSRWLockExclusive((PSRWLOCK)opaque_storage_);
+  CHECK_EQ(owner_, 0);
   owner_ = GetThreadSelf();
 }
 
 void BlockingMutex::Unlock() {
-  CHECK_EQ(owner_, GetThreadSelf());
-  owner_ = LOCK_READY;
-  LeaveCriticalSection((LPCRITICAL_SECTION)opaque_storage_);
+  CheckLocked();
+  owner_ = 0;
+  ReleaseSRWLockExclusive((PSRWLOCK)opaque_storage_);
 }
 
 void BlockingMutex::CheckLocked() {
Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h
===================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_mutex.h
@@ -73,13 +73,8 @@
 
 class BlockingMutex {
  public:
-#if SANITIZER_WINDOWS
-  // Windows does not currently support LinkerInitialized
-  explicit BlockingMutex(LinkerInitialized);
-#else
   explicit constexpr BlockingMutex(LinkerInitialized)
-      : opaque_storage_ {0, }, owner_(0) {}
-#endif
+      : opaque_storage_ {0, }, owner_{0} {}
   BlockingMutex();
   void Lock();
   void Unlock();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49893.158130.patch
Type: text/x-patch
Size: 2748 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180730/86559175/attachment.bin>