[PATCH] x86emu: Fix more mis-decoding of the data prefix

[Adam Jackson]

On Mon, 2010-12-20 at 19:41 +0100, Julien Cristau wrote:
> On Fri, Dec 10, 2010 at 14:47:20 -0500, Adam Jackson wrote:
> > @@ -7084,8 +7089,12 @@ static void x86emuOp_call_far_IMM(u8 X86EMU_UNUSED(op1))
> >      TRACE_AND_STEP();
> >      push_word(M.x86.R_CS);
> >      M.x86.R_CS = farseg;
> > -    push_word(M.x86.R_IP);
> > -    M.x86.R_IP = faroff;
> > +    if (M.x86.mode & SYSMODE_PREFIX_DATA) {
> > +	push_long(M.x86.R_EIP);
> > +    } else {
> > +	push_word(M.x86.R_IP);
> > +    }
> > +    M.x86.R_EIP = faroff & 0xffff;
> >      DECODE_CLEAR_SEGOVR();
> >      END_OF_INSTR();
> >  }
> 
> The intel doc says (for far calls in virtual-8086 mode):
> 
> IF OperandSize = 32
>   THEN
>     ...
>     EIP ← DEST[31:0]; (* DEST is ptr16:32 or [m16:32] *)
>   ELSE
>     ...
>     EIP ← DEST[15:0]; (* DEST is ptr16:16 or [m16:16]; clear upper 16 bits *)
> FI;
> 
> which suggests that you don't need the & 0xffff in the last assignment?

Bit of artistic license on my part.  The relevant excerpt is:

IF OperandSize = 32
THEN
	IF stack not large enough for a 6-byte return address
	THEN #SS(0); FI;
@	IF DEST[31:16] is not zero THEN #GP(0); FI;
	Push(CS); (* Padded with 16 high-order bits *)
	Push(EIP);
	CS <- DEST[47:32]; (* DEST is ptr16:32 or [m16:32] *)
	EIP <- DEST[31:0]; (* DEST is ptr16:32 or [m16:32] *)
ELSE (* OperandSize = 16 *)
	IF stack not large enough for a 4-byte return address
	THEN #SS(0); FI;
	Push(CS);
	Push(IP);
	CS <- DEST[31:16]; (* DEST is ptr16:16 or [m16:16] *)
	EIP <- DEST[15:0]; (* DEST is ptr16:16 or [m16:16]; clear upper 16 bits *)
FI;

The @'d line is where I'm taking a small liberty, as x86emu doesn't
emulate faults.  We'll assume (since x86emu tends to work) that the
number of BIOSes that need fault handling is very low.  So by the time
you hit the EIP load, either:

- operand size is 16, high word of EIP explicitly 0
- operand size is 32 but you'd have faulted if the high word wasn't 0

Practically I think this matters only if the later code looks at the
high word of EIP, instruction fetch is only going to look at cs:ip not
cs:eip.  But, might as well be screechingly correct.

- ajax