Re: [Qemu-devel] [PATCH] disas/libvixl: Fix VIXL_FALLTHROUGH macro for QEMU

[Stefan Weil <sw@weilnetz.de>]

Am 15.05.2018 um 14:13 schrieb Peter Maydell:
> On 13 May 2018 at 10:57, Stefan Weil <sw@weilnetz.de> wrote:
>> Even with the above patch, disas/libvixl raises a compiler warning for a
>> fall through case. The patch below fixes that warning, but I am not sure
>> whether a fall through is correct there.
>>
>> Stefan
>>
>>
>> diff --git a/disas/libvixl/vixl/a64/disasm-a64.cc
>> b/disas/libvixl/vixl/a64/disasm-a64.cc
>> index 7a58a5c087..5481d94209 100644
>> --- a/disas/libvixl/vixl/a64/disasm-a64.cc
>> +++ b/disas/libvixl/vixl/a64/disasm-a64.cc
>> @@ -2986,6 +2986,7 @@ int Disassembler::SubstituteImmediateField(const
>> Instruction* instr,
>>            return 3;
>>          }
>>        }
>> +      VIXL_FALLTHROUGH(); // ???
>>      }
>>      case 'C': {  // ICondB - Immediate Conditional Branch.
>>        int64_t offset = instr->ImmCondBranch() << 2;
> This is fixed in upstream vixl, in fact:
>
> https://git.linaro.org/arm/vixl.git/commit/?id=de326f850f736c3a337fda52845ed3d2e620cc02
>
> thanks
> -- PMM


That fix will work for the moment, but is not sufficient for the future
because it only supports gcc 7.x, but not gcc 8.x or later:

+#elif __GNUC__ == 7

How do we proceed with the code in QEMU? Do you have a plan to update
the vixl code? As vixl is obviously no longer maintained on GitHub, I am
not sure whom I could contact.

And what about my other question / suggestion:

"I suggest to add and use a similar macro QEMU_FALLTHROUGH() for the
rest of the code and can provide a patch if that's fine for everyone."

gcc gives lots of fallthrough warnings, and many code locations don't contain a comment stating that the fall through is fine.

Thanks
Stefan