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

[Richard Henderson]

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~