[PATCH] D75436: [profile] Remove fork management from code coverage

Thu Mar 5 04:56:42 PST 2020

calixte added a comment.

In D75436#1903566 <https://reviews.llvm.org/D75436#1903566>, @jfb wrote:

> Doing fork will still share an output file, right? Doe we currently end up interleaving content from both forks, or does one of them overwrite the other entirely?
>  I understand that crashing is undesirable, but wrong data isn't really any better.

There is a crash when there are two forks in two threads because of the global variables used to dump in `compiler_rt/lib/profile/GCDAProfiling.cpp`.
We could probably just remove all the global variables used to dump and creating them in `__llvm_gcov_writeout` and pass them as arguments to the different functions used to dump.
But we still have the global lists (`writeout_fn_list` or `flush_fn_list`) containing the functions to call when writing out or flushing.
These lists can be updated concurently: 2 threads are loading 2 CUs and then the global ctors for each of them are executed and so they're updating concurrently theses list. Is it correct ?
When forking in a thread when another is loading a CU may result in a wrong list too in the child process.
So it's why I think we need to lock when the list are updated and when there is a fork.

Anyway, currently we can crash because of fork (reproductible in clang 9) and we discovered it when we tryed to switch from gcc to clang on our linux toolchains.
I think we must avoid it: I really prefer having inaccurate counters instead of a crash.
And let me work on this again to fix these issues and then have something good: no crash and accurate counters.
FYI, we actively tested my previous patches in our CI on our test-suites with ccov enabled and everything was fine.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75436/new/

https://reviews.llvm.org/D75436