| Message ID | 20201028041819.2169003-4-kuhn.chenqun@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | silence the compiler warnings | expand |
On 28/10/2020 05.18, Chen Qun wrote: > When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning: > ../accel/tcg/user-exec.c: In function ‘handle_cpu_signal’: > ../accel/tcg/user-exec.c:169:13: warning: this statement may fall through [-Wimplicit-fallthrough=] > 169 | cpu_exit_tb_from_sighandler(cpu, old_set); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > ../accel/tcg/user-exec.c:172:9: note: here > 172 | default: > > This exception branch fall through the 'default' branch and run the 'g_assert_not_reached' statement. > So we could use "fall through" instead of "NORETURN" here. > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > --- > Cc: Riku Voipio <riku.voipio@iki.fi> > Cc: Richard Henderson <richard.henderson@linaro.org> > Cc: Paolo Bonzini <pbonzini@redhat.com> > --- > accel/tcg/user-exec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c > index 4ebe25461a..330468e990 100644 > --- a/accel/tcg/user-exec.c > +++ b/accel/tcg/user-exec.c > @@ -167,7 +167,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, > */ > clear_helper_retaddr(); > cpu_exit_tb_from_sighandler(cpu, old_set); > - /* NORETURN */ > + /* fall through */ There should not be a fall through here since the previous function should never return. Does the warning go away if you mark the cpu_exit_tb_from_sighandler() function with QEMU_NORETURN ? If so, I think that would be the better fix. Thomas
On 10/28/20 6:52 AM, Thomas Huth wrote: > On 28/10/2020 05.18, Chen Qun wrote: >> When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning: >> ../accel/tcg/user-exec.c: In function ‘handle_cpu_signal’: >> ../accel/tcg/user-exec.c:169:13: warning: this statement may fall through [-Wimplicit-fallthrough=] >> 169 | cpu_exit_tb_from_sighandler(cpu, old_set); >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> ../accel/tcg/user-exec.c:172:9: note: here >> 172 | default: >> >> This exception branch fall through the 'default' branch and run the 'g_assert_not_reached' statement. >> So we could use "fall through" instead of "NORETURN" here. >> >> Reported-by: Euler Robot <euler.robot@huawei.com> >> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> >> --- >> Cc: Riku Voipio <riku.voipio@iki.fi> >> Cc: Richard Henderson <richard.henderson@linaro.org> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> --- >> accel/tcg/user-exec.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c >> index 4ebe25461a..330468e990 100644 >> --- a/accel/tcg/user-exec.c >> +++ b/accel/tcg/user-exec.c >> @@ -167,7 +167,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, >> */ >> clear_helper_retaddr(); >> cpu_exit_tb_from_sighandler(cpu, old_set); >> - /* NORETURN */ >> + /* fall through */ > > There should not be a fall through here since the previous function should > never return. Does the warning go away if you mark the > cpu_exit_tb_from_sighandler() function with QEMU_NORETURN ? If so, I think > that would be the better fix. The compiler should have figured that out itself, due to cpu_loop_exit_noexc being marked QEMU_NORETURN. However, if adding a second QEMU_NORETURN works, I'm fine with that. As a very last resort, we can change the comment to /* no return, but fall through to assert not reached */ which correctly documents both the function preceding and also contains the regexp that the compiler is using for the warning. r~
> -----Original Message----- > From: Richard Henderson [mailto:richard.henderson@linaro.org] > Sent: Wednesday, October 28, 2020 11:38 PM > To: Thomas Huth <thuth@redhat.com>; Chenqun (kuhn) > <kuhn.chenqun@huawei.com>; qemu-devel@nongnu.org; > qemu-trivial@nongnu.org > Cc: Zhanghailiang <zhang.zhanghailiang@huawei.com>; Riku Voipio > <riku.voipio@iki.fi>; Paolo Bonzini <pbonzini@redhat.com>; ganqixin > <ganqixin@huawei.com>; Euler Robot <euler.robot@huawei.com> > Subject: Re: [PATCH 3/9] accel/tcg/user-exec: silence the compiler warnings > > On 10/28/20 6:52 AM, Thomas Huth wrote: > > On 28/10/2020 05.18, Chen Qun wrote: > >> When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed > warning: > >> ../accel/tcg/user-exec.c: In function ‘handle_cpu_signal’: > >> ../accel/tcg/user-exec.c:169:13: warning: this statement may fall through > [-Wimplicit-fallthrough=] > >> 169 | cpu_exit_tb_from_sighandler(cpu, old_set); > >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >> ../accel/tcg/user-exec.c:172:9: note: here > >> 172 | default: > >> > >> This exception branch fall through the 'default' branch and run the > 'g_assert_not_reached' statement. > >> So we could use "fall through" instead of "NORETURN" here. > >> > >> Reported-by: Euler Robot <euler.robot@huawei.com> > >> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> > >> --- > >> Cc: Riku Voipio <riku.voipio@iki.fi> > >> Cc: Richard Henderson <richard.henderson@linaro.org> > >> Cc: Paolo Bonzini <pbonzini@redhat.com> > >> --- > >> accel/tcg/user-exec.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index > >> 4ebe25461a..330468e990 100644 > >> --- a/accel/tcg/user-exec.c > >> +++ b/accel/tcg/user-exec.c > >> @@ -167,7 +167,7 @@ static inline int handle_cpu_signal(uintptr_t pc, > siginfo_t *info, > >> */ > >> clear_helper_retaddr(); > >> cpu_exit_tb_from_sighandler(cpu, old_set); > >> - /* NORETURN */ > >> + /* fall through */ > > > > There should not be a fall through here since the previous function > > should never return. Does the warning go away if you mark the > > cpu_exit_tb_from_sighandler() function with QEMU_NORETURN ? If so, I > > think that would be the better fix. > > The compiler should have figured that out itself, due to cpu_loop_exit_noexc > being marked QEMU_NORETURN. However, if adding a second > QEMU_NORETURN works, I'm fine with that. > I tried to add QEMU_NORETURN to the cpu_exit_tb_from_sighandler() function. And then the compiler warning was cleared. It seems to be a better fix. Thanks, Chen Qun > As a very last resort, we can change the comment to > > /* no return, but fall through to assert not reached */ > > which correctly documents both the function preceding and also contains the > regexp that the compiler is using for the warning. >
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index 4ebe25461a..330468e990 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -167,7 +167,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t *info, */ clear_helper_retaddr(); cpu_exit_tb_from_sighandler(cpu, old_set); - /* NORETURN */ + /* fall through */ default: g_assert_not_reached();
When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning: ../accel/tcg/user-exec.c: In function ‘handle_cpu_signal’: ../accel/tcg/user-exec.c:169:13: warning: this statement may fall through [-Wimplicit-fallthrough=] 169 | cpu_exit_tb_from_sighandler(cpu, old_set); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../accel/tcg/user-exec.c:172:9: note: here 172 | default: This exception branch fall through the 'default' branch and run the 'g_assert_not_reached' statement. So we could use "fall through" instead of "NORETURN" here. Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com> --- Cc: Riku Voipio <riku.voipio@iki.fi> Cc: Richard Henderson <richard.henderson@linaro.org> Cc: Paolo Bonzini <pbonzini@redhat.com> --- accel/tcg/user-exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)