[TSan] Remove ignore_interceptors_accesses flag

Summary:
It has been superseded by the `ignore_noninstrumented_modules` flag and is no longer needed.

Also simplify a test that checks that `mmap_interceptor` respects ignore annotations (`thr->ignore_reads_and_writes `).
Relevant: https://reviews.llvm.org/rL269855

<rdar://problem/46263073> Remove obsolete Apple-specific suppression option

Reviewers: dcoughlin, kubamracek, dvyukov, delcypher

Reviewed By: dvyukov

Subscribers: jfb, llvm-commits, #sanitizers

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D55075

llvm-svn: 350883

compiler-rt/lib/tsan/rtl/tsan_flags.inc

@@ -77,8 +77,6 @@ TSAN_FLAG(int, io_sync, 1,
 TSAN_FLAG(bool, die_after_fork, true,
           "Die after multi-threaded fork if the child creates new threads.")
 TSAN_FLAG(const char *, suppressions, "", "Suppressions file name.")
-TSAN_FLAG(bool, ignore_interceptors_accesses, false,
-          "Ignore reads and writes from all interceptors.")
 TSAN_FLAG(bool, ignore_noninstrumented_modules, SANITIZER_MAC ? true : false,
           "Interceptors should only detect races when called from instrumented "
           "modules.")

compiler-rt/lib/tsan/rtl/tsan_interceptors.cc

@@ -254,8 +254,7 @@ ScopedInterceptor::ScopedInterceptor(ThreadState *thr, const char *fname,
   if (!thr_->ignore_interceptors) FuncEntry(thr, pc);
   DPrintf("#%d: intercept %s()\n", thr_->tid, fname);
   ignoring_ =
-      !thr_->in_ignored_lib && (flags()->ignore_interceptors_accesses ||
-                                libignore()->IsIgnored(pc, &in_ignored_lib_));
+      !thr_->in_ignored_lib && libignore()->IsIgnored(pc, &in_ignored_lib_);
   EnableIgnores();
 }
 

compiler-rt/test/tsan/Darwin/ignore-noninstrumented.mm

@@ -1,4 +1,7 @@
-// Check that ignore_noninstrumented_modules=1 suppresses races from system libraries on OS X.
+// Check that ignore_noninstrumented_modules=1 suppresses reporting races from
+// system libraries on OS X. There are currently false positives coming from
+// libxpc, libdispatch, CoreFoundation and others, because these libraries use
+// TSan-invisible atomics as synchronization.
 
 // RUN: %clang_tsan %s -o %t -framework Foundation
 

compiler-rt/test/tsan/ignored-interceptors-mmap.cc

@@ -1,10 +1,12 @@
 // RUN: %clangxx_tsan -O0 %s -o %t
-// RUN: not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NORMAL
-// RUN: %env_tsan_opts=ignore_interceptors_accesses=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-IGNORE
+// RUN: not %run %t        2>&1 | FileCheck %s --check-prefix=CHECK-RACE
+// RUN:     %run %t ignore 2>&1 | FileCheck %s --check-prefix=CHECK-IGNORE
 // XFAIL: freebsd,netbsd
 
-#include <errno.h>
 #include <sys/mman.h>
+#include <string.h>
+#include <assert.h>
+#include <atomic>
 
 #include "test.h"
 
@@ -15,48 +17,45 @@ void AnnotateIgnoreWritesBegin(const char *f, int l);
 void AnnotateIgnoreWritesEnd(const char *f, int l);
 }
 
-void *global_p;
+// Use atomic to ensure we do not have a race for the pointer value itself.  We
+// only want to check races in the mmap'd memory to isolate the test that mmap
+// respects ignore annotations.
+std::atomic<int*> global_p;
 
-int mmap_and_ignore_reads_and_writes() {
+void mmap_ignored(bool ignore) {
   const size_t kSize = sysconf(_SC_PAGESIZE);
-  void *p = mmap(0, kSize, PROT_READ|PROT_WRITE,
-                 MAP_PRIVATE|MAP_ANON, -1, 0);
-  if (p == MAP_FAILED)
-    return printf("mmap failed with %d\n", errno);
-  munmap(p, kSize);
-
-  void *new_p = mmap(p, kSize, PROT_READ|PROT_WRITE,
-                     MAP_PRIVATE|MAP_ANON, -1, 0);
-  if (p == MAP_FAILED || p != new_p)
-    return printf("second mmap failed with %d\n", errno);
-
-  AnnotateIgnoreWritesBegin(__FILE__, __LINE__);
-  global_p = p;
-  AnnotateIgnoreWritesEnd(__FILE__, __LINE__);
+
+  if (ignore) AnnotateIgnoreWritesBegin(__FILE__, __LINE__);
+  void *p = mmap(0, kSize, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANON, -1, 0);
+  if (ignore) AnnotateIgnoreWritesEnd(__FILE__, __LINE__);
+
+  // Use relaxed to retain the race between the mmap call and the memory write
+  global_p.store((int *)p, std::memory_order_relaxed);
   barrier_wait(&barrier);
-  return 0;
 }
 
-void *Thread(void *a) {
+void *WriteToMemory(void *unused) {
   barrier_wait(&barrier);
-
-  ((int*)global_p)[1] = 10;
-  printf("Read the zero value from mmapped memory %d\n", ((int*)global_p)[1]);
+  global_p[0] = 7;
   return 0;
 }
 
-int main() {
+// Create race between allocating (mmap) and writing memory
+int main(int argc, const char *argv[]) {
+  bool ignore = (argc > 1) && (strcmp(argv[1], "ignore") == 0);
+
   barrier_init(&barrier, 2);
   pthread_t t;
-  pthread_create(&t, 0, Thread, 0);
-  if (mmap_and_ignore_reads_and_writes())
-    return 1;
+  pthread_create(&t, 0, WriteToMemory, 0);
+  mmap_ignored(ignore);
   pthread_join(t, 0);
+
+  assert(global_p[0] == 7);
   printf("OK\n");
   return 0;
 }
 
-// CHECK-NORMAL: WARNING: ThreadSanitizer: data race
-// CHECK-NORMAL: OK
-// CHECK-IGNORE_NOT: WARNING: ThreadSanitizer: data race
+// CHECK-RACE: WARNING: ThreadSanitizer: data race
+// CHECK-RACE: OK
+// CHECK-IGNORE-NOT: WARNING: ThreadSanitizer: data race
 // CHECK-IGNORE: OK