public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/3] Generalize gdb.reverse/aarch64.exp
  2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
  2016-02-22 16:53 ` [PATCH 2/3] Record right reg num of thumb special data instructions Yao Qi
  2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi
@ 2016-02-22 16:53 ` Yao Qi
  2016-02-24 11:22   ` Pedro Alves
  2016-02-26 15:01 ` [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
  3 siblings, 1 reply; 11+ messages in thread
From: Yao Qi @ 2016-02-22 16:53 UTC (permalink / raw)
  To: gdb-patches

I said we can generialize gdb.reverse/aarch64.exp for other
architectures https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html
and here is the patch to change aarch64.exp to insn-reverse.exp.

gdb/testsuite:

2016-02-22  Yao Qi  <yao.qi@linaro.org>

	* gdb.reverse/aarch64.c: Rename it ...
	* gdb.reverse/insn-reverse.c: to ...
	[__aarch64__] Include arm_neon.h.
	(testcase_ftype): New.
	(testcases): New array.
	(n_testcases): New.
	(main): Call each element in testcases.
	* gdb.reverse/aarch64.exp: Rename it ...
	* gdb.reverse/insn-reverse.exp: to ...
	Remove is_aarch64_target check.
	(read_testcase): New.
	Do the tests in a loop.
---
 gdb/testsuite/gdb.reverse/aarch64.c        |  99 ----------------------
 gdb/testsuite/gdb.reverse/aarch64.exp      | 115 -------------------------
 gdb/testsuite/gdb.reverse/insn-reverse.c   | 121 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.reverse/insn-reverse.exp | 130 +++++++++++++++++++++++++++++
 4 files changed, 251 insertions(+), 214 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.c
 delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.exp
 create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.exp

diff --git a/gdb/testsuite/gdb.reverse/aarch64.c b/gdb/testsuite/gdb.reverse/aarch64.c
deleted file mode 100644
index ae8509a..0000000
--- a/gdb/testsuite/gdb.reverse/aarch64.c
+++ /dev/null
@@ -1,99 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2015-2016 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/>.  */
-
-#include <arm_neon.h>
-
-static void
-load (void)
-{
-  int buf[8];
-
-  asm ("ld1 { v1.8b }, [%[buf]]\n"
-       "ld1 { v2.8b, v3.8b }, [%[buf]]\n"
-       "ld1 { v3.8b, v4.8b, v5.8b }, [%[buf]]\n"
-       :
-       : [buf] "r" (buf)
-       : /* No clobbers */);
-}
-
-static void
-move (void)
-{
-  float32x2_t b1_ = vdup_n_f32(123.0f);
-  float32_t a1_ = 0;
-  float64x1_t b2_ = vdup_n_f64(456.0f);
-  float64_t a2_ = 0;
-
-  asm ("ins %0.s[0], %w1\n"
-       : "=w"(b1_)
-       : "r"(a1_), "0"(b1_)
-       : /* No clobbers */);
-
-  asm ("ins %0.d[1], %x1\n"
-       : "=w"(b2_)
-       : "r"(a2_), "0"(b2_)
-       : /* No clobbers */);
-}
-
-static void
-adv_simd_mod_imm (void)
-{
-  float32x2_t a1 = {2.0, 4.0};
-
-  asm ("bic %0.2s, #1\n"
-       "bic %0.2s, #1, lsl #8\n"
-       : "=w"(a1)
-       : "0"(a1)
-       : /* No clobbers */);
-}
-
-static void
-adv_simd_scalar_index (void)
-{
-  float64x2_t b_ = {0.0, 0.0};
-  float64_t a_ = 1.0;
-  float64_t result;
-
-  asm ("fmla %d0,%d1,%2.d[1]"
-       : "=w"(result)
-       : "w"(a_), "w"(b_)
-       : /* No clobbers */);
-}
-
-static void
-adv_simd_smlal (void)
-{
-  asm ("smlal v13.2d, v8.2s, v0.2s");
-}
-
-static void
-adv_simd_vect_shift (void)
-{
-  asm ("fcvtzs s0, s0, #1");
-}
-
-int
-main ()
-{
-  load ();
-  move ();
-  adv_simd_mod_imm ();
-  adv_simd_scalar_index ();
-  adv_simd_smlal ();
-  adv_simd_vect_shift ();
-  return 0;
-}
diff --git a/gdb/testsuite/gdb.reverse/aarch64.exp b/gdb/testsuite/gdb.reverse/aarch64.exp
deleted file mode 100644
index 2906d4b..0000000
--- a/gdb/testsuite/gdb.reverse/aarch64.exp
+++ /dev/null
@@ -1,115 +0,0 @@
-# Copyright (C) 2015-2016 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/>.
-
-if ![supports_reverse] {
-    return
-}
-
-# Test aarch64 instruction recording.
-
-if {![is_aarch64_target]} then {
-    verbose "Skipping aarch64 instruction recording tests."
-    return
-}
-
-standard_testfile
-
-if {[prepare_for_testing $testfile.exp $testfile $srcfile \
-	 [list debug]]} {
-    untested ${testfile}.exp
-    return -1
-}
-if { ![runto main] } then {
-    fail "run to main"
-    return
-}
-
-# In each function FUNC, GDB turns on process record, and single step
-# until program goes to the end of the function.  Then, single step
-# backward.  In each of forward single step and backward single step,
-# the contents of registers are saved, and test compares them.  If
-# there is any differences, a FAIL is emitted.
-
-proc test { func } {
-    global hex decimal
-    global gdb_prompt
-
-    with_test_prefix "$func" {
-	gdb_breakpoint $func
-	gdb_test "continue"
-
-	set last_insn ""
-	set test "disassemble $func"
-	gdb_test_multiple $test $test {
-	    -re ".*($hex) <\\+$decimal>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" {
-		set last_insn $expect_out(1,string)
-	    }
-	}
-	if { $last_insn == "" } {
-	    fail "find the last instruction of function $func"
-	}
-
-	# Activate process record/replay
-	gdb_test_no_output "record" "Turn on process record"
-
-	# Registers contents before each forward single step.
-	set count 0
-	for {} {$count < 500} {incr count} {
-	    gdb_test_multiple "x/i \$pc" "" {
-		-re ".* ($hex) <.*>:\[ \t\]*(.*)\r\n$gdb_prompt $" {
-		    set insn_addr $expect_out(1,string)
-
-		    if [expr {$last_insn == $insn_addr}] {
-			break
-		    }
-
-		    set insn_array($count) $expect_out(2,string)
-		}
-	    }
-
-	    set pre_regs($count) [capture_command_output "info all-registers" ""]
-	    gdb_test "si" "" ""
-	}
-
-	incr count -1
-	# Registers contents after each backward single step.
-	for {set i $count} {$i >= 0} {incr i -1} {
-	    gdb_test "reverse-stepi" "" ""
-	    set post_regs($i) [capture_command_output "info all-registers" ""]
-	}
-
-	# Compare the register contents.
-	for {set i 0} {$i < $count} {incr i} {
-	    if { ![gdb_assert { [string compare $pre_regs($i) $post_regs($i)] == 0 } \
-		      "compare registers on insn $i:$insn_array($i)"] } {
-
-		foreach pre_line [split $pre_regs($i) \n] post_line [split $post_regs($i) \n] {
-		    if { [string compare $pre_line $post_line] } {
-			verbose -log " -:$pre_line"
-			verbose -log " +:$post_line"
-		    }
-		}
-	    }
-	}
-	gdb_test "record stop"
-    }
-}
-
-test "load"
-test "move"
-test "adv_simd_mod_imm"
-test "adv_simd_scalar_index"
-test "adv_simd_smlal"
-test "adv_simd_vect_shift"
diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.c b/gdb/testsuite/gdb.reverse/insn-reverse.c
new file mode 100644
index 0000000..1bfb8b0
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/insn-reverse.c
@@ -0,0 +1,121 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015-2016 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/>.  */
+
+#if (defined __aarch64__)
+#include <arm_neon.h>
+#endif
+
+#if (defined __aarch64__)
+static void
+load (void)
+{
+  int buf[8];
+
+  asm ("ld1 { v1.8b }, [%[buf]]\n"
+       "ld1 { v2.8b, v3.8b }, [%[buf]]\n"
+       "ld1 { v3.8b, v4.8b, v5.8b }, [%[buf]]\n"
+       :
+       : [buf] "r" (buf)
+       : /* No clobbers */);
+}
+
+static void
+move (void)
+{
+  float32x2_t b1_ = vdup_n_f32(123.0f);
+  float32_t a1_ = 0;
+  float64x1_t b2_ = vdup_n_f64(456.0f);
+  float64_t a2_ = 0;
+
+  asm ("ins %0.s[0], %w1\n"
+       : "=w"(b1_)
+       : "r"(a1_), "0"(b1_)
+       : /* No clobbers */);
+
+  asm ("ins %0.d[1], %x1\n"
+       : "=w"(b2_)
+       : "r"(a2_), "0"(b2_)
+       : /* No clobbers */);
+}
+
+static void
+adv_simd_mod_imm (void)
+{
+  float32x2_t a1 = {2.0, 4.0};
+
+  asm ("bic %0.2s, #1\n"
+       "bic %0.2s, #1, lsl #8\n"
+       : "=w"(a1)
+       : "0"(a1)
+       : /* No clobbers */);
+}
+
+static void
+adv_simd_scalar_index (void)
+{
+  float64x2_t b_ = {0.0, 0.0};
+  float64_t a_ = 1.0;
+  float64_t result;
+
+  asm ("fmla %d0,%d1,%2.d[1]"
+       : "=w"(result)
+       : "w"(a_), "w"(b_)
+       : /* No clobbers */);
+}
+
+static void
+adv_simd_smlal (void)
+{
+  asm ("smlal v13.2d, v8.2s, v0.2s");
+}
+
+static void
+adv_simd_vect_shift (void)
+{
+  asm ("fcvtzs s0, s0, #1");
+}
+#endif
+
+typedef void (*testcase_ftype) (void);
+
+/* Functions testing instruction decodings.  GDB will read n_testcases
+   to know how many functions to test.  */
+
+static testcase_ftype testcases[] =
+{
+#if (defined __aarch64__)
+  load,
+  move,
+  adv_simd_mod_imm,
+  adv_simd_scalar_index,
+  adv_simd_smlal,
+  adv_simd_vect_shift,
+#endif
+};
+
+static int n_testcases = (sizeof (testcases) / sizeof (testcase_ftype));
+
+int
+main ()
+{
+  int i = 0;
+
+  for (i = 0; i < n_testcases; i++)
+    testcases[i] ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.exp b/gdb/testsuite/gdb.reverse/insn-reverse.exp
new file mode 100644
index 0000000..f52b40c
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/insn-reverse.exp
@@ -0,0 +1,130 @@
+# Copyright (C) 2015-2016 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/>.
+
+if ![supports_reverse] {
+    return
+}
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile \
+	 [list debug]]} {
+    untested ${testfile}.exp
+    return -1
+}
+if { ![runto main] } then {
+    fail "run to main"
+    return
+}
+
+# Read function name from testcases[N].
+
+proc read_testcase { n } {
+    global gdb_prompt
+
+    set result -1
+    gdb_test_multiple "print testcases\[${n}\]" "read name of test case ${n}" {
+	-re "\[$\].*= .*<(.*)>.*$gdb_prompt $" {
+	    set result $expect_out(1,string)
+	}
+	-re "$gdb_prompt $" { }
+    }
+
+    return $result
+}
+
+# In each function FUNC, GDB turns on process record, and single step
+# until program goes to the end of the function.  Then, single step
+# backward.  In each of forward single step and backward single step,
+# the contents of registers are saved, and test compares them.  If
+# there is any differences, a FAIL is emitted.
+
+proc test { func } {
+    global hex decimal
+    global gdb_prompt
+
+    with_test_prefix "$func" {
+	gdb_breakpoint $func
+	gdb_test "continue"
+
+	set last_insn ""
+	set test "disassemble $func"
+	gdb_test_multiple $test $test {
+	    -re ".*($hex) <\\+$decimal>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" {
+		set last_insn $expect_out(1,string)
+	    }
+	}
+	if { $last_insn == "" } {
+	    fail "find the last instruction of function $func"
+	}
+
+	# Activate process record/replay
+	gdb_test_no_output "record" "Turn on process record"
+
+	# Registers contents before each forward single step.
+	set count 0
+	for {} {$count < 500} {incr count} {
+	    gdb_test_multiple "x/i \$pc" "" {
+		-re ".* ($hex) <.*>:\[ \t\]*(.*)\r\n$gdb_prompt $" {
+		    set insn_addr $expect_out(1,string)
+
+		    if [expr {$last_insn == $insn_addr}] {
+			break
+		    }
+
+		    set insn_array($count) $expect_out(2,string)
+		}
+	    }
+
+	    set pre_regs($count) [capture_command_output "info all-registers" ""]
+	    gdb_test "si" "" ""
+	}
+
+	incr count -1
+	# Registers contents after each backward single step.
+	for {set i $count} {$i >= 0} {incr i -1} {
+	    gdb_test "reverse-stepi" "" ""
+	    set post_regs($i) [capture_command_output "info all-registers" ""]
+	}
+
+	# Compare the register contents.
+	for {set i 0} {$i < $count} {incr i} {
+	    if { ![gdb_assert { [string compare $pre_regs($i) $post_regs($i)] == 0 } \
+		      "compare registers on insn $i:$insn_array($i)"] } {
+
+		foreach pre_line [split $pre_regs($i) \n] post_line [split $post_regs($i) \n] {
+		    if { [string compare $pre_line $post_line] } {
+			verbose -log " -:$pre_line"
+			verbose -log " +:$post_line"
+		    }
+		}
+	    }
+	}
+	gdb_test "record stop"
+    }
+}
+
+set n_testcases [get_integer_valueof "n_testcases" 0]
+
+if { ${n_testcases} == 0 } {
+    untested "No test"
+    return 1
+}
+
+for { set i 0 } { ${i} < ${n_testcases} } { incr i } {
+    set testcase [read_testcase $i]
+
+    test $testcase
+}
-- 
1.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 0/3] Bug fixes in arm reverse debugging
@ 2016-02-22 16:53 Yao Qi
  2016-02-22 16:53 ` [PATCH 2/3] Record right reg num of thumb special data instructions Yao Qi
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Yao Qi @ 2016-02-22 16:53 UTC (permalink / raw)
  To: gdb-patches

This patch series extends test case gdb.reverse/aarch64.exp for general
test (it covers aarch64 and arm, and I plan to merge i386 tests to it
too) and fix some bugs on instruction decoding exposed the new tests.

*** BLURB HERE ***

Yao Qi (3):
  Generalize gdb.reverse/aarch64.exp
  Record right reg num of thumb special data instructions
  Fix various bugs in arm_record_exreg_ld_st_insn

 gdb/arm-tdep.c                             | 101 +++++++++++-------
 gdb/testsuite/gdb.reverse/aarch64.c        |  99 -----------------
 gdb/testsuite/gdb.reverse/aarch64.exp      | 115 --------------------
 gdb/testsuite/gdb.reverse/insn-reverse.c   | 164 +++++++++++++++++++++++++++++
 gdb/testsuite/gdb.reverse/insn-reverse.exp | 130 +++++++++++++++++++++++
 5 files changed, 356 insertions(+), 253 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.c
 delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.exp
 create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.exp

-- 
1.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn
  2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
  2016-02-22 16:53 ` [PATCH 2/3] Record right reg num of thumb special data instructions Yao Qi
