[ovs-dev,lib:] Avoid clobbered variable warning on ppc64le.
diff mbox series

Message ID 20191008170417.11823-1-dwilder@us.ibm.com
State New
Headers show
Series
  • [ovs-dev,lib:] Avoid clobbered variable warning on ppc64le.
Related show

Commit Message

dwilder Oct. 8, 2019, 5:04 p.m. UTC
Since commit e2ed6fbeb1, Ci on ppc64le with Ubuntu 16.04.6 LTS throws
this error:

lib/fatal-signal.c: In function 'send_backtrace_to_monitor':
lib/fatal-signal.c:168:9: error: variable 'dep' might be clobbered by
'longjmp' or 'vfork' [-Werror=clobbered]
     int dep;

Declaring dep as a volatile int.

Signed-off-by: David Wilder <wilder@us.ibm.com>
---
 lib/fatal-signal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

0-day Robot Oct. 8, 2019, 5:59 p.m. UTC | #1
Bleep bloop.  Greetings David Wilder, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author David Wilder <dwilder@us.ibm.com> needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: David Wilder <wilder@us.ibm.com>
Lines checked: 37, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
William Tu Oct. 8, 2019, 7:44 p.m. UTC | #2
On Tue, Oct 08, 2019 at 10:04:17AM -0700, David Wilder wrote:
> Since commit e2ed6fbeb1, Ci on ppc64le with Ubuntu 16.04.6 LTS throws
> this error:
> 
> lib/fatal-signal.c: In function 'send_backtrace_to_monitor':
> lib/fatal-signal.c:168:9: error: variable 'dep' might be clobbered by
> 'longjmp' or 'vfork' [-Werror=clobbered]
>      int dep;
> 
> Declaring dep as a volatile int.
> 
> Signed-off-by: David Wilder <wilder@us.ibm.com>
> ---
>  lib/fatal-signal.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
> index 7733850d5..09f7c6ecf 100644
> --- a/lib/fatal-signal.c
> +++ b/lib/fatal-signal.c
> @@ -165,7 +165,8 @@ fatal_signal_add_hook(void (*hook_cb)(void *aux), void (*cancel_cb)(void *aux),
>   */
>  static inline void
>  send_backtrace_to_monitor(void) {
> -    int dep;
> +    /* volatile added to prevent a "clobbered" error on ppc64le with gcc */
> +    volatile int dep;
>      struct unw_backtrace unw_bt[UNW_MAX_DEPTH];
>      unw_cursor_t cursor;
>      unw_context_t uc;
> -- 
> 2.23.0.162.gf1d4a28
> 
Thank you for testing it on ppc64!
I'm not able to reproduce the issue on x86_64.

And I don't understand where do we call setjmp and longjmp?
Is it the case that:
unw_getcontext/unw_init_local is calling setjmp
and
unw_get_reg/unw_get_proc_name is calling longjmp while it's
scanning back the stack?

Looking at the source code of libunwind, I couldn't quite
understand how setjmp/longjmp is used here.

Regards,
William
dwilder Oct. 8, 2019, 8:12 p.m. UTC | #3
On 2019-10-08 12:44, William Tu wrote:
> On Tue, Oct 08, 2019 at 10:04:17AM -0700, David Wilder wrote:
>> Since commit e2ed6fbeb1, Ci on ppc64le with Ubuntu 16.04.6 LTS throws
>> this error:
>> 
>> lib/fatal-signal.c: In function 'send_backtrace_to_monitor':
>> lib/fatal-signal.c:168:9: error: variable 'dep' might be clobbered by
>> 'longjmp' or 'vfork' [-Werror=clobbered]
>>      int dep;
>> 
>> Declaring dep as a volatile int.
>> 
>> Signed-off-by: David Wilder <wilder@us.ibm.com>
>> ---
>>  lib/fatal-signal.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
>> index 7733850d5..09f7c6ecf 100644
>> --- a/lib/fatal-signal.c
>> +++ b/lib/fatal-signal.c
>> @@ -165,7 +165,8 @@ fatal_signal_add_hook(void (*hook_cb)(void *aux), 
>> void (*cancel_cb)(void *aux),
>>   */
>>  static inline void
>>  send_backtrace_to_monitor(void) {
>> -    int dep;
>> +    /* volatile added to prevent a "clobbered" error on ppc64le with 
>> gcc */
>> +    volatile int dep;
>>      struct unw_backtrace unw_bt[UNW_MAX_DEPTH];
>>      unw_cursor_t cursor;
>>      unw_context_t uc;
>> --
>> 2.23.0.162.gf1d4a28
>> 
> Thank you for testing it on ppc64!
> I'm not able to reproduce the issue on x86_64.
> 
> And I don't understand where do we call setjmp and longjmp?
> Is it the case that:
> unw_getcontext/unw_init_local is calling setjmp
> and
> unw_get_reg/unw_get_proc_name is calling longjmp while it's
> scanning back the stack?
> 
> Looking at the source code of libunwind, I couldn't quite
> understand how setjmp/longjmp is used here.
> 
> Regards,
> William

Hi William
This is an issue with gcc incorrectly generating the warning. I found 
reports against gcc for this issue. I tried but was unable to isolate 
the problem to a gcc version.  Gcc v8.2.1 in rhel8 did not have the 
problem, however v8 in ubuntu 16.04 still had the issue.  As I was 
unable to flag the version gcc I chose this solution over a #pragma.

PS: I recently starting running Ci against ppc64le, travis_ci.org is now 
supporting (in beta) ppc64le.  I hope to soon push changes to enable a 
ppc64le Ci for ovs.

Regards
David

Patch
diff mbox series

diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index 7733850d5..09f7c6ecf 100644
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -165,7 +165,8 @@  fatal_signal_add_hook(void (*hook_cb)(void *aux), void (*cancel_cb)(void *aux),
  */
 static inline void
 send_backtrace_to_monitor(void) {
-    int dep;
+    /* volatile added to prevent a "clobbered" error on ppc64le with gcc */
+    volatile int dep;
     struct unw_backtrace unw_bt[UNW_MAX_DEPTH];
     unw_cursor_t cursor;
     unw_context_t uc;