diff mbox

[4/7] target-i386: Dump illegal opcodes with -d unimp

Message ID 1456983051-14707-5-git-send-email-rth@twiddle.net (mailing list archive)
State New, archived
Headers show

Commit Message

Richard Henderson March 3, 2016, 5:30 a.m. UTC
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 target-i386/translate.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

Comments

Hervé Poussineau March 3, 2016, 6:57 a.m. UTC | #1
Le 03/03/2016 06:30, Richard Henderson a écrit :
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>   target-i386/translate.c | 22 +++++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/target-i386/translate.c b/target-i386/translate.c
> index b73c237..aa423cb 100644
> --- a/target-i386/translate.c
> +++ b/target-i386/translate.c
> @@ -99,6 +99,7 @@ typedef struct DisasContext {
>       int prefix;
>       TCGMemOp aflag;
>       TCGMemOp dflag;
> +    target_ulong pc_start;
>       target_ulong pc; /* pc = eip + cs_base */
>       int is_jmp; /* 1 = means jump (stop translation), 2 means CPU
>                      static state change (stop translation) */
> @@ -2368,6 +2369,21 @@ static void gen_exception(DisasContext *s, int trapno, target_ulong cur_eip)
>       s->is_jmp = DISAS_TB_JUMP;
>   }
>
> +static void gen_illop(CPUX86State *env, DisasContext *s)
> +{
> +    target_ulong pc = s->pc_start;
> +    gen_exception(s, EXCP06_ILLOP, pc - s->cs_base);
> +
> +    if (qemu_loglevel_mask(LOG_UNIMP)) {

Do you want LOG_UNIMP or LOG_GUEST_ERROR?
Both are possible. Either you decide that guest works well, and an unknown instruction is a valid instruction unimplemented in QEMU side,
you decide that guest can do invalid things, and LOG_GUEST_ERROR is probably better.

> +        target_ulong end = s->pc;
> +        qemu_log("ILLOPC: " TARGET_FMT_lx ":", pc);
> +        for (; pc < end; ++pc) {
> +            qemu_log(" %02x", cpu_ldub_code(env, pc));
> +        }
> +        qemu_log("\n");
> +    }
> +}
> +
>   /* an interrupt is different from an exception because of the
>      privilege checks */
>   static void gen_interrupt(DisasContext *s, int intno,
> @@ -2893,7 +2909,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
>       }
>       if (s->flags & HF_EM_MASK) {
>       illegal_op:
> -        gen_exception(s, EXCP06_ILLOP, pc_start - s->cs_base);
> +        gen_illop(env, s);
>           return;
>       }
>       if (is_xmm && !(s->flags & HF_OSFXSR_MASK))
> @@ -4293,7 +4309,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>       target_ulong next_eip, tval;
>       int rex_w, rex_r;
>
> -    s->pc = pc_start;
> +    s->pc_start = s->pc = pc_start;
>       prefixes = 0;
>       s->override = -1;
>       rex_w = -1;
> @@ -8031,7 +8047,7 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
>       if (s->prefix & PREFIX_LOCK)
>           gen_helper_unlock();
>       /* XXX: ensure that no lock was generated */
> -    gen_exception(s, EXCP06_ILLOP, pc_start - s->cs_base);
> +    gen_illop(env, s);
>       return s->pc;
>   }
>
>


This patch is not quiet on some operating systems:
OS/2:
ILLOPC: 000172e1: 0f a6

Windows XP:
ILLOPC: 00020d1a: c4 c4

And very verbose in Windows 3.11, Windows 9x:
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000027fe: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 00011b36: 63
ILLOPC: 000ffb17: 63
ILLOPC: 00011b3d: 63
ILLOPC: 00011b36: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 0001e3b9: 0f ff
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 00011b36: 63
ILLOPC: 00011b3d: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 00014d8a: 0f ff
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000ffb17: 63
ILLOPC: 000118ca: 63
ILLOPC: 000ffb17: 63

Is it normal?

Regards,

Hervé
Paolo Bonzini March 3, 2016, 10:08 a.m. UTC | #2
> Do you want LOG_UNIMP or LOG_GUEST_ERROR? 

