Re: [PATCH] Set the correct env->fpip for x86 float instructions [cleaned]

[Richard Henderson <richard.henderson@linaro.org>]

On 4/16/21 8:34 AM, Ziqiao Kong wrote:
> +++ b/target/i386/tcg/translate.c
> @@ -6337,7 +6337,10 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
>                   goto unknown_op;
>               }
>           }
> +        tcg_gen_movi_tl(s->tmp0, pc_start - s->cs_base);
> +        tcg_gen_st_tl(s->tmp0, cpu_env, offsetof(CPUX86State, fpip));

This placement is wrong because it catches instructions that should not modify 
FIP, like FINIT.

It might be best to set a flag around this case like

   bool update_fip;

   case 0xd8 .. 0xdf:
     ...
     update_fip = true;
     if (mod != 3) {
         ...
     } else {
         ...
     }
     if (update_fip) {
         ...
     }
     break;

and set update_fip to false for the set of insns that either do not update FIP 
or clear it (8.1.8 x87 fpu instruction and data (operand) pointers).

I notice you're not saving FCS to go along with this, at least for 
CPUID.(EAX=07H,ECX=0H):EBX[bit 13] = 0.

And if you're going to this trouble, you might want to think about FDP+FDS as 
well.  It should be about the same amount of effort.


r~