[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

git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@350883 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 35 insertions, 91 deletions

diff --git a/lib/tsan/rtl/tsan_flags.inc b/lib/tsan/rtl/tsan_flags.inc
index e9b3e35f0..d3b678fdd 100644
--- a/lib/tsan/rtl/tsan_flags.inc
+++ b/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.")
diff --git a/lib/tsan/rtl/tsan_interceptors.cc b/lib/tsan/rtl/tsan_interceptors.cc
index 4d0051bb2..9e49dfe5d 100644
--- a/lib/tsan/rtl/tsan_interceptors.cc
+++ b/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();
 }
 
diff --git a/test/tsan/Darwin/ignore-noninstrumented.mm b/test/tsan/Darwin/ignore-noninstrumented.mm
index 668a76a46..88d39268f 100644
--- a/test/tsan/Darwin/ignore-noninstrumented.mm
+++ b/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
 
diff --git a/test/tsan/Darwin/ignored-interceptors.mm b/test/tsan/Darwin/ignored-interceptors.mm
deleted file mode 100644
index b2e40f07d..000000000
--- a/test/tsan/Darwin/ignored-interceptors.mm
+++ /dev/null
@@ -1,55 +0,0 @@
-// Check that ignore_interceptors_accesses=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
-
-// Check that without the flag, there are false positives.
-// RUN: %env_tsan_opts=ignore_noninstrumented_modules=0 %deflake %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-RACE
-
-// With ignore_interceptors_accesses=1, no races are reported.
-// RUN: %env_tsan_opts=ignore_noninstrumented_modules=0:ignore_interceptors_accesses=1 %run %t 2>&1 | FileCheck %s
-
-// With ignore_interceptors_accesses=1, races in user's code are still reported.
-// RUN: %env_tsan_opts=ignore_noninstrumented_modules=0:ignore_interceptors_accesses=1 %deflake %run %t race 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-RACE
-
-#import <Foundation/Foundation.h>
-
-#import "../test.h"
-
-long global;
-
-void *Thread1(void *x) {
-  barrier_wait(&barrier);
-  global = 42;
-  return NULL;
-}
-
-void *Thread2(void *x) {
-  global = 43;
-  barrier_wait(&barrier);
-  return NULL;
-}
-
-int main(int argc, char *argv[]) {
-  fprintf(stderr, "Hello world.\n");
-  
-  // NSUserDefaults uses XPC which triggers the false positive.
-  NSDictionary *d = [[NSUserDefaults standardUserDefaults] dictionaryRepresentation];
-
-  if (argc > 1 && strcmp(argv[1], "race") == 0) {
-    barrier_init(&barrier, 2);
-    pthread_t t[2];
-    pthread_create(&t[0], NULL, Thread1, NULL);
-    pthread_create(&t[1], NULL, Thread2, NULL);
-    pthread_join(t[0], NULL);
-    pthread_join(t[1], NULL);
-  }
-
-  fprintf(stderr, "Done.\n");
-}
-
-// CHECK: Hello world.
-// CHECK-RACE: SUMMARY: ThreadSanitizer: data race
-// CHECK: Done.
diff --git a/test/tsan/ignored-interceptors-mmap.cc b/test/tsan/ignored-interceptors-mmap.cc
index 4686e8eb3..bb43250a6 100644
--- a/test/tsan/ignored-interceptors-mmap.cc
+++ b/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