* [PULL 01/12] disas/libvixl: Fix fall-through annotation for GCC >= 7
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 02/12] target/unicore32/translate: Add missing fallthrough annotations Thomas Huth
` (12 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201211152426.350966-2-thuth@redhat.com>
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] 22+ messages in thread* [PULL 02/12] target/unicore32/translate: Add missing fallthrough annotations
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
2020-12-16 17:29 ` [PULL 01/12] disas/libvixl: Fix fall-through annotation for GCC >= 7 Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 03/12] hw/rtc/twl92230: Silence warnings about missing fallthrough statements Thomas Huth
` (11 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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 <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201211152426.350966-3-thuth@redhat.com>
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] 22+ messages in thread* [PULL 03/12] hw/rtc/twl92230: Silence warnings about missing fallthrough statements
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
2020-12-16 17:29 ` [PULL 01/12] disas/libvixl: Fix fall-through annotation for GCC >= 7 Thomas Huth
2020-12-16 17:29 ` [PULL 02/12] target/unicore32/translate: Add missing fallthrough annotations Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 04/12] hw/timer/renesas_tmr: silence the compiler warnings Thomas Huth
` (10 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Message-Id: <20201211152426.350966-4-thuth@redhat.com>
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] 22+ messages in thread* [PULL 04/12] hw/timer/renesas_tmr: silence the compiler warnings
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (2 preceding siblings ...)
2020-12-16 17:29 ` [PULL 03/12] hw/rtc/twl92230: Silence warnings about missing fallthrough statements Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 05/12] target/i386: silence the compiler warnings in gen_shiftd_rm_T1 Thomas Huth
` (9 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20201211152426.350966-5-thuth@redhat.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] 22+ messages in thread* [PULL 05/12] target/i386: silence the compiler warnings in gen_shiftd_rm_T1
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (3 preceding siblings ...)
2020-12-16 17:29 ` [PULL 04/12] hw/timer/renesas_tmr: silence the compiler warnings Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 06/12] hw/intc/arm_gicv3_kvm: silence the compiler warnings Thomas Huth
` (8 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20201211152426.350966-6-thuth@redhat.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 e8f5f5803a..25fec8a150 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] 22+ messages in thread* [PULL 06/12] hw/intc/arm_gicv3_kvm: silence the compiler warnings
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (4 preceding siblings ...)
2020-12-16 17:29 ` [PULL 05/12] target/i386: silence the compiler warnings in gen_shiftd_rm_T1 Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 07/12] accel/tcg/user-exec: " Thomas Huth
` (7 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201211152426.350966-7-thuth@redhat.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] 22+ messages in thread* [PULL 07/12] accel/tcg/user-exec: silence the compiler warnings
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (5 preceding siblings ...)
2020-12-16 17:29 ` [PULL 06/12] hw/intc/arm_gicv3_kvm: silence the compiler warnings Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 08/12] target/sparc/translate: " Thomas Huth
` (6 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20201211152426.350966-8-thuth@redhat.com>
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] 22+ messages in thread* [PULL 08/12] target/sparc/translate: silence the compiler warnings
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (6 preceding siblings ...)
2020-12-16 17:29 ` [PULL 07/12] accel/tcg/user-exec: " Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 09/12] target/sparc/win_helper: " Thomas Huth
` (5 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201211152426.350966-9-thuth@redhat.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] 22+ messages in thread* [PULL 09/12] target/sparc/win_helper: silence the compiler warnings
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (7 preceding siblings ...)
2020-12-16 17:29 ` [PULL 08/12] target/sparc/translate: " Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 10/12] tcg/optimize: Add fallthrough annotations Thomas Huth
` (4 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20201211152426.350966-10-thuth@redhat.com>
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] 22+ messages in thread* [PULL 10/12] tcg/optimize: Add fallthrough annotations
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (8 preceding siblings ...)
2020-12-16 17:29 ` [PULL 09/12] target/sparc/win_helper: " Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests Thomas Huth
` (3 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201211152426.350966-11-thuth@redhat.com>
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 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;
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PULL 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (9 preceding siblings ...)
2020-12-16 17:29 ` [PULL 10/12] tcg/optimize: Add fallthrough annotations Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 17:29 ` [PULL 12/12] configure: Compile with -Wimplicit-fallthrough=2 Thomas Huth
` (2 subsequent siblings)
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Chen Qun <kuhn.chenqun@huawei.com>
Message-Id: <20201211152426.350966-12-thuth@redhat.com>
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] 22+ messages in thread* [PULL 12/12] configure: Compile with -Wimplicit-fallthrough=2
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (10 preceding siblings ...)
2020-12-16 17:29 ` [PULL 11/12] tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests Thomas Huth
@ 2020-12-16 17:29 ` Thomas Huth
2020-12-16 23:01 ` [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough no-reply
2020-12-17 12:51 ` Peter Maydell
13 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-16 17:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel; +Cc: Chen Qun, Alex Bennée
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 <thuth@redhat.com>
Reviewed-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20201211152426.350966-13-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
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
--
2.27.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (11 preceding siblings ...)
2020-12-16 17:29 ` [PULL 12/12] configure: Compile with -Wimplicit-fallthrough=2 Thomas Huth
@ 2020-12-16 23:01 ` no-reply
2020-12-17 6:09 ` Thomas Huth
2020-12-17 12:51 ` Peter Maydell
13 siblings, 1 reply; 22+ messages in thread
From: no-reply @ 2020-12-16 23:01 UTC (permalink / raw)
To: thuth; +Cc: peter.maydell, alex.bennee, qemu-devel, kuhn.chenqun
Patchew URL: https://patchew.org/QEMU/20201216172949.57380-1-thuth@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20201216172949.57380-1-thuth@redhat.com
Subject: [PULL 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
* [new tag] patchew/20201216172949.57380-1-thuth@redhat.com -> patchew/20201216172949.57380-1-thuth@redhat.com
Switched to a new branch 'test'
7bedbc8 configure: Compile with -Wimplicit-fallthrough=2
e14bb9d tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests
ebd3c45 tcg/optimize: Add fallthrough annotations
cfe5662 target/sparc/win_helper: silence the compiler warnings
8ef9335 target/sparc/translate: silence the compiler warnings
4588bf9 accel/tcg/user-exec: silence the compiler warnings
be2108e hw/intc/arm_gicv3_kvm: silence the compiler warnings
7d033d0 target/i386: silence the compiler warnings in gen_shiftd_rm_T1
284b00a hw/timer/renesas_tmr: silence the compiler warnings
c3d2957 hw/rtc/twl92230: Silence warnings about missing fallthrough statements
1b1609c target/unicore32/translate: Add missing fallthrough annotations
99bc0f0 disas/libvixl: Fix fall-through annotation for GCC >= 7
=== OUTPUT BEGIN ===
1/12 Checking commit 99bc0f0e92b7 (disas/libvixl: Fix fall-through annotation for GCC >= 7)
ERROR: do not use C99 // comments
#49: FILE: disas/libvixl/vixl/globals.h:111:
+// Fallthrough annotation for Clang and C++11(201103L).
ERROR: do not use C99 // comments
#52: 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 1b1609c7573a (target/unicore32/translate: Add missing fallthrough annotations)
3/12 Checking commit c3d2957383b8 (hw/rtc/twl92230: Silence warnings about missing fallthrough statements)
4/12 Checking commit 284b00aef566 (hw/timer/renesas_tmr: silence the compiler warnings)
5/12 Checking commit 7d033d02b90d (target/i386: silence the compiler warnings in gen_shiftd_rm_T1)
6/12 Checking commit be2108e641c9 (hw/intc/arm_gicv3_kvm: silence the compiler warnings)
7/12 Checking commit 4588bf97482b (accel/tcg/user-exec: silence the compiler warnings)
8/12 Checking commit 8ef9335f2838 (target/sparc/translate: silence the compiler warnings)
9/12 Checking commit cfe56623ece8 (target/sparc/win_helper: silence the compiler warnings)
10/12 Checking commit ebd3c45fd052 (tcg/optimize: Add fallthrough annotations)
WARNING: architecture specific defines should be avoided
#35: FILE: include/qemu/compiler.h:230:
+#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 e14bb9ddd6f3 (tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests)
12/12 Checking commit 7bedbc83bcfa (configure: Compile with -Wimplicit-fallthrough=2)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20201216172949.57380-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] 22+ messages in thread* Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough
2020-12-16 23:01 ` [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough no-reply
@ 2020-12-17 6:09 ` Thomas Huth
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Huth @ 2020-12-17 6:09 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee, kuhn.chenqun
On 17/12/2020 00.01, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20201216172949.57380-1-thuth@redhat.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 20201216172949.57380-1-thuth@redhat.com
> Subject: [PULL 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
> * [new tag] patchew/20201216172949.57380-1-thuth@redhat.com -> patchew/20201216172949.57380-1-thuth@redhat.com
> Switched to a new branch 'test'
> 7bedbc8 configure: Compile with -Wimplicit-fallthrough=2
> e14bb9d tests/fp: Do not emit implicit-fallthrough warnings in the softfloat tests
> ebd3c45 tcg/optimize: Add fallthrough annotations
> cfe5662 target/sparc/win_helper: silence the compiler warnings
> 8ef9335 target/sparc/translate: silence the compiler warnings
> 4588bf9 accel/tcg/user-exec: silence the compiler warnings
> be2108e hw/intc/arm_gicv3_kvm: silence the compiler warnings
> 7d033d0 target/i386: silence the compiler warnings in gen_shiftd_rm_T1
> 284b00a hw/timer/renesas_tmr: silence the compiler warnings
> c3d2957 hw/rtc/twl92230: Silence warnings about missing fallthrough statements
> 1b1609c target/unicore32/translate: Add missing fallthrough annotations
> 99bc0f0 disas/libvixl: Fix fall-through annotation for GCC >= 7
>
> === OUTPUT BEGIN ===
> 1/12 Checking commit 99bc0f0e92b7 (disas/libvixl: Fix fall-through annotation for GCC >= 7)
> ERROR: do not use C99 // comments
> #49: FILE: disas/libvixl/vixl/globals.h:111:
> +// Fallthrough annotation for Clang and C++11(201103L).
>
> ERROR: do not use C99 // comments
> #52: FILE: disas/libvixl/vixl/globals.h:114:
> +// Fallthrough annotation for GCC >= 7.
Well, libvixl is C++ code and the upstream patch used these comments, too,
so this error can be ignored.
> 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 1b1609c7573a (target/unicore32/translate: Add missing fallthrough annotations)
> 3/12 Checking commit c3d2957383b8 (hw/rtc/twl92230: Silence warnings about missing fallthrough statements)
> 4/12 Checking commit 284b00aef566 (hw/timer/renesas_tmr: silence the compiler warnings)
> 5/12 Checking commit 7d033d02b90d (target/i386: silence the compiler warnings in gen_shiftd_rm_T1)
> 6/12 Checking commit be2108e641c9 (hw/intc/arm_gicv3_kvm: silence the compiler warnings)
> 7/12 Checking commit 4588bf97482b (accel/tcg/user-exec: silence the compiler warnings)
> 8/12 Checking commit 8ef9335f2838 (target/sparc/translate: silence the compiler warnings)
> 9/12 Checking commit cfe56623ece8 (target/sparc/win_helper: silence the compiler warnings)
> 10/12 Checking commit ebd3c45fd052 (tcg/optimize: Add fallthrough annotations)
> WARNING: architecture specific defines should be avoided
> #35: FILE: include/qemu/compiler.h:230:
> +#if __has_attribute(fallthrough)
Maybe we should teach checkpatch.pl to allow these in compiler.h?
Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough
2020-12-16 17:29 [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough Thomas Huth
` (12 preceding siblings ...)
2020-12-16 23:01 ` [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough no-reply
@ 2020-12-17 12:51 ` Peter Maydell
2020-12-17 13:03 ` Thomas Huth
13 siblings, 1 reply; 22+ messages in thread
From: Peter Maydell @ 2020-12-17 12:51 UTC (permalink / raw)
To: Thomas Huth; +Cc: Chen Qun, Alex Bennée, QEMU Developers
On Wed, 16 Dec 2020 at 17:29, Thomas Huth <thuth@redhat.com> wrote:
>
> Hi!
>
> The following changes since commit af3f37319cb1e1ca0c42842ecdbd1bcfc64a4b6f:
>
> Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-12-15 21:24:31 +0000)
>
> are available in the Git repository at:
>
> https://gitlab.com/huth/qemu.git tags/pull-request-2020-12-16
>
> for you to fetch changes up to cbbedfeeb77e25b065f8a2b0c33e81403edaf728:
>
> configure: Compile with -Wimplicit-fallthrough=2 (2020-12-16 12:52:20 +0100)
>
> ----------------------------------------------------------------
> * Compile QEMU with -Wimplicit-fallthrough=2 to avoid bugs in
> switch-case statements
> ----------------------------------------------------------------
Hi; this generates a new warning on the NetBSD build:
../src/bsd-user/main.c: In function 'cpu_loop':
../src/bsd-user/main.c:513:16: warning: this statement may fall
through [-Wimplicit-fallthrough=]
if (bsd_type != target_freebsd)
^
../src/bsd-user/main.c:515:9: note: here
case 0x100:
^~~~
thanks
-- PMM
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough
2020-12-17 12:51 ` Peter Maydell
@ 2020-12-17 13:03 ` Thomas Huth
2020-12-17 14:00 ` Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough) Daniel P. Berrangé
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Huth @ 2020-12-17 13:03 UTC (permalink / raw)
To: Peter Maydell
Cc: Ed Maste, QEMU Developers, Kamil Rytarowski, Daniel P. Berrange,
Chen Qun, Alex Bennée, Li-Wen Hsu, Brad Smith
On 17/12/2020 13.51, Peter Maydell wrote:
> On Wed, 16 Dec 2020 at 17:29, Thomas Huth <thuth@redhat.com> wrote:
>>
>> Hi!
>>
>> The following changes since commit af3f37319cb1e1ca0c42842ecdbd1bcfc64a4b6f:
>>
>> Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-12-15 21:24:31 +0000)
>>
>> are available in the Git repository at:
>>
>> https://gitlab.com/huth/qemu.git tags/pull-request-2020-12-16
>>
>> for you to fetch changes up to cbbedfeeb77e25b065f8a2b0c33e81403edaf728:
>>
>> configure: Compile with -Wimplicit-fallthrough=2 (2020-12-16 12:52:20 +0100)
>>
>> ----------------------------------------------------------------
>> * Compile QEMU with -Wimplicit-fallthrough=2 to avoid bugs in
>> switch-case statements
>> ----------------------------------------------------------------
>
> Hi; this generates a new warning on the NetBSD build:
>
> ../src/bsd-user/main.c: In function 'cpu_loop':
> ../src/bsd-user/main.c:513:16: warning: this statement may fall
> through [-Wimplicit-fallthrough=]
> if (bsd_type != target_freebsd)
> ^
> ../src/bsd-user/main.c:515:9: note: here
> case 0x100:
> ^~~~
Oh man, can't we just ditch the bsd-user folder now? It's known to be broken
since many releases, so it's currently only causing additional effort to
keep this code compilable (also with regards to the automatic code scan tool
reports that we've seen during the past months), without real benefit. Even
if the BSD folks finally upstream their fixed version again, it's more
likely that they will start from scratch again instead of fixing the old
folder, I guess?
Thomas
^ permalink raw reply [flat|nested] 22+ messages in thread
* Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
2020-12-17 13:03 ` Thomas Huth
@ 2020-12-17 14:00 ` Daniel P. Berrangé
2020-12-17 16:03 ` Warner Losh
0 siblings, 1 reply; 22+ messages in thread
From: Daniel P. Berrangé @ 2020-12-17 14:00 UTC (permalink / raw)
To: Thomas Huth
Cc: Peter Maydell, Ed Maste, Sean Bruno, QEMU Developers,
Kamil Rytarowski, Chen Qun, Alex Bennée, Li-Wen Hsu,
Brad Smith
On Thu, Dec 17, 2020 at 02:03:47PM +0100, Thomas Huth wrote:
> On 17/12/2020 13.51, Peter Maydell wrote:
> > On Wed, 16 Dec 2020 at 17:29, Thomas Huth <thuth@redhat.com> wrote:
> >>
> >> Hi!
> >>
> >> The following changes since commit af3f37319cb1e1ca0c42842ecdbd1bcfc64a4b6f:
> >>
> >> Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-12-15 21:24:31 +0000)
> >>
> >> are available in the Git repository at:
> >>
> >> https://gitlab.com/huth/qemu.git tags/pull-request-2020-12-16
> >>
> >> for you to fetch changes up to cbbedfeeb77e25b065f8a2b0c33e81403edaf728:
> >>
> >> configure: Compile with -Wimplicit-fallthrough=2 (2020-12-16 12:52:20 +0100)
> >>
> >> ----------------------------------------------------------------
> >> * Compile QEMU with -Wimplicit-fallthrough=2 to avoid bugs in
> >> switch-case statements
> >> ----------------------------------------------------------------
> >
> > Hi; this generates a new warning on the NetBSD build:
> >
> > ../src/bsd-user/main.c: In function 'cpu_loop':
> > ../src/bsd-user/main.c:513:16: warning: this statement may fall
> > through [-Wimplicit-fallthrough=]
> > if (bsd_type != target_freebsd)
> > ^
> > ../src/bsd-user/main.c:515:9: note: here
> > case 0x100:
> > ^~~~
>
> Oh man, can't we just ditch the bsd-user folder now? It's known to be broken
> since many releases, so it's currently only causing additional effort to
> keep this code compilable (also with regards to the automatic code scan tool
> reports that we've seen during the past months), without real benefit. Even
> if the BSD folks finally upstream their fixed version again, it's more
> likely that they will start from scratch again instead of fixing the old
> folder, I guess?
Yeah, it has been a while since we last discussed this:
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00171.html
Meanwhile their out of free bsd-user impl continues to be developed
until Dec 2019 at least:
https://github.com/seanbruno/qemu-bsd-user/commits/bsd-user
I don't recall what happened after that initial discussion about
merging the new impl. Did Sean simply not have the time to invest
in the merge ? I'll CC him here to see what opinion he has on the
future of bsd-user in QEMU.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
2020-12-17 14:00 ` Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough) Daniel P. Berrangé
@ 2020-12-17 16:03 ` Warner Losh
2020-12-17 16:20 ` Peter Maydell
0 siblings, 1 reply; 22+ messages in thread
From: Warner Losh @ 2020-12-17 16:03 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Peter Maydell, Thomas Huth, Ed Maste, Sean Bruno,
QEMU Developers, Kamil Rytarowski, Chen Qun, Alex Bennée,
Li-Wen Hsu, Brad Smith
[-- Attachment #1: Type: text/plain, Size: 3974 bytes --]
On Thu, Dec 17, 2020 at 7:02 AM Daniel P. Berrangé <berrange@redhat.com>
wrote:
> On Thu, Dec 17, 2020 at 02:03:47PM +0100, Thomas Huth wrote:
> > On 17/12/2020 13.51, Peter Maydell wrote:
> > > On Wed, 16 Dec 2020 at 17:29, Thomas Huth <thuth@redhat.com> wrote:
> > >>
> > >> Hi!
> > >>
> > >> The following changes since commit
> af3f37319cb1e1ca0c42842ecdbd1bcfc64a4b6f:
> > >>
> > >> Merge remote-tracking branch
> 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-12-15
> 21:24:31 +0000)
> > >>
> > >> are available in the Git repository at:
> > >>
> > >> https://gitlab.com/huth/qemu.git tags/pull-request-2020-12-16
> > >>
> > >> for you to fetch changes up to
> cbbedfeeb77e25b065f8a2b0c33e81403edaf728:
> > >>
> > >> configure: Compile with -Wimplicit-fallthrough=2 (2020-12-16
> 12:52:20 +0100)
> > >>
> > >> ----------------------------------------------------------------
> > >> * Compile QEMU with -Wimplicit-fallthrough=2 to avoid bugs in
> > >> switch-case statements
> > >> ----------------------------------------------------------------
> > >
> > > Hi; this generates a new warning on the NetBSD build:
> > >
> > > ../src/bsd-user/main.c: In function 'cpu_loop':
> > > ../src/bsd-user/main.c:513:16: warning: this statement may fall
> > > through [-Wimplicit-fallthrough=]
> > > if (bsd_type != target_freebsd)
> > > ^
> > > ../src/bsd-user/main.c:515:9: note: here
> > > case 0x100:
> > > ^~~~
> >
> > Oh man, can't we just ditch the bsd-user folder now? It's known to be
> broken
> > since many releases, so it's currently only causing additional effort to
> > keep this code compilable (also with regards to the automatic code scan
> tool
> > reports that we've seen during the past months), without real benefit.
> Even
> > if the BSD folks finally upstream their fixed version again, it's more
> > likely that they will start from scratch again instead of fixing the old
> > folder, I guess?
>
> Yeah, it has been a while since we last discussed this:
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg00171.html
>
> Meanwhile their out of free bsd-user impl continues to be developed
> until Dec 2019 at least:
>
> https://github.com/seanbruno/qemu-bsd-user/commits/bsd-user
You should check out the bsd-user-rebase-3.1 branch. The most recent commit
was this month...
>
> I don't recall what happened after that initial discussion about
> merging the new impl. Did Sean simply not have the time to invest
> in the merge ? I'll CC him here to see what opinion he has on the
> future of bsd-user in QEMU.
>
I've actually taken over for Sean Bruno managing this.
I spent some time and rebased our extensive work up through QEMU 3.1, but I
hit some snags with changes in the underlying QEMU, and we had a few bugs
we had to sort out to make it stable, which took some time. There were
other people doing the sorting out since it affected them, but not me
directly. By the time they were sorted out, 4.1 was being released. Plus I
blew about a week trying to make every single commit compile rather than
just the tip of the branch. I spent quite a bit of time curating the
changes into more logical bits, which also chewed up a few weeks and in the
end was only partially complete. In the end,
The main problem with the merge, which I've done a couple of times, is that
this is so tied to the structure of QEMU, a part that has a high velocity
is that it takes so long to do the merge that by the time you're done,
things have changed again in the code base and you have to do a lot more
work to catch up.
The code that's in the tree is so bit-rotted I'd be surprised if it could
run anything beyond trivial programs.
I'd love to hear from people ways that I can speed things up. I'd love to
get this back into the tree since the version we have in the
Warner
[-- Attachment #2: Type: text/html, Size: 5387 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
2020-12-17 16:03 ` Warner Losh
@ 2020-12-17 16:20 ` Peter Maydell
2020-12-17 17:10 ` Warner Losh
0 siblings, 1 reply; 22+ messages in thread
From: Peter Maydell @ 2020-12-17 16:20 UTC (permalink / raw)
To: Warner Losh
Cc: Thomas Huth, Ed Maste, Sean Bruno, QEMU Developers,
Kamil Rytarowski, Chen Qun, Daniel P. Berrangé,
Alex Bennée, Li-Wen Hsu, Brad Smith
On Thu, 17 Dec 2020 at 16:03, Warner Losh <imp@bsdimp.com> wrote:
> On Thu, Dec 17, 2020 at 7:02 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>> I don't recall what happened after that initial discussion about
>> merging the new impl. Did Sean simply not have the time to invest
>> in the merge ? I'll CC him here to see what opinion he has on the
>> future of bsd-user in QEMU.
>
>
> I've actually taken over for Sean Bruno managing this.
> I'd love to hear from people ways that I can speed things up.
There was a bit of discussion about this on #qemu IRC the other
day, coincidentally. I think the conclusion we (upstream QEMU)
came to was that we'd be happy with a "delete all of bsd-user
and reinstate" approach, assuming that the "reinstate" part is
in reasonably logical chunks and not one big "here's what we
have all in one lump" patch.
AIUI from IRC this is being primarily driven by FreeBSD and
NetBSD/OpenBSD support is merely "we hope it is not broken
by the delete-and-reinstate but it was probably broken anyway" ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
2020-12-17 16:20 ` Peter Maydell
@ 2020-12-17 17:10 ` Warner Losh
2020-12-17 17:59 ` Warner Losh
0 siblings, 1 reply; 22+ messages in thread
From: Warner Losh @ 2020-12-17 17:10 UTC (permalink / raw)
To: Peter Maydell
Cc: Thomas Huth, Ed Maste, Sean Bruno, QEMU Developers,
Kamil Rytarowski, Chen Qun, Daniel P. Berrangé,
Alex Bennée, Li-Wen Hsu, Brad Smith
[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]
On Thu, Dec 17, 2020 at 9:21 AM Peter Maydell <peter.maydell@linaro.org>
wrote:
> On Thu, 17 Dec 2020 at 16:03, Warner Losh <imp@bsdimp.com> wrote:
> > On Thu, Dec 17, 2020 at 7:02 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> >> I don't recall what happened after that initial discussion about
> >> merging the new impl. Did Sean simply not have the time to invest
> >> in the merge ? I'll CC him here to see what opinion he has on the
> >> future of bsd-user in QEMU.
> >
> >
> > I've actually taken over for Sean Bruno managing this.
>
> > I'd love to hear from people ways that I can speed things up.
>
> There was a bit of discussion about this on #qemu IRC the other
> day, coincidentally. I think the conclusion we (upstream QEMU)
> came to was that we'd be happy with a "delete all of bsd-user
> and reinstate" approach, assuming that the "reinstate" part is
> in reasonably logical chunks and not one big "here's what we
> have all in one lump" patch.
>
> AIUI from IRC this is being primarily driven by FreeBSD and
> NetBSD/OpenBSD support is merely "we hope it is not broken
> by the delete-and-reinstate but it was probably broken anyway" ?
>
Yea, I don't think it actually works for anything non-trivial on the other
BSDs.
Warner
[-- Attachment #2: Type: text/html, Size: 1908 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Status/future of QEMU bsd-user impl ? (Wea Re: [PULL 00/12] Compile QEMU with -Wimplicit-fallthrough)
2020-12-17 17:10 ` Warner Losh
@ 2020-12-17 17:59 ` Warner Losh
0 siblings, 0 replies; 22+ messages in thread
From: Warner Losh @ 2020-12-17 17:59 UTC (permalink / raw)
To: Peter Maydell
Cc: Thomas Huth, Ed Maste, Sean Bruno, QEMU Developers,
Kamil Rytarowski, Chen Qun, Daniel P. Berrangé,
Alex Bennée, Li-Wen Hsu, Brad Smith
[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]
On Thu, Dec 17, 2020 at 10:10 AM Warner Losh <imp@bsdimp.com> wrote:
>
>
> On Thu, Dec 17, 2020 at 9:21 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On Thu, 17 Dec 2020 at 16:03, Warner Losh <imp@bsdimp.com> wrote:
>> > On Thu, Dec 17, 2020 at 7:02 AM Daniel P. Berrangé <berrange@redhat.com>
>> wrote:
>> >> I don't recall what happened after that initial discussion about
>> >> merging the new impl. Did Sean simply not have the time to invest
>> >> in the merge ? I'll CC him here to see what opinion he has on the
>> >> future of bsd-user in QEMU.
>> >
>> >
>> > I've actually taken over for Sean Bruno managing this.
>>
>> > I'd love to hear from people ways that I can speed things up.
>>
>> There was a bit of discussion about this on #qemu IRC the other
>> day, coincidentally. I think the conclusion we (upstream QEMU)
>> came to was that we'd be happy with a "delete all of bsd-user
>> and reinstate" approach, assuming that the "reinstate" part is
>> in reasonably logical chunks and not one big "here's what we
>> have all in one lump" patch.
>>
>> AIUI from IRC this is being primarily driven by FreeBSD and
>> NetBSD/OpenBSD support is merely "we hope it is not broken
>> by the delete-and-reinstate but it was probably broken anyway" ?
>>
>
> Yea, I don't think it actually works for anything non-trivial on the other
> BSDs.
>
Looking at the changes, it may be possible to get the first dozen or so
into a recent tree. It's not until after that that the changes touch areas
that have the high churn rate, but it may mean things like threaded apps
may have issues... I'll see what I can do over the holidays.
Warner
[-- Attachment #2: Type: text/html, Size: 2609 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread