[gdb/tdep] Fix vmovdqu decoding

PR tdep/31952 reports that displaced stepping over an instruction pointer
relative insn "vmovdqu 0x20(%rip),%ymm1" gives the wrong results.

This is caused by misclassification of the insn in amd64_get_insn_details,
which results in details.modrm_offset == -1, while the instruction in fact
does have a modrm byte.

The instruction is encoded as follows:
...
  400557:       c5 fe 6f 0d 20 00 00 00    vmovdqu 0x20(%rip),%ymm1
...
where:
- "0xc5 0xfe" is the vex2 prefix,
- "0x6f" is the opcode,
- "0x0d" is the modrm byte, and
- "0x20 0x00 0x00 0x00" is a 32-bit displacement.

The problem is related to details.opcode_len, which is 1.

While it is true that the length of the opcode in the insn (0x6f) is 1 byte,
the vex2 prefix implies that we're encoding an 2-byte opcode beginnning
with 0x0f [1].

Consequently, we should be using the twobyte_has_modrm map rather than the
onebyte_has_modrm map.

Fix this in amd64_get_insn_details, and add a selftest to check this.

Tested on x86_64-linux.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31952

[1] https://en.wikipedia.org/wiki/VEX_prefix

1 files changed, 77 insertions, 6 deletions

diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index a01b97b..e6fedec 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1336,10 +1336,49 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details)
       details->enc_prefix_offset = insn - start;
       insn += 3;
     }
+  gdb_byte *prefix = (details->enc_prefix_offset == -1
+		      ? nullptr
+		      : &start[details->enc_prefix_offset]);
 
   details->opcode_offset = insn - start;
 
