calixte created this revision.
calixte added a reviewer: marco-c.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, jfb, hiraditya.
Herald added projects: clang, Sanitizers, LLVM.
When a program is forked with code coverage enabled, all the counters are
flushed.
To avoid data race, a critical section has been added around `__gcov_flush`
(https://reviews.llvm.org/D70910) but when forking the mutex is just duplicated
and if it was in a locked state (in the parent process) then it's locked in the
child process which leads to a deadlock when the child is trying to flush
counters.
This patch aims to initialize the mutex in the child process when forking.
So a custom fork (`__gcov_fork`) is called from instrumented code and the mutex
is then initialized in the child process.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74071
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
compiler-rt/lib/profile/GCDAProfiling.c
llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
Index: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp
@@ -631,20 +631,22 @@
}
void GCOVProfiler::AddFlushBeforeForkAndExec() {
- SmallVector<Instruction *, 2> ForkAndExecs;
+ SmallVector<std::pair<bool, CallInst *>, 2> ForksAndExecs;
for (auto &F : M->functions()) {
auto *TLI = &GetTLI(F);
for (auto &I : instructions(F)) {
if (CallInst *CI = dyn_cast<CallInst>(&I)) {
if (Function *Callee = CI->getCalledFunction()) {
LibFunc LF;
- if (TLI->getLibFunc(*Callee, LF) &&
- (LF == LibFunc_fork || LF == LibFunc_execl ||
- LF == LibFunc_execle || LF == LibFunc_execlp ||
- LF == LibFunc_execv || LF == LibFunc_execvp ||
- LF == LibFunc_execve || LF == LibFunc_execvpe ||
- LF == LibFunc_execvP)) {
- ForkAndExecs.push_back(&I);
+ if (TLI->getLibFunc(*Callee, LF)) {
+ if (LF == LibFunc_fork) {
+ ForksAndExecs.push_back({true, CI});
+ } else if (LF == LibFunc_execl || LF == LibFunc_execle ||
+ LF == LibFunc_execlp || LF == LibFunc_execv ||
+ LF == LibFunc_execvp || LF == LibFunc_execve ||
+ LF == LibFunc_execvpe || LF == LibFunc_execvP) {
+ ForksAndExecs.push_back({false, CI});
+ }
}
}
}
@@ -654,12 +656,21 @@
// We need to split the block after the fork/exec call
// because else the counters for the lines after will be
// the same as before the call.
- for (auto I : ForkAndExecs) {
- IRBuilder<> Builder(I);
+ for (auto F : ForksAndExecs) {
+ IRBuilder<> Builder(F.second);
FunctionType *FTy = FunctionType::get(Builder.getVoidTy(), {}, false);
FunctionCallee GCOVFlush = M->getOrInsertFunction("__gcov_flush", FTy);
Builder.CreateCall(GCOVFlush);
- I->getParent()->splitBasicBlock(I);
+
+ if (F.first) {
+ // We've a fork, so we call a custom fork which will initialize the mutex
+ // used when flushing
+ FTy = FunctionType::get(Builder.getInt32Ty(), {}, false);
+ FunctionCallee GCOVFork = M->getOrInsertFunction("__gcov_fork", FTy);
+ F.second->setCalledFunction(GCOVFork);
+ }
+
+ F.second->getParent()->splitBasicBlock(F.second);
}
}
Index: compiler-rt/lib/profile/GCDAProfiling.c
===================================================================
--- compiler-rt/lib/profile/GCDAProfiling.c
+++ compiler-rt/lib/profile/GCDAProfiling.c
@@ -71,6 +71,9 @@
static __inline void gcov_flush_unlock() {
pthread_mutex_unlock(&gcov_flush_mutex);
}
+static __inline void gcov_flush_init_mutex() {
+ pthread_mutex_init(&gcov_flush_mutex, NULL);
+}
#else
#include <windows.h>
static SRWLOCK gcov_flush_mutex = SRWLOCK_INIT;
@@ -80,6 +83,9 @@
static __inline void gcov_flush_unlock() {
ReleaseSRWLockExclusive(&gcov_flush_mutex);
}
+static __inline void gcov_flush_init_mutex() {
+ InitializeSRWLock(&gcov_flush_mutex);
+}
#endif
/* #define DEBUG_GCDAPROFILING */
@@ -638,6 +644,16 @@
fn_list_insert(&flush_fn_list, fn);
}
+pid_t __gcov_fork() {
+ pid_t pid = fork();
+
+ if (pid == 0) {
+ gcov_flush_init_mutex();
+ }
+
+ return pid;
+}
+
void __gcov_flush() {
gcov_flush_lock();
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1143,6 +1143,7 @@
// runtime's functionality.
if (hasExportSymbolDirective(Args)) {
if (ForGCOV) {
+ addExportedSymbol(CmdArgs, "___gcov_fork");
addExportedSymbol(CmdArgs, "___gcov_flush");
addExportedSymbol(CmdArgs, "_flush_fn_list");
addExportedSymbol(CmdArgs, "_writeout_fn_list");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
