[lttng-dev] lttng-ust with --std=c99 -pedantic

[John Steele Scott]

Mathieu Desnoyers <mathieu.desnoyers at efficios.com> 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