calixte updated this revision to Diff 259854.
calixte added a comment.
Fix nits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78477/new/
https://reviews.llvm.org/D78477
Files:
clang/lib/Driver/ToolChains/Darwin.cpp
compiler-rt/lib/profile/GCDAProfiling.c
compiler-rt/test/profile/Inputs/instrprof-gcov-multithread_fork.cpp
compiler-rt/test/profile/instrprof-gcov-multithread_fork.test
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
@@ -115,7 +115,8 @@
// list.
Function *
insertCounterWriteout(ArrayRef<std::pair<GlobalVariable *, MDNode *>>);
- Function *insertFlush(ArrayRef<std::pair<GlobalVariable *, MDNode *>>);
+ Function *insertReset(ArrayRef<std::pair<GlobalVariable *, MDNode *>>);
+ Function *insertFlush(Function *ResetF);
void AddFlushBeforeForkAndExec();
@@ -631,35 +632,76 @@
}
void GCOVProfiler::AddFlushBeforeForkAndExec() {
- SmallVector<Instruction *, 2> ForkAndExecs;
+ SmallVector<CallInst *, 2> Forks;
+ SmallVector<CallInst *, 2> Execs;
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) {
+#if !defined(_WIN32)
+ Forks.push_back(CI);
+#endif
+ } 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) {
+ Execs.push_back(CI);
+ }
}
}
}
}
}
- // 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 : Forks) {
+ IRBuilder<> Builder(F);
+ BasicBlock *Parent = F->getParent();
+ auto NextInst = ++F->getIterator();
+
+ // We've a fork so just reset the counters in the child process
+ FunctionType *FTy = FunctionType::get(Builder.getInt32Ty(), {}, false);
+ FunctionCallee GCOVFork = M->getOrInsertFunction("__gcov_fork", FTy);
+ F->setCalledFunction(GCOVFork);
+
+ // We split just after the fork to have a counter for the lines after
+ // Anyway there's a bug:
+ // void foo() { fork(); }
+ // void bar() { foo(); blah(); }
+ // then "blah();" will be called 2 times but showed as 1
+ // because "blah()" belongs to the same block as "foo();"
+ Parent->splitBasicBlock(NextInst);
+
+ // back() is a br instruction with a debug location
+ // equals to the one from NextAfterFork
+ // So to avoid to have two debug locs on two blocks just change it
+ DebugLoc Loc = F->getDebugLoc();
+ Parent->back().setDebugLoc(Loc);
+ }
+
+ for (auto E : Execs) {
+ IRBuilder<> Builder(E);
+ BasicBlock *Parent = E->getParent();
+ auto NextInst = ++E->getIterator();
+
+ // Since the process is replaced by a new one we need to write out gcdas
+ // No need to reset the counters since they'll be lost after the exec**
FunctionType *FTy = FunctionType::get(Builder.getVoidTy(), {}, false);
- FunctionCallee GCOVFlush = M->getOrInsertFunction("__gcov_flush", FTy);
- Builder.CreateCall(GCOVFlush);
- I->getParent()->splitBasicBlock(I);
+ FunctionCallee WriteoutF =
+ M->getOrInsertFunction("llvm_writeout_files", FTy);
+ Builder.CreateCall(WriteoutF);
+
+ DebugLoc Loc = E->getDebugLoc();
+ Builder.SetInsertPoint(&*NextInst);
+ // If the exec** fails we must reset the counters since they've been
+ // dumped
+ FunctionCallee ResetF = M->getOrInsertFunction("llvm_reset_counters", FTy);
+ Builder.CreateCall(ResetF)->setDebugLoc(Loc);
+ Parent->splitBasicBlock(NextInst);
+ Parent->back().setDebugLoc(Loc);
}
}
@@ -851,7 +893,8 @@
}
Function *WriteoutF = insertCounterWriteout(CountersBySP);
- Function *FlushF = insertFlush(CountersBySP);
+ Function *ResetF = insertReset(CountersBySP);
+ Function *FlushF = insertFlush(ResetF);
// Create a small bit of code that registers the "__llvm_gcov_writeout" to
// be executed at exit and the "__llvm_gcov_flush" function to be executed
@@ -869,16 +912,14 @@
IRBuilder<> Builder(BB);
FTy = FunctionType::get(Type::getVoidTy(*Ctx), false);
- Type *Params[] = {
- PointerType::get(FTy, 0),
- PointerType::get(FTy, 0)
- };
+ Type *Params[] = {PointerType::get(FTy, 0), PointerType::get(FTy, 0),
+ PointerType::get(FTy, 0)};
FTy = FunctionType::get(Builder.getVoidTy(), Params, false);
- // Initialize the environment and register the local writeout and flush
- // functions.
+ // Initialize the environment and register the local writeout, flush and
+ // reset functions.
FunctionCallee GCOVInit = M->getOrInsertFunction("llvm_gcov_init", FTy);
- Builder.CreateCall(GCOVInit, {WriteoutF, FlushF});
+ Builder.CreateCall(GCOVInit, {WriteoutF, FlushF, ResetF});
Builder.CreateRetVoid();
appendToGlobalCtors(*M, F, 0);
@@ -1191,8 +1232,43 @@
return WriteoutF;
}
-Function *GCOVProfiler::
-insertFlush(ArrayRef<std::pair<GlobalVariable*, MDNode*> > CountersBySP) {
+Function *GCOVProfiler::insertReset(
+ ArrayRef<std::pair<GlobalVariable *, MDNode *>> CountersBySP) {
+ FunctionType *FTy = FunctionType::get(Type::getVoidTy(*Ctx), false);
+ Function *ResetF = M->getFunction("__llvm_gcov_reset");
+ if (!ResetF)
+ ResetF = Function::Create(FTy, GlobalValue::InternalLinkage,
+ "__llvm_gcov_reset", M);
+ else
+ ResetF->setLinkage(GlobalValue::InternalLinkage);
+ ResetF->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
+ ResetF->addFnAttr(Attribute::NoInline);
+ if (Options.NoRedZone)
+ ResetF->addFnAttr(Attribute::NoRedZone);
+
+ BasicBlock *Entry = BasicBlock::Create(*Ctx, "entry", ResetF);
+ IRBuilder<> Builder(Entry);
+
+ // Zero out the counters.
+ for (const auto &I : CountersBySP) {
+ GlobalVariable *GV = I.first;
+ Constant *Null = Constant::getNullValue(GV->getValueType());
+ Builder.CreateStore(Null, GV);
+ }
+
+ Type *RetTy = ResetF->getReturnType();
+ if (RetTy->isVoidTy())
+ Builder.CreateRetVoid();
+ else if (RetTy->isIntegerTy())
+ // Used if __llvm_gcov_reset was implicitly declared.
+ Builder.CreateRet(ConstantInt::get(RetTy, 0));
+ else
+ report_fatal_error("invalid return type for __llvm_gcov_reset");
+
+ return ResetF;
+}
+
+Function *GCOVProfiler::insertFlush(Function *ResetF) {
FunctionType *FTy = FunctionType::get(Type::getVoidTy(*Ctx), false);
Function *FlushF = M->getFunction("__llvm_gcov_flush");
if (!FlushF)
@@ -1213,16 +1289,10 @@
IRBuilder<> Builder(Entry);
Builder.CreateCall(WriteoutF, {});
-
- // Zero out the counters.
- for (const auto &I : CountersBySP) {
- GlobalVariable *GV = I.first;
- Constant *Null = Constant::getNullValue(GV->getValueType());
- Builder.CreateStore(Null, GV);
- }
+ Builder.CreateCall(ResetF, {});
Type *RetTy = FlushF->getReturnType();
- if (RetTy == Type::getVoidTy(*Ctx))
+ if (RetTy->isVoidTy())
Builder.CreateRetVoid();
else if (RetTy->isIntegerTy())
// Used if __llvm_gcov_flush was implicitly declared.
Index: compiler-rt/test/profile/instrprof-gcov-multithread_fork.test
===================================================================
--- /dev/null
+++ compiler-rt/test/profile/instrprof-gcov-multithread_fork.test
@@ -0,0 +1,11 @@
+UNSUPPORTED: windows
+
+RUN: mkdir -p %t.d
+RUN: cd %t.d
+
+RUN: %clangxx --coverage -lpthread -o %t %S/Inputs/instrprof-gcov-multithread_fork.cpp
+RUN: test -f instrprof-gcov-multithread_fork.gcno
+
+RUN: rm -f instrprof-gcov-multithread_fork.gcda
+RUN: %run %t
+RUN: llvm-cov gcov instrprof-gcov-multithread_fork.gcda
Index: compiler-rt/test/profile/Inputs/instrprof-gcov-multithread_fork.cpp
===================================================================
--- /dev/null
+++ compiler-rt/test/profile/Inputs/instrprof-gcov-multithread_fork.cpp
@@ -0,0 +1,35 @@
+#include <sys/types.h>
+#include <thread>
+#include <unistd.h>
+#include <vector>
+
+template <typename T>
+void launcher(T func) {
+ std::vector<std::thread> pool;
+
+ for (int i = 0; i < 10; i++) {
+ pool.emplace_back(std::thread(func));
+ }
+
+ for (auto &t : pool) {
+ t.join();
+ }
+}
+
+void h() {}
+
+void g() {
+ fork();
+ launcher<>(h);
+}
+
+void f() {
+ fork();
+ launcher<>(g);
+}
+
+int main() {
+ launcher<>(f);
+
+ return 0;
+}
Index: compiler-rt/lib/profile/GCDAProfiling.c
===================================================================
--- compiler-rt/lib/profile/GCDAProfiling.c
+++ compiler-rt/lib/profile/GCDAProfiling.c
@@ -32,8 +32,10 @@
#include <windows.h>
#include "WindowsMMap.h"
#else
-#include <sys/mman.h>
#include <sys/file.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <unistd.h>
#endif
#if defined(__FreeBSD__) && defined(__i386__)
@@ -119,6 +121,11 @@
*/
struct fn_list flush_fn_list;
+/*
+ * A list of reset functions, shared between all dynamic objects.
+ */
+struct fn_list reset_fn_list;
+
static void fn_list_insert(struct fn_list* list, fn_ptr fn) {
struct fn_node* new_node = malloc(sizeof(struct fn_node));
new_node->fn = fn;
@@ -643,7 +650,46 @@
}
COMPILER_RT_VISIBILITY
-void llvm_gcov_init(fn_ptr wfn, fn_ptr ffn) {
+void llvm_register_reset_function(fn_ptr fn) {
+ fn_list_insert(&reset_fn_list, fn);
+}
+
+COMPILER_RT_VISIBILITY
+void llvm_delete_reset_function_list(void) { fn_list_remove(&reset_fn_list); }
+
+COMPILER_RT_VISIBILITY
+void llvm_reset_counters(void) {
+ struct fn_node *curr = reset_fn_list.head;
+
+ while (curr) {
+ if (curr->id == CURRENT_ID) {
+ curr->fn();
+ }
+ curr = curr->next;
+ }
+}
+
+#if !defined(_WIN32)
+COMPILER_RT_VISIBILITY
+pid_t __gcov_fork() {
+ pid_t parent_pid = getpid();
+ pid_t pid = fork();
+
+ if (pid == 0) {
+ pid_t child_pid = getpid();
+ if (child_pid != parent_pid) {
+ // The pid changed so we've a fork (one could have its own fork function)
+ // Just reset the counters for this child process
+ // threads.
+ llvm_reset_counters();
+ }
+ }
+ return pid;
+}
+#endif
+
+COMPILER_RT_VISIBILITY
+void llvm_gcov_init(fn_ptr wfn, fn_ptr ffn, fn_ptr rfn) {
static int atexit_ran = 0;
if (wfn)
@@ -652,10 +698,14 @@
if (ffn)
llvm_register_flush_function(ffn);
+ if (rfn)
+ llvm_register_reset_function(rfn);
+
if (atexit_ran == 0) {
atexit_ran = 1;
/* Make sure we write out the data and delete the data structures. */
+ atexit(llvm_delete_reset_function_list);
atexit(llvm_delete_flush_function_list);
atexit(llvm_delete_writeout_function_list);
atexit(llvm_writeout_files);
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1173,6 +1173,7 @@
addExportedSymbol(CmdArgs, "___gcov_flush");
addExportedSymbol(CmdArgs, "_flush_fn_list");
addExportedSymbol(CmdArgs, "_writeout_fn_list");
+ addExportedSymbol(CmdArgs, "_reset_fn_list");
} else {
addExportedSymbol(CmdArgs, "___llvm_profile_filename");
addExportedSymbol(CmdArgs, "___llvm_profile_raw_version");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
