Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
From: toojays@toojays.net (John Steele Scott)
Subject: [lttng-dev] lttng-ust with --std=c99 -pedantic
Date: Wed, 20 Jun 2012 22:40:12 +0930	[thread overview]
Message-ID: <4FE1CBB4.7040007@toojays.net> (raw)
In-Reply-To: <20120619174518.GB8776@Krystal>

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).

>>
>> ------------------
>>
>> In file included from hello.c:34:0:
>> ust_tests_hello.h: In function ?__tracepoint_cb_ust_tests_hello___tptest?:
>> ust_tests_hello.h:28:1: warning: ISO C forbids braced-groups within expressions [-pedantic]
>> ust_tests_hello.h:28:1: warning: ISO C forbids conversion of object pointer to function pointer type [-pedantic]
>> ust_tests_hello.h: In function ?__tracepoint_cb_ust_tests_hello___tptest_sighandler?:
>> ust_tests_hello.h:52:1: warning: ISO C forbids braced-groups within expressions [-pedantic]
>> ust_tests_hello.h:52:1: warning: ISO C forbids conversion of object pointer to function pointer type [-pedantic]
>>
>> ---> see include/lttng/tracepoint.h 
>>
>> #define _DECLARE_TRACEPOINT(_provider, _name, ...)                                      \
>> extern struct tracepoint __tracepoint_##_provider##___##_name;                          \
>> static inline void __tracepoint_cb_##_provider##___##_name(_TP_ARGS_PROTO(__VA_ARGS__)) \
>> [...]
>>
>> ----> this is caused by use of "tp_rcu_dereference_bp()", which is
>> defined in include/lttng/tracepoint-rcu.h (in the #else clause) :
>>
>> #define tp_rcu_dereference_bp(p)                                             \
>>         ({                                                                   \
>>                 __typeof__(p) _________p1 = URCU_FORCE_CAST(__typeof__(p),   \
>>                         tracepoint_dlopen.rcu_dereference_sym_bp(URCU_FORCE_CAST(void *, p))); \
>>                 (_________p1);                                               \
>>         })
>>
>> For this one, we should be able to change it into a single-expression
>> and remove the braced-groups, since we are just really evaluating a
>> single expression here. Fixed by the following commit in lttng-ust
>> master:
>>
>> commit a4eaf8eabe829be8f7d7432ffaf83291a068b0ed
>> Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
>> Date:   Mon Jun 18 10:10:36 2012 -0400
>>
>>     Fix c99 compatibility: tp_rcu_dereference_bp() should not use braced-groups within expressions
>>
>> ------------------
>>
>> hello.c: In function ?inthandler?:
>> hello.c:39:47: warning: ISO C99 requires rest arguments to be used [enabled by default]
>> ---> see include/lttng/tracepoint.h  tracepoint() macro.
>> ----> this is caused by not passing any argument to the tracepoint.
>> Ideas on how to fix this are welcome, as I don't see any way to do it in
>> C99-pedantic.
> I think I found a way to fix the last pedantic warnings. Can you try
> with this master branch commit ?
>
>
> commit fbdeb5ecb8ff9b7d73a72de9fc66d07f7797d93f
> Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> Date:   Tue Jun 19 13:45:05 2012 -0400
>
>     Fix C99 strict compatibility: don't use void * for function pointers
>     
>     compiling public headers with --std=x99 -pedantic shows:
>     
>     warning: ISO C forbids conversion of object pointer to function pointer type
>     
>     Use "void (*func)(void)" to represent a generic function pointer rather
>     than "void *".
>     
>     Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
>

Unfortunately, this isn't sufficient to fix these warnings on Centos 6.2. However, I can now make the warnings go away by using dlsym+memcpy (instead of dlsym+cast), as mentioned above. I haven't had a chance to actually use the resulting binary yet, but I don't see why it shouldn't work.

Thanks,

John



  reply	other threads:[~2012-06-20 13:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-09 23:37 John Steele Scott
2012-06-12 15:32 ` Mathieu Desnoyers
2012-06-12 15:55   ` Mathieu Desnoyers
2012-06-18 12:10     ` John Steele Scott
2012-06-18 14:11       ` Mathieu Desnoyers
2012-06-19 17:45         ` Mathieu Desnoyers
2012-06-20 13:10           ` John Steele Scott [this message]
2012-06-20 13:47             ` Mathieu Desnoyers
2012-06-21  0:51               ` John Steele Scott

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FE1CBB4.7040007@toojays.net \
    --to=toojays@toojays.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox