This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5624e86ae0fb: [tsan] Respect !nosanitize metadata and remove 
gcov special case (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158385/new/

https://reviews.llvm.org/D158385

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/code-coverage-tsan.c
  clang/test/Driver/fprofile-update.c
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
  llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll

Index: llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll
===================================================================
--- llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll
+++ llvm/test/Instrumentation/ThreadSanitizer/do-not-instrument-memory-access.ll
@@ -1,6 +1,6 @@
 ; 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
 
@@ -18,42 +18,44 @@
 
 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
Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -364,11 +364,6 @@
               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
@@ -522,6 +517,9 @@
   // 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))
Index: clang/test/Driver/fprofile-update.c
===================================================================
--- clang/test/Driver/fprofile-update.c
+++ clang/test/Driver/fprofile-update.c
@@ -1,6 +1,5 @@
 /// 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
 
Index: clang/test/CodeGen/code-coverage-tsan.c
===================================================================
--- clang/test/CodeGen/code-coverage-tsan.c
+++ clang/test/CodeGen/code-coverage-tsan.c
@@ -1,5 +1,4 @@
-/// -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
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -869,8 +869,6 @@
     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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to