I would actually use LOG_IN_ASM.  As you noticed, guests sometimes use
illegal opcodes; another example is Xen's hypercall interface.

On 03/03/2016 07:57, Hervé Poussineau wrote:
> This patch is not quiet on some operating systems:
> OS/2:
> ILLOPC: 000172e1: 0f a6
> 
> Windows XP:
> ILLOPC: 00020d1a: c4 c4
> 
> And very verbose in Windows 3.11, Windows 9x:
> ILLOPC: 000ffb17: 63
> ILLOPC: 000ffb17: 63
> 
> Is it normal?

Yes, it is.  As usual, Raymond Chen explains what's going on:

https://blogs.msdn.microsoft.com/oldnewthing/20041215-00/?p=37003

Alternatively, you can buy Unauthorized Windows 95 on Amazon for about
10 euros.  Which of course I just did. :)

Paolo
Richard Henderson March 3, 2016, 7:06 p.m. UTC | #3
On 03/03/2016 02:08 AM, Paolo Bonzini wrote:
>> Do you want LOG_UNIMP or LOG_GUEST_ERROR?
>
> I would actually use LOG_IN_ASM.  As you noticed, guests sometimes use
> illegal opcodes; another example is Xen's hypercall interface.
>
> On 03/03/2016 07:57, Hervé Poussineau wrote:
>> This patch is not quiet on some operating systems:
>> OS/2:
>> ILLOPC: 000172e1: 0f a6
>>
>> Windows XP:
>> ILLOPC: 00020d1a: c4 c4
>>
>> And very verbose in Windows 3.11, Windows 9x:
>> ILLOPC: 000ffb17: 63
>> ILLOPC: 000ffb17: 63
>>
>> Is it normal?
>
> Yes, it is.  As usual, Raymond Chen explains what's going on:
>
> https://blogs.msdn.microsoft.com/oldnewthing/20041215-00/?p=37003

Wow.  That's... interesting.

I think maybe I'll re-do the patch to distinguish between those opcodes that 
are completely unrecognized (which is what I was expecting to find) and those 
that raise #UD due to cpu state (e.g. this arpl in vm86 mode).


r~
Paolo Bonzini March 4, 2016, 10:41 a.m. UTC | #4
On 03/03/2016 20:06, Richard Henderson wrote:
> On 03/03/2016 02:08 AM, Paolo Bonzini wrote:
>>> Do you want LOG_UNIMP or LOG_GUEST_ERROR?
>>
>> I would actually use LOG_IN_ASM.  As you noticed, guests sometimes use
>> illegal opcodes; another example is Xen's hypercall interface.
>>
>> On 03/03/2016 07:57, Hervé Poussineau wrote:
>>> This patch is not quiet on some operating systems:
>>> OS/2:
>>> ILLOPC: 000172e1: 0f a6
>>>
>>> Windows XP:
>>> ILLOPC: 00020d1a: c4 c4
>>>
>>> And very verbose in Windows 3.11, Windows 9x:
>>> ILLOPC: 000ffb17: 63
>>> ILLOPC: 000ffb17: 63
>>>
>>> Is it normal?
>>
>> Yes, it is.  As usual, Raymond Chen explains what's going on:
>>
>> https://blogs.msdn.microsoft.com/oldnewthing/20041215-00/?p=37003
> 
> Wow.  That's... interesting.
> 
> I think maybe I'll re-do the patch to distinguish between those opcodes
> that are completely unrecognized (which is what I was expecting to find)
> and those that raise #UD due to cpu state (e.g. this arpl in vm86 mode).

Good idea.  UD2 should not warn too, and also VEX prefixes outside
64-bit mode.

Any thoughts about patch 7?

Paolo
Paolo Bonzini March 4, 2016, 12:15 p.m. UTC | #5
On 03/03/2016 20:06, Richard Henderson wrote:
> On 03/03/2016 02:08 AM, Paolo Bonzini wrote:
>>> Do you want LOG_UNIMP or LOG_GUEST_ERROR?
>>
>> I would actually use LOG_IN_ASM.  As you noticed, guests sometimes use
>> illegal opcodes; another example is Xen's hypercall interface.
>>
>> On 03/03/2016 07:57, Hervé Poussineau wrote:
>>> This patch is not quiet on some operating systems:
>>> OS/2:
>>> ILLOPC: 000172e1: 0f a6
>>>
>>> Windows XP:
>>> ILLOPC: 00020d1a: c4 c4
>>>
>>> And very verbose in Windows 3.11, Windows 9x:
>>> ILLOPC: 000ffb17: 63
>>> ILLOPC: 000ffb17: 63
>>>
>>> Is it normal?
>>
>> Yes, it is.  As usual, Raymond Chen explains what's going on:
>>
>> https://blogs.msdn.microsoft.com/oldnewthing/20041215-00/?p=37003
> 
> Wow.  That's... interesting.

It's actually even more interesting (the explanation is in the book) if
you notice that 0xffb17 is in the middle of the BIOS.  Indeed Windows 95
first locates a single 0x63 in the BIOS (so that it's ROM and no one can
write a different byte).  Then the 32-bit code can use a system service
that allocates a callback from 16-bit MS-DOS.  That service gets a
32-bit address for the 32-bit code and returns a real-mode address to be
used in 16-bit code.

The kick is that all the real-mode addresses point to that single 0x63
that was found in ROM.  For example in the case above the real-mode
addresses could be FFB1:07, FFB0:17, FFAF:27, etc.  The illegal opcode
exception handler looks at the segment to figure out which 32-bit
address to jump to.

There are also cases where the ARPL is patched into existing code (like
a breakpoint) to trap that code to 32-bit.  But this one using the ROM
is much cooler.

Paolo
diff mbox

Patch

diff --git a/target-i386/translate.c b/target-i386/translate.c
index b73c237..aa423cb 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -99,6 +99,7 @@  typedef struct DisasContext {
     int prefix;
     TCGMemOp aflag;
     TCGMemOp dflag;
+    target_ulong pc_start;
     target_ulong pc; /* pc = eip + cs_base */
     int is_jmp; /* 1 = means jump (stop translation), 2 means CPU
                    static state change (stop translation) */
@@ -2368,6 +2369,21 @@  static void gen_exception(DisasContext *s, int trapno, target_ulong cur_eip)
     s->is_jmp = DISAS_TB_JUMP;
 }
 
+static void gen_illop(CPUX86State *env, DisasContext *s)
+{
+    target_ulong pc = s->pc_start;
+    gen_exception(s, EXCP06_ILLOP, pc - s->cs_base);
+
+    if (qemu_loglevel_mask(LOG_UNIMP)) {
+        target_ulong end = s->pc;
+        qemu_log("ILLOPC: " TARGET_FMT_lx ":", pc);
+        for (; pc < end; ++pc) {
+            qemu_log(" %02x", cpu_ldub_code(env, pc));
+        }
+        qemu_log("\n");
+    }
+}
+
 /* an interrupt is different from an exception because of the
    privilege checks */
 static void gen_interrupt(DisasContext *s, int intno,
@@ -2893,7 +2909,7 @@  static void gen_sse(CPUX86State *env, DisasContext *s, int b,
     }
     if (s->flags & HF_EM_MASK) {
     illegal_op:
-        gen_exception(s, EXCP06_ILLOP, pc_start - s->cs_base);
+        gen_illop(env, s);
         return;
     }
     if (is_xmm && !(s->flags & HF_OSFXSR_MASK))
@@ -4293,7 +4309,7 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
     target_ulong next_eip, tval;
     int rex_w, rex_r;
 
-    s->pc = pc_start;
+    s->pc_start = s->pc = pc_start;
     prefixes = 0;
     s->override = -1;
     rex_w = -1;
@@ -8031,7 +8047,7 @@  static target_ulong disas_insn(CPUX86State *env, DisasContext *s,
     if (s->prefix & PREFIX_LOCK)
         gen_helper_unlock();
     /* XXX: ensure that no lock was generated */
-    gen_exception(s, EXCP06_ILLOP, pc_start - s->cs_base);
+    gen_illop(env, s);
     return s->pc;
 }