This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f46269f0c1c: [profile] Don't dump counters when
forking and don't reset when calling exec**… (authored by Calixte Denizet
<calixte.deni...@gmail.com>).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74953/new/
https://reviews.llvm.org/D74953
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
@@ -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,74 @@
}
void GCOVProfiler::AddFlushBeforeForkAndExec() {
- SmallVector<Instruction *, 2> ForkAndExecs;
+ SmallVector<std::pair<bool, CallInst *>, 2> ForkAndExecs;
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)
+ ForkAndExecs.push_back({true, 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) {
+ ForkAndExecs.push_back({false, 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);
- FunctionType *FTy = FunctionType::get(Builder.getVoidTy(), {}, false);
- FunctionCallee GCOVFlush = M->getOrInsertFunction("__gcov_flush", FTy);
- Builder.CreateCall(GCOVFlush);
- I->getParent()->splitBasicBlock(I);
+ for (auto F : ForkAndExecs) {
+ IRBuilder<> Builder(F.second);
+ BasicBlock *Parent = F.second->getParent();
+ auto NextInst = ++F.second->getIterator();
+
+ if (F.first) {
+ // 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.second->setCalledFunction(GCOVFork);
+ if (NextInst != Parent->end()) {
+ // 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.second->getDebugLoc();
+ Parent->back().setDebugLoc(Loc);
+ }
+ } else {
+ // 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 WriteoutF =
+ M->getOrInsertFunction("llvm_writeout_files", FTy);
+ Builder.CreateCall(WriteoutF);
+ if (NextInst != Parent->end()) {
+ DebugLoc Loc = F.second->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 +891,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 +910,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 +1230,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 == Type::getVoidTy(*Ctx))
+ 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)
@@ -1206,20 +1280,13 @@
FlushF->addFnAttr(Attribute::NoRedZone);
BasicBlock *Entry = BasicBlock::Create(*Ctx, "entry", FlushF);
+ IRBuilder<> Builder(Entry);
- // Write out the current counters.
Function *WriteoutF = M->getFunction("__llvm_gcov_writeout");
assert(WriteoutF && "Need to create the writeout function first!");
- 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(WriteoutF);
+ Builder.CreateCall(ResetF);
Type *RetTy = FlushF->getReturnType();
if (RetTy == Type::getVoidTy(*Ctx))
Index: compiler-rt/lib/profile/GCDAProfiling.c
===================================================================
--- compiler-rt/lib/profile/GCDAProfiling.c
+++ compiler-rt/lib/profile/GCDAProfiling.c
@@ -32,8 +32,9 @@
#include <windows.h>
#include "WindowsMMap.h"
#else
-#include <sys/mman.h>
#include <sys/file.h>
+#include <sys/mman.h>
+#include <unistd.h>
#endif
#if defined(__FreeBSD__) && defined(__i386__)
@@ -62,27 +63,20 @@
#include "InstrProfiling.h"
#include "InstrProfilingUtil.h"
+/* #define DEBUG_GCDAPROFILING */
+
#ifndef _WIN32
#include <pthread.h>
-static pthread_mutex_t gcov_flush_mutex = PTHREAD_MUTEX_INITIALIZER;
-static __inline void gcov_flush_lock() {
- pthread_mutex_lock(&gcov_flush_mutex);
-}
-static __inline void gcov_flush_unlock() {
- pthread_mutex_unlock(&gcov_flush_mutex);
-}
+pthread_mutex_t gcov_mutex = PTHREAD_MUTEX_INITIALIZER;
+static __inline void gcov_lock() { pthread_mutex_lock(&gcov_mutex); }
+static __inline void gcov_unlock() { pthread_mutex_unlock(&gcov_mutex); }
#else
#include <windows.h>
-static SRWLOCK gcov_flush_mutex = SRWLOCK_INIT;
-static __inline void gcov_flush_lock() {
- AcquireSRWLockExclusive(&gcov_flush_mutex);
-}
-static __inline void gcov_flush_unlock() {
- ReleaseSRWLockExclusive(&gcov_flush_mutex);
-}
+SRWLOCK gcov_mutex = SRWLOCK_INIT;
+static __inline void gcov_lock() { AcquireSRWLockExclusive(&gcov_mutex); }
+static __inline void gcov_unlock() { ReleaseSRWLockExclusive(&gcov_mutex); }
#endif
-/* #define DEBUG_GCDAPROFILING */
/*
* --- GCOV file format I/O primitives ---
*/
@@ -138,6 +132,12 @@
*/
struct fn_list flush_fn_list;
+/*
+ * A list of reset functions that our __gcov_reset() function should call,
+ * 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;
@@ -638,8 +638,25 @@
fn_list_insert(&flush_fn_list, fn);
}
+COMPILER_RT_VISIBILITY
+void llvm_register_reset_function(fn_ptr fn) {
+ fn_list_insert(&reset_fn_list, fn);
+}
+
+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;
+ }
+}
+
void __gcov_flush() {
- gcov_flush_lock();
+ gcov_lock();
struct fn_node* curr = flush_fn_list.head;
@@ -648,30 +665,69 @@
curr = curr->next;
}
- gcov_flush_unlock();
+ gcov_unlock();
}
+#if !defined(_WIN32)
+pid_t __gcov_fork() {
+ pid_t parent_pid = getpid();
+ pid_t pid;
+
+ gcov_lock();
+ // Avoid a concurrent modification of the lists during the fork.
+ // For example, a thread is making a fork while another one is
+ // loading a CU and so executing global initializer in this case
+ // the child process could inherit a bad list (e.g. bad tail)
+ // or could have the malloc in a wrong state.
+ pid = fork();
+ gcov_unlock();
+
+ 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
+ // No need to lock here since we just forked and cannot have any other
+ // threads.
+ llvm_reset_counters();
+ }
+ }
+ return pid;
+}
+#endif
+
COMPILER_RT_VISIBILITY
void llvm_delete_flush_function_list(void) {
fn_list_remove(&flush_fn_list);
}
COMPILER_RT_VISIBILITY
-void llvm_gcov_init(fn_ptr wfn, fn_ptr ffn) {
+void llvm_delete_reset_function_list(void) { fn_list_remove(&reset_fn_list); }
+
+COMPILER_RT_VISIBILITY
+void llvm_gcov_init(fn_ptr wfn, fn_ptr ffn, fn_ptr rfn) {
static int atexit_ran = 0;
+ gcov_lock();
+
if (wfn)
llvm_register_writeout_function(wfn);
if (ffn)
llvm_register_flush_function(ffn);
+ if (rfn)
+ llvm_register_reset_function(rfn);
+
+ gcov_unlock();
+
if (atexit_ran == 0) {
atexit_ran = 1;
/* Make sure we write out the data and delete the data structures. */
atexit(llvm_delete_flush_function_list);
atexit(llvm_delete_writeout_function_list);
+ atexit(llvm_delete_reset_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
@@ -1144,8 +1144,10 @@
if (hasExportSymbolDirective(Args)) {
if (ForGCOV) {
addExportedSymbol(CmdArgs, "___gcov_flush");
+ addExportedSymbol(CmdArgs, "___gcov_fork");
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