@ 2016-02-22 16:53 ` Yao Qi
  2016-02-23 19:59   ` Luis Machado
  2016-02-22 16:53 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Yao Qi
  2016-02-26 15:01 ` [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
  3 siblings, 1 reply; 11+ messages in thread
From: Yao Qi @ 2016-02-22 16:53 UTC (permalink / raw)
  To: gdb-patches

This patch fixes various bugs in arm_record_exreg_ld_st_insn, and use
gdb.reverse/insn-reverse.c to test more arm instructions.

 - Set flag SINGLE_REG correctly.  In the arch reference manual,
   SING_REG is true when the bit 8 of instruction is zero.
 - Record the right D registers for instructions changing S registers.
 - Fix the order of length and address in record_buf_mem array.
 - Shift the offset by 2 instead of by 24.

This patch also fixes one internal error,

(gdb) PASS: gdb.reverse/finish-precsave.exp: BP at end of main
continue^M
Continuing.^M
../../binutils-gdb/gdb/utils.c:1072: internal-error: virtual memory exhausted.^M
A problem internal to GDB has been detected,FAIL: gdb.reverse/finish-precsave.exp: run to end of main (GDB internal error)

gdb:

2016-02-22  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c (arm_record_exreg_ld_st_insn): Set 'single_reg'
	per bit 8.  Check bit 20 instead of bit 4 for VMOV
	instruction.  Record D registers for instructions changing
	S registers.  Change of the order of length and address
	in record_buf_mem array.

gdb/testsuite:

2016-02-22  Yao Qi  <yao.qi@linaro.org>

	* gdb.reverse/insn-reverse.c [__arm__] (ext_reg_load): New.
	[__arm__] (ext_reg_mov, ext_reg_push_pop): New.
	(testcases): Update.
---
 gdb/arm-tdep.c                           | 93 ++++++++++++++++++++------------
 gdb/testsuite/gdb.reverse/insn-reverse.c | 43 +++++++++++++++
 2 files changed, 101 insertions(+), 35 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 0c0e1a5..5c5524b 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -10909,13 +10909,13 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
   const int num_regs = gdbarch_num_regs (arm_insn_r->gdbarch);
 
   opcode = bits (arm_insn_r->arm_insn, 20, 24);
-  single_reg = bit (arm_insn_r->arm_insn, 8);
+  single_reg = !bit (arm_insn_r->arm_insn, 8);
   op_vldm_vstm = opcode & 0x1b;
 
   /* Handle VMOV instructions.  */
   if ((opcode & 0x1e) == 0x04)
     {
-      if (bit (arm_insn_r->arm_insn, 4))
+      if (bit (arm_insn_r->arm_insn, 20)) /* to_arm_registers bit 20? */
 	{
 	  record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15);
 	  record_buf[1] = bits (arm_insn_r->arm_insn, 16, 19);
@@ -10923,18 +10923,29 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
 	}
       else
 	{
-	  uint8_t reg_m = ((bits (arm_insn_r->arm_insn, 0, 3) << 1)
-			   | bit (arm_insn_r->arm_insn, 5));
+	  uint8_t reg_m = bits (arm_insn_r->arm_insn, 0, 3);
+	  uint8_t bit_m = bit (arm_insn_r->arm_insn, 5);
 
-	  if (!single_reg)
+	  if (single_reg)
 	    {
-	      record_buf[0] = num_regs + reg_m;
-	      record_buf[1] = num_regs + reg_m + 1;
-	      arm_insn_r->reg_rec_count = 2;
+	      /* The first S register number m is REG_M:M (M is bit 5),
+		 the corresponding D register number is REG_M:M / 2, which
+		 is REG_M.  */
+	      record_buf[arm_insn_r->reg_rec_count++] = ARM_D0_REGNUM + reg_m;
+	      /* The second S register number is REG_M:M + 1, the
+		 corresponding D register number is (REG_M:M + 1) / 2.
+		 IOW, if bit M is 1, the first and second S registers
+		 are mapped to different D registers, otherwise, they are
+		 in the same D register.  */
+	      if (bit_m)
+		{
+		  record_buf[arm_insn_r->reg_rec_count++]
+		    = ARM_D0_REGNUM + reg_m + 1;
+		}
 	    }
 	  else
 	    {
-	      record_buf[0] = reg_m + ARM_D0_REGNUM;
+	      record_buf[0] = ((bit_m << 4) + reg_m + ARM_D0_REGNUM);
 	      arm_insn_r->reg_rec_count = 1;
 	    }
 	}
@@ -10949,7 +10960,7 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
       reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
       regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
       imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
-      imm_off32 = imm_off8 << 24;
+      imm_off32 = imm_off8 << 2;
       memory_count = imm_off8;
 
       if (bit (arm_insn_r->arm_insn, 23))
@@ -10965,19 +10976,19 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
 
       while (memory_count > 0)
 	{
-	  if (!single_reg)
+	  if (single_reg)
 	    {
-	      record_buf_mem[memory_index] = start_address;
-	      record_buf_mem[memory_index + 1] = 4;
+	      record_buf_mem[memory_index] = 4;
+	      record_buf_mem[memory_index + 1] = start_address;
 	      start_address = start_address + 4;
 	      memory_index = memory_index + 2;
 	    }
 	  else
 	    {
-	      record_buf_mem[memory_index] = start_address;
-	      record_buf_mem[memory_index + 1] = 4;
-	      record_buf_mem[memory_index + 2] = start_address + 4;
-	      record_buf_mem[memory_index + 3] = 4;
+	      record_buf_mem[memory_index] = 4;
+	      record_buf_mem[memory_index + 1] = start_address;
+	      record_buf_mem[memory_index + 2] = 4;
+	      record_buf_mem[memory_index + 3] = start_address + 4;
 	      start_address = start_address + 8;
 	      memory_index = memory_index + 4;
 	    }
@@ -10991,25 +11002,36 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
     {
       uint32_t reg_count, reg_vd;
       uint32_t reg_index = 0;
+      uint32_t bit_d = bit (arm_insn_r->arm_insn, 22);
 
       reg_vd = bits (arm_insn_r->arm_insn, 12, 15);
       reg_count = bits (arm_insn_r->arm_insn, 0, 7);
 
-      if (single_reg)
-	reg_vd = reg_vd | (bit (arm_insn_r->arm_insn, 22) << 4);
-      else
-	reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 22);
+      /* REG_VD is the first D register number.  If the instruction
+	 loads memory to S registers (SINGLE_REG is TRUE), the register
+	 number is (REG_VD << 1 | bit D), so the corresponding D
+	 register number is (REG_VD << 1 | bit D) / 2 = REG_VD.  */
+      if (!single_reg)
+	reg_vd = reg_vd | (bit_d << 4);
 
-      if (bit (arm_insn_r->arm_insn, 21))
+      if (bit (arm_insn_r->arm_insn, 21) /* write back */)
 	record_buf[reg_index++] = bits (arm_insn_r->arm_insn, 16, 19);
 
-      while (reg_count > 0)
+      /* If the instruction loads memory to D register, REG_COUNT should
+	 divide 2, according to the ARM Architecture Reference Manual.
+	 If the instruction loads memory to S register, divide 2 as well
+	 because two S registers are mapped to D register.  */
+      reg_count  = reg_count / 2;
+      if (single_reg && bit_d)
 	{
-	  if (single_reg)
-	    record_buf[reg_index++] = num_regs + reg_vd + reg_count - 1;
-	  else
-	    record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
+	  /* Increase the register count if S register list starts from
+	     odd number (bit d is one).  */
+	  reg_count++;
+	}
 
+      while (reg_count > 0)
+	{
+	  record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
 	  reg_count--;
 	}
       arm_insn_r->reg_rec_count = reg_index;
@@ -11023,7 +11045,7 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
       reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
       regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
       imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
-      imm_off32 = imm_off8 << 24;
+      imm_off32 = imm_off8 << 2;
 
       if (bit (arm_insn_r->arm_insn, 23))
 	start_address = u_regval + imm_off32;
@@ -11032,16 +11054,16 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
 
       if (single_reg)
 	{
-	  record_buf_mem[memory_index] = start_address;
-	  record_buf_mem[memory_index + 1] = 4;
+	  record_buf_mem[memory_index] = 4;
+	  record_buf_mem[memory_index + 1] = start_address;
 	  arm_insn_r->mem_rec_count = 1;
 	}
       else
 	{
-	  record_buf_mem[memory_index] = start_address;
-	  record_buf_mem[memory_index + 1] = 4;
-	  record_buf_mem[memory_index + 2] = start_address + 4;
-	  record_buf_mem[memory_index + 3] = 4;
+	  record_buf_mem[memory_index] = 4;
+	  record_buf_mem[memory_index + 1] = start_address;
+	  record_buf_mem[memory_index + 2] = 4;
+	  record_buf_mem[memory_index + 3] = start_address + 4;
 	  arm_insn_r->mem_rec_count = 2;
 	}
     }
@@ -11058,7 +11080,8 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
       else
 	{
 	  reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 22);
-	  record_buf[0] = num_regs + reg_vd;
+	  /* Record register D rather than pseudo register S.  */
+	  record_buf[0] = ARM_D0_REGNUM + reg_vd / 2;
 	}
       arm_insn_r->reg_rec_count = 1;
     }
diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.c b/gdb/testsuite/gdb.reverse/insn-reverse.c
index 1bfb8b0..22cd267 100644
--- a/gdb/testsuite/gdb.reverse/insn-reverse.c
+++ b/gdb/testsuite/gdb.reverse/insn-reverse.c
@@ -88,6 +88,45 @@ adv_simd_vect_shift (void)
 {
   asm ("fcvtzs s0, s0, #1");
 }
+#elif (defined __arm__)
+static void
+ext_reg_load (void)
+{
+  char in[8];
+
+  asm ("vldr d0, [%0]" : : "r" (in));
+  asm ("vldr s3, [%0]" : : "r" (in));
+
+  asm ("vldm %0, {d3-d4}" : : "r" (in));
+  asm ("vldm %0, {s9-s11}" : : "r" (in));
+}
+
+static void
+ext_reg_mov (void)
+{
+  int i, j;
+  double d;
+
+  i = 1;
+  j = 2;
+
+  asm ("vmov s4, s5, %0, %1" : "=r" (i), "=r" (j): );
+  asm ("vmov s7, s8, %0, %1" : "=r" (i), "=r" (j): );
+  asm ("vmov %0, %1, s10, s11" : : "r" (i), "r" (j));
+  asm ("vmov %0, %1, s1, s2" : : "r" (i), "r" (j));
+
+  asm ("vmov %P2, %0, %1" : "=r" (i), "=r" (j): "w" (d));
+  asm ("vmov %1, %2, %P0" : "=w" (d) : "r" (i), "r" (j));
+}
+
+static void
+ext_reg_push_pop (void)
+{
+  double d;
+
+  asm ("vpush {%P0}" : : "w" (d));
+  asm ("vpop {%P0}" : : "w" (d));
+}
 #endif
 
 typedef void (*testcase_ftype) (void);
@@ -104,6 +143,10 @@ static testcase_ftype testcases[] =
   adv_simd_scalar_index,
   adv_simd_smlal,
   adv_simd_vect_shift,
+#elif (defined __arm__)
+  ext_reg_load,
+  ext_reg_mov,
+  ext_reg_push_pop,
 #endif
 };
 
-- 
1.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/3] Record right reg num of thumb special data instructions
  2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
@ 2016-02-22 16:53 ` Yao Qi
  2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Yao Qi @ 2016-02-22 16:53 UTC (permalink / raw)
  To: gdb-patches

