marco-c added a comment.
Also, as we discussed, https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93623#c5
regarding exec.
================
Comment at: compiler-rt/lib/profile/GCDAProfiling.c:665
+ gcov_lock();
+ // Avoid a concurrent modification of the lists during the fork
+ pid = fork();
----------------
Could you expand the comment explaining a situation where this could fail if we
didn't lock?
================
Comment at: compiler-rt/lib/profile/GCDAProfiling.c:671
+ pid_t child_pid = getpid();
+ if (child_pid != parent_pid) {
+ // The pid changed so we've a fork
----------------
Nit: do we need this check or can we just use the earlier one on pid == 0?
================
Comment at: compiler-rt/lib/profile/GCDAProfiling.c:675
+ // No need to lock here since we just forked and cannot have any other
+ // threads.
+ struct fn_node *curr = reset_fn_list.head;
----------------
What if we have a thread in another process making modifications to the list?
================
Comment at: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp:635
void GCOVProfiler::AddFlushBeforeForkAndExec() {
- SmallVector<Instruction *, 2> ForkAndExecs;
+ SmallVector<std::pair<bool, CallInst *>, 2> ForkAndExecs;
for (auto &F : M->functions()) {
----------------
Since we are now mostly doing different things on forks and execs, we could
remove this vector and just do the operations directly instead of adding to the
vec.
================
Comment at: llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp:671
+ // We split just after the fork to have a counter for the lines after
+ // Anyway there's a bug:
+ // void foo() { fork(); }
----------------
Isn't this bug fixed by splitting the block?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74953/new/
https://reviews.llvm.org/D74953
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
