Re: [PATCH v22 12/17] i386: separate fpu_helper into user and sysemu parts

[Richard Henderson]

On 2/24/21 5:34 AM, Claudio Fontana wrote:
>  #if !defined(CONFIG_USER_ONLY)
> -    else if (ferr_irq && !(env->hflags2 & HF2_IGNNE_MASK)) {
> -        qemu_irq_raise(ferr_irq);
> +    else {
> +        (void)fpu_check_raise_ferr_irq(env);

Given this is the only use, why split out the function at all, much less make
it return bool and then cast the return value to void?


> -#if defined(CONFIG_USER_ONLY)
> -void cpu_x86_fsave(CPUX86State *env, target_ulong ptr, int data32)
> -{
> -    helper_fsave(env, ptr, data32);
> -}

Not your bug, obviously, but these usages are broken.

For the record:

In helper_fsave() we reference GETPC().  Which is *only* valid when directly
called from code_gen_buffer.

The signature of cpu_x86_foo must be changed to add a "uintptr_t retaddr"
argument, the callers from linux-user/i386/signal.c must pass 0 for this new
argument (meaning no unwind required), and the helpers must do

void helper_fsave(CPUX86State *env, target_ulong ptr, int data32)
{
    cpu_x86_fsave(env, ptr, data32, GETPC());
}

etc.


r~