[PATCH] D48538: Make __gcov_flush flush counters for all shared libraries

[Marco Castelluccio via llvm-commits]

Il 04/07/2018 00:52, Xinliang David Li ha scritto:

>
>
> On Tue, Jul 3, 2018 at 4:29 PM Marco Castelluccio via Phabricator
> <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
>
>     marco-c updated this revision to Diff 154015.
>     marco-c added a comment.
>
>     I've taken another approach which looks simpler to me. Instead of
>     having two lists, I only have a shared list and I use a static
>     variable's address as an identifier for a dynamic object.
>
>     I've also added another test (which would have failed with the
>     previous iteration of the patch) which dlopens three libraries.
>
>     I will file follow-up bugs for things that don't look right in the
>     llvm-cov output which are not regressions from this patch:
>
>     - instrprof-shared-main-gcov-flush_no-writeout.c.gcov: lines 23,
>     26 and 30 should not be covered; lines 33 and 35 should not be
>     covered (they are considered as uncoverable instead).
>     - instrprof-dlopen-func{2,3}.c.gcov: the only line in the function
>     should be covered once and not thrice
>
>     Regarding removing writeout and only always using __gcov_flush, do
>     you see any disadvantage if we do that?
>
>
> writeout function only writes the profile for the current dynamic
> module which is different from the behavior of __gcov_flush. The form
> is expected when shared lib is unloaded -- i.e., do not dump other
> load module's profile unless being explicitly told so (to use
> gcov_flush).  The write out is similar to gcc's gcov_exit. In short,
> we should keep it as it is.
Sorry, what I meant is that we could remove __llvm_gcov_writeout and
only keep __llvm_gcov_flush (they are defined in
lib/Transforms/Instrumentation/GCOVProfiling.cpp and passed to
llvm_gcov_init). We currently have writeout functions and flush
functions, where the difference is just that the flush function is also
resetting the counters to 0.
At exit, instead of calling __llvm_gcov_writeout, we could call
__llvm_gcov_flush. Since we are at exit, the fact that we reset the
counters to 0 for the current module doesn't matter (as the module is
not usable anymore after that).
At exit we would still call __llvm_gcov_flush only for the module that
is exiting, at __gcov_flush we would call __llvm_gcov_flush for all modules.


>  
>
>     Regarding llvm_gcov_flush that was recently added, should I remove
>     it since we can now unhide __gcov_flush?
>
>
> I suggest we keep it -- there is no harm keeping it. llvm_gcov_flush
> can be documented to be always non hidden.  There might also be a use
> case that user want to discover the dumper using dlsym, but only want
> to dump profile for that shared library. In that case, a symbol with
> protected visibility is needed which is a wrapper to the write-out method.
Should we make llvm_gcov_flush hidden and __gcov_flush unhidden then?
__gcov_flush unhidden would be for compatibility with GCC.

- Marco.




>
>
> David
>
>
>  
>
>
>     https://reviews.llvm.org/D48538
>
>     Files:
>       lib/profile/GCDAProfiling.c
>       test/profile/Inputs/instrprof-dlopen-dlclose-main.c
>       test/profile/Inputs/instrprof-dlopen-dlclose-main.c.gcov
>       test/profile/Inputs/instrprof-dlopen-dlclose-main_three-libs.c.gcov
>       test/profile/Inputs/instrprof-dlopen-func.c.gcov
>       test/profile/Inputs/instrprof-dlopen-func2.c.gcov
>       test/profile/Inputs/instrprof-dlopen-func3.c
>       test/profile/Inputs/instrprof-dlopen-func3.c.gcov
>       test/profile/Inputs/instrprof-shared-lib.c.gcov
>       test/profile/Inputs/instrprof-shared-lib_called-twice.c.gcov
>       test/profile/Inputs/instrprof-shared-lib_in-loop.c.gcov
>       test/profile/Inputs/instrprof-shared-main-gcov-flush.c
>      
>     test/profile/Inputs/instrprof-shared-main-gcov-flush_no-writeout.c.gcov
>      
>     test/profile/Inputs/instrprof-shared-main-gcov-flush_shared-call-after.c.gcov
>      
>     test/profile/Inputs/instrprof-shared-main-gcov-flush_shared-call-before-after.c.gcov
>      
>     test/profile/Inputs/instrprof-shared-main-gcov-flush_shared-call-before.c.gcov
>       test/profile/Inputs/instrprof-shared-main.c.gcov
>       test/profile/instrprof-dlopen-dlclose-gcov.test
>       test/profile/instrprof-gcov-two-objects.test
>       test/profile/instrprof-shared-gcov-flush.test
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180704/c186b335/attachment.html>