On 08/29/2018 01:18 PM, Richard Biener wrote:
> On Wed, Aug 29, 2018 at 12:44 PM Martin Liška <mli...@suse.cz> wrote:
>>
>> On 08/29/2018 11:17 AM, Richard Biener wrote:
>>> On Wed, Aug 29, 2018 at 10:31 AM Martin Liška <mli...@suse.cz> wrote:
>>>>
>>>> Hello.
>>>>
>>>> Moving the thread from gcc ML into gcc-patches. That's first implementation
>>>> of shared libgcov library. Currently inclusion of t-libgcov is added only
>>>> to *-linux targets. Is it fine to add to all configurations that already
>>>> include 't-slibgcc t-slibgcc-elf-ver'?
>>>>
>>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>>
>>> I understand this is to make profiling work with multiple DSOs (all
>>> instrumented).
>>
>> Yes.
>>
>>> But does this also make the case work when those DSOs are dlopened/dlclosed
>>> multiple times?
>>
>> That works fine even now, when a DSO is loaded, __gcov_init is called and
>> it registers gcov_info into __gcov_root.
>>
>> Following sample:
>>
>> #include <dlfcn.h>
>> #include <assert.h>
>> #include <unistd.h>
>>
>> int main()
>> {
>> for (int i = 0; i < 10; i++)
>> {
>> void *handle = dlopen ("libfoo.so", RTLD_NOW);
>> assert (handle);
>>
>> void (*fn) (void) = dlsym (handle, "foo");
>> assert (fn);
>>
>> fn ();
>> sleep (1);
>> }
>>
>> return 0;
>> }
>
> That doesn't dlclose ;)
Yep, I tested that but with compiler (static libgcov) :/
I noticed that make check for postgres generated invalid *.gcda files.
If's caused by fact that:
libgcov-driver.c:
/* Per-dynamic-object gcov state. */
struct gcov_root __gcov_root;
is not longer per-DSO, but global. So when a shared library is unloaded
with dlclose, it computes histogram of all gcov_info files. As the program
still runs, arcs counters are being updated.
>
>> has:
>>
>> $ gcov-dump -l foo.gcda
>> foo.gcda:data:magic `gcda':version `A82*'
>> foo.gcda:stamp 2235622999
>> foo.gcda: a3000000: 22:PROGRAM_SUMMARY checksum=0x02bfe640
>> foo.gcda: counts=2, runs=1, sum_all=20, run_max=10, sum_max=10
>> foo.gcda: counter histogram:
>> foo.gcda: 9: num counts=2, min counter=10, cum_counter=20
>> foo.gcda: 01000000: 3:FUNCTION ident=1636255671,
>> lineno_checksum=0x2172766e, cfg_checksum=0xc0bbb23e
>> foo.gcda: 01a10000: 4:COUNTERS arcs 2 counts
>> foo.gcda: 0: 10 10
>> foo.gcda: 01af0000: 2:COUNTERS time_profiler 1 counts
>> foo.gcda: 0: 1
>>
>> which is fine, runs == 1 and arcs counter executed 10x.
>>
>>
>> I understand that with the shared libgcov implementation this
>>> library must be finalized last?
>>
>> If you do profiling, then you depend on libgcov, thus it's loaded before a
>> DSO (or executable)
>> is loaded.
>>
>> Wouldn't it be better to have some additional
>>> object that lives in an executable only? Or do we want profiled DSOs
>>> without
>>> profiling the executable using it?
>>
>> That should be rare, but it will be possible as __gcov_exit will be
>> eventually called.
>
> I'm not too familiar but I guess each DSO _fini will call __gcov_exit?
Yes.
>
>> Does that case work with the shared libgcov
>>> (with dlopen/dlclose)?
>>
>> Yes, I've just tested that.
>>
>> Does it work when DSO1 is compiled with GCC N
>>> and DSO2 is compiled with GCC N+1 where the libgcov ABI changed?
>>
>> For such case we have:
>>
>> /* Exactly one of these will be live in the process image. */
>> struct gcov_master __gcov_master =
>> {GCOV_VERSION, 0};
>> ...
>> and it's check in __gcov_init.
>
> So it doesn't work. Too bad, in the past we shipped a profiling (-pg)
> glibc build
> but that would then only work for the same compiler.
>
> I guess using a shared libgcov makes profiling the dynamic loader DSO
> impossible?
You mean ld.so?
> So we'd likely want a -static-libgcov option as well?
If you use -static, that works now.
>
>>>
>>> I think we need a better understanding of the situation before jumping to
>>> the conclusion that a shared libgcov is the solution. Maybe better
>>> isolating
>>> the individual instances is better?
>>
>> It's undesired as seen in the PR. When doing profiling of indirect jumps
>> we need have exactly one __gcov_indirect_call_callee. That's because one can
>> do indirect calls that cross DSO boundaries.
>
> But usually you don't inline across DSO boundaries and after devirtualizing
> you still need to go through the PLT...
>
> Btw, when you dlopen/dlcose a DSO multiple times the same function can
> end up at different addresses, does gcov somehow deal with this? It seems
> that the profile functions get the callee address.
That's fine, profile_id is saved:
if (__gcov_indirect_call_callee != NULL)
__gcov_indirect_call_profiler_v2 (profile_id, ¤t_function_decl);
tree_uid = build_int_cst
(gcov_type_node,
cgraph_node::get (current_function_decl)->profile_id);
>
> Can you shortly tell why the testcase in the PR segfaults? Does the issue
> only affect indirect call profiling?
What happens is that there will exist 2 instances of:
void * __gcov_indirect_call_callee;
one in main executable, and one another in DSO loaded via dlopen.
Then caller sets __gcov_indirect_call_callee to a function pointer, but when
the actual function is called it has it's own __gcov_indirect_call_callee
instance
and it's zero. Then following dereferencing happens:
if (cur_func == __gcov_indirect_call_callee
|| (__LIBGCC_VTABLE_USES_DESCRIPTORS__
&& *(void **) cur_func == *(void **) __gcov_indirect_call_callee))
__gcov_one_value_profiler_body (__gcov_indirect_call_counters, value, 0);
Well, yes, it's only related to indirect profiling. And only benefit is that we
maybe
miss a target of an indirect call. Profile for indirect call is beneficial only
for speculative inlining, which will not be possible because the callee lives in
a different DSO. Thus the code of such function will not be probably available
in compilation unit of a caller.
That said, it looks the shared library libgcov.so is overkill.
Alexander may explain how that can be beneficial?
Note that I can fix the segfault being caused by inter-DSO calls.
Martin
>
> Richard.
>
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Martin
>>>>
>>>> libgcc/ChangeLog:
>>>>
>>>> 2018-08-28 Martin Liska <mli...@suse.cz>
>>>>
>>>> PR gcov-profile/84107
>>>> * Makefile.in: Build libgcov.so objects separately,
>>>> add rule for libgcov.map and how the library
>>>> is linked.
>>>> * config.host: Add t-libgcov and t-libgcov-elf-ver
>>>> to *-linux targets.
>>>> * config/t-libgcov: New file.
>>>> * config/t-libgcov-elf-ver: New file.
>>>> * libgcov-driver.c: Declare function if new L_gcov_shared
>>>> is defined.
>>>> * libgcov-interface.c: Likewise.
>>>> * libgcov-merge.c: Likewise.
>>>> * libgcov-profiler.c: Likewise.
>>>> * libgcov-std.ver.in: New file.
>>>> * libgcov.h (__gcov_init): Remove ATTRIBUTE_HIDDEN.
>>>> (__gcov_exit): Likewise.
>>>> (__gcov_merge_add): Likewise.
>>>> (__gcov_merge_time_profile): Likewise.
>>>> (__gcov_merge_single): Likewise.
>>>> (__gcov_merge_ior): Likewise.
>>>> (__gcov_merge_icall_topn): Likewise.
>>>> (gcov_sort_n_vals): Likewise.
>>>> (__gcov_fork): Likewise.
>>>> (__gcov_execl): Likewise.
>>>> (__gcov_execlp): Likewise.
>>>> (__gcov_execle): Likewise.
>>>> (__gcov_execv): Likewise.
>>>> (__gcov_execvp): Likewise.
>>>> (__gcov_execve): Likewise.
>>>> ---
>>>> libgcc/Makefile.in | 56 ++++++++++++++++++++++++++++++---
>>>> libgcc/config.host | 2 +-
>>>> libgcc/config/t-libgcov | 46 +++++++++++++++++++++++++++
>>>> libgcc/config/t-libgcov-elf-ver | 3 ++
>>>> libgcc/libgcov-driver.c | 4 +--
>>>> libgcc/libgcov-interface.c | 28 ++++++++---------
>>>> libgcc/libgcov-merge.c | 14 ++++-----
>>>> libgcc/libgcov-profiler.c | 34 ++++++++++----------
>>>> libgcc/libgcov-std.ver.in | 53 +++++++++++++++++++++++++++++++
>>>> libgcc/libgcov.h | 31 +++++++++---------
>>>> 10 files changed, 209 insertions(+), 62 deletions(-)
>>>> create mode 100644 libgcc/config/t-libgcov
>>>> create mode 100644 libgcc/config/t-libgcov-elf-ver
>>>> create mode 100644 libgcc/libgcov-std.ver.in
>>>>
>>>>
>>