-  if (*insn == TWO_BYTE_OPCODE_ESCAPE)
+  if (prefix != nullptr && vex2_prefix_p (*prefix))
+    {
+      need_modrm = twobyte_has_modrm[*insn];
+      details->opcode_len = 2;
+    }
+  else if (prefix != nullptr && vex3_prefix_p (*prefix))
+    {
+      need_modrm = twobyte_has_modrm[*insn];
+
+      gdb_byte m = prefix[1] & 0x1f;
+      if (m == 0)
+	{
+	  /* Todo: Xeon Phi-specific JKZD/JKNZD.  */
+	  return;
+	}
+      else if (m == 1)
+	{
+	  /* Escape 0x0f.  */
+	  details->opcode_len = 2;
+	}
+      else if (m == 2 || m == 3)
+	{
+	  /* Escape 0x0f 0x38 or 0x0f 0x3a.  */
+	  details->opcode_len = 3;
+	}
+      else if (m == 7)
+	{
+	  /* Todo: URDMSR/UWRMSR instructions.  */
+	  return;
+	}
+      else
+	{
+	  /* Unknown opcode map.  */
+	  return;
+	}
+    }
+  else if (*insn == TWO_BYTE_OPCODE_ESCAPE)
     {
       /* Two or three-byte opcode.  */
       ++insn;
@@ -1513,6 +1552,8 @@ amd64_displaced_step_copy_insn (struct gdbarch *gdbarch,
   memset (buf + len, 0, fixup_sentinel_space);
 
   amd64_get_insn_details (buf, details);
+  if (details->opcode_len == -1)
+    return nullptr;
 
   /* GDB may get control back after the insn after the syscall.
      Presumably this is a kernel bug.
@@ -3475,7 +3516,7 @@ static void
 test_amd64_get_insn_details (void)
 {
   struct amd64_insn details;
-  gdb::byte_vector insn;
+  gdb::byte_vector insn, tmp;
 
   /* INSN: add %eax,(%rcx).  */
   insn = { 0x01, 0x01 };
@@ -3521,7 +3562,7 @@ test_amd64_get_insn_details (void)
   /* INSN: vzeroall, vex2 prefix.  */
   vex2 = { 0xc5, 0xfc, 0x77 };
   amd64_get_insn_details (vex2.data (), &details);
-  SELF_CHECK (details.opcode_len == 1);
+  SELF_CHECK (details.opcode_len == 2);
   SELF_CHECK (details.enc_prefix_offset == 0);
   SELF_CHECK (details.opcode_offset == 2);
   SELF_CHECK (details.modrm_offset == -1);
@@ -3529,7 +3570,7 @@ test_amd64_get_insn_details (void)
   /* INSN: vzeroall, vex3 prefix.  */
   vex2_to_vex3 (vex2, vex3);
   amd64_get_insn_details (vex3.data (), &details);
-  SELF_CHECK (details.opcode_len == 1);
+  SELF_CHECK (details.opcode_len == 2);
   SELF_CHECK (details.enc_prefix_offset == 0);
   SELF_CHECK (details.opcode_offset == 3);
   SELF_CHECK (details.modrm_offset == -1);
@@ -3537,7 +3578,7 @@ test_amd64_get_insn_details (void)
   /* INSN: vzeroupper, vex2 prefix.  */
   vex2 = { 0xc5, 0xf8, 0x77 };
   amd64_get_insn_details (vex2.data (), &details);
-  SELF_CHECK (details.opcode_len == 1);
+  SELF_CHECK (details.opcode_len == 2);
   SELF_CHECK (details.enc_prefix_offset == 0);
   SELF_CHECK (details.opcode_offset == 2);
   SELF_CHECK (details.modrm_offset == -1);
@@ -3545,10 +3586,40 @@ test_amd64_get_insn_details (void)
   /* INSN: vzeroupper, vex3 prefix.  */
   vex2_to_vex3 (vex2, vex3);
   amd64_get_insn_details (vex3.data (), &details);
-  SELF_CHECK (details.opcode_len == 1);
+  SELF_CHECK (details.opcode_len == 2);
   SELF_CHECK (details.enc_prefix_offset == 0);
   SELF_CHECK (details.opcode_offset == 3);
   SELF_CHECK (details.modrm_offset == -1);
+
+  /* INSN: vmovdqu 0x9(%rip),%ymm3, vex2 prefix.  */
+  vex2 = { 0xc5, 0xfe, 0x6f, 0x1d, 0x09, 0x00, 0x00, 0x00 };
+  amd64_get_insn_details (vex2.data (), &details);
+  SELF_CHECK (details.opcode_len == 2);
+  SELF_CHECK (details.enc_prefix_offset == 0);
+  SELF_CHECK (details.opcode_offset == 2);
+  SELF_CHECK (details.modrm_offset == 3);
+
+  /* INSN: vmovdqu 0x9(%rcx),%ymm3, vex2 prefix.  */
+  gdb::byte_vector updated_vex2
+    = { 0xc5, 0xfe, 0x6f, 0x99, 0x09, 0x00, 0x00, 0x00 };
+  tmp = vex2;
+  fixup_riprel (details, tmp.data (), ECX_REG_NUM);
+  SELF_CHECK (tmp == updated_vex2);
+
+  /* INSN: vmovdqu 0x9(%rip),%ymm3, vex3 prefix.  */
+  vex2_to_vex3 (vex2, vex3);
+  amd64_get_insn_details (vex3.data (), &details);
+  SELF_CHECK (details.opcode_len == 2);
+  SELF_CHECK (details.enc_prefix_offset == 0);
+  SELF_CHECK (details.opcode_offset == 3);
+  SELF_CHECK (details.modrm_offset == 4);
+
+  /* INSN: vmovdqu 0x9(%rcx),%ymm3, vex3 prefix.  */
+  gdb::byte_vector updated_vex3;
+  vex2_to_vex3 (updated_vex2, updated_vex3);
+  tmp = vex3;
+  fixup_riprel (details, tmp.data (), ECX_REG_NUM);
+  SELF_CHECK (tmp == updated_vex3);
 }
 
 static void