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

[Stefan Weil]

Am 15.05.2018 um 14:13 schrieb Peter Maydell:

On 13 May 2018 at 10:57, Stefan Weil <address@hidden> 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