On 2/14/2012 6:12 PM, Julian Seward wrote:

>> So ... I am running tests with the decoder revised to allow 0x66 on the fpu
>> instructions.

>> Does anyone know of a reason why doing this would be bad/wrong?

> Giving a blanket OK for 0x66 on FPU instructions makes me nervous, that we
> might inadvertantly accept an 0x66 prefix that had some significance, and
> then ignore it.

I understand your skittishness about it, but I quote from the AMD manual
(pages 8-9 of Volume 3 (URL below)):

"The operand-size prefix should be used only with general-purpose instructions
and the x87 FLDENV, FNSTENV, FNSAVE, and FRSTOR instructions, in which the
prefix selects between 16-bit and 32-bit operand size. The prefix is ignored
by all other x87 instructions and by 64-bit media floating-point (3DNow!)
instructions."

So, this says that the four indicated instructions should check for the size
explicitly, and the rest should ignore it. I have verified in the case of FLD
that the machine in fact ignores it. It would be painful to go verify it with
the rest of the opcodes, though possible in principle.  For FNSAVE and FRSTOR
I have implemented both the 16- and 32-bit versions -- the application that
valgrind was failing to work with used 16-bit FRSTOR (and maybe FNSAVE too,
but I put them bth in at once, and FRSTOR was the one it had failed on).  Then
it failed on 0x66 FLDL and I went to check up on things and found the quoted
statement. I will make sure that FNSTENV and FLDENV check for size (or
implemented their 16-bit version as well, which should be easy given the
implementations of FNSAVE and FRSTOR).

I *do* make sure that the 0xF2 and 0xF3 (opcode extension) prefixes are *not*
given, since they do affect the semantics. Here's the code I am testing now:

in dis_ESC_NONE, for the 0xD8 through 0xDF cases of the switch on opc:

   Bool redundantREXWok = False;

   if (haveF2orF3(pfx))
      goto decode_failure;

   /* kludge to tolerate redundant rex.w prefixes (should do this
      properly one day) */
   /* mono 1.1.18.1 produces 48 D9 FA, which is rex.w fsqrt */
   if ( (opc == 0xD9 && getUChar(delta+0) == 0xFA)/*fsqrt*/ )
      redundantREXWok = True;

   /* AMD manual says 0x66 size override is ignored, except where it is 
meaningful */
   if ( (sz == 2 || sz == 4 || (sz == 8 && redundantREXWok)) ) {
      Bool decode_OK = False;
      delta = dis_FPU ( &decode_OK, vbi, pfx, delta );
      if (!decode_OK)
         goto decode_failure;
   } else {
      goto decode_failure;
   }
   return delta;

The if could probably read: if ( (sz != 8) || redundantREXWok ) ...

> Is it possible you can add the specific necessary exemptions in the place
> where dis_FPU is called (shown below), so that we continue to disallow 0x66
> for FPU instructions in the general case?

You didn't show it. But given what AMD says, there are no "specific
exemptions". If we're willing to put in patches for each case people
run into in practice, I have coded it and tested on my programs.
My app does not appear to have more of these (admittedly silly) 0x66
prefixed fpu opcodes.

   Bool redundantREXWok = False;

   if (haveF2orF3(pfx))
      goto decode_failure;

   /* kludge to tolerate redundant rex.w prefixes (should do this
      properly one day) */
   /* mono 1.1.18.1 produces 48 D9 FA, which is rex.w fsqrt */
   if ( (opc == 0xD9 && getUChar(delta+0) == 0xFA)/*fsqrt*/ )
      redundantREXWok = True;

   Bool size_OK = False;
   if ( sz == 4 )
      size_OK = True;
   else if ( sz == 8 )
      size_OK = redundantREXWok;
   else if ( sz == 2 ) {
      int mod_rm = getUChar(delta+0);
      int reg = gregLO3ofRM(mod_rm);
      if ( (opc == 0xDD) && (reg == 0 || reg == 4 || reg == 6) ) {
         size_OK = True;
      }
   }
   /* AMD manual says 0x66 size override is ignored, except where it is 
meaningful */
   if ( size_OK ) {
      Bool decode_OK = False;
      delta = dis_FPU ( &decode_OK, vbi, pfx, delta );
      if (!decode_OK)
         goto decode_failure;
   } else {
      goto decode_failure;
   }
   return delta;

What do you think at this point?

Regards -- Eliot

------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
Valgrind-users mailing list
Valgrind-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/valgrind-users

Reply via email to