* [PATCH 01/12] disas/libvixl: Fix fall-through annotation for GCC >= 7
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:35 ` Peter Maydell
2020-12-11 15:24 ` [PATCH 02/12] target/unicore32/translate: Add missing fallthrough annotations Thomas Huth
` (12 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
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 <thuth@redhat.com>
---
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
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 02/12] target/unicore32/translate: Add missing fallthrough annotations
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
2020-12-11 15:24 ` [PATCH 01/12] disas/libvixl: Fix fall-through annotation for GCC >= 7 Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:24 ` [PATCH 03/12] hw/rtc/twl92230: Silence warnings about missing fallthrough statements Thomas Huth
` (11 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
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.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
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;
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 03/12] hw/rtc/twl92230: Silence warnings about missing fallthrough statements
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
2020-12-11 15:24 ` [PATCH 01/12] disas/libvixl: Fix fall-through annotation for GCC >= 7 Thomas Huth
2020-12-11 15:24 ` [PATCH 02/12] target/unicore32/translate: Add missing fallthrough annotations Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:37 ` Peter Maydell
2020-12-11 15:24 ` [PATCH 04/12] hw/timer/renesas_tmr: silence the compiler warnings Thomas Huth
` (10 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
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 <thuth@redhat.com>
---
hw/rtc/twl92230.c | 43 +++++++++++++------------------------------
1 file changed, 13 insertions(+), 30 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_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_DCDC_CTRL1 ... MENELAUS_DCDC_CTRL3:
+ return s->dcdc[addr - MENELAUS_DCDC_CTRL1];
+
+ 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:
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 03/12] hw/rtc/twl92230: Silence warnings about missing fallthrough statements
2020-12-11 15:24 ` [PATCH 03/12] hw/rtc/twl92230: Silence warnings about missing fallthrough statements Thomas Huth
@ 2020-12-11 15:37 ` Peter Maydell
0 siblings, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2020-12-11 15:37 UTC (permalink / raw)
To: Thomas Huth; +Cc: Chen Qun, Richard Henderson, QEMU Developers, Paolo Bonzini
On Fri, 11 Dec 2020 at 15:24, Thomas Huth <thuth@redhat.com> wrote:
>
> 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 <thuth@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 04/12] hw/timer/renesas_tmr: silence the compiler warnings
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (2 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 03/12] hw/rtc/twl92230: Silence warnings about missing fallthrough statements Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:38 ` Peter Maydell
2020-12-11 15:24 ` [PATCH 05/12] target/i386: silence the compiler warnings in gen_shiftd_rm_T1 Thomas Huth
` (9 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
From: Chen Qun <kuhn.chenqun@huawei.com>
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 <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Message-Id: <20201028041819.2169003-10-kuhn.chenqun@huawei.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
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];
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 04/12] hw/timer/renesas_tmr: silence the compiler warnings
2020-12-11 15:24 ` [PATCH 04/12] hw/timer/renesas_tmr: silence the compiler warnings Thomas Huth
@ 2020-12-11 15:38 ` Peter Maydell
2020-12-11 15:46 ` Thomas Huth
0 siblings, 1 reply; 24+ messages in thread
From: Peter Maydell @ 2020-12-11 15:38 UTC (permalink / raw)
To: Thomas Huth; +Cc: Chen Qun, Richard Henderson, QEMU Developers, Paolo Bonzini
On Fri, 11 Dec 2020 at 15:24, Thomas Huth <thuth@redhat.com> wrote:
>
> From: Chen Qun <kuhn.chenqun@huawei.com>
>
> 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 <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> Message-Id: <20201028041819.2169003-10-kuhn.chenqun@huawei.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> 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];
Yes, but maybe we should just get the patch that
refactors this code in instead ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 04/12] hw/timer/renesas_tmr: silence the compiler warnings
2020-12-11 15:38 ` Peter Maydell
@ 2020-12-11 15:46 ` Thomas Huth
0 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:46 UTC (permalink / raw)
To: Peter Maydell
Cc: Chen Qun, Richard Henderson, QEMU Developers, Yoshinori Sato,
Paolo Bonzini
On 11/12/2020 16.38, Peter Maydell wrote:
> On Fri, 11 Dec 2020 at 15:24, Thomas Huth <thuth@redhat.com> wrote:
>>
>> From: Chen Qun <kuhn.chenqun@huawei.com>
>>
>> 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 <euler.robot@huawei.com>
>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>> Message-Id: <20201028041819.2169003-10-kuhn.chenqun@huawei.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> 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];
>
> Yes, but maybe we should just get the patch that
> refactors this code in instead ?
I think that patch had a bug in it:
https://lore.kernel.org/qemu-devel/28b04149-bbd9-12ed-0e40-3c3da9fee672@redhat.com/
So I was hoping that Yoshinori Sato would send a new version or at least a
reply with a clarification, but I've never seen a response... so for the
time being, I'd suggest to go with Chen Qun's patch instead, which certainly
does not hurt.
Thomas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 05/12] target/i386: silence the compiler warnings in gen_shiftd_rm_T1
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (3 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 04/12] hw/timer/renesas_tmr: silence the compiler warnings Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:24 ` [PATCH 06/12] hw/intc/arm_gicv3_kvm: silence the compiler warnings Thomas Huth
` (8 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
From: Chen Qun <kuhn.chenqun@huawei.com>
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 <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20201030004046.2191790-2-kuhn.chenqun@huawei.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
target/i386/translate.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 4c57307e42..dcf48f053f 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -1777,9 +1777,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) {
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 06/12] hw/intc/arm_gicv3_kvm: silence the compiler warnings
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (4 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 05/12] target/i386: silence the compiler warnings in gen_shiftd_rm_T1 Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:24 ` [PATCH 07/12] accel/tcg/user-exec: " Thomas Huth
` (7 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
From: Chen Qun <kuhn.chenqun@huawei.com>
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 <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201030004046.2191790-3-kuhn.chenqun@huawei.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
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;
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 07/12] accel/tcg/user-exec: silence the compiler warnings
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (5 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 06/12] hw/intc/arm_gicv3_kvm: silence the compiler warnings Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:24 ` [PATCH 08/12] target/sparc/translate: " Thomas Huth
` (6 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
From: Chen Qun <kuhn.chenqun@huawei.com>
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 <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20201030004046.2191790-4-kuhn.chenqun@huawei.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
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);
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 08/12] target/sparc/translate: silence the compiler warnings
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (6 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 07/12] accel/tcg/user-exec: " Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:24 ` [PATCH 09/12] target/sparc/win_helper: " Thomas Huth
` (5 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
From: Chen Qun <kuhn.chenqun@huawei.com>
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 <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201030004046.2191790-6-kuhn.chenqun@huawei.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
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);
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 09/12] target/sparc/win_helper: silence the compiler warnings
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (7 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 08/12] target/sparc/translate: " Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:24 ` [PATCH 10/12] tcg/optimize: Add fallthrough annotations Thomas Huth
` (4 subsequent siblings)
13 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
From: Chen Qun <kuhn.chenqun@huawei.com>
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 <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Message-Id: <20201030004046.2191790-7-kuhn.chenqun@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
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:
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 10/12] tcg/optimize: Add fallthrough annotations
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (8 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 09/12] target/sparc/win_helper: " Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:45 ` Richard Henderson
2020-12-11 15:24 ` [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests Thomas Huth
` (3 subsequent siblings)
13 siblings, 1 reply; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
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 <thuth@redhat.com>
---
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 c76281f354..d831a25da4 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -243,4 +243,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;
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 10/12] tcg/optimize: Add fallthrough annotations
2020-12-11 15:24 ` [PATCH 10/12] tcg/optimize: Add fallthrough annotations Thomas Huth
@ 2020-12-11 15:45 ` Richard Henderson
0 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2020-12-11 15:45 UTC (permalink / raw)
To: Thomas Huth, Peter Maydell, qemu-devel; +Cc: Chen Qun, Paolo Bonzini
On 12/11/20 9:24 AM, Thomas Huth wrote:
> 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 <thuth@redhat.com>
> ---
> include/qemu/compiler.h | 11 +++++++++++
> tcg/optimize.c | 4 ++++
> 2 files changed, 15 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (9 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 10/12] tcg/optimize: Add fallthrough annotations Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:46 ` Richard Henderson
2020-12-12 7:58 ` Chenqun (kuhn)
2020-12-11 15:24 ` [PATCH 12/12] configure: Compile with -Wimplicit-fallthrough=2 Thomas Huth
` (2 subsequent siblings)
13 siblings, 2 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
The softfloat tests are external repositories, so we do not care
about implicit fallthrough warnings in this code.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
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',
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests
2020-12-11 15:24 ` [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests Thomas Huth
@ 2020-12-11 15:46 ` Richard Henderson
2020-12-12 7:58 ` Chenqun (kuhn)
1 sibling, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2020-12-11 15:46 UTC (permalink / raw)
To: Thomas Huth, Peter Maydell, qemu-devel; +Cc: Chen Qun, Paolo Bonzini
On 12/11/20 9:24 AM, Thomas Huth wrote:
> The softfloat tests are external repositories, so we do not care
> about implicit fallthrough warnings in this code.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> tests/fp/meson.build | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests
2020-12-11 15:24 ` [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests Thomas Huth
2020-12-11 15:46 ` Richard Henderson
@ 2020-12-12 7:58 ` Chenqun (kuhn)
1 sibling, 0 replies; 24+ messages in thread
From: Chenqun (kuhn) @ 2020-12-12 7:58 UTC (permalink / raw)
To: Thomas Huth, Peter Maydell, qemu-devel@nongnu.org
Cc: Paolo Bonzini, Richard Henderson
> -----Original Message-----
> From: Thomas Huth [mailto:thuth@redhat.com]
> Sent: Friday, December 11, 2020 11:24 PM
> To: Peter Maydell <peter.maydell@linaro.org>; qemu-devel@nongnu.org
> Cc: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; Richard Henderson
> <richard.henderson@linaro.org>; Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 11/12] 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 <thuth@redhat.com>
> ---
Reviewed-by: Chen Qun <kuhn.chenqun@huawei.com>
The warnings are frequently generated for files in this directory.
This is a good solution for the warnings.
Thanks,
Chen Qun
> 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',
> --
> 2.27.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 12/12] configure: Compile with -Wimplicit-fallthrough=2
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (10 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests Thomas Huth
@ 2020-12-11 15:24 ` Thomas Huth
2020-12-11 15:42 ` Peter Maydell
2020-12-12 9:19 ` Chenqun (kuhn)
2020-12-11 15:55 ` [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
2020-12-11 17:06 ` no-reply
13 siblings, 2 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:24 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
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.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
configure | 1 +
1 file changed, 1 insertion(+)
diff --git a/configure b/configure
index 18c26e0389..dc2bc3c2f0 100755
--- a/configure
+++ b/configure
@@ -2007,6 +2007,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
--
2.27.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 12/12] configure: Compile with -Wimplicit-fallthrough=2
2020-12-11 15:24 ` [PATCH 12/12] configure: Compile with -Wimplicit-fallthrough=2 Thomas Huth
@ 2020-12-11 15:42 ` Peter Maydell
2020-12-12 9:19 ` Chenqun (kuhn)
1 sibling, 0 replies; 24+ messages in thread
From: Peter Maydell @ 2020-12-11 15:42 UTC (permalink / raw)
To: Thomas Huth; +Cc: Chen Qun, Richard Henderson, QEMU Developers, Paolo Bonzini
On Fri, 11 Dec 2020 at 15:24, Thomas Huth <thuth@redhat.com> wrote:
>
> 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.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> configure | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/configure b/configure
> index 18c26e0389..dc2bc3c2f0 100755
> --- a/configure
> +++ b/configure
> @@ -2007,6 +2007,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
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Maybe mention in the commit message why =2 is our preference ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 12/12] configure: Compile with -Wimplicit-fallthrough=2
2020-12-11 15:24 ` [PATCH 12/12] configure: Compile with -Wimplicit-fallthrough=2 Thomas Huth
2020-12-11 15:42 ` Peter Maydell
@ 2020-12-12 9:19 ` Chenqun (kuhn)
1 sibling, 0 replies; 24+ messages in thread
From: Chenqun (kuhn) @ 2020-12-12 9:19 UTC (permalink / raw)
To: Thomas Huth, Peter Maydell, qemu-devel@nongnu.org
Cc: Paolo Bonzini, Richard Henderson
> -----Original Message-----
> From: Thomas Huth [mailto:thuth@redhat.com]
> Sent: Friday, December 11, 2020 11:24 PM
> To: Peter Maydell <peter.maydell@linaro.org>; qemu-devel@nongnu.org
> Cc: Chenqun (kuhn) <kuhn.chenqun@huawei.com>; Richard Henderson
> <richard.henderson@linaro.org>; Paolo Bonzini <pbonzini@redhat.com>
> Subject: [PATCH 12/12] 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.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Chen Qun <kuhn.chenqun@huawei.com>
Good job, we'll never see such warnings again.
Thanks,
Chen Qun
> ---
> configure | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/configure b/configure
> index 18c26e0389..dc2bc3c2f0 100755
> --- a/configure
> +++ b/configure
> @@ -2007,6 +2007,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
> --
> 2.27.0
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (11 preceding siblings ...)
2020-12-11 15:24 ` [PATCH 12/12] configure: Compile with -Wimplicit-fallthrough=2 Thomas Huth
@ 2020-12-11 15:55 ` Thomas Huth
2020-12-11 17:06 ` no-reply
13 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-12-11 15:55 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Richard Henderson, Paolo Bonzini
On 11/12/2020 16.24, Thomas Huth wrote:
> Coverity is already reporting switch-case statements where code
> can fall through from one case to another without a proper comment
> (since this could indicate a missing "break" and thus a bug).
> However, it's cumbersome to fix these issues after they have been
> merged already, it would be better if the author of the code would
> already take care of this when writing the patch. Fortunately,
> GCC and Clang can already warn about those code spots, too.
> So let's fix our remaining statements that fall through without
> a proper comment, so we can finally turn on -Wimplicit-fallthrough
> for all compilation runs.
>
> Chen Qun (6):
> hw/timer/renesas_tmr: silence the compiler warnings
> target/i386: silence the compiler warnings in gen_shiftd_rm_T1
> hw/intc/arm_gicv3_kvm: silence the compiler warnings
> accel/tcg/user-exec: silence the compiler warnings
> target/sparc/translate: silence the compiler warnings
> target/sparc/win_helper: silence the compiler warnings
>
> Thomas Huth (6):
> disas/libvixl: Fix fall-through annotation for GCC >= 7
> target/unicore32/translate: Add missing fallthrough annotations
> hw/rtc/twl92230: Silence warnings about missing fallthrough statements
> tcg/optimize: Add fallthrough annotations
> tests/fp: Do not emit implicit-fallthrough warnings in the softfloat
> tests
> configure: Compile with -Wimplicit-fallthrough=2
I forgot to mention: There are two more spots in the ppc code that currently
cause some warnings, but the patches to fix those are already in David
Gibson's ppc pull request, so they should be handled soon.
Thomas
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough
2020-12-11 15:24 [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (12 preceding siblings ...)
2020-12-11 15:55 ` [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
@ 2020-12-11 17:06 ` no-reply
13 siblings, 0 replies; 24+ messages in thread
From: no-reply @ 2020-12-11 17:06 UTC (permalink / raw)
To: thuth; +Cc: peter.maydell, pbonzini, richard.henderson, qemu-devel,
kuhn.chenqun
Patchew URL: https://patchew.org/QEMU/20201211152426.350966-1-thuth@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20201211152426.350966-1-thuth@redhat.com
Subject: [PATCH 00/12] Compile QEMU with -Wimplicit-fallthrough
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20201204220758.2879-1-vsementsov@virtuozzo.com -> patchew/20201204220758.2879-1-vsementsov@virtuozzo.com
* [new tag] patchew/20201211152426.350966-1-thuth@redhat.com -> patchew/20201211152426.350966-1-thuth@redhat.com
Switched to a new branch 'test'
2daf9e7 configure: Compile with -Wimplicit-fallthrough=2
bf473af tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests
e7b216b tcg/optimize: Add fallthrough annotations
c1d0232 target/sparc/win_helper: silence the compiler warnings
fc18091 target/sparc/translate: silence the compiler warnings
9610d75 accel/tcg/user-exec: silence the compiler warnings
580a2fc hw/intc/arm_gicv3_kvm: silence the compiler warnings
3f4ee3d target/i386: silence the compiler warnings in gen_shiftd_rm_T1
1343e8c hw/timer/renesas_tmr: silence the compiler warnings
198be4c hw/rtc/twl92230: Silence warnings about missing fallthrough statements
17c473a target/unicore32/translate: Add missing fallthrough annotations
697b282 disas/libvixl: Fix fall-through annotation for GCC >= 7
=== OUTPUT BEGIN ===
1/12 Checking commit 697b282c9a19 (disas/libvixl: Fix fall-through annotation for GCC >= 7)
ERROR: do not use C99 // comments
#46: FILE: disas/libvixl/vixl/globals.h:111:
+// Fallthrough annotation for Clang and C++11(201103L).
ERROR: do not use C99 // comments
#49: FILE: disas/libvixl/vixl/globals.h:114:
+// Fallthrough annotation for GCC >= 7.
total: 2 errors, 0 warnings, 24 lines checked
Patch 1/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/12 Checking commit 17c473af89a3 (target/unicore32/translate: Add missing fallthrough annotations)
3/12 Checking commit 198be4c4dd79 (hw/rtc/twl92230: Silence warnings about missing fallthrough statements)
4/12 Checking commit 1343e8c24457 (hw/timer/renesas_tmr: silence the compiler warnings)
5/12 Checking commit 3f4ee3deca5a (target/i386: silence the compiler warnings in gen_shiftd_rm_T1)
6/12 Checking commit 580a2fc632d5 (hw/intc/arm_gicv3_kvm: silence the compiler warnings)
7/12 Checking commit 9610d7588b9c (accel/tcg/user-exec: silence the compiler warnings)
8/12 Checking commit fc1809134418 (target/sparc/translate: silence the compiler warnings)
9/12 Checking commit c1d0232b4185 (target/sparc/win_helper: silence the compiler warnings)
10/12 Checking commit e7b216b31c4e (tcg/optimize: Add fallthrough annotations)
WARNING: architecture specific defines should be avoided
#32: FILE: include/qemu/compiler.h:251:
+#if __has_attribute(fallthrough)
total: 0 errors, 1 warnings, 43 lines checked
Patch 10/12 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/12 Checking commit bf473af503d5 (tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests)
12/12 Checking commit 2daf9e78b889 (configure: Compile with -Wimplicit-fallthrough=2)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20201211152426.350966-1-thuth@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 24+ messages in thread