<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Jul 3, 2018 at 4:29 PM Marco Castelluccio via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">marco-c updated this revision to Diff 154015.<br>
marco-c added a comment.<br>
<br>
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.<br>
<br>
I've also added another test (which would have failed with the previous iteration of the patch) which dlopens three libraries.<br>
<br>
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:<br>
<br>
- 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).<br>
- instrprof-dlopen-func{2,3}.c.gcov: the only line in the function should be covered once and not thrice<br>
<br>
Regarding removing writeout and only always using __gcov_flush, do you see any disadvantage if we do that?<br>
<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Regarding llvm_gcov_flush that was recently added, should I remove it since we can now unhide __gcov_flush?<br>
<br></blockquote><div><br></div><div>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.</div><div><br></div><div><br></div><div>David</div><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<a href="https://reviews.llvm.org/D48538" rel="noreferrer" target="_blank">https://reviews.llvm.org/D48538</a><br>
<br>
Files:<br>
  lib/profile/GCDAProfiling.c<br>
  test/profile/Inputs/instrprof-dlopen-dlclose-main.c<br>
  test/profile/Inputs/instrprof-dlopen-dlclose-main.c.gcov<br>
  test/profile/Inputs/instrprof-dlopen-dlclose-main_three-libs.c.gcov<br>
  test/profile/Inputs/instrprof-dlopen-func.c.gcov<br>
  test/profile/Inputs/instrprof-dlopen-func2.c.gcov<br>
  test/profile/Inputs/instrprof-dlopen-func3.c<br>
  test/profile/Inputs/instrprof-dlopen-func3.c.gcov<br>
  test/profile/Inputs/instrprof-shared-lib.c.gcov<br>
  test/profile/Inputs/instrprof-shared-lib_called-twice.c.gcov<br>
  test/profile/Inputs/instrprof-shared-lib_in-loop.c.gcov<br>
  test/profile/Inputs/instrprof-shared-main-gcov-flush.c<br>
  test/profile/Inputs/instrprof-shared-main-gcov-flush_no-writeout.c.gcov<br>
  test/profile/Inputs/instrprof-shared-main-gcov-flush_shared-call-after.c.gcov<br>
  test/profile/Inputs/instrprof-shared-main-gcov-flush_shared-call-before-after.c.gcov<br>
  test/profile/Inputs/instrprof-shared-main-gcov-flush_shared-call-before.c.gcov<br>
  test/profile/Inputs/instrprof-shared-main.c.gcov<br>
  test/profile/instrprof-dlopen-dlclose-gcov.test<br>
  test/profile/instrprof-gcov-two-objects.test<br>
  test/profile/instrprof-shared-gcov-flush.test<br>
<br>
</blockquote></div></div>