Fix displaced-stepping RIP-relative VEX-encoded instructions (AVX) (PR gdb/22499)

PR gdb/22499 is about a latent bug exposed by the switch to "maint set
target-non-stop on" by default on x86-64 GNU/Linux, a while ago.  With
that on, GDB is also preferring to use displaced-stepping by default.

The testcase in the bug is failing because GDB ends up incorrectly
displaced-stepping over a RIP-relative VEX-encoded instruction, like
this:

 0x00000000004007f5 <+15>:    c5 fb 10 05 8b 01 00 00 vmovsd 0x18b(%rip),%xmm0        # 0x400988

While RIP-relative instructions need adjustment when relocated to the
scratch pad, GDB ends up just copying VEX-encoded instructions to the
scratch pad unmodified, with the end result that the inferior ends up
executing an instruction that fetches/writes memory from the wrong
address...

This patch teaches GDB about the VEX-encoding prefixes, fixing the
problem, and adds a testcase that fails without the GDB fix.

I think we may need a similar treatment for EVEX-encoded instructions,
but I didn't address that simply because I couldn't find any
EVEX-encoded RIP-relative instruction in the gas testsuite.  In any
case, this commit is forward progress as-is already.

gdb/ChangeLog:
2017-12-04  Pedro Alves  <palves@redhat.com>

	PR gdb/22499
	* amd64-tdep.c (amd64_insn::rex_offset): Rename to...
	(amd64_insn::enc_prefix_offset): ... this, and tweak comment.
	(vex2_prefix_p, vex3_prefix_p): New functions.
	(amd64_get_insn_details): Adjust to rename.  Also skip VEX2 and
	VEX3 prefixes.
	(fixup_riprel): Set VEX3.!B.

gdb/testsuite/ChangeLog:
2017-12-04  Pedro Alves  <palves@redhat.com>

	PR gdb/22499
	* gdb.arch/amd64-disp-step-avx.S: New file.
	* gdb.arch/amd64-disp-step-avx.exp: New file.

5 files changed, 274 insertions, 9 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d1704c0..84f54a9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,13 @@
+2017-12-04  Pedro Alves  <palves@redhat.com>
+
+	PR gdb/22499
+	* amd64-tdep.c (amd64_insn::rex_offset): Rename to...
+	(amd64_insn::enc_prefix_offset): ... this, and tweak comment.
+	(vex2_prefix_p, vex3_prefix_p): New functions.
+	(amd64_get_insn_details): Adjust to rename.  Also skip VEX2 and
+	VEX3 prefixes.
+	(fixup_riprel): Set VEX3.!B.
+
 2017-12-03  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* target.h (mem_region_vector): Remove.
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index bcd37ef..0eb3670 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -1037,8 +1037,9 @@ struct amd64_insn
 {
   /* The number of opcode bytes.  */
   int opcode_len;
-  /* The offset of the rex prefix or -1 if not present.  */
-  int rex_offset;
+  /* The offset of the REX/VEX instruction encoding prefix or -1 if
+     not present.  */
+  int enc_prefix_offset;
   /* The offset to the first opcode byte.  */
   int opcode_offset;
   /* The offset to the modrm byte or -1 if not present.  */
@@ -1124,6 +1125,22 @@ rex_prefix_p (gdb_byte pfx)
   return REX_PREFIX_P (pfx);
 }
 
+/* True if PFX is the start of the 2-byte VEX prefix.  */
+
+static bool
+vex2_prefix_p (gdb_byte pfx)
+{
+  return pfx == 0xc5;
+}
+
+/* True if PFX is the start of the 3-byte VEX prefix.  */
+
+static bool
+vex3_prefix_p (gdb_byte pfx)
+{
+  return pfx == 0xc4;
+}
+
 /* 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.  */
@@ -1242,19 +1259,30 @@ amd64_get_insn_details (gdb_byte *insn, struct amd64_insn *details)
   details->raw_insn = insn;
 
   details->opcode_len = -1;
-  details->rex_offset = -1;
+  details->enc_prefix_offset = -1;
   details->opcode_offset = -1;
   details->modrm_offset = -1;
 
   /* Skip legacy instruction prefixes.  */
   insn = amd64_skip_prefixes (insn);
 
