1. Patchew
  2. QEMU
  3. [PATCH v2 0/4] build qemu with gcc and tsan
  4. View patch

[PATCH v2 2/4] target/i386: fix build warning (gcc-12 -fsanitize=thread)

Pierrick Bouvier posted 4 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH v2 2/4] target/i386: fix build warning (gcc-12 -fsanitize=thread)
Posted by Pierrick Bouvier 3 months, 1 week ago 
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;
-- 
2.39.2
Re: [PATCH v2 2/4] target/i386: fix build warning (gcc-12 -fsanitize=thread)
Posted by Philippe Mathieu-Daudé 3 months, 1 week ago 
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);
Re: [PATCH v2 2/4] target/i386: fix build warning (gcc-12 -fsanitize=thread)
Posted by Pierrick Bouvier 3 months, 1 week ago 
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.
Re: [PATCH v2 2/4] target/i386: fix build warning (gcc-12 -fsanitize=thread)
Posted by Thomas Huth 3 months, 1 week ago 
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>
Re: [PATCH v2 2/4] target/i386: fix build warning (gcc-12 -fsanitize=thread)
Posted by Richard Henderson 3 months, 1 week ago 
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~
Re: [PATCH v2 2/4] target/i386: fix build warning (gcc-12 -fsanitize=thread)
Posted by Pierrick Bouvier 3 months, 1 week ago 
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~

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.