| Message ID | 20240814224132.897098-3-pierrick.bouvier@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | build qemu with gcc and tsan | expand |
On 8/15/24 08:41, Pierrick Bouvier wrote: > Found on debian stable. > > ../target/i386/kvm/kvm.c: In function ‘kvm_handle_rdmsr’: > ../target/i386/kvm/kvm.c:5345:1: error: control reaches end of non-void function [-Werror=return-type] > 5345 | } > | ^ > ../target/i386/kvm/kvm.c: In function ‘kvm_handle_wrmsr’: > ../target/i386/kvm/kvm.c:5364:1: error: control reaches end of non-void function [-Werror=return-type] > 5364 | } > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > target/i386/kvm/kvm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index 31f149c9902..ddec27edd5b 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -5770,7 +5770,7 @@ static int kvm_handle_rdmsr(X86CPU *cpu, struct kvm_run *run) > } > } > > - assert(false); > + g_assert_not_reached(); While a good change, and while I have always hated the assert(false) idiom, I believe this points to a compiler bug and might be worth reporting -- assuming a later version of gcc still warns. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 15/08/2024 00.41, Pierrick Bouvier wrote: > Found on debian stable. > > ../target/i386/kvm/kvm.c: In function ‘kvm_handle_rdmsr’: > ../target/i386/kvm/kvm.c:5345:1: error: control reaches end of non-void function [-Werror=return-type] > 5345 | } > | ^ > ../target/i386/kvm/kvm.c: In function ‘kvm_handle_wrmsr’: > ../target/i386/kvm/kvm.c:5364:1: error: control reaches end of non-void function [-Werror=return-type] > 5364 | } > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > target/i386/kvm/kvm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c > index 31f149c9902..ddec27edd5b 100644 > --- a/target/i386/kvm/kvm.c > +++ b/target/i386/kvm/kvm.c > @@ -5770,7 +5770,7 @@ static int kvm_handle_rdmsr(X86CPU *cpu, struct kvm_run *run) > } > } > > - assert(false); > + g_assert_not_reached(); > } > > static int kvm_handle_wrmsr(X86CPU *cpu, struct kvm_run *run) > @@ -5789,7 +5789,7 @@ static int kvm_handle_wrmsr(X86CPU *cpu, struct kvm_run *run) > } > } > > - assert(false); > + g_assert_not_reached(); > } > > static bool has_sgx_provisioning; Reviewed-by: Thomas Huth <thuth@redhat.com>
On 8/14/24 15:47, Richard Henderson wrote: > On 8/15/24 08:41, Pierrick Bouvier wrote: >> Found on debian stable. >> >> ../target/i386/kvm/kvm.c: In function ‘kvm_handle_rdmsr’: >> ../target/i386/kvm/kvm.c:5345:1: error: control reaches end of non-void function [-Werror=return-type] >> 5345 | } >> | ^ >> ../target/i386/kvm/kvm.c: In function ‘kvm_handle_wrmsr’: >> ../target/i386/kvm/kvm.c:5364:1: error: control reaches end of non-void function [-Werror=return-type] >> 5364 | } >> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> target/i386/kvm/kvm.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c >> index 31f149c9902..ddec27edd5b 100644 >> --- a/target/i386/kvm/kvm.c >> +++ b/target/i386/kvm/kvm.c >> @@ -5770,7 +5770,7 @@ static int kvm_handle_rdmsr(X86CPU *cpu, struct kvm_run *run) >> } >> } >> >> - assert(false); >> + g_assert_not_reached(); > > While a good change, and while I have always hated the assert(false) idiom, I believe this > points to a compiler bug and might be worth reporting -- assuming a later version of gcc > still warns. > Reported it: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116386 > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > > r~
On 15/8/24 00:41, Pierrick Bouvier wrote: > Found on debian stable. > > ../target/i386/kvm/kvm.c: In function ‘kvm_handle_rdmsr’: > ../target/i386/kvm/kvm.c:5345:1: error: control reaches end of non-void function [-Werror=return-type] > 5345 | } > | ^ > ../target/i386/kvm/kvm.c: In function ‘kvm_handle_wrmsr’: > ../target/i386/kvm/kvm.c:5364:1: error: control reaches end of non-void function [-Werror=return-type] > 5364 | } > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > target/i386/kvm/kvm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> But what about the other cases? $ git grep 'assert(false)' block/qcow2.c:5302: assert(false); hw/hyperv/hyperv_testdev.c:91: assert(false); hw/hyperv/hyperv_testdev.c:190: assert(false); hw/hyperv/hyperv_testdev.c:240: assert(false); hw/hyperv/vmbus.c:1877: assert(false); hw/hyperv/vmbus.c:1892: assert(false); hw/hyperv/vmbus.c:1934: assert(false); hw/hyperv/vmbus.c:1949: assert(false); hw/hyperv/vmbus.c:1999: assert(false); hw/hyperv/vmbus.c:2023: assert(false); hw/net/e1000e_core.c:564: assert(false); hw/net/igb_core.c:400: assert(false); hw/net/net_rx_pkt.c:378: assert(false); hw/nvme/ctrl.c:1819: assert(false); hw/nvme/ctrl.c:1873: assert(false); hw/nvme/ctrl.c:4657: assert(false); hw/nvme/ctrl.c:7208: assert(false); hw/pci/pci-stub.c:49: g_assert(false); hw/pci/pci-stub.c:55: g_assert(false); hw/ppc/spapr_events.c:648: g_assert(false); include/hw/s390x/cpu-topology.h:60: assert(false); include/qemu/osdep.h:240: * assert(false) as unused. We rely on this within the code base to delete migration/dirtyrate.c:231: assert(false); /* unreachable */ target/i386/kvm/kvm.c:5773: assert(false); target/i386/kvm/kvm.c:5792: assert(false);
On 8/16/24 03:59, Philippe Mathieu-Daudé wrote: > On 15/8/24 00:41, Pierrick Bouvier wrote: >> Found on debian stable. >> >> ../target/i386/kvm/kvm.c: In function ‘kvm_handle_rdmsr’: >> ../target/i386/kvm/kvm.c:5345:1: error: control reaches end of non-void function [-Werror=return-type] >> 5345 | } >> | ^ >> ../target/i386/kvm/kvm.c: In function ‘kvm_handle_wrmsr’: >> ../target/i386/kvm/kvm.c:5364:1: error: control reaches end of non-void function [-Werror=return-type] >> 5364 | } >> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> target/i386/kvm/kvm.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > But what about the other cases? > > $ git grep 'assert(false)' > block/qcow2.c:5302: assert(false); > hw/hyperv/hyperv_testdev.c:91: assert(false); > hw/hyperv/hyperv_testdev.c:190: assert(false); > hw/hyperv/hyperv_testdev.c:240: assert(false); > hw/hyperv/vmbus.c:1877: assert(false); > hw/hyperv/vmbus.c:1892: assert(false); > hw/hyperv/vmbus.c:1934: assert(false); > hw/hyperv/vmbus.c:1949: assert(false); > hw/hyperv/vmbus.c:1999: assert(false); > hw/hyperv/vmbus.c:2023: assert(false); > hw/net/e1000e_core.c:564: assert(false); > hw/net/igb_core.c:400: assert(false); > hw/net/net_rx_pkt.c:378: assert(false); > hw/nvme/ctrl.c:1819: assert(false); > hw/nvme/ctrl.c:1873: assert(false); > hw/nvme/ctrl.c:4657: assert(false); > hw/nvme/ctrl.c:7208: assert(false); > hw/pci/pci-stub.c:49: g_assert(false); > hw/pci/pci-stub.c:55: g_assert(false); > hw/ppc/spapr_events.c:648: g_assert(false); > include/hw/s390x/cpu-topology.h:60: assert(false); > include/qemu/osdep.h:240: * assert(false) as unused. We rely on this > within the code base to delete > migration/dirtyrate.c:231: assert(false); /* unreachable */ > target/i386/kvm/kvm.c:5773: assert(false); > target/i386/kvm/kvm.c:5792: assert(false); > They don't seem to be a problem, but I'll do a series to clean this completely from the code base, so assert(false) is eradicated.
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 31f149c9902..ddec27edd5b 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -5770,7 +5770,7 @@ static int kvm_handle_rdmsr(X86CPU *cpu, struct kvm_run *run) } } - assert(false); + g_assert_not_reached(); } static int kvm_handle_wrmsr(X86CPU *cpu, struct kvm_run *run) @@ -5789,7 +5789,7 @@ static int kvm_handle_wrmsr(X86CPU *cpu, struct kvm_run *run) } } - assert(false); + g_assert_not_reached(); } static bool has_sgx_provisioning;
Found on debian stable. ../target/i386/kvm/kvm.c: In function ‘kvm_handle_rdmsr’: ../target/i386/kvm/kvm.c:5345:1: error: control reaches end of non-void function [-Werror=return-type] 5345 | } | ^ ../target/i386/kvm/kvm.c: In function ‘kvm_handle_wrmsr’: ../target/i386/kvm/kvm.c:5364:1: error: control reaches end of non-void function [-Werror=return-type] 5364 | } Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- target/i386/kvm/kvm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)