tsan: implement suppressions for top frame only

[dvyukov]

Summary

The new suppression type is called "race_top" and is matched only against top frame in report stacks.
This is required for situations when we want to suppress a race in a "thread pool" or "event loop" implementation.
If we simply use "race:ThreadPool::Execute" suppression, that can suppress everything in the program.

[glider]

General question: you're creating a TSan-specific suppression type that matches the topmost frame.
Don't you think other tools may require such functionality as well?

[dvyukov]

I don't know. Everything is possible.

[glider]

LGTM

[glider]

Adding other folks for the case they have ideas about the _top suffix.

[samsonov]

lib/tsan/rtl/tsan_suppressions.cc

118–119

you can totally factor this out to

bool IsSuppressed(const AddressInfo &info, const char *suppression_type, Suppression **sp);

124

stack->frames != nullptr

test/tsan/race_top_suppression.cc.supp

1 ↗

(On Diff #28334)

I'd prefer to get rid of extra file, and create it with "echo" command in the RUN-line.

test/tsan/race_top_suppression1.cc.supp

1 ↗

(On Diff #28334)

Ditto

[glider]

Alexey, wdyt about this functionality in general? Shall we limit it to TSan?

sent from phone

[samsonov]

I see no reasonable way to make this non-limited to TSan, given how generic our suppression machinery is.

[dvyukov]

addressed comments

[dvyukov]

ptal

lib/tsan/rtl/tsan_suppressions.cc

118–119

done

124

done

test/tsan/race_top_suppression.cc.supp

1 ↗

(On Diff #28334)

done

[samsonov]

LGTM

lib/tsan/rtl/tsan_suppressions.cc

98

make this function static?

[dvyukov]

lib/tsan/rtl/tsan_suppressions.cc

98

done

[dvyukov]

Submitted in 240949

lib/tsan/rtl/tsan_suppressions.h

 
 #include "sanitizer_common/sanitizer_suppressions.h"
 #include "tsan_report.h"
 
 namespace __tsan {
 
 const char kSuppressionNone[] = "none";
 const char kSuppressionRace[] = "race";
+const char kSuppressionRaceTop[] = "race_top";
 const char kSuppressionMutex[] = "mutex";
 const char kSuppressionThread[] = "thread";
 const char kSuppressionSignal[] = "signal";
 const char kSuppressionLib[] = "called_from_lib";
 const char kSuppressionDeadlock[] = "deadlock";
 
 void InitializeSuppressions();
 SuppressionContext *Suppressions();
 void PrintMatchedSuppressions();
 uptr IsSuppressed(ReportType typ, const ReportStack *stack, Suppression **sp);
 uptr IsSuppressed(ReportType typ, const ReportLocation *loc, Suppression **sp);
 
 }  // namespace __tsan
 
 #endif  // TSAN_SUPPRESSIONS_H

lib/tsan/rtl/tsan_suppressions.cc

 }
 #endif
 
 namespace __tsan {
 
 ALIGNED(64) static char suppression_placeholder[sizeof(SuppressionContext)];
 static SuppressionContext *suppression_ctx = nullptr;
 static const char *kSuppressionTypes[] = {
-    kSuppressionRace,   kSuppressionMutex, kSuppressionThread,
-    kSuppressionSignal, kSuppressionLib,   kSuppressionDeadlock};
+    kSuppressionRace,   kSuppressionRaceTop, kSuppressionMutex,
+    kSuppressionThread, kSuppressionSignal, kSuppressionLib,
+    kSuppressionDeadlock};
 
 void InitializeSuppressions() {
   CHECK_EQ(nullptr, suppression_ctx);
   suppression_ctx = new (suppression_placeholder) // NOLINT
       SuppressionContext(kSuppressionTypes, ARRAY_SIZE(kSuppressionTypes));
   suppression_ctx->ParseFromFile(flags()->suppressions);
 #ifndef SANITIZER_GO
   suppression_ctx->Parse(__tsan_default_suppressions());
static const char *conv(ReportType typ) {
   else if (typ == ReportTypeErrnoInSignal)
     return kSuppressionNone;
   else if (typ == ReportTypeDeadlock)
     return kSuppressionDeadlock;
   Printf("ThreadSanitizer: unknown report type %d\n", typ),
   Die();
 }
 
+uptr IsSuppressed(const char *stype, const AddressInfo &info,
+    Suppression **sp) {
+  if (suppression_ctx->Match(info.function, stype, sp) ||
+      suppression_ctx->Match(info.file, stype, sp) ||
+      suppression_ctx->Match(info.module, stype, sp)) {
+    DPrintf("ThreadSanitizer: matched suppression '%s'\n", (*sp)->templ);
+    (*sp)->hit_count++;
+    return info.address;
+  }
+  return 0;
+}
+
 uptr IsSuppressed(ReportType typ, const ReportStack *stack, Suppression **sp) {
   CHECK(suppression_ctx);
   if (!suppression_ctx->SuppressionCount() || stack == 0 ||
       !stack->suppressable)
     return 0;
   const char *stype = conv(typ);
   if (0 == internal_strcmp(stype, kSuppressionNone))
     return 0;
-  Suppression *s;
   for (const SymbolizedStack *frame = stack->frames; frame;
-       frame = frame->next) {
-    const AddressInfo &info = frame->info;
-    if (suppression_ctx->Match(info.function, stype, &s) ||
-        suppression_ctx->Match(info.file, stype, &s) ||
-        suppression_ctx->Match(info.module, stype, &s)) {
-      DPrintf("ThreadSanitizer: matched suppression '%s'\n", s->templ);
-      s->hit_count++;
-      *sp = s;
-      return info.address;
-    }
+    uptr pc = IsSuppressed(stype, frame->info, sp);
+    if (pc != 0)
+      return pc;
   }
+  if (0 == internal_strcmp(stype, kSuppressionRace) && stack->frames != nullptr)
+    return IsSuppressed(kSuppressionRaceTop, stack->frames->info, sp);
   return 0;
 }
 
 uptr IsSuppressed(ReportType typ, const ReportLocation *loc, Suppression **sp) {
   CHECK(suppression_ctx);
   if (!suppression_ctx->SuppressionCount() || loc == 0 ||
       loc->type != ReportLocationGlobal || !loc->suppressable)
     return 0;

test/tsan/race_top_suppression.cc

+// RUN: echo "race_top:TopFunction" > %t.supp
+// RUN: %clangxx_tsan -O1 %s -o %t
+// RUN: TSAN_OPTIONS="$TSAN_OPTIONS suppressions='%t.supp'" %run %t 2>&1 | FileCheck %s
+// RUN: rm %t.supp
+#include "test.h"
+
+int Global;
+
+void TopFunction(int *p) {
+  *p = 1;
+}
+
+void *Thread(void *x) {
+  barrier_wait(&barrier);
+  TopFunction(&Global);
+  return 0;
+}
+
+int main() {
+  barrier_init(&barrier, 2);
+  pthread_t t;
+  pthread_create(&t, 0, Thread, 0);
+  Global--;
+  barrier_wait(&barrier);
+  pthread_join(t, 0);
+  fprintf(stderr, "DONE\n");
+}
+
+// CHECK-NOT: WARNING: ThreadSanitizer: data race

test/tsan/race_top_suppression1.cc

+// RUN: echo "race_top:TopFunction" > %t.supp
+// RUN: %clangxx_tsan -O1 %s -o %t
+// RUN: TSAN_OPTIONS="$TSAN_OPTIONS suppressions='%t.supp'" %deflake %run %t 2>&1 | FileCheck %s
+// RUN: rm %t.supp
+#include "test.h"
+
+int Global;
+
+void AnotherFunction(int *p) {
+  *p = 1;
+}
+
+void TopFunction(int *p) {
+  AnotherFunction(p);
+}
+
+void *Thread(void *x) {
+  barrier_wait(&barrier);
+  TopFunction(&Global);
+  return 0;
+}
+
+int main() {
+  barrier_init(&barrier, 2);
+  pthread_t t;
+  pthread_create(&t, 0, Thread, 0);
+  Global--;
+  barrier_wait(&barrier);
+  pthread_join(t, 0);
+}
+
+// CHECK: WARNING: ThreadSanitizer: data race