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) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
