[PATCH] D29728: Remove strict tid checks from the mac implementation of BlockingMutex

[Francis Ricci via Phabricator via llvm-commits]

fjricci updated this revision to Diff 87822.
fjricci added a comment.

Add comment explaining CheckLocked behavior


https://reviews.llvm.org/D29728

Files:
  lib/sanitizer_common/sanitizer_mac.cc


Index: lib/sanitizer_common/sanitizer_mac.cc
===================================================================
--- lib/sanitizer_common/sanitizer_mac.cc
+++ lib/sanitizer_common/sanitizer_mac.cc
@@ -348,20 +348,22 @@
 void BlockingMutex::Lock() {
   CHECK(sizeof(OSSpinLock) <= sizeof(opaque_storage_));
   CHECK_EQ(OS_SPINLOCK_INIT, 0);
-  CHECK_NE(owner_, (uptr)pthread_self());
+  CHECK_EQ(owner_, 0);
   OSSpinLockLock((OSSpinLock*)&opaque_storage_);
-  CHECK(!owner_);
-  owner_ = (uptr)pthread_self();
 }
 
 void BlockingMutex::Unlock() {
-  CHECK(owner_ == (uptr)pthread_self());
-  owner_ = 0;
   OSSpinLockUnlock((OSSpinLock*)&opaque_storage_);
 }
 
+// This function explicitly avoids checking whether the mutex is owned by
+// the calling thread. This behavior, while more strictly correct, causes
+// problems in cases like StopTheWorld, where a parent thread owns the mutex
+// but a child checks that it is locked. Rather than maintaining complex state
+// to work around those situations, the check only checks that the mutex is
+// owned, and assumes callers to be generally well-behaved.
 void BlockingMutex::CheckLocked() {
-  CHECK_EQ((uptr)pthread_self(), owner_);
+  CHECK_NE(*(OSSpinLock*)&opaque_storage_, 0);
 }
 
 u64 NanoTime() {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29728.87822.patch
Type: text/x-patch
Size: 1258 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170209/ecc5a7b4/attachment.bin>