| Message ID | 20191008170417.11823-1-dwilder@us.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,lib:] Avoid clobbered variable warning on ppc64le. | expand |
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
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
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
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;
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(-)