[llvm] b5ef137 - [gcov] Increment counters with atomicrmw if -fsanitize=thread
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 28 16:33:31 PDT 2020
Author: Fangrui Song
Date: 2020-08-28T16:32:35-07:00
New Revision: b5ef137c11b1cc6ae839ee75b49233825772bdd0
URL: https://github.com/llvm/llvm-project/commit/b5ef137c11b1cc6ae839ee75b49233825772bdd0
DIFF: https://github.com/llvm/llvm-project/commit/b5ef137c11b1cc6ae839ee75b49233825772bdd0.diff
LOG: [gcov] Increment counters with atomicrmw if -fsanitize=thread
Without this patch, `clang --coverage -fsanitize=thread` may fail spuriously
because non-atomic counter increments can be detected as data races.
Added:
clang/test/CodeGen/code-coverage-tsan.c
llvm/test/Transforms/GCOVProfiling/atomic-counter.ll
Modified:
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/Transforms/Instrumentation.h
llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
Removed:
################################################################################
diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 1d4763fff80e..258f5fe69ff8 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -553,7 +553,9 @@ static void initTargetOptions(DiagnosticsEngine &Diags,
Options.MCOptions.Argv0 = CodeGenOpts.Argv0;
Options.MCOptions.CommandLineArgs = CodeGenOpts.CommandLineArgs;
}
-static Optional<GCOVOptions> getGCOVOptions(const CodeGenOptions &CodeGenOpts) {
+
+static Optional<GCOVOptions> getGCOVOptions(const CodeGenOptions &CodeGenOpts,
+ const LangOptions &LangOpts) {
if (CodeGenOpts.DisableGCov)
return None;
if (!CodeGenOpts.EmitGcovArcs && !CodeGenOpts.EmitGcovNotes)
@@ -567,6 +569,7 @@ static Optional<GCOVOptions> getGCOVOptions(const CodeGenOptions &CodeGenOpts) {
Options.NoRedZone = CodeGenOpts.DisableRedZone;
Options.Filter = CodeGenOpts.ProfileFilterFiles;
Options.Exclude = CodeGenOpts.ProfileExcludeFiles;
+ Options.Atomic = LangOpts.Sanitize.has(SanitizerKind::Thread);
return Options;
}
@@ -763,7 +766,7 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager &MPM,
MPM.add(createUniqueInternalLinkageNamesPass());
}
- if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts)) {
+ if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts, LangOpts)) {
MPM.add(createGCOVProfilerPass(*Options));
if (CodeGenOpts.getDebugInfo() == codegenoptions::NoDebugInfo)
MPM.add(createStripSymbolsPass(true));
@@ -1229,7 +1232,7 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
MPM.addPass(LowerTypeTestsPass(/*ExportSummary=*/nullptr,
/*ImportSummary=*/nullptr,
/*DropTypeTests=*/true));
- if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts))
+ if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts, LangOpts))
MPM.addPass(GCOVProfilerPass(*Options));
if (Optional<InstrProfOptions> Options =
getInstrProfOptions(CodeGenOpts, LangOpts))
@@ -1349,7 +1352,7 @@ void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
/*CompileKernel=*/false, Recover, UseAfterScope)));
});
}
- if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts))
+ if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts, LangOpts))
PB.registerPipelineStartEPCallback([Options](ModulePassManager &MPM) {
MPM.addPass(GCOVProfilerPass(*Options));
});
diff --git a/clang/test/CodeGen/code-coverage-tsan.c b/clang/test/CodeGen/code-coverage-tsan.c
new file mode 100644
index 000000000000..023a99598075
--- /dev/null
+++ b/clang/test/CodeGen/code-coverage-tsan.c
@@ -0,0 +1,12 @@
+/// -fsanitize=thread requires the (potentially concurrent) counter updates to be atomic.
+// RUN: %clang_cc1 %s -triple x86_64 -emit-llvm -fsanitize=thread -femit-coverage-notes -femit-coverage-data \
+// 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: call i64 @__tsan_atomic64_fetch_add
+// CHECK-NEXT: call i64 @__tsan_atomic64_fetch_add
+// CHECK-NEXT: call i32 @__tsan_atomic32_fetch_sub
+
+_Atomic(int) cnt;
+void foo() { cnt--; }
diff --git a/llvm/include/llvm/Transforms/Instrumentation.h b/llvm/include/llvm/Transforms/Instrumentation.h
index 0453cb428bc0..c960d5b0ab50 100644
--- a/llvm/include/llvm/Transforms/Instrumentation.h
+++ b/llvm/include/llvm/Transforms/Instrumentation.h
@@ -66,6 +66,9 @@ struct GCOVOptions {
// Add the 'noredzone' attribute to added runtime library calls.
bool NoRedZone;
+ // Use atomic profile counter increments.
+ bool Atomic = false;
+
// Regexes separated by a semi-colon to filter the files to instrument.
std::string Filter;
diff --git a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
index 53a89f7348de..3773c3e19ef6 100644
--- a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
@@ -63,6 +63,9 @@ static cl::opt<std::string> DefaultGCOVVersion("default-gcov-version",
cl::init("408*"), cl::Hidden,
cl::ValueRequired);
+static cl::opt<bool> AtomicCounter("gcov-atomic-counter", cl::Hidden,
+ cl::desc("Make counter updates atomic"));
+
// Returns the number of words which will be used to represent this string.
static unsigned wordsOfString(StringRef s) {
// Length + NUL-terminated string + 0~3 padding NULs.
@@ -74,6 +77,7 @@ GCOVOptions GCOVOptions::getDefault() {
Options.EmitNotes = true;
Options.EmitData = true;
Options.NoRedZone = false;
+ Options.Atomic = AtomicCounter;
if (DefaultGCOVVersion.size() != 4) {
llvm::report_fatal_error(std::string("Invalid -default-gcov-version: ") +
@@ -883,9 +887,15 @@ bool GCOVProfiler::emitProfileArcs() {
// Skip phis, landingpads.
IRBuilder<> Builder(&*BB.getFirstInsertionPt());
- Value *Count = Builder.CreateLoad(Builder.getInt64Ty(), Phi);
- Count = Builder.CreateAdd(Count, Builder.getInt64(1));
- Builder.CreateStore(Count, Phi);
+ if (Options.Atomic) {
+ Builder.CreateAtomicRMW(AtomicRMWInst::Add, Phi,
+ Builder.getInt64(1),
+ AtomicOrdering::Monotonic);
+ } else {
+ Value *Count = Builder.CreateLoad(Builder.getInt64Ty(), Phi);
+ Count = Builder.CreateAdd(Count, Builder.getInt64(1));
+ Builder.CreateStore(Count, Phi);
+ }
Instruction *TI = BB.getTerminator();
if (isa<ReturnInst>(TI)) {
@@ -894,9 +904,15 @@ bool GCOVProfiler::emitProfileArcs() {
const unsigned Edge = It->second;
Value *Counter = Builder.CreateConstInBoundsGEP2_64(
Counters->getValueType(), Counters, 0, Edge);
- Value *Count = Builder.CreateLoad(Builder.getInt64Ty(), Counter);
- Count = Builder.CreateAdd(Count, Builder.getInt64(1));
- Builder.CreateStore(Count, Counter);
+ if (Options.Atomic) {
+ Builder.CreateAtomicRMW(AtomicRMWInst::Add, Counter,
+ Builder.getInt64(1),
+ AtomicOrdering::Monotonic);
+ } else {
+ Value *Count = Builder.CreateLoad(Builder.getInt64Ty(), Counter);
+ Count = Builder.CreateAdd(Count, Builder.getInt64(1));
+ Builder.CreateStore(Count, Counter);
+ }
}
}
}
diff --git a/llvm/test/Transforms/GCOVProfiling/atomic-counter.ll b/llvm/test/Transforms/GCOVProfiling/atomic-counter.ll
new file mode 100644
index 000000000000..f293bac9a142
--- /dev/null
+++ b/llvm/test/Transforms/GCOVProfiling/atomic-counter.ll
@@ -0,0 +1,30 @@
+;; -fsanitize=thread requires the (potentially concurrent) counter updates to be atomic.
+; RUN: opt < %s -S -passes=insert-gcov-profiling -gcov-atomic-counter | FileCheck %s
+
+; CHECK-LABEL: void @empty()
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label %0, !dbg [[DBG:![0-9]+]]
+; CHECK: 0:
+; CHECK-NEXT: %1 = phi i64* [ getelementptr inbounds ([2 x i64], [2 x i64]* @__llvm_gcov_ctr, i64 0, i64 0), %entry ], !dbg [[DBG]]
+; CHECK-NEXT: %2 = atomicrmw add i64* %1, i64 1 monotonic, !dbg [[DBG]]
+;; Counter for the exit.
+; CHECK-NEXT: %3 = atomicrmw add i64* getelementptr inbounds ([2 x i64], [2 x i64]* @__llvm_gcov_ctr, i64 0, i64 1), i64 1 monotonic, !dbg [[DBG]]
+; CHECK-NEXT: ret void, !dbg [[DBG]]
+
+define dso_local void @empty() !dbg !5 {
+entry:
+ ret void, !dbg !8
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "a.c", directory: "")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 5}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = distinct !DISubprogram(name: "empty", scope: !1, file: !1, line: 1, type: !6, scopeLine: 1, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!6 = !DISubroutineType(types: !7)
+!7 = !{null}
+!8 = !DILocation(line: 2, column: 1, scope: !5)
More information about the llvm-commits
mailing list