When GDB decodes these thumb special data instructions, such as 'mov sp, r7'
the Rd is got incorrectly.  According to the arch reference manual, the Rd
is DN:Rdn, in which DN is bit 7 and Rdn is bits 0 to 2.  This patch fixes it.

gdb:

2016-02-22  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c (thumb_record_ld_st_reg_offset): Fix the register
	number of Rd.
---
 gdb/arm-tdep.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 1a3a209..0c0e1a5 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -11512,10 +11512,10 @@ thumb_record_ld_st_reg_offset (insn_decode_record *thumb_insn_r)
         }
       else
         {
-          /* Format 8; special data processing insns.  */
-          reg_src1 = bits (thumb_insn_r->arm_insn, 0, 2);
-          record_buf[0] = ARM_PS_REGNUM;
-          record_buf[1] = reg_src1;
+	  /* Format 8; special data processing insns.  */
+	  record_buf[0] = ARM_PS_REGNUM;
+	  record_buf[1] = (bit (thumb_insn_r->arm_insn, 7) << 3
+			   | bits (thumb_insn_r->arm_insn, 0, 2));
           thumb_insn_r->reg_rec_count = 2;
         }
     }
-- 
1.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn
  2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi
@ 2016-02-23 19:59   ` Luis Machado
  2016-02-26 14:54     ` Yao Qi
  0 siblings, 1 reply; 11+ messages in thread
