Fix emulator detection (#1966)

* Add a quick test helper macro to test_x86.c

* Add regression tests for bswap and rex prefixes

* Properly ignore REX prefixes when appropriate

* Fix bswap ax emulator detection

qemu/target/i386/translate.c

@@ -4792,12 +4792,12 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
 {
     TCGContext *tcg_ctx = s->uc->tcg_ctx;
     CPUX86State *env = cpu->env_ptr;
-    int b, prefixes;
+    int b, prefixes, prefix_count;
     int shift;
     MemOp ot, aflag, dflag;
     int modrm, reg, rm, mod, op, opreg, val;
     target_ulong next_eip, tval;
-    int rex_w, rex_r;
+    int rex_w, rex_r, rex_byte, rex_index;
     target_ulong pc_start = s->base.pc_next;
     TCGOp *tcg_op, *prev_op = NULL;
     bool insn_hook = false;
@@ -4854,6 +4854,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
     prefixes = 0;
     rex_w = -1;
     rex_r = 0;
+    rex_byte = 0;
+    rex_index = -1;
+    prefix_count = 0;
 
  next_byte:
     b = x86_ldub_code(env, s);
@@ -4861,36 +4864,47 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
     switch (b) {
     case 0xf3:
         prefixes |= PREFIX_REPZ;
+        prefix_count++;
         goto next_byte;
     case 0xf2:
         prefixes |= PREFIX_REPNZ;
+        prefix_count++;
         goto next_byte;
     case 0xf0:
         prefixes |= PREFIX_LOCK;
+        prefix_count++;
         goto next_byte;
     case 0x2e:
         s->override = R_CS;
+        prefix_count++;
         goto next_byte;
     case 0x36:
         s->override = R_SS;
+        prefix_count++;
         goto next_byte;
     case 0x3e:
         s->override = R_DS;
+        prefix_count++;
         goto next_byte;
     case 0x26:
         s->override = R_ES;
+        prefix_count++;
         goto next_byte;
     case 0x64:
         s->override = R_FS;
+        prefix_count++;
         goto next_byte;
     case 0x65:
         s->override = R_GS;
+        prefix_count++;
         goto next_byte;
     case 0x66:
         prefixes |= PREFIX_DATA;
+        prefix_count++;
         goto next_byte;
     case 0x67:
         prefixes |= PREFIX_ADR;
+        prefix_count++;
         goto next_byte;
 #ifdef TARGET_X86_64
     case 0x40:
@@ -4910,13 +4924,9 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
     case 0x4e:
     case 0x4f:
         if (CODE64(s)) {
-            /* REX prefix */
+            rex_byte = b;
-            rex_w = (b >> 3) & 1;
+            rex_index = prefix_count;
-            rex_r = (b & 0x4) << 1;
+            prefix_count++;
-            s->rex_x = (b & 0x2) << 2;
-            REX_B(s) = (b & 0x1) << 3;
-            /* select uniform byte register addressing */
-            s->x86_64_hregs = true;
             goto next_byte;
         }
         break;
@@ -4944,7 +4954,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
                 goto illegal_op;
             }
 #ifdef TARGET_X86_64
-            if (s->x86_64_hregs) {
+            if (rex_byte != 0) {
                 goto illegal_op;
             }
 #endif
@@ -4979,11 +4989,23 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
             s->vex_l = (vex3 >> 2) & 1;
             prefixes |= pp_prefix[vex3 & 3] | PREFIX_VEX;
         }
+        prefix_count++;
         break;
     }
 
     /* Post-process prefixes.  */
     if (CODE64(s)) {
+        /* 2.2.1: A REX prefix is ignored when it does not immediately precede the opcode byte */
+        if (rex_byte != 0 && rex_index + 1 == prefix_count) {
+            /* REX prefix */
+            rex_w = (rex_byte >> 3) & 1;
+            rex_r = (rex_byte & 0x4) << 1;
+            s->rex_x = (rex_byte & 0x2) << 2;
+            REX_B(s) = (rex_byte & 0x1) << 3;
+            /* select uniform byte register addressing */
+            s->x86_64_hregs = true;
+        }
+
         /* In 64-bit mode, the default data size is 32-bit.  Select 64-bit
            data with rex_w, and 16-bit data with 0x66; rex_w takes precedence
            over 0x66 if both are present.  */
@@ -7807,12 +7829,16 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
             gen_op_mov_reg_v(s, MO_64, reg, s->T0);
         } else
 #endif
-        {
+        if (dflag == MO_32) {
             gen_op_mov_v_reg(s, MO_32, s->T0, reg);
             tcg_gen_ext32u_tl(tcg_ctx, s->T0, s->T0);
             tcg_gen_bswap32_tl(tcg_ctx, s->T0, s->T0);
             gen_op_mov_reg_v(s, MO_32, reg, s->T0);
         }
+        else {
+            tcg_gen_movi_tl(tcg_ctx, s->T0, 0);
+            gen_op_mov_reg_v(s, MO_16, reg, s->T0);
+        }
         break;
     case 0xd6: /* salc */
         if (CODE64(s))

tests/unit/test_x86.c

@@ -3,6 +3,11 @@
 const uint64_t code_start = 0x1000;
 const uint64_t code_len = 0x4000;
 
+#define MEM_BASE 0x40000000
+#define MEM_SIZE 1024 * 1024
+#define MEM_STACK MEM_BASE + (MEM_SIZE / 2)
+#define MEM_TEXT MEM_STACK + 4096
+
 static void uc_common_setup(uc_engine **uc, uc_arch arch, uc_mode mode,
                             const char *code, uint64_t size)
 {
@@ -11,6 +16,90 @@ static void uc_common_setup(uc_engine **uc, uc_arch arch, uc_mode mode,
     OK(uc_mem_write(*uc, code_start, code, size));
 }
 
+typedef struct RegInfo_t {
+    const char *file;
+    int line;
+    const char *name;
+    uc_x86_reg reg;
+    uint64_t value;
+} RegInfo;
+
+typedef struct QuickTest_t {
+    uc_mode mode;
+    uint8_t *code_data;
+    size_t code_size;
+    size_t in_count;
+    RegInfo in_regs[32];
+    size_t out_count;
+    RegInfo out_regs[32];
+} QuickTest;
+
+static void QuickTest_run(QuickTest *test)
+{
+    uc_engine *uc;
+
+    // initialize emulator in X86-64bit mode
+    OK(uc_open(UC_ARCH_X86, test->mode, &uc));
+
+    // map 1MB of memory for this emulation
+    OK(uc_mem_map(uc, MEM_BASE, MEM_SIZE, UC_PROT_ALL));
+    OK(uc_mem_write(uc, MEM_TEXT, test->code_data, test->code_size));
+    if (test->mode == UC_MODE_64) {
+        uint64_t stack_top = MEM_STACK;
+        OK(uc_reg_write(uc, UC_X86_REG_RSP, &stack_top));
+    } else {
+        uint32_t stack_top = MEM_STACK;
+        OK(uc_reg_write(uc, UC_X86_REG_ESP, &stack_top));
+    }
+    for (size_t i = 0; i < test->in_count; i++) {
+        OK(uc_reg_write(uc, test->in_regs[i].reg, &test->in_regs[i].value));
+    }
+    OK(uc_emu_start(uc, MEM_TEXT, MEM_TEXT + test->code_size, 0, 0));
+    for (size_t i = 0; i < test->out_count; i++) {
+        RegInfo *out = &test->out_regs[i];
+        if (test->mode == UC_MODE_64) {
+            uint64_t value = 0;
+            OK(uc_reg_read(uc, out->reg, &value));
+            acutest_check_(value == out->value, out->file, out->line,
+                           "OUT_REG(%s, 0x%llX) = 0x%llX", out->name,
+                           out->value, value);
+        } else {
+            uint32_t value = 0;
+            OK(uc_reg_read(uc, out->reg, &value));
+            acutest_check_(value == (uint32_t)out->value, out->file, out->line,
+                           "OUT_REG(%s, 0x%X) = 0x%X", out->name,
+                           (uint32_t)out->value, value);
+        }
+    }
+    OK(uc_mem_unmap(uc, MEM_BASE, MEM_SIZE));
+    OK(uc_close(uc));
+}
+
+#define TEST_CODE(MODE, CODE)                                                  \
+    QuickTest t;                                                               \
+    memset(&t, 0, sizeof(t));                                                  \
+    t.mode = MODE;                                                             \
+    t.code_data = CODE;                                                        \
+    t.code_size = sizeof(CODE)
+
+#define TEST_IN_REG(NAME, VALUE)                                               \
+    t.in_regs[t.in_count].file = __FILE__;                                     \
+    t.in_regs[t.in_count].line = __LINE__;                                     \
+    t.in_regs[t.in_count].name = #NAME;                                        \
+    t.in_regs[t.in_count].reg = UC_X86_REG_##NAME;                             \
+    t.in_regs[t.in_count].value = VALUE;                                       \
+    t.in_count++
+
+#define TEST_OUT_REG(NAME, VALUE)                                              \
+    t.out_regs[t.out_count].file = __FILE__;                                   \
+    t.out_regs[t.out_count].line = __LINE__;                                   \
+    t.out_regs[t.out_count].name = #NAME;                                      \
+    t.out_regs[t.out_count].reg = UC_X86_REG_##NAME;                           \
+    t.out_regs[t.out_count].value = VALUE;                                     \
+    t.out_count++
+
+#define TEST_RUN() QuickTest_run(&t)
+
 typedef struct _INSN_IN_RESULT {
     uint32_t port;
     int size;
@@ -1427,7 +1516,7 @@ static void test_x86_vtlb(void)
     OK(uc_close(uc));
 }
 
-static void test_x86_segmentation()
+static void test_x86_segmentation(void)
 {
     uc_engine *uc;
     uint64_t fs = 0x53;
@@ -1446,7 +1535,7 @@ static void test_x86_0xff_lcall_callback(uc_engine *uc, uint64_t address,
 }
 
 // This aborts prior to a7a5d187e77f7853755eff4768658daf8095c3b7
-static void test_x86_0xff_lcall()
+static void test_x86_0xff_lcall(void)
 {
     uc_engine *uc;
     uc_hook hk;
@@ -1483,7 +1572,7 @@ static bool test_x86_64_not_overwriting_tmp0_for_pc_update_cb(
 
 // https://github.com/unicorn-engine/unicorn/issues/1717
 // https://github.com/unicorn-engine/unicorn/issues/1862
-static void test_x86_64_not_overwriting_tmp0_for_pc_update()
+static void test_x86_64_not_overwriting_tmp0_for_pc_update(void)
 {
     uc_engine *uc;
     uc_hook hk;
@@ -1513,11 +1602,6 @@ static void test_x86_64_not_overwriting_tmp0_for_pc_update()
     OK(uc_close(uc));
 }
 
-#define MEM_BASE 0x40000000
-#define MEM_SIZE 1024 * 1024
-#define MEM_STACK MEM_BASE + (MEM_SIZE / 2)
-#define MEM_TEXT MEM_STACK + 4096
-
 static void test_fxsave_fpip_x86(void)
 {
     // note: fxsave was introduced in Pentium II
@@ -1590,6 +1674,110 @@ static void test_fxsave_fpip_x64(void)
     OK(uc_close(uc));
 }
 
+static void test_bswap_ax(void)
+{
+    // References:
+    // - https://gynvael.coldwind.pl/?id=268
+    // - https://github.com/JonathanSalwan/Triton/issues/1131
+    {
+        uint8_t code[] = {
+            // bswap ax
+            0x66, 0x0F, 0xC8,
+        };
+        TEST_CODE(UC_MODE_32, code);
+        TEST_IN_REG(EAX, 0x44332211);
+        TEST_OUT_REG(EAX, 0x44330000);
+        TEST_RUN();
+    }
+    {
+        uint8_t code[] = {
+            // bswap ax
+            0x66, 0x0F, 0xC8,
+        };
+        TEST_CODE(UC_MODE_64, code);
+        TEST_IN_REG(RAX, 0x8877665544332211);
+        TEST_OUT_REG(RAX, 0x8877665544330000);
+        TEST_RUN();
+    }
+    {
+        uint8_t code[] = {
+            // bswap rax (66h ignored)
+            0x66, 0x48, 0x0F, 0xC8,
+        };
+        TEST_CODE(UC_MODE_64, code);
+        TEST_IN_REG(RAX, 0x8877665544332211);
+        TEST_OUT_REG(RAX, 0x1122334455667788);
+        TEST_RUN();
+    }
+    {
+        uint8_t code[] = {
+            // bswap ax (rex ignored)
+            0x48, 0x66, 0x0F, 0xC8,
+        };
+        TEST_CODE(UC_MODE_64, code);
+        TEST_IN_REG(RAX, 0x8877665544332211);
+        TEST_OUT_REG(RAX, 0x8877665544330000);
+        TEST_RUN();
+    }
+    {
+        uint8_t code[] = {
+            // bswap eax
+            0x0F, 0xC8,
+        };
+        TEST_CODE(UC_MODE_32, code);
+        TEST_IN_REG(EAX, 0x44332211);
+        TEST_OUT_REG(EAX, 0x11223344);
+        TEST_RUN();
+    }
+    {
+        uint8_t code[] = {
+            // bswap eax
+            0x0F, 0xC8,
+        };
+        TEST_CODE(UC_MODE_64, code);
+        TEST_IN_REG(RAX, 0x8877665544332211);
+        TEST_OUT_REG(RAX, 0x0000000011223344);
+        TEST_RUN();
+    }
+}
+
+static void test_rex_x64(void)
+{
+    {
+        uint8_t code[] = {
+            // mov ax, bx (rex.w ignored)
+            0x48, 0x66, 0x89, 0xD8,
+        };
+        TEST_CODE(UC_MODE_64, code);
+        TEST_IN_REG(RAX, 0x8877665544332211);
+        TEST_IN_REG(RBX, 0x1122334455667788);
+        TEST_OUT_REG(RAX, 0x8877665544337788);
+        TEST_RUN();
+    }
+    {
+        uint8_t code[] = {
+            // mov rax, rbx (66h ignored)
+            0x66, 0x48, 0x89, 0xD8,
+        };
+        TEST_CODE(UC_MODE_64, code);
+        TEST_IN_REG(RAX, 0x8877665544332211);
+        TEST_IN_REG(RBX, 0x1122334455667788);
+        TEST_OUT_REG(RAX, 0x1122334455667788);
+        TEST_RUN();
+    }
+    {
+        uint8_t code[] = {
+            // mov ax, bx (expected encoding)
+            0x66, 0x89, 0xD8,
+        };
+        TEST_CODE(UC_MODE_64, code);
+        TEST_IN_REG(RAX, 0x8877665544332211);
+        TEST_IN_REG(RBX, 0x1122334455667788);
+        TEST_OUT_REG(RAX, 0x8877665544337788);
+        TEST_RUN();
+    }
+}
+
 TEST_LIST = {
     {"test_x86_in", test_x86_in},
     {"test_x86_out", test_x86_out},
@@ -1641,4 +1829,6 @@ TEST_LIST = {
      test_x86_64_not_overwriting_tmp0_for_pc_update},
     {"test_fxsave_fpip_x86", test_fxsave_fpip_x86},
     {"test_fxsave_fpip_x64", test_fxsave_fpip_x64},
+    {"test_bswap_x64", test_bswap_ax},
+    {"test_rex_x64", test_rex_x64},
     {NULL, NULL}};