[PATCH 1/6] dlls/msvcrt*: ensure variable sse2_cw is set for all code paths in _control87 (GCC11)

[Zebediah Figura (she/her)]

On 9/29/21 12:59, Eric Pouech wrote:
> Le 29/09/2021 à 17:57, Zebediah Figura (she/her) a écrit :
>> On 9/29/21 02:43, Eric Pouech wrote:
>>> Le 28/09/2021 à 20:01, Zebediah Figura (she/her) a écrit :
>>>> On 9/28/21 11:49, Eric Pouech wrote:
>>>>> Signed-off-by: Eric Pouech <eric.pouech at gmail.com>
>>>>>
>>>>> ---
>>>>>   dlls/msvcrt/math.c |    2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/dlls/msvcrt/math.c b/dlls/msvcrt/math.c
>>>>> index 7f59a4d20d4..ad632e70548 100644
>>>>> --- a/dlls/msvcrt/math.c
>>>>> +++ b/dlls/msvcrt/math.c
>>>>> @@ -5643,7 +5643,7 @@ unsigned int CDECL _control87(unsigned int 
>>>>> newval, unsigned int mask)
>>>>>   {
>>>>>       unsigned int flags = 0;
>>>>>   #ifdef __i386__
>>>>> -    unsigned int sse2_cw;
>>>>> +    unsigned int sse2_cw = 0;
>>>>>         __control87_2( newval, mask, &flags, &sse2_cw );
>>>>>
>>>>>
>>>>
>>>> Wouldn't it be better to check for failure from __control87_2()?
>>>>
>>> unfortunately, gcc11 still complains when checking for failure of 
>>> _control87_2()
>>>
>>> gcc doesn't seem to be smart enough to infer that ss2_cw is always 
>>> when _control87_2() returns 1
>>
>> That doesn't match what I have here. With the attached patch gcc 11.1 
>> doesn't complain.
> 
> what I tried is:
> 
> diff --git a/dlls/msvcrt/math.c b/dlls/msvcrt/math.c
> index 7f59a4d20d4..4560040eb9f 100644
> --- a/dlls/msvcrt/math.c
> +++ b/dlls/msvcrt/math.c
> @@ -5645,10 +5645,11 @@ unsigned int CDECL _control87(unsigned int 
> newval, unsigned int mask)
>   #ifdef __i386__
>       unsigned int sse2_cw;
> 
> -    __control87_2( newval, mask, &flags, &sse2_cw );
> -
> -    if ((flags ^ sse2_cw) & (_MCW_EM | _MCW_RC)) flags |= _EM_AMBIGUOUS;
> -    flags |= sse2_cw;
> +    if (__control87_2( newval, mask, &flags, &sse2_cw ))
> +    {
> +        if ((flags ^ sse2_cw) & (_MCW_EM | _MCW_RC)) flags |= 
> _EM_AMBIGUOUS;
> +        flags |= sse2_cw;
> +    }
>   #else
>       flags = newval;
>       _setfp(&flags, mask, NULL, 0);
> 
> 
> which still gives me the warnings, when compiling the 32bit part of a 
> wow64 conf
> 
> (but not on a pure 32bit conf)
> 
> your solution doesn't generate warnings on neither of the two
> 
> so will need further investigation on:
> 
> - discrepency wrt wow
> 
> - why the difference between the two patches

I don't get warnings with that diff either. What version of GCC do you have?