[llvm-commits] [PATCH] tsan: add mutexsets to reports

[Kostya Serebryany]

  Great in general!
  Please add the comments about the entire idea (to tsan_mutexset.h?) and let us review again.
  Also, is this patch dividable into 2-3 separate patches? (maybe not, but please try to send shorter patches when possible).
  E.g. GetOrCreateAndLock change could go as a separate trivial refactoring.
  MutexSet class could go as yet another separate change.


================
Comment at: lit_tests/mutexset1.cc:33
@@ +32,3 @@
+// CHECK: WARNING: ThreadSanitizer: data race
+// CHECK:   Write of size 4 at {{.*}} by thread 1 (mutexes: W1):
+// CHECK:   Previous write of size 4 at {{.*}} by thread 2:
----------------
Maybe like this: (mutexes M1-w) ?
This will help searching for M1 as a token



================
Comment at: lit_tests/mutexset1.cc:35
@@ +34,3 @@
+// CHECK:   Previous write of size 4 at {{.*}} by thread 2:
+// CHECK:   Mutex 1 created at:
+// CHECK:     #0 pthread_mutex_init
----------------
Mutex M1?

================
Comment at: rtl/tsan_mutexset.h:11
@@ +10,3 @@
+// This file is a part of ThreadSanitizer (TSan), a race detector.
+//
+//===----------------------------------------------------------------------===//
----------------
Comments, please!! 

================
Comment at: rtl/tsan_mutexset.h:22
@@ +21,3 @@
+ public:
+  static const uptr kMaxSize = 64;
+  struct Desc {
----------------
Maybe a template parameter?

================
Comment at: rtl/tsan_rtl_report.cc:153
@@ +152,3 @@
+    uptr addr = SyncVar::SplitId(d.id, &uid);
+    SyncVar *s = ctx_->synctab.GetAndLock(0, 0, addr, false, false);
+    if (s && s->CheckId(uid)) {
----------------
This part looks tricky. Comments?

================
Comment at: rtl/tsan_sync.cc:52
@@ -50,3 +51,3 @@
 SyncVar* SyncTab::GetAndLock(ThreadState *thr, uptr pc,
-                             uptr addr, bool write_lock) {
+                             uptr addr, bool write_lock, bool create) {
 #ifndef TSAN_GO
----------------
GetOrCreateAndLock / GetIfExistsAndLock ?


http://llvm-reviews.chandlerc.com/D182