[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe
From: |
Eric Auger |
Subject: |
Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized |
Date: |
Thu, 22 Dec 2022 12:18:28 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 |
Hi All,
On 12/22/22 12:09, Daniel P. Berrangé wrote:
> On Thu, Dec 22, 2022 at 11:07:31AM +0100, Eric Auger wrote:
>> Hi Philippe,
>>
>> On 12/22/22 10:01, Philippe Mathieu-Daudé wrote:
>>> On 22/12/22 09:18, Paolo Bonzini wrote:
>>>> On 12/21/22 17:36, Eric Auger wrote:
>>>>> To avoid compilation errors when -Werror=maybe-uninitialized is used,
>>>>> replace 'case 3' by 'default'.
>>>>>
>>>>> Otherwise we get:
>>>>>
>>>>> ../target/i386/ops_sse.h: In function ‘helper_vpermdq_ymm’:
>>>>> ../target/i386/ops_sse.h:2495:13: error: ‘r3’ may be used
>>>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>>> 2495 | d->Q(3) = r3;
>>>>> | ~~~~~~~~^~~~
>>>>> ../target/i386/ops_sse.h:2494:13: error: ‘r2’ may be used
>>>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>>> 2494 | d->Q(2) = r2;
>>>>> | ~~~~~~~~^~~~
>>>>> ../target/i386/ops_sse.h:2493:13: error: ‘r1’ may be used
>>>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>>> 2493 | d->Q(1) = r1;
>>>>> | ~~~~~~~~^~~~
>>>>> ../target/i386/ops_sse.h:2492:13: error: ‘r0’ may be used
>>>>> uninitialized in this function [-Werror=maybe-uninitialized]
>>>>> 2492 | d->Q(0) = r0;
>>>>> | ~~~~~~~~^~~~
>>> With what compiler? Is that a supported one?
>> 3aab489e-9d90-c1ad-0b6b-b2b5d80db723@redhat.com/">https://lore.kernel.org/qemu-devel/3aab489e-9d90-c1ad-0b6b-b2b5d80db723@redhat.com/
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>> Suggested-by: Stefan Weil <sw@weilnetz.de>
>>>>> Fixes: 790684776861 ("target/i386: reimplement 0x0f 0x3a, add AVX")
>>>>> ---
>>>>> target/i386/ops_sse.h | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
>>>>> index 3cbc36a59d..c442c8c10c 100644
>>>>> --- a/target/i386/ops_sse.h
>>>>> +++ b/target/i386/ops_sse.h
>>>>> @@ -2466,7 +2466,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg
>>>>> *s, uint32_t order)
>>>>> r0 = s->Q(0);
>>>>> r1 = s->Q(1);
>>>>> break;
>>>>> - case 3:
>>>>> + default:
>>>>> r0 = s->Q(2);
>>>>> r1 = s->Q(3);
>>>>> break;
>>>>> @@ -2484,7 +2484,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg
>>>>> *s, uint32_t order)
>>>>> r2 = s->Q(0);
>>>>> r3 = s->Q(1);
>>>>> break;
>>>>> - case 3:
>>>>> + default:
>>>>> r2 = s->Q(2);
>>>>> r3 = s->Q(3);
>>>>> break;
>>>> Queued, but this compiler sucks. :)
>>> Can't we simply add a dumb 'default' case? So when reviewing we don't
>>> have to evaluate 'default' means 3 here.
>>>
>>> -- >8 --
>>> --- a/target/i386/ops_sse.h
>>> +++ b/target/i386/ops_sse.h
>>> @@ -2470,6 +2470,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s,
>>> uint32_t order)
>>> r0 = s->Q(2);
>>> r1 = s->Q(3);
>>> break;
>>> + default:
>>> + qemu_build_not_reached();
>>> }
>>> switch ((order >> 4) & 3) {
>>> case 0:
>>> @@ -2488,6 +2490,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s,
>>> uint32_t order)
>>> r2 = s->Q(2);
>>> r3 = s->Q(3);
>>> break;
>>> + default:
>>> + qemu_build_not_reached();
>>> }
>> I guess this won't fix the fact r0, r1, r2, r3 are not initialized, will it?
> This ultimately expands to assert() and the compiler should see that it
> terminates the control flow at this point, so shouldn't have a reason
> to warn.
OK so with qemu_build_not_reached(); I get
/home/augere/UPSTREAM/qemu/include/qemu/osdep.h:184:35: error: call to
‘qemu_build_not_reached_always’ declared with attribute error: code path
is reachable
184 | #define qemu_build_not_reached() qemu_build_not_reached_always()
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
However with g_assert_not_reached(), it does not complain and errors are
removed. So I will respin with g_assert_not_reached() if nobody advises
me against that.
Thanks
Eric
>
>
> With regards,
> Daniel
- [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Eric Auger, 2022/12/21
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Stefan Weil, 2022/12/21
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Paolo Bonzini, 2022/12/22
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Philippe Mathieu-Daudé, 2022/12/22
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Eric Auger, 2022/12/22
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Philippe Mathieu-Daudé, 2022/12/22
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Daniel P . Berrangé, 2022/12/22
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized,
Eric Auger <=
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Philippe Mathieu-Daudé, 2022/12/22
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Stefan Weil, 2022/12/22
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Philippe Mathieu-Daudé, 2022/12/22
- Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized, Bernhard Beschow, 2022/12/22