|
From: Eliot M. <mo...@cs...> - 2012-02-14 14:47:37
|
In my continuing quest to get the HotSpot Java virtual machine to work under valgrind, I found that it wants to execute this opcode: 0x66 0xDD 0x4 This is (as far as I have been able to determine) a floating point load (fldl) with a 0x66 size prefix. Now the size prefix is useless with this instruction. The AMD decoder in the svn head of valgrind explicitly says that putting 0x66 with any of the fpu-related opcodes is bad. However, I needed to put in special dispensation for fnsave and frstor, since 0x66 has meaning for them (and HotSpot appears to use that sequence). The AMD manual explicitly says that 0x66 is *ignored* if presented with fpu opcodes on which it has no effect. 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? Regards -- Eliot |
|
From: Julian S. <js...@ac...> - 2012-02-14 23:20:18
|
> 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. 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? J |
|
From: Eliot M. <mo...@cs...> - 2012-02-15 02:18:13
|
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
|