[tsan] Respect !nosanitize metadata and remove gcov special case

[MaskRay]

Summary

Certain instrumentations set the !nosanitize metadata for inserted
instructions, which are generally not interested for sanitizers. Skip
tsan instrumentation like we do for asan (D126294)/msan/hwasan.

-fprofile-arcs instrumentation has data race unless
-fprofile-update=atomic is specified. Let's remove the the __llvm_gcov
special case from commit 0222adbcd25779a156399bcc16fde9f6d083a809 (2016)
as the racy instructions have the !nosanitize metadata.
(-fprofile-arcs instrumentation does not use __llvm_gcda as global variables.)

std::atomic<int> c;
void foo() { c++; }
int main() {
  std::thread th(foo);
  c++;
  th.join();
}

Tested that clang++ --coverage -fsanitize=thread a.cc && ./a.out does
not report spurious tsan errors.

Also remove the default CC1 option -fprofile-update=atomic for
-fsanitize=thread to make options more orthogonal.

clang/lib/Driver/ToolChains/Clang.cpp

static void addPGOAndCoverageFlags(const ToolChain &TC, Compilation &C,
 
   if (const auto *A = Args.getLastArg(options::OPT_fprofile_update_EQ)) {
     StringRef Val = A->getValue();
     if (Val == "atomic" || Val == "prefer-atomic")
       CmdArgs.push_back("-fprofile-update=atomic");
     else if (Val != "single")
       D.Diag(diag::err_drv_unsupported_option_argument)
           << A->getSpelling() << Val;
-  } else if (SanArgs.needsTsanRt()) {
-    CmdArgs.push_back("-fprofile-update=atomic");
   }
 
   int FunctionGroups = 1;
   int SelectedFunctionGroup = 0;
   if (const auto *A = Args.getLastArg(options::OPT_fprofile_function_groups)) {
     StringRef Val = A->getValue();
     if (Val.getAsInteger(0, FunctionGroups) || FunctionGroups < 1)
       D.Diag(diag::err_drv_invalid_int_value) << A->getAsString(Args) << Val;

clang/test/CodeGen/code-coverage-tsan.c

-/// -fprofile-update=atomic (implied by -fsanitize=thread) requires the
-/// (potentially concurrent) counter updates to be atomic.
+/// -fprofile-update=atomic requires the (potentially concurrent) counter updates to be atomic.
 // RUN: %clang_cc1 %s -triple x86_64 -emit-llvm -fprofile-update=atomic \
 // RUN:   -coverage-notes-file=/dev/null -coverage-data-file=/dev/null -o - | FileCheck %s
 
 // CHECK-LABEL: void @foo()
 /// Two counters are incremented by __tsan_atomic64_fetch_add.
 // CHECK:         atomicrmw add ptr @__llvm_gcov_ctr{{.*}} monotonic, align 8
 // CHECK-NEXT:    atomicrmw sub ptr
 
 _Atomic(int) cnt;
 void foo(void) { cnt--; }

clang/test/Driver/fprofile-update.c

 /// For -fprofile-instr-generate and -fprofile-arcs, increment counters atomically
-/// if -fprofile-update={atomic,prefer-atomic} or -fsanitize=thread is specified.
-// RUN: %clang -### %s -c -target x86_64-linux -fsanitize=thread %s 2>&1 | FileCheck %s
+/// if -fprofile-update={atomic,prefer-atomic} is specified.
 // RUN: %clang -### %s -c -fprofile-update=atomic 2>&1 | FileCheck %s
 // RUN: %clang -### %s -c -fprofile-update=prefer-atomic 2>&1 | FileCheck %s
 
 // CHECK: "-fprofile-update=atomic"
 
 // RUN: %clang -### %s -c -fprofile-update=atomic -fprofile-update=single 2>&1 | FileCheck %s --check-prefix=SINGLE
 
 // SINGLE-NOT: "-fprofile-update=atomic"
 
 // RUN: not %clang %s -c -fprofile-update=unknown 2>&1 | FileCheck %s --check-prefix=ERROR
 
 // ERROR: error: unsupported argument 'unknown' to option '-fprofile-update='

llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp

if (GlobalVariable *GV = dyn_cast<GlobalVariable>(Addr)) {
     if (GV->hasSection()) {
       StringRef SectionName = GV->getSection();
       // Check if the global is in the PGO counters section.
       auto OF = Triple(M->getTargetTriple()).getObjectFormat();
       if (SectionName.endswith(
               getInstrProfSectionName(IPSK_cnts, OF, /*AddSegmentInfo=*/false)))
         return false;
     }
-
-    // Check if the global is private gcov data.
-    if (GV->getName().startswith("__llvm_gcov") ||
-        GV->getName().startswith("__llvm_gcda"))
-      return false;
   }
 
   // Do not instrument accesses from different address spaces; we cannot deal
   // with them.
   if (Addr) {
     Type *PtrTy = cast<PointerType>(Addr->getType()->getScalarType());
     if (PtrTy->getPointerAddressSpace() != 0)
       return false;
bool ThreadSanitizer::sanitizeFunction(Function &F,
   bool Res = false;
   bool HasCalls = false;
   bool SanitizeFunction = F.hasFnAttribute(Attribute::SanitizeThread);
   const DataLayout &DL = F.getParent()->getDataLayout();
 
   // Traverse all instructions, collect loads/stores/returns, check for calls.
   for (auto &BB : F) {
     for (auto &Inst : BB) {
+      // Skip instructions inserted by another instrumentation.
+      if (Inst.hasMetadata(LLVMContext::MD_nosanitize))
+        continue;
       if (isTsanAtomic(&Inst))
         AtomicAccesses.push_back(&Inst);
       else if (isa<LoadInst>(Inst) || isa<StoreInst>(Inst))
         LocalLoadsAndStores.push_back(&Inst);
       else if ((isa<CallInst>(Inst) && !isa<DbgInfoIntrinsic>(Inst)) ||
                isa<InvokeInst>(Inst)) {
         if (CallInst *CI = dyn_cast<CallInst>(&Inst))
           maybeMarkSanitizerLibraryCallNoBuiltin(CI, &TLI);

llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll

 ; This test checks that we are not instrumenting unwanted acesses to globals:
+; - Instructions with the !nosanitize metadata (e.g. -fprofile-arcs instrumented counter accesses)
 ; - Instruction profiler counter instrumentation has known intended races.
-; - The gcov counters array has a known intended race.
 ;
 ; RUN: opt < %s -passes='function(tsan),module(tsan-module)' -S | FileCheck %s
 
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-apple-macosx10.9"
 
 @__profc_test_gep = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 @__profc_test_bitcast = private global [2 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 @__profc_test_bitcast_foo = private global [1 x i64] zeroinitializer, section "__DATA,__llvm_prf_cnts", align 8
 
 @__llvm_gcov_ctr = internal global [1 x i64] zeroinitializer
 @__llvm_gcov_ctr.1 = internal global [1 x i64] zeroinitializer
 @__llvm_gcov_global_state_pred = internal global i32 0
 @__llvm_gcda_foo = internal global i32 0
 
 define i32 @test_gep() sanitize_thread {
 entry:
-  %pgocount = load i64, ptr @__profc_test_gep
+  %pgocount = load i64, ptr @__profc_test_gep, !nosanitize !0
   %0 = add i64 %pgocount, 1
-  store i64 %0, ptr @__profc_test_gep
+  store i64 %0, ptr @__profc_test_gep, !nosanitize !0
 
-  %gcovcount = load i64, ptr @__llvm_gcov_ctr
+  %gcovcount = load i64, ptr @__llvm_gcov_ctr, !nosanitize !0
   %1 = add i64 %gcovcount, 1
-  store i64 %1, ptr @__llvm_gcov_ctr
+  store i64 %1, ptr @__llvm_gcov_ctr, !nosanitize !0
 
-  %gcovcount.1 = load i64, ptr @__llvm_gcov_ctr.1
+  %gcovcount.1 = load i64, ptr @__llvm_gcov_ctr.1, !nosanitize !0
   %2 = add i64 %gcovcount.1, 1
-  store i64 %2, ptr @__llvm_gcov_ctr.1
+  store i64 %2, ptr @__llvm_gcov_ctr.1, !nosanitize !0
 
   ret i32 1
 }
 
 define i32 @test_bitcast() sanitize_thread {
 entry:
-  %0 = load <2 x i64>, ptr @__profc_test_bitcast, align 8
-  %.promoted5 = load i64, ptr @__profc_test_bitcast_foo, align 8
+  %0 = load <2 x i64>, ptr @__profc_test_bitcast, align 8, !nosanitize !0
+  %.promoted5 = load i64, ptr @__profc_test_bitcast_foo, align 8, !nosanitize !0
   %1 = add i64 %.promoted5, 10
   %2 = add <2 x i64> %0, <i64 1, i64 10>
-  store <2 x i64> %2, ptr @__profc_test_bitcast, align 8
-  store i64 %1, ptr @__profc_test_bitcast_foo, align 8
+  store <2 x i64> %2, ptr @__profc_test_bitcast, align 8, !nosanitize !0
+  store i64 %1, ptr @__profc_test_bitcast_foo, align 8, !nosanitize !0
   ret i32 undef
 }
 
 define void @test_load() sanitize_thread {
 entry:
-  %0 = load i32, ptr @__llvm_gcov_global_state_pred
-  store i32 1, ptr @__llvm_gcov_global_state_pred
+  %0 = load i32, ptr @__llvm_gcov_global_state_pred, !nosanitize !0
+  store i32 1, ptr @__llvm_gcov_global_state_pred, !nosanitize !0
 
-  %1 = load i32, ptr @__llvm_gcda_foo
-  store i32 1, ptr @__llvm_gcda_foo
+  %1 = load i32, ptr @__llvm_gcda_foo, !nosanitize !0
+  store i32 1, ptr @__llvm_gcda_foo, !nosanitize !0
 
   ret void
 }
 
+!0 = !{}
+
 ; CHECK-NOT: {{call void @__tsan_write}}
 ; CHECK: __tsan_init