public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: "GDB" <gdb-patches@sourceware.org>
Subject: [PATCH] x86-64: fix displaced stepping for VEX, XOP, and EVEX encoded insns
Date: Thu, 07 Feb 2019 14:10:00 -0000	[thread overview]
Message-ID: <5C5C3C360200007800214AC3@prv1-mh.provo.novell.com> (raw)

Improper decoding has lead to wrong use of onebyte_has_modrm[] in all
those cases, leading to misbehavior in the debuggee (wrong data accessed
or faults raised) when RIP-relative operands were used. Fix VEX handling
and add XOP and EVEX recognition.

I apologize for this not being accompanied by a testsuite extension, but
it was hard enough already to find a couple of hours to investigate and
fix the actual issue here.

(For XOP there may be another problem with the immediates two of the
forms use. I didn't look into that aspect yet.)

gdb/
2019-02-07  Jan Beulich  <jbeulich@suse.com>

	* amd64-tdep.c (xop_prefix_p, evex_prefix_p): New.
	(fixup_riprel): Use them.
	(amd64_get_insn_details): Likewise. Handle VEX/XOP/EVEX cases
	independent of legacy encoding ones.

--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1147,6 +1147,22 @@ vex3_prefix_p (gdb_byte pfx)
   return pfx == 0xc4;
 }
 
+/* True if PFX is the start of an XOP prefix.  */
+
+static bool
+xop_prefix_p (const gdb_byte *pfx)
+{
+  return pfx[0] == 0x8f && (pfx[1] & 0x38);
+}
+
+/* True if PFX is the start of an EVEX prefix.  */
+
+static bool
+evex_prefix_p (const gdb_byte *pfx)
+{
+  return pfx[0] == 0x62 && !(pfx[1] & 0x0c) && (pfx[2] & 4);
+}
+
 /* Skip the legacy instruction prefixes in INSN.
    We assume INSN is properly sentineled so we don't have to worry
    about falling off the end of the buffer.  */
@@ -1260,11 +1276,11 @@ static void
 amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details)
 {
   gdb_byte *start = insn;
-  int need_modrm;
+  int need_modrm = 0;
 
   details->raw_insn = insn;
 
-  details->opcode_len = -1;
+  details->opcode_len = 1;
   details->enc_prefix_offset = -1;
   details->opcode_offset = -1;
   details->modrm_offset = -1;
@@ -1283,16 +1299,35 @@ amd64_get_insn_details (gdb_byte *insn,
       /* Don't record the offset in this case because this prefix has
 	 no REX.B equivalent.  */
       insn += 2;
+      need_modrm = -1;
     }
   else if (vex3_prefix_p (*insn))
     {
       details->enc_prefix_offset = insn - start;
+      need_modrm = (insn[1] & 0x1f) == 1 ? -1 : 1;
+      insn += 3;
+    }
+  else if (xop_prefix_p (insn))
+    {
+      details->enc_prefix_offset = insn - start;
       insn += 3;
+      need_modrm = 1;
+    }
+  else if (evex_prefix_p (insn))
+    {
+      details->enc_prefix_offset = insn - start;
+      need_modrm = (insn[1] & 3) == 1 ? -1 : 1;
+      insn += 4;
     }
 
   details->opcode_offset = insn - start;
 
-  if (*insn == TWO_BYTE_OPCODE_ESCAPE)
+  if (need_modrm < 0)
+    {
+      /* VEX/EVEX-encoded would-be-two-byte opcode.  */
+      need_modrm = twobyte_has_modrm[*insn];
+    }
+  else if (!need_modrm && *insn == TWO_BYTE_OPCODE_ESCAPE)
     {
       /* Two or three-byte opcode.  */
       ++insn;
@@ -1315,11 +1350,10 @@ amd64_get_insn_details (gdb_byte *insn,
 	  break;
 	}
     }
-  else
+  else if (!need_modrm)
     {
       /* One-byte opcode.  */
       need_modrm = onebyte_has_modrm[*insn];
-      details->opcode_len = 1;
     }
 
   if (need_modrm)
@@ -1363,7 +1397,8 @@ fixup_riprel (struct gdbarch *gdbarch, a
   arch_tmp_regno = amd64_get_unused_input_int_reg (insn_details);
   tmp_regno = amd64_arch_reg_to_regnum (arch_tmp_regno);
 
-  /* Position of the not-B bit in the 3-byte VEX prefix (in byte 1).  */
+  /* Position of the not-B bit in the 3-byte VEX prefix, the EVEX prefix,
+     and the XOP prefix (always in byte 1).  */
   static constexpr gdb_byte VEX3_NOT_B = 0x20;
 
   /* REX.B should be unset (VEX.!B set) as we were using rip-relative
@@ -1374,7 +1409,8 @@ fixup_riprel (struct gdbarch *gdbarch, a
       gdb_byte *pfx = &dsc->insn_buf[insn_details->enc_prefix_offset];
       if (rex_prefix_p (pfx[0]))
 	pfx[0] &= ~REX_B;
-      else if (vex3_prefix_p (pfx[0]))
+      else if (vex3_prefix_p (pfx[0]) || xop_prefix_p (pfx)
+	       || evex_prefix_p (pfx))
 	pfx[1] |= VEX3_NOT_B;
       else
 	gdb_assert_not_reached ("unhandled prefix");



             reply	other threads:[~2019-02-07 14:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-07 14:10 Jan Beulich [this message]
2019-02-07 16:09 ` Pedro Alves
2019-02-08  9:49   ` Jan Beulich
2019-02-08 12:40     ` Pedro Alves
2019-02-26 12:26 ` Ping: " Jan Beulich
2019-02-26 17:13   ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5C5C3C360200007800214AC3@prv1-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).