----- Original Message ----- > From: "Gerlando Falauto" <[email protected]> > To: "Mathieu Desnoyers" <[email protected]> > Cc: [email protected] > Sent: Tuesday, June 10, 2014 3:13:54 AM > Subject: Re: [lttng-dev] [PATCH lttng-ust] Fix: remove weak attribute from > __start___tracepoints_ptrs and > __stop___tracepoints_ptrs > > Hi Mathieu, > > On 06/05/2014 07:27 PM, Mathieu Desnoyers wrote: > > ----- Original Message ----- > >> From: "Gerlando Falauto" <[email protected]> > >> To: [email protected] > >> Cc: "Gerlando Falauto" <[email protected]> > >> Sent: Wednesday, May 28, 2014 7:05:52 PM > >> Subject: [lttng-dev] [PATCH lttng-ust] Fix: remove weak attribute from > >> __start___tracepoints_ptrs and > >> __stop___tracepoints_ptrs > >> > >> Some OpenEmbedded GCC releases (namely 4.7.2) incorrectly emit those > >> symbols with default visibility if both weak and hidden attributes > >> are used. > >> When tracepoints are distributed among the main application and one or > >> several shared objects (e.g. lttng_ust_tracef:event in liblttng-ust.so > >> AND your own tracepoint provider, statically or dynamically linked), > >> this results in a subtle name clash at runtime, causing only the > >> tracepoints from one particular binary to be active, silently breaking > >> all others. > >> These symbols are indeed only declared and need not be defined (contrary > >> to __tracepoint_registered and friends), as they are automatically > >> PROVIDE-d by the linker as pointers to the "_tracepoint_ptrs" orphaned > >> section. > > > > Does it work well if the program and/or libraries do the following: > > > > - they include tracepoint.h, > > - but they don't contain any tracepoint (so there is technically no > > _tracepoint_ptrs section) > > > > Has this case been tested ? I remember that the weak was there for this > > peculiar corner case. > > I tested it exactly the way you described, and yes, it works. > *HOWEVER*, if prior to including tracepoint.h the program also #defines > TRACEPOINT_DEFINE, *THEN* we have the problem you described: > > root@kmeter1:~# cat foo.c > > #define TRACEPOINT_DEFINE > #include <lttng/tracepoint.h> > > int main(void) > { > printf("Hello world!\n"); > } > root@kmeter1:~# gcc foo.c -llttng-ust -ldl > /tmp/cc92SYHX.o: In function `__tracepoints__ptrs_init': > foo.c:(.text+0x39a): undefined reference to `__stop___tracepoints_ptrs' > foo.c:(.text+0x39e): undefined reference to `__stop___tracepoints_ptrs' > foo.c:(.text+0x3a2): undefined reference to `__start___tracepoints_ptrs' > foo.c:(.text+0x3a6): undefined reference to `__start___tracepoints_ptrs' > foo.c:(.text+0x3b2): undefined reference to `__start___tracepoints_ptrs' > foo.c:(.text+0x3b6): undefined reference to `__start___tracepoints_ptrs' > /tmp/cc92SYHX.o: In function `__tracepoints__ptrs_destroy': > foo.c:(.text+0x446): undefined reference to `__start___tracepoints_ptrs' > /tmp/cc92SYHX.o:foo.c:(.text+0x44a): more undefined references to > `__start___tracepoints_ptrs' follow > /usr/lib/gcc/powerpc-linux/4.7.2/../../../../powerpc-linux/bin/ld: > a.out: hidden symbol `__stop___tracepoints_ptrs' isn't defined > /usr/lib/gcc/powerpc-linux/4.7.2/../../../../powerpc-linux/bin/ld: final > link failed: Bad value > collect2: error: ld returned 1 exit status > > Re-adding the weak attribute "fixes" this (though it leaves us with a > much bigger issue, in my opinion). > > However, I'm not sure whether: > > a) this is really a use case (defining TRACEPOINT_DEFINE and then not > really defining any), and even so, whether > > b) what we have with the weak attribute is really what we want. In the > end, we would be defining a function with __attribute__((constructor)) > (oh wait, potentially even more than one!) without actually having any > tracepoint available. OK, the above complaints are probably not the most > intuitive in the world, but isn't that better than an application > silently "failing" (to provide any tracepoint) at runtime? > > After all, the issue with the above messages is about the linker trying > to resolve those symbols; but that's because they are used by > __tracepoints__ptrs_init() and friends, not because they are merely > declared. > > Any thoughts?
I'm really not keen on changing the behavior of existing public APIs to work around specific compiler bugs. Perhaps we could find a way to tell users that they are facing this issue by adding a check in lttng-ust configure.ac ? 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 <[email protected]> > >> Thanks-to: Henrik Wallin <[email protected]> > >> Cc: Paul Woegerer <[email protected]> > >> Signed-off-by: Gerlando Falauto <[email protected]> > >> --- > >> 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 > >> [email protected] > >> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev > >> > > > > -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
