[lttng-dev] [PATCH lttng-ust] Fix: remove weak attribute from __start___tracepoints_ptrs and __stop___tracepoints_ptrs

[Gerlando Falauto]

Hi Mathieu,

[... removing weak from __attribute__(...) of __start___tracepoints_ptrs 
and __stop___tracepoints_ptrs]

>> Any thoughts?
>
> I'm really not keen on changing the behavior of existing public APIs
> to work around specific compiler bugs.

I'm not keen on that, either. But how can you be so sure this is really 
the case? I'm not a compiler expert, and I don't understand the inner 
workings of lttng-ust either, but I really don't see a reason why those 
symbols should be both weak *and* hidden.
As far as I could understand, those symbols *have* to be defined in 
order for the various contructors to work properly. It's just *the way* 
they are defined that's a bit weak (or weird), in that the linker's the 
one defining them instead of the user (through the compiler).
If they're not defined (i.e. because no tracepoint is defined at all), 
no one should use them either (i.e. no 
__tracepoints__ptrs_init()/__tracepoints__ptrs_destroy() function should 
be emitted, either). If someone inadvertently changes the section nome 
to something else, the linker should complain.
To me, just having the linker shut up and swallow it, seems plain wrong.

 > Perhaps we could find a way to
> tell users that they are facing this issue by adding a check in lttng-ust
> configure.ac ?

I'd thought of that, too. Maybe a #define symbol, generated by the 
configure script to distinguish between the two cases?
I believe the test case to detect this bug was quite trivial; I would 
just have to dig it up. But I'm not very fond of autotools, either. ;-)
Plus, I still don't see the issue with removing weak, really.

But I'm open to any suggestion.

Thanks again!
Gerlando

>
> Thanks,
>
> Mathieu
>
>>
>> Thank you!
>> Gerlando
>>
>>
>>>
>>> Thanks,
>>>
>>> Mathieu
>>>
>>>>
>>>> So even though the issue is only observed with a buggy compiler, removing
>>>> the weak attribute also enforces the correct strong linking to the symbols
>>>> provided by the linker (which would otherwise, by weak semantics, default
>>>> to a value of all-zeroes).
>>>>
>>>> References: https://gcc.gnu.org/ml/gcc-help/2014-05/msg00005.html
>>>> References:
>>>> http://lists.lttng.org/pipermail/lttng-dev/2014-May/023090.html
>>>> Reported-by: Martin Ünsal <martinunsal at gmail.com>
>>>> Thanks-to: Henrik Wallin <henrikwallin72 at gmail.com>
>>>> Cc: Paul Woegerer <Paul_Woegerer at mentor.com>
>>>> Signed-off-by: Gerlando Falauto <gerlando.falauto at keymile.com>
>>>> ---
>>>>    include/lttng/tracepoint.h | 15 ++++++++++-----
>>>>    1 file changed, 10 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
>>>> index c5a09d8..229ea72 100644
>>>> --- a/include/lttng/tracepoint.h
>>>> +++ b/include/lttng/tracepoint.h
>>>> @@ -308,14 +308,19 @@ __tracepoints__destroy(void)
>>>>    #ifdef TRACEPOINT_DEFINE
>>>>
>>>>    /*
>>>> - * These weak symbols, the constructor, and destructor take care of
>>>> - * registering only _one_ instance of the tracepoints per shared-ojbect
>>>> - * (or for the whole main program).
>>>> + * These strong hidden symbols are automatically provided by the linker
>>>> + * for each shared-object (or for the whole main program) as pointers
>>>> + * to the orphaned section "_tracepoints_ptrs" and must not be visible
>>>> + * from other shared objects to avoid name clashes at runtime which would
>>>> + * silently enable only the tracepoints from the object loaded first.
>>>> + * NOTICE: Some OpenEmbedded GCC releases (namely 4.7.2) incorrectly
>>>> + * emit those symbols with default visibility if both weak and hidden
>>>> + * are used.
>>>>     */
>>>>    extern struct tracepoint * const __start___tracepoints_ptrs[]
>>>> -	__attribute__((weak, visibility("hidden")));
>>>> +	__attribute__((visibility("hidden")));
>>>>    extern struct tracepoint * const __stop___tracepoints_ptrs[]
>>>> -	__attribute__((weak, visibility("hidden")));
>>>> +	__attribute__((visibility("hidden")));
>>>>
>>>>    /*
>>>>     * When TRACEPOINT_PROBE_DYNAMIC_LINKAGE is defined, we do not emit a
>>>> --
>>>> 1.8.0.1
>>>>
>>>>
>>>> _______________________________________________
>>>> lttng-dev mailing list
>>>> lttng-dev at lists.lttng.org
>>>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>>>
>>>
>>
>>
>