Re: [PATCH v2 6/7] alpha: Fix fesetexceptflag (BZ 30998)

[Carlos O'Donell <carlos@redhat.com>]

On 11/6/23 08:27, Adhemerval Zanella wrote:
> From: Bruno Haible <bruno@clisp.org>
> 
> It clears some exception flags that are outside the EXCEPTS argument.
> 
> It fixes math/test-fexcept on qemu-user.

> ---
>  sysdeps/alpha/fpu/fsetexcptflg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/alpha/fpu/fsetexcptflg.c b/sysdeps/alpha/fpu/fsetexcptflg.c
> index 70f3666a6e..63eb06845d 100644
> --- a/sysdeps/alpha/fpu/fsetexcptflg.c
> +++ b/sysdeps/alpha/fpu/fsetexcptflg.c
> @@ -27,7 +27,7 @@ __fesetexceptflag (const fexcept_t *flagp, int excepts)
>    tmp = __ieee_get_fp_control ();
>  
>    /* Set all the bits that were called for.  */
> -  tmp = (tmp & ~SWCR_STATUS_MASK) | (*flagp & excepts & SWCR_STATUS_MASK);
> +  tmp ^= (tmp ^ *flagp) & excepts & SWCR_STATUS_MASK;

Does this actually work?

Assume excepts is FE_INVALID, and *flagp bit 17 is 0.
Assume currently bit 17 is 1.

tmp ^ *flagp       => bit 17 is still 1, even though bit 17 in flagp is 0.
& excepts          => bit 17 is still 1.
& SWCR_STATUS_MASK => bit 17 is still 1.
^=                 => bit 17 is still 1.

The operation will set a bit, but won't clear it?

I would expect (taken from hppa code I wrote for that port):

/* Clear all status bits we care about.  */
tmp = tmp & ~(excepts & SWCR_STATUS_MASK);
/* Install the new ones.  */
tmp |= *flagp & excepts & SWCR_STATUS_MASK;

>  
>    /* And store it back.  */
>    __ieee_set_fp_control (tmp);

-- 
Cheers,
Carlos.