RE: [PATCH 3/9] accel/tcg/user-exec: silence the compiler warnings

[Chenqun (kuhn)]

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