From: Luis Machado @ 2016-02-23 19:59 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

Just internal comment nits.

On 02/22/2016 01:53 PM, Yao Qi wrote:
> This patch fixes various bugs in arm_record_exreg_ld_st_insn, and use
> gdb.reverse/insn-reverse.c to test more arm instructions.
>
>   - Set flag SINGLE_REG correctly.  In the arch reference manual,
>     SING_REG is true when the bit 8 of instruction is zero.
>   - Record the right D registers for instructions changing S registers.
>   - Fix the order of length and address in record_buf_mem array.
>   - Shift the offset by 2 instead of by 24.
>
> This patch also fixes one internal error,
>
> (gdb) PASS: gdb.reverse/finish-precsave.exp: BP at end of main
> continue^M
> Continuing.^M
> ../../binutils-gdb/gdb/utils.c:1072: internal-error: virtual memory exhausted.^M
> A problem internal to GDB has been detected,FAIL: gdb.reverse/finish-precsave.exp: run to end of main (GDB internal error)
>

...

> @@ -10991,25 +11002,36 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
>       {
>         uint32_t reg_count, reg_vd;
>         uint32_t reg_index = 0;
> +      uint32_t bit_d = bit (arm_insn_r->arm_insn, 22);
>
>         reg_vd = bits (arm_insn_r->arm_insn, 12, 15);
>         reg_count = bits (arm_insn_r->arm_insn, 0, 7);
>
> -      if (single_reg)
> -	reg_vd = reg_vd | (bit (arm_insn_r->arm_insn, 22) << 4);
> -      else
> -	reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 22);
> +      /* REG_VD is the first D register number.  If the instruction
> +	 loads memory to S registers (SINGLE_REG is TRUE), the register
> +	 number is (REG_VD << 1 | bit D), so the corresponding D
> +	 register number is (REG_VD << 1 | bit D) / 2 = REG_VD.  */
> +      if (!single_reg)
> +	reg_vd = reg_vd | (bit_d << 4);
>
> -      if (bit (arm_insn_r->arm_insn, 21))
> +      if (bit (arm_insn_r->arm_insn, 21) /* write back */)
>   	record_buf[reg_index++] = bits (arm_insn_r->arm_insn, 16, 19);
>
> -      while (reg_count > 0)
> +      /* If the instruction loads memory to D register, REG_COUNT should
> +	 divide 2, according to the ARM Architecture Reference Manual.

"...should be divided by 2..."?

> +	 If the instruction loads memory to S register, divide 2 as well

"... divide by 2..."

> +	 because two S registers are mapped to D register.  */
> +      reg_count  = reg_count / 2;
> +      if (single_reg && bit_d)
>   	{
> -	  if (single_reg)
> -	    record_buf[reg_index++] = num_regs + reg_vd + reg_count - 1;
> -	  else
> -	    record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
> +	  /* Increase the register count if S register list starts from
> +	     odd number (bit d is one).  */

"...from an odd number..."?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] Generalize gdb.reverse/aarch64.exp
  2016-02-22 16:53 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Yao Qi
@ 2016-02-24 11:22   ` Pedro Alves
  2016-02-24 14:40     ` Yao Qi
  0 siblings, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2016-02-24 11:22 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 02/22/2016 04:53 PM, Yao Qi wrote:
> I said we can generialize gdb.reverse/aarch64.exp for other
> architectures https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html
> and here is the patch to change aarch64.exp to insn-reverse.exp.
> 
> gdb/testsuite:
> 
> 2016-02-22  Yao Qi  <yao.qi@linaro.org>
> 
> 	* gdb.reverse/aarch64.c: Rename it ...
> 	* gdb.reverse/insn-reverse.c: to ...
> 	[__aarch64__] Include arm_neon.h.
> 	(testcase_ftype): New.
> 	(testcases): New array.
> 	(n_testcases): New.
> 	(main): Call each element in testcases.
> 	* gdb.reverse/aarch64.exp: Rename it ...
> 	* gdb.reverse/insn-reverse.exp: to ...
> 	Remove is_aarch64_target check.
> 	(read_testcase): New.
> 	Do the tests in a loop.
> ---
>  gdb/testsuite/gdb.reverse/aarch64.c        |  99 ----------------------
>  gdb/testsuite/gdb.reverse/aarch64.exp      | 115 -------------------------
>  gdb/testsuite/gdb.reverse/insn-reverse.c   | 121 +++++++++++++++++++++++++++
>  gdb/testsuite/gdb.reverse/insn-reverse.exp | 130 +++++++++++++++++++++++++++++


Can you send a "git diff -M" so we can easily see the actual
changes?  (You can make that the default with git config.)