-  /* Skip REX instruction prefix.  */
+  /* Skip REX/VEX instruction encoding prefixes.  */
   if (rex_prefix_p (*insn))
     {
-      details->rex_offset = insn - start;
+      details->enc_prefix_offset = insn - start;
       ++insn;
     }
+  else if (vex2_prefix_p (*insn))
+    {
+      /* Don't record the offset in this case because this prefix has
+	 no REX.B equivalent.  */
+      insn += 2;
+    }
+  else if (vex3_prefix_p (*insn))
+    {
+      details->enc_prefix_offset = insn - start;
+      insn += 3;
+    }
 
   details->opcode_offset = insn - start;
 
@@ -1329,10 +1357,22 @@ fixup_riprel (struct gdbarch *gdbarch, amd64_displaced_step_closure *dsc,
   arch_tmp_regno = amd64_get_unused_input_int_reg (insn_details);
   tmp_regno = amd64_arch_reg_to_regnum (arch_tmp_regno);
 
-  /* REX.B should be unset as we were using rip-relative addressing,
-     but ensure it's unset anyway, tmp_regno is not r8-r15.  */
-  if (insn_details->rex_offset != -1)
-    dsc->insn_buf[insn_details->rex_offset] &= ~REX_B;
+  /* Position of the not-B bit in the 3-byte VEX prefix (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
+     addressing, but ensure it's unset (set for VEX) anyway, tmp_regno
+     is not r8-r15.  */
+  if (insn_details->enc_prefix_offset != -1)
+    {
+      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]))
+	pfx[1] |= VEX3_NOT_B;
+      else
+	gdb_assert_not_reached ("unhandled prefix");
+    }
 
   regcache_cooked_read_unsigned (regs, tmp_regno, &orig_value);
   dsc->tmp_regno = tmp_regno;
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index fa096b3..30ab10c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2017-12-04  Pedro Alves  <palves@redhat.com>
+
+	PR gdb/22499
+	* gdb.arch/amd64-disp-step-avx.S: New file.
+	* gdb.arch/amd64-disp-step-avx.exp: New file.
+
 2017-12-03  Pedro Alves  <palves@redhat.com>
 
 	* gdb.threads/process-dies-while-detaching.c: Include <errno.h>
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S
new file mode 100644
index 0000000..5677717
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S
@@ -0,0 +1,70 @@
+/* Copyright 2009-2017 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   This file is part of the gdb testsuite.
+
+   Test displaced stepping over VEX-encoded RIP-relative AVX
+   instructions.  */
+
+	.text
+
+	.global main
+main:
+	nop
+
+/***********************************************/
+
+/* Test a VEX2-encoded RIP-relative instruction.  */
+
+	.global test_rip_vex2
+test_rip_vex2:
+	vmovsd ro_var(%rip),%xmm0
+	.global test_rip_vex2
+test_rip_vex2_end:
+	nop
+
+/* Test a VEX3-encoded RIP-relative instruction.  */
+
+	.global test_rip_vex3
+test_rip_vex3:
+	vextractf128 $0x0,%ymm0,var128(%rip)
+	.global test_rip_vex3
+test_rip_vex3_end:
+	nop
+
+	/* skip over test data */
+	jmp done
+
+/* RIP-relative ro-data for VEX2 test above.  */
+
+ro_var:
+	.8byte 0x1122334455667788
+	.8byte 0x8877665544332211
+
+/***********************************************/
+
+/* All done.  */
+
+done:
+	mov $0,%rdi
+	call exit
+	hlt
+
+/* RIP-relative data for VEX3 test above.  */
+
+.data
+var128:
+	.8byte 0xaa55aa55aa55aa55
+	.8byte 0x55aa55aa55aa55aa
diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp
new file mode 100644
index 0000000..b2e13d1
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp
@@ -0,0 +1,139 @@
+# Copyright 2009-2017 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test displaced stepping over VEX-encoded RIP-relative AVX
+# instructions.
+
+if { ![istarget x86_64-*-* ] || ![is_lp64_target] } {
+    verbose "Skipping x86_64 displaced stepping tests."
+    return
+}
+
+standard_testfile .S
+
+set additional_flags "-Wa,-g"
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+	  [list debug $additional_flags]] } {
+    return -1
+}
+
+# Get things started.
+
+gdb_test "set displaced-stepping on" ""
+gdb_test "show displaced-stepping" ".* displaced stepping .* is on.*"
+
+if ![runto_main] then {
+    fail "can't run to main"
+    return 0
+}
+
+# GDB picks a spare register from this list to hold the RIP-relative
+# address.
+set rip_regs { "rax" "rbx" "rcx" "rdx" "rbp" "rsi" "rdi" }
+
+# Assign VAL to all the RIP_REGS.
+
+proc set_regs { val } {
+    global gdb_prompt
+    global rip_regs
+
+    foreach reg ${rip_regs} {
+	gdb_test_no_output "set \$${reg} = ${val}"
+    }
+}
+
+# Verify all RIP_REGS print as HEX_VAL_RE in hex.
+
+proc verify_regs { hex_val_re } {
+    global rip_regs
+
+    foreach reg ${rip_regs} {
+	gdb_test "p /x \$${reg}" " = ${hex_val_re}" "${reg} expected value"
+    }
+}
+
+# Set a break at FUNC, which starts with a RIP-relative instruction
+# that we want to displaced-step over, and then continue over the
+# breakpoint, forcing a displaced-stepping sequence.
+
+proc disp_step_func { func } {
+    global srcfile
+
+    set test_start_label "${func}"
+    set test_end_label "${func}_end"
+
+    gdb_test "break ${test_start_label}" \
+	"Breakpoint.*at.* file .*$srcfile, line.*" \
+	"break ${test_start_label}"
+    gdb_test "break ${test_end_label}" \
+	"Breakpoint.*at.* file .*$srcfile, line.*" \
+	"break ${test_end_label}"
+
+    gdb_test "continue" \
+	"Continuing.*Breakpoint.*, ${test_start_label} ().*" \
+	"continue to ${test_start_label}"
+
+    # GDB picks a spare register to hold the RIP-relative address.
+    # Ensure the spare register value is restored properly (rax-rdi,
+    # sans rsp).
+    set value "0xdeadbeefd3adb33f"
+    set_regs $value
+
+    gdb_test "continue" \
+	"Continuing.*Breakpoint.*, ${test_end_label} ().*" \
+	"continue to ${test_end_label}"
+
+    verify_regs $value
+}
+
+# Test a VEX2-encoded RIP-relative instruction.
+with_test_prefix "vex2" {
+    # This case writes to the 'xmm0' register.  Confirm the register's
+    # value is what we believe it is before the AVX instruction runs.
+    gdb_test "p /x \$xmm0.uint128" " = 0x0" \
+	"xmm0 has expected value before"
+
+    disp_step_func "test_rip_vex2"
+
+    # Confirm the instruction's expected side effects.  It should have
+    # modified xmm0.
+    gdb_test "p /x \$xmm0.uint128" " = 0x1122334455667788" \
+	"xmm0 has expected value after"
+}
+
+# Test a VEX3-encoded RIP-relative instruction.
+with_test_prefix "vex3" {
+    # This case writes to the 'var128' variable.  Confirm the
+    # variable's value is what we believe it is before the AVX
+    # instruction runs.
+    gdb_test "p /x (unsigned long long \[2\]) var128" \
+	" = \\{0xaa55aa55aa55aa55, 0x55aa55aa55aa55aa\\}" \
+	"var128 has expected value before"
+
+    # Run the AVX instruction.
+    disp_step_func "test_rip_vex3"
+
+    # Confirm the instruction's expected side effects.  It should have
+    # modifed the 'var128' variable.
+    gdb_test "p /x (unsigned long long \[2\]) var128" \
+	" = \\{0x1122334455667788, 0x0\\}" \
+	"var128 has expected value after"
+}
+
+# Done, run program to exit.
+gdb_continue_to_end "amd64-disp-step-avx"