From 4f02d49a80fdb3c5898dd32b5879289425ee158f Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 11 Dec 2020 16:24:15 +0100 Subject: [PATCH 01/14] disas/libvixl: Fix fall-through annotation for GCC >= 7 For compiling with -Wimplicit-fallthrough we need to fix the fallthrough annotations in the libvixl code. This is based on the following upstream vixl commit by Martyn Capewell: https://git.linaro.org/arm/vixl.git/commit/?id=de326f850f736c3a337 "GCC 7 enables switch/case fallthrough checking, but this fails in VIXL, because the annotation we use is Clang specific. Also, fix a missing annotation in the disassembler." Signed-off-by: Thomas Huth Reviewed-by: Peter Maydell Message-Id: <20201211152426.350966-2-thuth@redhat.com> Signed-off-by: Thomas Huth --- disas/libvixl/vixl/a64/disasm-a64.cc | 4 ++++ disas/libvixl/vixl/globals.h | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/disas/libvixl/vixl/a64/disasm-a64.cc b/disas/libvixl/vixl/a64/disasm-a64.cc index 7a58a5c087..f34d1d68da 100644 --- a/disas/libvixl/vixl/a64/disasm-a64.cc +++ b/disas/libvixl/vixl/a64/disasm-a64.cc @@ -2985,6 +2985,10 @@ int Disassembler::SubstituteImmediateField(const Instruction* instr, } return 3; } + default: { + VIXL_UNIMPLEMENTED(); + return 0; + } } } case 'C': { // ICondB - Immediate Conditional Branch. diff --git a/disas/libvixl/vixl/globals.h b/disas/libvixl/vixl/globals.h index 61dc9f7f7e..7099aa599f 100644 --- a/disas/libvixl/vixl/globals.h +++ b/disas/libvixl/vixl/globals.h @@ -108,10 +108,12 @@ inline void USE(T1, T2, T3, T4) {} #define __has_warning(x) 0 #endif -// Note: This option is only available for Clang. And will only be enabled for -// C++11(201103L). +// Fallthrough annotation for Clang and C++11(201103L). #if __has_warning("-Wimplicit-fallthrough") && __cplusplus >= 201103L #define VIXL_FALLTHROUGH() [[clang::fallthrough]] //NOLINT +// Fallthrough annotation for GCC >= 7. +#elif __GNUC__ >= 7 + #define VIXL_FALLTHROUGH() __attribute__((fallthrough)) #else #define VIXL_FALLTHROUGH() do {} while (0) #endif From 51c915674d8cf9de596d96ac31226c4a374fcb95 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 11 Dec 2020 16:24:16 +0100 Subject: [PATCH 02/14] target/unicore32/translate: Add missing fallthrough annotations Looking at the way the code is formatted here (there is an empty line after break statements, but none where the break is missing), and the instruction set overview at https://en.wikipedia.org/wiki/Unicore the fallthrough is very likely intended here. So add a fallthrough comment to make the it compilable with -Werror=implicit-fallthrough. Signed-off-by: Thomas Huth Reviewed-by: Richard Henderson Message-Id: <20201211152426.350966-3-thuth@redhat.com> Signed-off-by: Thomas Huth --- target/unicore32/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/unicore32/translate.c b/target/unicore32/translate.c index d4b06df672..962f9877a0 100644 --- a/target/unicore32/translate.c +++ b/target/unicore32/translate.c @@ -1801,6 +1801,7 @@ static void disas_uc32_insn(CPUUniCore32State *env, DisasContext *s) do_misc(env, s, insn); break; } + /* fallthrough */ case 0x1: if (((UCOP_OPCODES >> 2) == 2) && !UCOP_SET_S) { do_misc(env, s, insn); @@ -1817,6 +1818,7 @@ static void disas_uc32_insn(CPUUniCore32State *env, DisasContext *s) if (UCOP_SET(8) || UCOP_SET(5)) { ILLEGAL; } + /* fallthrough */ case 0x3: do_ldst_ir(env, s, insn); break; From 216776099b087edfdf8cca3cb3cbfee1edbb12e2 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 11 Dec 2020 16:24:17 +0100 Subject: [PATCH 03/14] hw/rtc/twl92230: Silence warnings about missing fallthrough statements When compiling with -Werror=implicit-fallthrough, gcc complains about missing fallthrough annotations in this file. Looking at the code, the fallthrough is indeed wanted here, but instead of adding the annotations, it can be done more efficiently by simply calculating the offset with a subtraction instead of increasing a local variable one by one. Signed-off-by: Thomas Huth Reviewed-by: Peter Maydell Message-Id: <20201211152426.350966-4-thuth@redhat.com> Signed-off-by: Thomas Huth --- hw/rtc/twl92230.c | 41 ++++++++++++----------------------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/hw/rtc/twl92230.c b/hw/rtc/twl92230.c index f838913b37..a787bd247d 100644 --- a/hw/rtc/twl92230.c +++ b/hw/rtc/twl92230.c @@ -271,37 +271,23 @@ static void menelaus_gpio_set(void *opaque, int line, int level) static uint8_t menelaus_read(void *opaque, uint8_t addr) { MenelausState *s = (MenelausState *) opaque; - int reg = 0; switch (addr) { case MENELAUS_REV: return 0x22; - case MENELAUS_VCORE_CTRL5: reg ++; - case MENELAUS_VCORE_CTRL4: reg ++; - case MENELAUS_VCORE_CTRL3: reg ++; - case MENELAUS_VCORE_CTRL2: reg ++; - case MENELAUS_VCORE_CTRL1: - return s->vcore[reg]; + case MENELAUS_VCORE_CTRL1 ... MENELAUS_VCORE_CTRL5: + return s->vcore[addr - MENELAUS_VCORE_CTRL1]; - case MENELAUS_DCDC_CTRL3: reg ++; - case MENELAUS_DCDC_CTRL2: reg ++; - case MENELAUS_DCDC_CTRL1: - return s->dcdc[reg]; + case MENELAUS_DCDC_CTRL1 ... MENELAUS_DCDC_CTRL3: + return s->dcdc[addr - MENELAUS_DCDC_CTRL1]; - case MENELAUS_LDO_CTRL8: reg ++; - case MENELAUS_LDO_CTRL7: reg ++; - case MENELAUS_LDO_CTRL6: reg ++; - case MENELAUS_LDO_CTRL5: reg ++; - case MENELAUS_LDO_CTRL4: reg ++; - case MENELAUS_LDO_CTRL3: reg ++; - case MENELAUS_LDO_CTRL2: reg ++; - case MENELAUS_LDO_CTRL1: - return s->ldo[reg]; + case MENELAUS_LDO_CTRL1 ... MENELAUS_LDO_CTRL8: + return s->ldo[addr - MENELAUS_LDO_CTRL1]; - case MENELAUS_SLEEP_CTRL2: reg ++; case MENELAUS_SLEEP_CTRL1: - return s->sleep[reg]; + case MENELAUS_SLEEP_CTRL2: + return s->sleep[addr - MENELAUS_SLEEP_CTRL1]; case MENELAUS_DEVICE_OFF: return 0; @@ -395,10 +381,8 @@ static uint8_t menelaus_read(void *opaque, uint8_t addr) case MENELAUS_S2_PULL_DIR: return s->pull[3]; - case MENELAUS_MCT_CTRL3: reg ++; - case MENELAUS_MCT_CTRL2: reg ++; - case MENELAUS_MCT_CTRL1: - return s->mmc_ctrl[reg]; + case MENELAUS_MCT_CTRL1 ... MENELAUS_MCT_CTRL3: + return s->mmc_ctrl[addr - MENELAUS_MCT_CTRL1]; case MENELAUS_MCT_PIN_ST: /* TODO: return the real Card Detect */ return 0; @@ -418,7 +402,6 @@ static void menelaus_write(void *opaque, uint8_t addr, uint8_t value) { MenelausState *s = (MenelausState *) opaque; int line; - int reg = 0; struct tm tm; switch (addr) { @@ -496,9 +479,9 @@ static void menelaus_write(void *opaque, uint8_t addr, uint8_t value) s->ldo[7] = value & 3; break; - case MENELAUS_SLEEP_CTRL2: reg ++; case MENELAUS_SLEEP_CTRL1: - s->sleep[reg] = value; + case MENELAUS_SLEEP_CTRL2: + s->sleep[addr - MENELAUS_SLEEP_CTRL1] = value; break; case MENELAUS_DEVICE_OFF: From 30982862b2d81e7b4a58bac319075d343c36e06a Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 11 Dec 2020 16:24:18 +0100 Subject: [PATCH 04/14] hw/timer/renesas_tmr: silence the compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning: ../hw/timer/renesas_tmr.c: In function ‘tmr_read’: ../hw/timer/renesas_tmr.c:221:19: warning: this statement may fall through [-Wimplicit-fallthrough=] 221 | } else if (ch == 0) {i | ^ ../hw/timer/renesas_tmr.c:224:5: note: here 224 | case A_TCORB: | ^~~~ Add the corresponding "fall through" comment to fix it. Reported-by: Euler Robot Signed-off-by: Chen Qun Signed-off-by: Thomas Huth Message-Id: <20201211152426.350966-5-thuth@redhat.com> Signed-off-by: Thomas Huth --- hw/timer/renesas_tmr.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/timer/renesas_tmr.c b/hw/timer/renesas_tmr.c index 446f2eacdd..e03a8155b2 100644 --- a/hw/timer/renesas_tmr.c +++ b/hw/timer/renesas_tmr.c @@ -221,6 +221,7 @@ static uint64_t tmr_read(void *opaque, hwaddr addr, unsigned size) } else if (ch == 0) { return concat_reg(tmr->tcora); } + /* fall through */ case A_TCORB: if (size == 1) { return tmr->tcorb[ch]; From bdddc1c425e8e39031bf726b54a6e7388337a7f0 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 11 Dec 2020 16:24:19 +0100 Subject: [PATCH 05/14] target/i386: silence the compiler warnings in gen_shiftd_rm_T1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current "#ifdef TARGET_X86_64" statement affects the compiler's determination of fall through. When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning: target/i386/translate.c: In function ‘gen_shiftd_rm_T1’: target/i386/translate.c:1773:12: warning: this statement may fall through [-Wimplicit-fallthrough=] if (is_right) { ^ target/i386/translate.c:1782:5: note: here case MO_32: ^~~~ Reported-by: Euler Robot Signed-off-by: Chen Qun Signed-off-by: Thomas Huth Reviewed-by: Richard Henderson Reviewed-by: Thomas Huth Message-Id: <20201211152426.350966-6-thuth@redhat.com> Signed-off-by: Thomas Huth --- target/i386/tcg/translate.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 750f75c257..11db2f3c8d 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -1778,9 +1778,12 @@ static void gen_shiftd_rm_T1(DisasContext *s, MemOp ot, int op1, } else { tcg_gen_deposit_tl(s->T1, s->T0, s->T1, 16, 16); } - /* FALLTHRU */ -#ifdef TARGET_X86_64 + /* + * If TARGET_X86_64 defined then fall through into MO_32 case, + * otherwise fall through default case. + */ case MO_32: +#ifdef TARGET_X86_64 /* Concatenate the two 32-bit values and use a 64-bit shift. */ tcg_gen_subi_tl(s->tmp0, count, 1); if (is_right) { From d85afd1eb5764722b8d46806d2d6acccc2b9d044 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 11 Dec 2020 16:24:20 +0100 Subject: [PATCH 06/14] hw/intc/arm_gicv3_kvm: silence the compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning: hw/intc/arm_gicv3_kvm.c: In function ‘kvm_arm_gicv3_put’: hw/intc/arm_gicv3_kvm.c:484:13: warning: this statement may fall through [-Wimplicit-fallthrough=] kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, ®64, true); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ hw/intc/arm_gicv3_kvm.c:485:9: note: here default: ^~~~~~~ hw/intc/arm_gicv3_kvm.c:495:13: warning: this statement may fall through [-Wimplicit-fallthrough=] kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, ®64, true); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ hw/intc/arm_gicv3_kvm.c:496:9: note: here case 6: ^~~~ hw/intc/arm_gicv3_kvm.c:498:13: warning: this statement may fall through [-Wimplicit-fallthrough=] kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, ®64, true); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ hw/intc/arm_gicv3_kvm.c:499:9: note: here default: ^~~~~~~ hw/intc/arm_gicv3_kvm.c: In function ‘kvm_arm_gicv3_get’: hw/intc/arm_gicv3_kvm.c:634:37: warning: this statement may fall through [-Wimplicit-fallthrough=] c->icc_apr[GICV3_G0][2] = reg64; ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ hw/intc/arm_gicv3_kvm.c:635:9: note: here case 6: ^~~~ hw/intc/arm_gicv3_kvm.c:637:37: warning: this statement may fall through [-Wimplicit-fallthrough=] c->icc_apr[GICV3_G0][1] = reg64; ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ hw/intc/arm_gicv3_kvm.c:638:9: note: here default: ^~~~~~~ hw/intc/arm_gicv3_kvm.c:648:39: warning: this statement may fall through [-Wimplicit-fallthrough=] c->icc_apr[GICV3_G1NS][2] = reg64; ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ hw/intc/arm_gicv3_kvm.c:649:9: note: here case 6: ^~~~ hw/intc/arm_gicv3_kvm.c:651:39: warning: this statement may fall through [-Wimplicit-fallthrough=] c->icc_apr[GICV3_G1NS][1] = reg64; ~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~ hw/intc/arm_gicv3_kvm.c:652:9: note: here default: ^~~~~~~ Reported-by: Euler Robot Signed-off-by: Chen Qun Signed-off-by: Thomas Huth Reviewed-by: Peter Maydell Message-Id: <20201211152426.350966-7-thuth@redhat.com> Signed-off-by: Thomas Huth --- hw/intc/arm_gicv3_kvm.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index 187eb054e0..d040a5d1e9 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -478,9 +478,11 @@ static void kvm_arm_gicv3_put(GICv3State *s) kvm_gicc_access(s, ICC_AP0R_EL1(3), ncpu, ®64, true); reg64 = c->icc_apr[GICV3_G0][2]; kvm_gicc_access(s, ICC_AP0R_EL1(2), ncpu, ®64, true); + /* fall through */ case 6: reg64 = c->icc_apr[GICV3_G0][1]; kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, ®64, true); + /* fall through */ default: reg64 = c->icc_apr[GICV3_G0][0]; kvm_gicc_access(s, ICC_AP0R_EL1(0), ncpu, ®64, true); @@ -492,9 +494,11 @@ static void kvm_arm_gicv3_put(GICv3State *s) kvm_gicc_access(s, ICC_AP1R_EL1(3), ncpu, ®64, true); reg64 = c->icc_apr[GICV3_G1NS][2]; kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, ®64, true); + /* fall through */ case 6: reg64 = c->icc_apr[GICV3_G1NS][1]; kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, ®64, true); + /* fall through */ default: reg64 = c->icc_apr[GICV3_G1NS][0]; kvm_gicc_access(s, ICC_AP1R_EL1(0), ncpu, ®64, true); @@ -631,9 +635,11 @@ static void kvm_arm_gicv3_get(GICv3State *s) c->icc_apr[GICV3_G0][3] = reg64; kvm_gicc_access(s, ICC_AP0R_EL1(2), ncpu, ®64, false); c->icc_apr[GICV3_G0][2] = reg64; + /* fall through */ case 6: kvm_gicc_access(s, ICC_AP0R_EL1(1), ncpu, ®64, false); c->icc_apr[GICV3_G0][1] = reg64; + /* fall through */ default: kvm_gicc_access(s, ICC_AP0R_EL1(0), ncpu, ®64, false); c->icc_apr[GICV3_G0][0] = reg64; @@ -645,9 +651,11 @@ static void kvm_arm_gicv3_get(GICv3State *s) c->icc_apr[GICV3_G1NS][3] = reg64; kvm_gicc_access(s, ICC_AP1R_EL1(2), ncpu, ®64, false); c->icc_apr[GICV3_G1NS][2] = reg64; + /* fall through */ case 6: kvm_gicc_access(s, ICC_AP1R_EL1(1), ncpu, ®64, false); c->icc_apr[GICV3_G1NS][1] = reg64; + /* fall through */ default: kvm_gicc_access(s, ICC_AP1R_EL1(0), ncpu, ®64, false); c->icc_apr[GICV3_G1NS][0] = reg64; From f190bf05f83c378958da84fe85c4df91c3941ce8 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 11 Dec 2020 16:24:21 +0100 Subject: [PATCH 07/14] accel/tcg/user-exec: silence the compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: Mark the cpu_exit_tb_from_sighandler() function with QEMU_NORETURN to fix it. Reported-by: Euler Robot Signed-off-by: Chen Qun Signed-off-by: Thomas Huth Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Reviewed-by: Alex Bennée Message-Id: <20201211152426.350966-8-thuth@redhat.com> Signed-off-by: Thomas Huth --- accel/tcg/user-exec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index 4ebe25461a..293ee86ea4 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -49,7 +49,8 @@ __thread uintptr_t helper_retaddr; /* exit the current TB from a signal handler. The host registers are restored in a state compatible with the CPU emulator */ -static void cpu_exit_tb_from_sighandler(CPUState *cpu, sigset_t *old_set) +static void QEMU_NORETURN cpu_exit_tb_from_sighandler(CPUState *cpu, + sigset_t *old_set) { /* XXX: use siglongjmp ? */ sigprocmask(SIG_SETMASK, old_set, NULL); From fc0cd867819a5cff9dd49b2fca4d927d765aa7ae Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 11 Dec 2020 16:24:22 +0100 Subject: [PATCH 08/14] target/sparc/translate: silence the compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning: target/sparc/translate.c: In function ‘gen_st_asi’: target/sparc/translate.c:2320:12: warning: this statement may fall through [-Wimplicit-fallthrough=] 2320 | if (!(dc->def->features & CPU_FEATURE_HYPV)) { | ^ target/sparc/translate.c:2329:5: note: here 2329 | case GET_ASI_DIRECT: | ^~~~ The "fall through" statement place is not correctly identified by the compiler. Reported-by: Euler Robot Signed-off-by: Chen Qun Signed-off-by: Thomas Huth Reviewed-by: Artyom Tarasenko Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20201211152426.350966-9-thuth@redhat.com> Signed-off-by: Thomas Huth --- target/sparc/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/sparc/translate.c b/target/sparc/translate.c index 30c73f8d2e..4bfa3179f8 100644 --- a/target/sparc/translate.c +++ b/target/sparc/translate.c @@ -2324,8 +2324,8 @@ static void gen_st_asi(DisasContext *dc, TCGv src, TCGv addr, } /* in OpenSPARC T1+ CPUs TWINX ASIs in store instructions * are ST_BLKINIT_ ASIs */ - /* fall through */ #endif + /* fall through */ case GET_ASI_DIRECT: gen_address_mask(dc, addr); tcg_gen_qemu_st_tl(src, addr, da.mem_idx, da.memop); From 9cf5a9cf60a3c5271dee05b6a81a05fb45bee622 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 11 Dec 2020 16:24:23 +0100 Subject: [PATCH 09/14] target/sparc/win_helper: silence the compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using -Wimplicit-fallthrough in our CFLAGS, the compiler showed warning: target/sparc/win_helper.c: In function ‘get_gregset’: target/sparc/win_helper.c:304:9: warning: this statement may fall through [-Wimplicit-fallthrough=] 304 | trace_win_helper_gregset_error(pstate); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ target/sparc/win_helper.c:306:5: note: here 306 | case 0: | ^~~~ Add the corresponding "fall through" comment to fix it. Reported-by: Euler Robot Signed-off-by: Chen Qun Signed-off-by: Thomas Huth Reviewed-by: Artyom Tarasenko Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20201211152426.350966-10-thuth@redhat.com> Signed-off-by: Thomas Huth --- target/sparc/win_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/sparc/win_helper.c b/target/sparc/win_helper.c index 5b57892a10..3a7c0ff943 100644 --- a/target/sparc/win_helper.c +++ b/target/sparc/win_helper.c @@ -302,7 +302,7 @@ static inline uint64_t *get_gregset(CPUSPARCState *env, uint32_t pstate) switch (pstate) { default: trace_win_helper_gregset_error(pstate); - /* pass through to normal set of global registers */ + /* fall through to normal set of global registers */ case 0: return env->bgregs; case PS_AG: From d84568b773fe1fc469c4d8419c3545be52eec82c Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 11 Dec 2020 16:24:24 +0100 Subject: [PATCH 10/14] tcg/optimize: Add fallthrough annotations To be able to compile this file with -Werror=implicit-fallthrough, we need to add some fallthrough annotations to the case statements that might fall through. Unfortunately, the typical "/* fallthrough */" comments do not work here as expected since some case labels are wrapped in macros and the compiler fails to match the comments in this case. But using __attribute__((fallthrough)) seems to work fine, so let's use that instead (by introducing a new QEMU_FALLTHROUGH macro in our compiler.h header file). Signed-off-by: Thomas Huth Reviewed-by: Richard Henderson Message-Id: <20201211152426.350966-11-thuth@redhat.com> Signed-off-by: Thomas Huth --- include/qemu/compiler.h | 11 +++++++++++ tcg/optimize.c | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index 1b9e58e82b..df9ec08f8a 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -222,4 +222,15 @@ extern void QEMU_NORETURN QEMU_ERROR("code path is reachable") #define qemu_build_not_reached() g_assert_not_reached() #endif +/** + * In most cases, normal "fallthrough" comments are good enough for + * switch-case statements, but sometimes the compiler has problems + * with those. In that case you can use QEMU_FALLTHROUGH instead. + */ +#if __has_attribute(fallthrough) +# define QEMU_FALLTHROUGH __attribute__((fallthrough)) +#else +# define QEMU_FALLTHROUGH do {} while (0) /* fallthrough */ +#endif + #endif /* COMPILER_H */ diff --git a/tcg/optimize.c b/tcg/optimize.c index 220f4601d5..7ca71de956 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -855,6 +855,7 @@ void tcg_optimize(TCGContext *s) if ((arg_info(op->args[1])->mask & 0x80) != 0) { break; } + QEMU_FALLTHROUGH; CASE_OP_32_64(ext8u): mask = 0xff; goto and_const; @@ -862,6 +863,7 @@ void tcg_optimize(TCGContext *s) if ((arg_info(op->args[1])->mask & 0x8000) != 0) { break; } + QEMU_FALLTHROUGH; CASE_OP_32_64(ext16u): mask = 0xffff; goto and_const; @@ -869,6 +871,7 @@ void tcg_optimize(TCGContext *s) if ((arg_info(op->args[1])->mask & 0x80000000) != 0) { break; } + QEMU_FALLTHROUGH; case INDEX_op_ext32u_i64: mask = 0xffffffffU; goto and_const; @@ -886,6 +889,7 @@ void tcg_optimize(TCGContext *s) if ((arg_info(op->args[1])->mask & 0x80000000) != 0) { break; } + QEMU_FALLTHROUGH; case INDEX_op_extu_i32_i64: /* We do not compute affected as it is a size changing op. */ mask = (uint32_t)arg_info(op->args[1])->mask; From 4f07e71bad905acc40771b81759ff10850325d99 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 11 Dec 2020 16:24:25 +0100 Subject: [PATCH 11/14] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests The softfloat tests are external repositories, so we do not care about implicit fallthrough warnings in this code. Signed-off-by: Thomas Huth Reviewed-by: Richard Henderson Reviewed-by: Chen Qun Message-Id: <20201211152426.350966-12-thuth@redhat.com> Signed-off-by: Thomas Huth --- tests/fp/meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/fp/meson.build b/tests/fp/meson.build index 3d4fb00f9d..8d739c4d59 100644 --- a/tests/fp/meson.build +++ b/tests/fp/meson.build @@ -27,6 +27,7 @@ tfdir = 'berkeley-testfloat-3/source' sfinc = include_directories(sfdir / 'include', sfspedir) tfcflags = [ + '-Wno-implicit-fallthrough', '-Wno-strict-prototypes', '-Wno-unknown-pragmas', '-Wno-uninitialized', @@ -209,6 +210,7 @@ libtestfloat = static_library( ) sfcflags = [ + '-Wno-implicit-fallthrough', '-Wno-missing-prototypes', '-Wno-redundant-decls', '-Wno-return-type', From 484bed05748d572a851e08412e4fb6bca8814342 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 17 Dec 2020 13:57:24 +0100 Subject: [PATCH 12/14] bsd-user: Silence warnings about missing fallthrough statement When compiling with -Werror=implicit-fallthrough, the compiler complains about a missing fallthrough annotation in this file. Looking at the code, the fallthrough is indeed wanted here, so let's add a proper comment. Message-Id: <20201217154138.1547274-1-thuth@redhat.com> Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- bsd-user/main.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bsd-user/main.c b/bsd-user/main.c index 0a918e8f74..9c700c6234 100644 --- a/bsd-user/main.c +++ b/bsd-user/main.c @@ -512,6 +512,7 @@ void cpu_loop(CPUSPARCState *env) case 0x141: if (bsd_type != target_freebsd) goto badtrap; + /* fallthrough */ case 0x100: #endif syscall_nr = env->gregs[1]; From 61e21b05de63e044dcc24b37ecec7514e326f62c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Fri, 11 Dec 2020 16:46:05 +0100 Subject: [PATCH 13/14] hw/rtc/twl92230: Add missing 'break' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add missing 'break' to fix: hw/rtc/twl92230.c: In function ‘menelaus_write’: hw/rtc/twl92230.c:713:5: error: label at end of compound statement 713 | default: | ^~~~~~~ Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Reviewed-by: Peter Maydell Message-Id: <20201211154605.511714-1-f4bug@amsat.org> Signed-off-by: Thomas Huth --- hw/rtc/twl92230.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/rtc/twl92230.c b/hw/rtc/twl92230.c index a787bd247d..006d75e174 100644 --- a/hw/rtc/twl92230.c +++ b/hw/rtc/twl92230.c @@ -697,6 +697,7 @@ static void menelaus_write(void *opaque, uint8_t addr, uint8_t value) #ifdef VERBOSE printf("%s: unknown register %02x\n", __func__, addr); #endif + break; } } From 0a2ebce92a3f10a89843e4a7a8e2f2eba4f7b109 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 11 Dec 2020 16:24:26 +0100 Subject: [PATCH 14/14] configure: Compile with -Wimplicit-fallthrough=2 Coverity always complains about switch-case statements that fall through the next one when there is no comment in between - which could indicate a forgotten "break" statement. Instead of handling these issues after they have been committed, it would be better to avoid them in the build process already. Thus let's enable the -Wimplicit-fallthrough warning now. The "=2" level seems to be a good compromise between being too strict and too generic about the possible comments, so we'll start with "=2" for now. Signed-off-by: Thomas Huth Reviewed-by: Chen Qun Reviewed-by: Peter Maydell Message-Id: <20201211152426.350966-13-thuth@redhat.com> Signed-off-by: Thomas Huth --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index c228f7c21e..881af4b6be 100755 --- a/configure +++ b/configure @@ -2023,6 +2023,7 @@ add_to warn_flags -Wempty-body add_to warn_flags -Wnested-externs add_to warn_flags -Wendif-labels add_to warn_flags -Wexpansion-to-defined +add_to warn_flags -Wimplicit-fallthrough=2 nowarn_flags= add_to nowarn_flags -Wno-initializer-overrides