Though I think even better would be to do the rename in one patch,
and the generalization changes in a separate patch.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] Generalize gdb.reverse/aarch64.exp
  2016-02-24 11:22   ` Pedro Alves
@ 2016-02-24 14:40     ` Yao Qi
  2016-02-24 14:40       ` [PATCH 1.5/3] Rename gdb.reverse/aarch64.{exp,c} to gdb.reverse/insn-reverse.{exp,c} Yao Qi
  2016-02-24 19:37       ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Pedro Alves
  0 siblings, 2 replies; 11+ messages in thread
From: Yao Qi @ 2016-02-24 14:40 UTC (permalink / raw)
  To: gdb-patches

I said we can generialize gdb.reverse/aarch64.exp for other
architectures https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html
and here is the patch to change aarch64.exp so that it can be used to
test for other architectures as well.

gdb/testsuite:

2016-02-24  Yao Qi  <yao.qi@linaro.org>

	* gdb.reverse/aarch64.c: [__aarch64__] Include arm_neon.h.
	(testcase_ftype): New.
	(testcases): New array.
	(n_testcases): New.
	(main): Call each element in testcases.
	* gdb.reverse/aarch64.exp: Remove is_aarch64_target check.
	(read_testcase): New.
	Do the tests in a loop.
---
 gdb/testsuite/gdb.reverse/aarch64.c   | 34 ++++++++++++++++++++++++-----
 gdb/testsuite/gdb.reverse/aarch64.exp | 41 ++++++++++++++++++++++++-----------
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/gdb/testsuite/gdb.reverse/aarch64.c b/gdb/testsuite/gdb.reverse/aarch64.c
index ae8509a..1bfb8b0 100644
--- a/gdb/testsuite/gdb.reverse/aarch64.c
+++ b/gdb/testsuite/gdb.reverse/aarch64.c
@@ -15,8 +15,11 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#if (defined __aarch64__)
 #include <arm_neon.h>
+#endif
 
+#if (defined __aarch64__)
 static void
 load (void)
 {
@@ -85,15 +88,34 @@ adv_simd_vect_shift (void)
 {
   asm ("fcvtzs s0, s0, #1");
 }
+#endif
+
+typedef void (*testcase_ftype) (void);
+
+/* Functions testing instruction decodings.  GDB will read n_testcases
+   to know how many functions to test.  */
+
+static testcase_ftype testcases[] =
+{
+#if (defined __aarch64__)
+  load,
+  move,
+  adv_simd_mod_imm,
+  adv_simd_scalar_index,
+  adv_simd_smlal,
+  adv_simd_vect_shift,
+#endif
+};
+
+static int n_testcases = (sizeof (testcases) / sizeof (testcase_ftype));
 
 int
 main ()
 {
-  load ();
-  move ();
-  adv_simd_mod_imm ();
-  adv_simd_scalar_index ();
-  adv_simd_smlal ();
-  adv_simd_vect_shift ();
+  int i = 0;
+
+  for (i = 0; i < n_testcases; i++)
+    testcases[i] ();
+
   return 0;
 }
diff --git a/gdb/testsuite/gdb.reverse/aarch64.exp b/gdb/testsuite/gdb.reverse/aarch64.exp
index 2906d4b..f52b40c 100644
--- a/gdb/testsuite/gdb.reverse/aarch64.exp
+++ b/gdb/testsuite/gdb.reverse/aarch64.exp
@@ -17,13 +17,6 @@ if ![supports_reverse] {
     return
 }
 
-# Test aarch64 instruction recording.
-
-if {![is_aarch64_target]} then {
-    verbose "Skipping aarch64 instruction recording tests."
-    return
-}
-
 standard_testfile
 
 if {[prepare_for_testing $testfile.exp $testfile $srcfile \
@@ -36,6 +29,22 @@ if { ![runto main] } then {
     return
 }
 
+# Read function name from testcases[N].
+
+proc read_testcase { n } {
+    global gdb_prompt
+
+    set result -1
+    gdb_test_multiple "print testcases\[${n}\]" "read name of test case ${n}" {
+	-re "\[$\].*= .*<(.*)>.*$gdb_prompt $" {
+	    set result $expect_out(1,string)
+	}
+	-re "$gdb_prompt $" { }
+    }
+
+    return $result
+}
+
 # In each function FUNC, GDB turns on process record, and single step
 # until program goes to the end of the function.  Then, single step
 # backward.  In each of forward single step and backward single step,
@@ -107,9 +116,15 @@ proc test { func } {
     }
 }
 
-test "load"
-test "move"
-test "adv_simd_mod_imm"
-test "adv_simd_scalar_index"
-test "adv_simd_smlal"
-test "adv_simd_vect_shift"
+set n_testcases [get_integer_valueof "n_testcases" 0]
+
+if { ${n_testcases} == 0 } {
+    untested "No test"
+    return 1
+}
+
+for { set i 0 } { ${i} < ${n_testcases} } { incr i } {
+    set testcase [read_testcase $i]
+
+    test $testcase
+}
-- 
1.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1.5/3] Rename gdb.reverse/aarch64.{exp,c} to gdb.reverse/insn-reverse.{exp,c}
  2016-02-24 14:40     ` Yao Qi
