[PATCH 5/7] riscv: Fix feenvupdate with FE_DFL_ENV (BZ 31022)

[Adhemerval Zanella Netto]

On 05/11/23 21:21, Bruno Haible wrote:
> This code gave me puzzles:
> 
>   long int env = (long int) envp - (long int) FE_DFL_ENV;
>   if (env != 0)
>     env = *envp;
> 
> * What is the intent of the code?
> * Can signed integer overflow happen in the first line? (It cannot, due to
>   the value of FE_DFL_ENV being -1 and envp being otherwise aligned mod 4.
>   But one should not have to think that far.)
> * It assumes that intptr_t is the same as 'long int'. Which is not
>   universally true.
> 
> I would suggest to replace the function with this code:
> 
> libc_feupdateenv_riscv (const fenv_t *envp)
> {
>   long int env = (envp != FE_DFL_ENV ? *envp : 0);
> 
>   _FPU_SETCW (env | riscv_getflags ());
> }
> 
> * It is clearer.
> * It does not make assumptions regarding FE_DFL_ENV or alignment.
> * It produces the exact same code; see attached foo.c and foo.s,
>   generated by "riscv64-linux-gnu-gcc -O2 -S foo.c".

Indeed I was not sure why libc_fesetenv_riscv used this specific idiom
to check whether the input is FE_DFL_ENV.  I will change to this, it
is clear.

> 
> Probably the idiom that you copied was meant to allow the use of a
> conditional load instruction. But I would avoid such micro-optimizations
> and instead rely on the compiler to choose the best instructions for a
> task.
> 
> Bruno