From mboxrd@z Thu Jan 1 00:00:00 1970 From: toojays@toojays.net (John Steele Scott) Date: Thu, 21 Jun 2012 10:21:01 +0930 Subject: [lttng-dev] lttng-ust with --std=c99 -pedantic In-Reply-To: <20120620134746.GB25924@Krystal> References: <20120612153257.GB14189@Krystal> <20120612155539.GA15859@Krystal> <4FDF1AC2.3070009@toojays.net> <20120618141156.GB6865@Krystal> <20120619174518.GB8776@Krystal> <4FE1CBB4.7040007@toojays.net> <20120620134746.GB25924@Krystal> Message-ID: <8762alsd1m.fsf@quantum.com> Mathieu Desnoyers writes: > * John Steele Scott (toojays at toojays.net) wrote: >> On 20/06/12 03:15, Mathieu Desnoyers wrote: >> > * Mathieu Desnoyers (mathieu.desnoyers at efficios.com) wrote: >> >> * John Steele Scott (toojays at toojays.net) wrote: >> >> >> snip snip >> >>> The thing I forgot to mention is: okay, I can build now with >> >>> -pedantic, but I can't build with "-pedantic -Werror". This >> >>> application builds with "--std=c99 -pedantic -Werror" (among many >> >>> other flags). I can of course remove -Werror for my tests, but in the >> >>> longer term I would like to see lttng-ust enabled in our regular >> >>> build, and disabling -Werror for that is not something we want to do. >> >>> If I only had to disable it for the trace provider, I could negotiate >> >>> that, but disabling it for any module which uses tracepoints is >> >>> undesirable. >> >>> >> >>> I haven't yet looked at how difficult it would be do eliminate these >> >>> pedantic warnings. Do you have a feel for what is involved? >> >>> Preprocessor tricks warp my mind. :( >> >> I don't think we'll want to make the lttng probe module build under >> >> --std=c99 -pedandic -Werror, but I think we should focus on making sure >> >> the tracepoint part that is built within the application (with >> >> TRACEPOINT_CREATE_PROBES _not_ defined) builds fine in --std=c99 >> >> -pedantic. >> >> >> >> Currently, building a simple test program with tracepoints under >> >> --std=c99 -pedantic gets me: >> >> >> >> >> >> In file included from ust_tests_hello.h:25:0, >> >> from hello.c:34: >> >> ../../include/lttng/tracepoint.h: In function ?__tracepoints__init?: >> >> ../../include/lttng/tracepoint.h:261:3: warning: ISO C forbids conversion of object pointer to function pointer type [-pedantic] >> >> ../../include/lttng/tracepoint.h:265:3: warning: ISO C forbids conversion of object pointer to function pointer type [-pedantic] >> >> ../../include/lttng/tracepoint.h:270:3: warning: ISO C forbids conversion of object pointer to function pointer type [-pedantic] >> >> ../../include/lttng/tracepoint.h:274:3: warning: ISO C forbids conversion of object pointer to function pointer type [-pedantic] >> >> ../../include/lttng/tracepoint.h:278:3: warning: ISO C forbids conversion of object pointer to function pointer type [-pedantic] >> >> >> >> ---> see include/lttng/tracepoint.h __tracepoints__init(): >> >> ----> This is caused by use of dlsym() to lookup a function pointer from >> >> a symbol. dlsym() returns "void *", and we cast it into function >> >> pointer type. Ideas on how to make this c99 pedantic compliant are >> >> welcome. >> >> I found this warning discussed in a GCC bug report. Jakub Jelinek's suggestion at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45289#c6 (load the result of dlsym into a void *, then memcpy to the function pointer), does make these go away (using GCC 4.4.6 on Centos 6.2). > > Can you check if these can be reproduced with gcc 4.5 or 4.6 ? (without > the memcpy trick) > > The bugzilla entry seems to imply that the warning went away from > -pedantic mode at some point. That was my reading of the bugzilla entry as well. However, I can still reproduce those warnings with GCC 4.5.4 and 4.6.1 (both installed on Ubuntu 11.10). I don't have a GCC 4.7 install to test with. An alternative approach to the memcpy thing is to wrap dlsym in a function which returns the symbol in an output parameter, rather than a return value. It looks a little less ugly. Something like: diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h index 5bab476..8d2afc1 100644 --- a/include/lttng/tracepoint.h +++ b/include/lttng/tracepoint.h @@ -249,6 +249,14 @@ int __tracepoint_registered struct tracepoint_dlopen tracepoint_dlopen __attribute__((weak, visibility("hidden"))); +#ifndef LTTNG_HAVE_PEDANTIC_C99_DLSYM_WORKAROUND +#define LTTNG_HAVE_PEDANTIC_C99_DLSYM_WORKAROUND +static void lttng_c99_dlsym(void *handle, const char* symbol, void **addr) +{ + *addr = dlsym(handle, symbol); +} +#endif + static void __attribute__((constructor)) __tracepoints__init(void) { if (__tracepoint_registered++) @@ -258,27 +266,22 @@ static void __attribute__((constructor)) __tracepoints__init(void) dlopen("liblttng-ust-tracepoint.so.0", RTLD_NOW | RTLD_GLOBAL); if (!tracepoint_dlopen.liblttngust_handle) return; - tracepoint_dlopen.tracepoint_register_lib = - URCU_FORCE_CAST(int (*)(struct tracepoint * const *, int), - dlsym(tracepoint_dlopen.liblttngust_handle, - "tracepoint_register_lib")); - tracepoint_dlopen.tracepoint_unregister_lib = - URCU_FORCE_CAST(int (*)(struct tracepoint * const *), - dlsym(tracepoint_dlopen.liblttngust_handle, - "tracepoint_unregister_lib")); + lttng_c99_dlsym(tracepoint_dlopen.liblttngust_handle, + "tracepoint_register_lib", + (void **)&tracepoint_dlopen.tracepoint_register_lib); + lttng_c99_dlsym(tracepoint_dlopen.liblttngust_handle, + "tracepoint_unregister_lib", + (void **)&tracepoint_dlopen.tracepoint_unregister_lib); #ifndef _LGPL_SOURCE - tracepoint_dlopen.rcu_read_lock_sym_bp = - URCU_FORCE_CAST(void (*)(void), - dlsym(tracepoint_dlopen.liblttngust_handle, - "tp_rcu_read_lock_bp")); - tracepoint_dlopen.rcu_read_unlock_sym_bp = - URCU_FORCE_CAST(void (*)(void), - dlsym(tracepoint_dlopen.liblttngust_handle, - "tp_rcu_read_unlock_bp")); - tracepoint_dlopen.rcu_dereference_sym_bp = - URCU_FORCE_CAST(void *(*)(void *p), - dlsym(tracepoint_dlopen.liblttngust_handle, - "tp_rcu_dereference_sym_bp")); + lttng_c99_dlsym(tracepoint_dlopen.liblttngust_handle, + "tp_rcu_read_lock_bp", + (void **)&tracepoint_dlopen.rcu_read_lock_sym_bp); + lttng_c99_dlsym(tracepoint_dlopen.liblttngust_handle, + "tp_rcu_read_unlock_bp", + (void **)&tracepoint_dlopen.rcu_read_unlock_sym_bp); + lttng_c99_dlsym(tracepoint_dlopen.liblttngust_handle, + "tp_rcu_dereference_sym_bp", + (void **)&tracepoint_dlopen.rcu_dereference_sym_bp); #endif tracepoint_dlopen.tracepoint_register_lib(__start___tracepoints_ptrs, __stop___tracepoints_ptrs - On the system I'm trying to build which has the aggressive GCC warnings enabled, this gets me down to just one warning, which is due to redundantly declaring the __tracepoint_provider_##_provider symbol. That one is from -Wredundant-decls, not -pedantic; and I'll leave it for another day. cheers, John