@ 2016-02-24 14:40       ` Yao Qi
  2016-02-24 19:37       ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Pedro Alves
  1 sibling, 0 replies; 11+ messages in thread
From: Yao Qi @ 2016-02-24 14:40 UTC (permalink / raw)
  To: gdb-patches

gdb/testsuite:

2016-02-24  Yao Qi  <yao.qi@linaro.org>

	* gdb.reverse/aarch64.c: Rename to ...
	* gdb.reverse/insn-reverse.c: ... it.
	* gdb.reverse/aarch64.exp: Rename to ...
	* gdb.reverse/insn-reverse.exp: ... it.
---
 gdb/testsuite/gdb.reverse/aarch64.c        | 121 ---------------------------
 gdb/testsuite/gdb.reverse/aarch64.exp      | 130 -----------------------------
 gdb/testsuite/gdb.reverse/insn-reverse.c   | 121 +++++++++++++++++++++++++++
 gdb/testsuite/gdb.reverse/insn-reverse.exp | 130 +++++++++++++++++++++++++++++
 4 files changed, 251 insertions(+), 251 deletions(-)
 delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.c
 delete mode 100644 gdb/testsuite/gdb.reverse/aarch64.exp
 create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.c
 create mode 100644 gdb/testsuite/gdb.reverse/insn-reverse.exp

diff --git a/gdb/testsuite/gdb.reverse/aarch64.c b/gdb/testsuite/gdb.reverse/aarch64.c
deleted file mode 100644
index 1bfb8b0..0000000
--- a/gdb/testsuite/gdb.reverse/aarch64.c
+++ /dev/null
@@ -1,121 +0,0 @@
-/* This testcase is part of GDB, the GNU debugger.
-
-   Copyright 2015-2016 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/>.  */
-
-#if (defined __aarch64__)
-#include <arm_neon.h>
-#endif
-
-#if (defined __aarch64__)
-static void
-load (void)
-{
-  int buf[8];
-
-  asm ("ld1 { v1.8b }, [%[buf]]\n"
-       "ld1 { v2.8b, v3.8b }, [%[buf]]\n"
-       "ld1 { v3.8b, v4.8b, v5.8b }, [%[buf]]\n"
-       :
-       : [buf] "r" (buf)
-       : /* No clobbers */);
-}
-
-static void
-move (void)
-{
-  float32x2_t b1_ = vdup_n_f32(123.0f);
-  float32_t a1_ = 0;
-  float64x1_t b2_ = vdup_n_f64(456.0f);
-  float64_t a2_ = 0;
-
-  asm ("ins %0.s[0], %w1\n"
-       : "=w"(b1_)
-       : "r"(a1_), "0"(b1_)
-       : /* No clobbers */);
-
-  asm ("ins %0.d[1], %x1\n"
-       : "=w"(b2_)
-       : "r"(a2_), "0"(b2_)
-       : /* No clobbers */);
-}
-
-static void
-adv_simd_mod_imm (void)
-{
-  float32x2_t a1 = {2.0, 4.0};
-
-  asm ("bic %0.2s, #1\n"
-       "bic %0.2s, #1, lsl #8\n"
-       : "=w"(a1)
-       : "0"(a1)
-       : /* No clobbers */);
-}
-
-static void
-adv_simd_scalar_index (void)
-{
-  float64x2_t b_ = {0.0, 0.0};
-  float64_t a_ = 1.0;
-  float64_t result;
-
-  asm ("fmla %d0,%d1,%2.d[1]"
-       : "=w"(result)
-       : "w"(a_), "w"(b_)
-       : /* No clobbers */);
-}
-
-static void
-adv_simd_smlal (void)
-{
-  asm ("smlal v13.2d, v8.2s, v0.2s");
-}
-
-static void
-adv_simd_vect_shift (void)
-{
-  asm ("fcvtzs s0, s0, #1");
-}
-#endif
-
-typedef void (*testcase_ftype) (void);
-
-/* Functions testing instruction decodings.  GDB will read n_testcases
-   to know how many functions to test.  */
-
-static testcase_ftype testcases[] =
-{
-#if (defined __aarch64__)
-  load,
-  move,
-  adv_simd_mod_imm,
-  adv_simd_scalar_index,
-  adv_simd_smlal,
-  adv_simd_vect_shift,
-#endif
-};
-
-static int n_testcases = (sizeof (testcases) / sizeof (testcase_ftype));
-
-int
-main ()
-{
-  int i = 0;
-
-  for (i = 0; i < n_testcases; i++)
-    testcases[i] ();
-
-  return 0;
-}
diff --git a/gdb/testsuite/gdb.reverse/aarch64.exp b/gdb/testsuite/gdb.reverse/aarch64.exp
deleted file mode 100644
index f52b40c..0000000
--- a/gdb/testsuite/gdb.reverse/aarch64.exp
+++ /dev/null
@@ -1,130 +0,0 @@
-# Copyright (C) 2015-2016 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/>.
-
-if ![supports_reverse] {
-    return
-}
-
-standard_testfile
-
-if {[prepare_for_testing $testfile.exp $testfile $srcfile \
-	 [list debug]]} {
-    untested ${testfile}.exp
-    return -1
-}
-if { ![runto main] } then {
-    fail "run to main"
-    return
-}
-
-# Read function name from testcases[N].
-
-proc read_testcase { n } {
-    global gdb_prompt
-
-    set result -1
-    gdb_test_multiple "print testcases\[${n}\]" "read name of test case ${n}" {
-	-re "\[$\].*= .*<(.*)>.*$gdb_prompt $" {
-	    set result $expect_out(1,string)
-	}
-	-re "$gdb_prompt $" { }
-    }
-
-    return $result
-}
-
-# In each function FUNC, GDB turns on process record, and single step
-# until program goes to the end of the function.  Then, single step
-# backward.  In each of forward single step and backward single step,
-# the contents of registers are saved, and test compares them.  If
-# there is any differences, a FAIL is emitted.
-
-proc test { func } {
-    global hex decimal
-    global gdb_prompt
-
-    with_test_prefix "$func" {
-	gdb_breakpoint $func
-	gdb_test "continue"
-
-	set last_insn ""
-	set test "disassemble $func"
-	gdb_test_multiple $test $test {
-	    -re ".*($hex) <\\+$decimal>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" {
-		set last_insn $expect_out(1,string)
-	    }
-	}
-	if { $last_insn == "" } {
-	    fail "find the last instruction of function $func"
-	}
-
-	# Activate process record/replay
-	gdb_test_no_output "record" "Turn on process record"
-
-	# Registers contents before each forward single step.
-	set count 0
-	for {} {$count < 500} {incr count} {
-	    gdb_test_multiple "x/i \$pc" "" {
-		-re ".* ($hex) <.*>:\[ \t\]*(.*)\r\n$gdb_prompt $" {
-		    set insn_addr $expect_out(1,string)
-
-		    if [expr {$last_insn == $insn_addr}] {
-			break
-		    }
-
-		    set insn_array($count) $expect_out(2,string)
-		}
-	    }
-
-	    set pre_regs($count) [capture_command_output "info all-registers" ""]
-	    gdb_test "si" "" ""
-	}
-
-	incr count -1
-	# Registers contents after each backward single step.
-	for {set i $count} {$i >= 0} {incr i -1} {
-	    gdb_test "reverse-stepi" "" ""
-	    set post_regs($i) [capture_command_output "info all-registers" ""]
-	}
-
-	# Compare the register contents.
-	for {set i 0} {$i < $count} {incr i} {
-	    if { ![gdb_assert { [string compare $pre_regs($i) $post_regs($i)] == 0 } \
-		      "compare registers on insn $i:$insn_array($i)"] } {
-
-		foreach pre_line [split $pre_regs($i) \n] post_line [split $post_regs($i) \n] {
-		    if { [string compare $pre_line $post_line] } {
-			verbose -log " -:$pre_line"
-			verbose -log " +:$post_line"
-		    }
-		}
-	    }
-	}
-	gdb_test "record stop"
-    }
-}
-
-set n_testcases [get_integer_valueof "n_testcases" 0]
-
-if { ${n_testcases} == 0 } {
-    untested "No test"
-    return 1
-}
-
-for { set i 0 } { ${i} < ${n_testcases} } { incr i } {
-    set testcase [read_testcase $i]
-
-    test $testcase
-}
diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.c b/gdb/testsuite/gdb.reverse/insn-reverse.c
new file mode 100644
index 0000000..1bfb8b0
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/insn-reverse.c
@@ -0,0 +1,121 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015-2016 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/>.  */
+
+#if (defined __aarch64__)
+#include <arm_neon.h>
+#endif
+
+#if (defined __aarch64__)
+static void
+load (void)
+{
+  int buf[8];
+
+  asm ("ld1 { v1.8b }, [%[buf]]\n"
+       "ld1 { v2.8b, v3.8b }, [%[buf]]\n"
+       "ld1 { v3.8b, v4.8b, v5.8b }, [%[buf]]\n"
+       :
+       : [buf] "r" (buf)
+       : /* No clobbers */);
+}
+
+static void
+move (void)
+{
+  float32x2_t b1_ = vdup_n_f32(123.0f);
+  float32_t a1_ = 0;
+  float64x1_t b2_ = vdup_n_f64(456.0f);
+  float64_t a2_ = 0;
+
+  asm ("ins %0.s[0], %w1\n"
+       : "=w"(b1_)
+       : "r"(a1_), "0"(b1_)
+       : /* No clobbers */);
+
+  asm ("ins %0.d[1], %x1\n"
+       : "=w"(b2_)
+       : "r"(a2_), "0"(b2_)
+       : /* No clobbers */);
+}
+
+static void
+adv_simd_mod_imm (void)
+{
+  float32x2_t a1 = {2.0, 4.0};
+
+  asm ("bic %0.2s, #1\n"
+       "bic %0.2s, #1, lsl #8\n"
+       : "=w"(a1)
+       : "0"(a1)
+       : /* No clobbers */);
+}
+
+static void
+adv_simd_scalar_index (void)
+{
+  float64x2_t b_ = {0.0, 0.0};
+  float64_t a_ = 1.0;
+  float64_t result;
+
+  asm ("fmla %d0,%d1,%2.d[1]"
+       : "=w"(result)
+       : "w"(a_), "w"(b_)
+       : /* No clobbers */);
+}
+
+static void
+adv_simd_smlal (void)
+{
+  asm ("smlal v13.2d, v8.2s, v0.2s");
+}
+
+static void
+adv_simd_vect_shift (void)
+{
+  asm ("fcvtzs s0, s0, #1");
+}
+#endif
+
+typedef void (*testcase_ftype) (void);
+
+/* Functions testing instruction decodings.  GDB will read n_testcases
+   to know how many functions to test.  */
+
+static testcase_ftype testcases[] =
+{
+#if (defined __aarch64__)
+  load,
+  move,
+  adv_simd_mod_imm,
+  adv_simd_scalar_index,
+  adv_simd_smlal,
+  adv_simd_vect_shift,
+#endif
+};
+
+static int n_testcases = (sizeof (testcases) / sizeof (testcase_ftype));
+
+int
+main ()
+{
+  int i = 0;
+
+  for (i = 0; i < n_testcases; i++)
+    testcases[i] ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.exp b/gdb/testsuite/gdb.reverse/insn-reverse.exp
new file mode 100644
index 0000000..f52b40c
--- /dev/null
+++ b/gdb/testsuite/gdb.reverse/insn-reverse.exp
@@ -0,0 +1,130 @@
+# Copyright (C) 2015-2016 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/>.
+
+if ![supports_reverse] {
+    return
+}
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile \
+	 [list debug]]} {
+    untested ${testfile}.exp
+    return -1
+}
+if { ![runto main] } then {
+    fail "run to main"
+    return
+}
+
+# Read function name from testcases[N].
+
+proc read_testcase { n } {
+    global gdb_prompt
+
+    set result -1
+    gdb_test_multiple "print testcases\[${n}\]" "read name of test case ${n}" {
+	-re "\[$\].*= .*<(.*)>.*$gdb_prompt $" {
+	    set result $expect_out(1,string)
+	}
+	-re "$gdb_prompt $" { }
+    }
+
+    return $result
+}
+
+# In each function FUNC, GDB turns on process record, and single step
+# until program goes to the end of the function.  Then, single step
+# backward.  In each of forward single step and backward single step,
+# the contents of registers are saved, and test compares them.  If
+# there is any differences, a FAIL is emitted.
+
+proc test { func } {
+    global hex decimal
+    global gdb_prompt
+
+    with_test_prefix "$func" {
+	gdb_breakpoint $func
+	gdb_test "continue"
+
+	set last_insn ""
+	set test "disassemble $func"
+	gdb_test_multiple $test $test {
+	    -re ".*($hex) <\\+$decimal>:\[^\r\n\]+\r\nEnd of assembler dump\.\r\n$gdb_prompt $" {
+		set last_insn $expect_out(1,string)
+	    }
+	}
+	if { $last_insn == "" } {
+	    fail "find the last instruction of function $func"
+	}
+
+	# Activate process record/replay
+	gdb_test_no_output "record" "Turn on process record"
+
+	# Registers contents before each forward single step.
+	set count 0
+	for {} {$count < 500} {incr count} {
+	    gdb_test_multiple "x/i \$pc" "" {
+		-re ".* ($hex) <.*>:\[ \t\]*(.*)\r\n$gdb_prompt $" {
+		    set insn_addr $expect_out(1,string)
+
+		    if [expr {$last_insn == $insn_addr}] {
+			break
+		    }
+
+		    set insn_array($count) $expect_out(2,string)
+		}
+	    }
+
+	    set pre_regs($count) [capture_command_output "info all-registers" ""]
+	    gdb_test "si" "" ""
+	}
+
+	incr count -1
+	# Registers contents after each backward single step.
+	for {set i $count} {$i >= 0} {incr i -1} {
+	    gdb_test "reverse-stepi" "" ""
+	    set post_regs($i) [capture_command_output "info all-registers" ""]
+	}
+
+	# Compare the register contents.
+	for {set i 0} {$i < $count} {incr i} {
+	    if { ![gdb_assert { [string compare $pre_regs($i) $post_regs($i)] == 0 } \
+		      "compare registers on insn $i:$insn_array($i)"] } {
+
+		foreach pre_line [split $pre_regs($i) \n] post_line [split $post_regs($i) \n] {
+		    if { [string compare $pre_line $post_line] } {
+			verbose -log " -:$pre_line"
+			verbose -log " +:$post_line"
+		    }
+		}
+	    }
+	}
+	gdb_test "record stop"
+    }
+}
+
+set n_testcases [get_integer_valueof "n_testcases" 0]
+
+if { ${n_testcases} == 0 } {
+    untested "No test"
+    return 1
+}
+
+for { set i 0 } { ${i} < ${n_testcases} } { incr i } {
+    set testcase [read_testcase $i]
+
+    test $testcase
+}
-- 
1.9.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] Generalize gdb.reverse/aarch64.exp
  2016-02-24 14:40     ` Yao Qi
  2016-02-24 14:40       ` [PATCH 1.5/3] Rename gdb.reverse/aarch64.{exp,c} to gdb.reverse/insn-reverse.{exp,c} Yao Qi
@ 2016-02-24 19:37       ` Pedro Alves
  1 sibling, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2016-02-24 19:37 UTC (permalink / raw)
  To: Yao Qi, gdb-patches

On 02/24/2016 02:40 PM, Yao Qi wrote:
> I said we can generialize gdb.reverse/aarch64.exp for other
> architectures https://sourceware.org/ml/gdb-patches/2015-05/msg00482.html
> and here is the patch to change aarch64.exp so that it can be used to
> test for other architectures as well.
> 
> gdb/testsuite:
> 
> 2016-02-24  Yao Qi  <yao.qi@linaro.org>
> 
> 	* gdb.reverse/aarch64.c: [__aarch64__] Include arm_neon.h.
> 	(testcase_ftype): New.
> 	(testcases): New array.
> 	(n_testcases): New.
> 	(main): Call each element in testcases.
> 	* gdb.reverse/aarch64.exp: Remove is_aarch64_target check.
> 	(read_testcase): New.
> 	Do the tests in a loop.

Thanks for splitting.  LGTM.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn
  2016-02-23 19:59   ` Luis Machado
@ 2016-02-26 14:54     ` Yao Qi
  0 siblings, 0 replies; 11+ messages in thread
From: Yao Qi @ 2016-02-26 14:54 UTC (permalink / raw)
  To: Luis Machado; +Cc: Yao Qi, gdb-patches

Luis Machado <lgustavo@codesourcery.com> writes:

>> -      while (reg_count > 0)
>> +      /* If the instruction loads memory to D register, REG_COUNT should
>> +	 divide 2, according to the ARM Architecture Reference Manual.
>
> "...should be divided by 2..."?
>
>> +	 If the instruction loads memory to S register, divide 2 as well
>
> "... divide by 2..."
>
>> +	 because two S registers are mapped to D register.  */
>> +      reg_count  = reg_count / 2;
>> +      if (single_reg && bit_d)
>>   	{
>> -	  if (single_reg)
>> -	    record_buf[reg_index++] = num_regs + reg_vd + reg_count - 1;
>> -	  else
>> -	    record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
>> +	  /* Increase the register count if S register list starts from
>> +	     odd number (bit d is one).  */
>
> "...from an odd number..."?

Hi Luis,
Thanks for the review.  I fixed them.

-- 
Yao (齐尧)
From 8600280115853d508d82ae47932b3cbb8d4c22ac Mon Sep 17 00:00:00 2001
From: Yao Qi <yao.qi@linaro.org>
Date: Tue, 16 Feb 2016 12:59:03 +0000
Subject: [PATCH] Fix various bugs in arm_record_exreg_ld_st_insn

This patch fixes various bugs in arm_record_exreg_ld_st_insn, and use
gdb.reverse/insn-reverse.c to test more arm instructions.

 - Set flag SINGLE_REG correctly.  In the arch reference manual,
   SING_REG is true when the bit 8 of instruction is zero.
 - Record the right D registers for instructions changing S registers.
 - Fix the order of length and address in record_buf_mem array.
 - Shift the offset by 2 instead of by 24.

This patch also fixes one internal error,

(gdb) PASS: gdb.reverse/finish-precsave.exp: BP at end of main
continue^M
Continuing.^M
../../binutils-gdb/gdb/utils.c:1072: internal-error: virtual memory exhausted.^M
A problem internal to GDB has been detected,FAIL: gdb.reverse/finish-precsave.exp: run to end of main (GDB internal error)

gdb:

2016-02-26  Yao Qi  <yao.qi@linaro.org>

	* arm-tdep.c (arm_record_exreg_ld_st_insn): Set 'single_reg'
	per bit 8.  Check bit 20 instead of bit 4 for VMOV
	instruction.  Record D registers for instructions changing
	S registers.  Change of the order of length and address
	in record_buf_mem array.

gdb/testsuite:

2016-02-26  Yao Qi  <yao.qi@linaro.org>

	* gdb.reverse/insn-reverse.c [__arm__] (ext_reg_load): New.
	[__arm__] (ext_reg_mov, ext_reg_push_pop): New.
	(testcases): Update.

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index bd0ee97..2eb7bb1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -10909,13 +10909,13 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
   const int num_regs = gdbarch_num_regs (arm_insn_r->gdbarch);
 
   opcode = bits (arm_insn_r->arm_insn, 20, 24);
-  single_reg = bit (arm_insn_r->arm_insn, 8);
+  single_reg = !bit (arm_insn_r->arm_insn, 8);
   op_vldm_vstm = opcode & 0x1b;
 
   /* Handle VMOV instructions.  */
   if ((opcode & 0x1e) == 0x04)
     {
-      if (bit (arm_insn_r->arm_insn, 4))
+      if (bit (arm_insn_r->arm_insn, 20)) /* to_arm_registers bit 20? */
 	{
 	  record_buf[0] = bits (arm_insn_r->arm_insn, 12, 15);
 	  record_buf[1] = bits (arm_insn_r->arm_insn, 16, 19);
@@ -10923,18 +10923,29 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
 	}
       else
 	{
-	  uint8_t reg_m = ((bits (arm_insn_r->arm_insn, 0, 3) << 1)
-			   | bit (arm_insn_r->arm_insn, 5));
+	  uint8_t reg_m = bits (arm_insn_r->arm_insn, 0, 3);
+	  uint8_t bit_m = bit (arm_insn_r->arm_insn, 5);
 
-	  if (!single_reg)
+	  if (single_reg)
 	    {
-	      record_buf[0] = num_regs + reg_m;
-	      record_buf[1] = num_regs + reg_m + 1;
-	      arm_insn_r->reg_rec_count = 2;
+	      /* The first S register number m is REG_M:M (M is bit 5),
+		 the corresponding D register number is REG_M:M / 2, which
+		 is REG_M.  */
+	      record_buf[arm_insn_r->reg_rec_count++] = ARM_D0_REGNUM + reg_m;
+	      /* The second S register number is REG_M:M + 1, the
+		 corresponding D register number is (REG_M:M + 1) / 2.
+		 IOW, if bit M is 1, the first and second S registers
+		 are mapped to different D registers, otherwise, they are
+		 in the same D register.  */
+	      if (bit_m)
+		{
+		  record_buf[arm_insn_r->reg_rec_count++]
+		    = ARM_D0_REGNUM + reg_m + 1;
+		}
 	    }
 	  else
 	    {
-	      record_buf[0] = reg_m + ARM_D0_REGNUM;
+	      record_buf[0] = ((bit_m << 4) + reg_m + ARM_D0_REGNUM);
 	      arm_insn_r->reg_rec_count = 1;
 	    }
 	}
@@ -10949,7 +10960,7 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
       reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
       regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
       imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
-      imm_off32 = imm_off8 << 24;
+      imm_off32 = imm_off8 << 2;
       memory_count = imm_off8;
 
       if (bit (arm_insn_r->arm_insn, 23))
@@ -10965,19 +10976,19 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
 
       while (memory_count > 0)
 	{
-	  if (!single_reg)
+	  if (single_reg)
 	    {
-	      record_buf_mem[memory_index] = start_address;
-	      record_buf_mem[memory_index + 1] = 4;
+	      record_buf_mem[memory_index] = 4;
+	      record_buf_mem[memory_index + 1] = start_address;
 	      start_address = start_address + 4;
 	      memory_index = memory_index + 2;
 	    }
 	  else
 	    {
-	      record_buf_mem[memory_index] = start_address;
-	      record_buf_mem[memory_index + 1] = 4;
-	      record_buf_mem[memory_index + 2] = start_address + 4;
-	      record_buf_mem[memory_index + 3] = 4;
+	      record_buf_mem[memory_index] = 4;
+	      record_buf_mem[memory_index + 1] = start_address;
+	      record_buf_mem[memory_index + 2] = 4;
+	      record_buf_mem[memory_index + 3] = start_address + 4;
 	      start_address = start_address + 8;
 	      memory_index = memory_index + 4;
 	    }
@@ -10991,25 +11002,36 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
     {
       uint32_t reg_count, reg_vd;
       uint32_t reg_index = 0;
+      uint32_t bit_d = bit (arm_insn_r->arm_insn, 22);
 
       reg_vd = bits (arm_insn_r->arm_insn, 12, 15);
       reg_count = bits (arm_insn_r->arm_insn, 0, 7);
 
-      if (single_reg)
-	reg_vd = reg_vd | (bit (arm_insn_r->arm_insn, 22) << 4);
-      else
-	reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 22);
+      /* REG_VD is the first D register number.  If the instruction
+	 loads memory to S registers (SINGLE_REG is TRUE), the register
+	 number is (REG_VD << 1 | bit D), so the corresponding D
+	 register number is (REG_VD << 1 | bit D) / 2 = REG_VD.  */
+      if (!single_reg)
+	reg_vd = reg_vd | (bit_d << 4);
 
-      if (bit (arm_insn_r->arm_insn, 21))
+      if (bit (arm_insn_r->arm_insn, 21) /* write back */)
 	record_buf[reg_index++] = bits (arm_insn_r->arm_insn, 16, 19);
 
-      while (reg_count > 0)
+      /* If the instruction loads memory to D register, REG_COUNT should
+	 be divided by 2, according to the ARM Architecture Reference
+	 Manual.  If the instruction loads memory to S register, divide by
+	 2 as well because two S registers are mapped to D register.  */
+      reg_count  = reg_count / 2;
+      if (single_reg && bit_d)
 	{
-	  if (single_reg)
-	    record_buf[reg_index++] = num_regs + reg_vd + reg_count - 1;
-	  else
-	    record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
+	  /* Increase the register count if S register list starts from
+	     an odd number (bit d is one).  */
+	  reg_count++;
+	}
 
+      while (reg_count > 0)
+	{
+	  record_buf[reg_index++] = ARM_D0_REGNUM + reg_vd + reg_count - 1;
 	  reg_count--;
 	}
       arm_insn_r->reg_rec_count = reg_index;
@@ -11023,7 +11045,7 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
       reg_rn = bits (arm_insn_r->arm_insn, 16, 19);
       regcache_raw_read_unsigned (reg_cache, reg_rn, &u_regval);
       imm_off8 = bits (arm_insn_r->arm_insn, 0, 7);
-      imm_off32 = imm_off8 << 24;
+      imm_off32 = imm_off8 << 2;
 
       if (bit (arm_insn_r->arm_insn, 23))
 	start_address = u_regval + imm_off32;
@@ -11032,16 +11054,16 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
 
       if (single_reg)
 	{
-	  record_buf_mem[memory_index] = start_address;
-	  record_buf_mem[memory_index + 1] = 4;
+	  record_buf_mem[memory_index] = 4;
+	  record_buf_mem[memory_index + 1] = start_address;
 	  arm_insn_r->mem_rec_count = 1;
 	}
       else
 	{
-	  record_buf_mem[memory_index] = start_address;
-	  record_buf_mem[memory_index + 1] = 4;
-	  record_buf_mem[memory_index + 2] = start_address + 4;
-	  record_buf_mem[memory_index + 3] = 4;
+	  record_buf_mem[memory_index] = 4;
+	  record_buf_mem[memory_index + 1] = start_address;
+	  record_buf_mem[memory_index + 2] = 4;
+	  record_buf_mem[memory_index + 3] = start_address + 4;
 	  arm_insn_r->mem_rec_count = 2;
 	}
     }
@@ -11058,7 +11080,8 @@ arm_record_exreg_ld_st_insn (insn_decode_record *arm_insn_r)
       else
 	{
 	  reg_vd = (reg_vd << 1) | bit (arm_insn_r->arm_insn, 22);
-	  record_buf[0] = num_regs + reg_vd;
+	  /* Record register D rather than pseudo register S.  */
+	  record_buf[0] = ARM_D0_REGNUM + reg_vd / 2;
 	}
       arm_insn_r->reg_rec_count = 1;
     }
diff --git a/gdb/testsuite/gdb.reverse/insn-reverse.c b/gdb/testsuite/gdb.reverse/insn-reverse.c
index 1bfb8b0..22cd267 100644
--- a/gdb/testsuite/gdb.reverse/insn-reverse.c
+++ b/gdb/testsuite/gdb.reverse/insn-reverse.c
@@ -88,6 +88,45 @@ adv_simd_vect_shift (void)
 {
   asm ("fcvtzs s0, s0, #1");
 }
+#elif (defined __arm__)
+static void
+ext_reg_load (void)
+{
+  char in[8];
+
+  asm ("vldr d0, [%0]" : : "r" (in));
+  asm ("vldr s3, [%0]" : : "r" (in));
+
+  asm ("vldm %0, {d3-d4}" : : "r" (in));
+  asm ("vldm %0, {s9-s11}" : : "r" (in));
+}
+
+static void
+ext_reg_mov (void)
+{
+  int i, j;
+  double d;
+
+  i = 1;
+  j = 2;
+
+  asm ("vmov s4, s5, %0, %1" : "=r" (i), "=r" (j): );
+  asm ("vmov s7, s8, %0, %1" : "=r" (i), "=r" (j): );
+  asm ("vmov %0, %1, s10, s11" : : "r" (i), "r" (j));
+  asm ("vmov %0, %1, s1, s2" : : "r" (i), "r" (j));
+
+  asm ("vmov %P2, %0, %1" : "=r" (i), "=r" (j): "w" (d));
+  asm ("vmov %1, %2, %P0" : "=w" (d) : "r" (i), "r" (j));
+}
+
+static void
+ext_reg_push_pop (void)
+{
+  double d;
+
+  asm ("vpush {%P0}" : : "w" (d));
+  asm ("vpop {%P0}" : : "w" (d));
+}
 #endif
 
 typedef void (*testcase_ftype) (void);
@@ -104,6 +143,10 @@ static testcase_ftype testcases[] =
   adv_simd_scalar_index,
   adv_simd_smlal,
   adv_simd_vect_shift,
+#elif (defined __arm__)
+  ext_reg_load,
+  ext_reg_mov,
+  ext_reg_push_pop,
 #endif
 };
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/3] Bug fixes in arm reverse debugging
  2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
                   ` (2 preceding siblings ...)
  2016-02-22 16:53 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Yao Qi
@ 2016-02-26 15:01 ` Yao Qi
  3 siblings, 0 replies; 11+ messages in thread
From: Yao Qi @ 2016-02-26 15:01 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

Yao Qi <qiyaoltc@gmail.com> writes:

> This patch series extends test case gdb.reverse/aarch64.exp for general
> test (it covers aarch64 and arm, and I plan to merge i386 tests to it
> too) and fix some bugs on instruction decoding exposed the new tests.

I pushed them in.

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-02-26 15:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-22 16:53 [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi
2016-02-22 16:53 ` [PATCH 2/3] Record right reg num of thumb special data instructions Yao Qi
2016-02-22 16:53 ` [PATCH 3/3] Fix various bugs in arm_record_exreg_ld_st_insn Yao Qi
2016-02-23 19:59   ` Luis Machado
2016-02-26 14:54     ` Yao Qi
2016-02-22 16:53 ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Yao Qi
2016-02-24 11:22   ` Pedro Alves
2016-02-24 14:40     ` Yao Qi
2016-02-24 14:40       ` [PATCH 1.5/3] Rename gdb.reverse/aarch64.{exp,c} to gdb.reverse/insn-reverse.{exp,c} Yao Qi
2016-02-24 19:37       ` [PATCH 1/3] Generalize gdb.reverse/aarch64.exp Pedro Alves
2016-02-26 15:01 ` [PATCH 0/3] Bug fixes in arm reverse debugging Yao Qi

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).