public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg.
@ 2015-11-10 11:21 Marcin Kościelnicki
  2015-11-10 11:21 ` [PATCH 2/2] gdb/testsuite/gdb.trace: Deduplicate set_point assembly Marcin Kościelnicki
  2015-11-10 11:35 ` [PATCH v2 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg Marcin Kościelnicki
  0 siblings, 2 replies; 7+ messages in thread
From: Marcin Kościelnicki @ 2015-11-10 11:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Marcin Kościelnicki

These variables were used in many gdb.trace tests.  Keep them in one place,
to reduce work needed for new targets.

gdb/testsuite/ChangeLog:

	* gdb.trace/backtrace.exp: Use global fpreg/spreg definition, add $
	in front.
	* gdb.trace/change-loc.exp: Use global pcreg definition.
	* gdb.trace/collection.exp: Use global pcreg/fpreg/spreg definition.
	* gdb.trace/entry-values.exp: Use global spreg definition, add $
	in front.
	* gdb.trace/mi-trace-frame-collected.exp: Use global pcreg definition.
	* gdb.trace/pending.exp: Likewise.
	* gdb.trace/report.exp: Use global pcreg/fpreg/spreg definition.
	* gdb.trace/trace-break.exp: Likewise.
	* gdb.trace/trace-condition.exp: Use global pcreg definition, add $
	in front.
	* gdb.trace/unavailable.exp: Use global pcreg/fpreg/spreg definition.
	* lib/trace-support.exp: Define fpreg, spreg, pcreg variables.
---
Preparation for s390 trace support.

 gdb/testsuite/ChangeLog                            | 17 ++++++++++++++++
 gdb/testsuite/gdb.trace/backtrace.exp              | 16 +--------------
 gdb/testsuite/gdb.trace/change-loc.exp             |  8 --------
 gdb/testsuite/gdb.trace/collection.exp             | 18 -----------------
 gdb/testsuite/gdb.trace/entry-values.exp           | 10 +---------
 .../gdb.trace/mi-trace-frame-collected.exp         | 14 -------------
 gdb/testsuite/gdb.trace/pending.exp                |  8 +-------
 gdb/testsuite/gdb.trace/report.exp                 | 18 -----------------
 gdb/testsuite/gdb.trace/trace-break.exp            | 16 ---------------
 gdb/testsuite/gdb.trace/trace-condition.exp        | 12 +----------
 gdb/testsuite/gdb.trace/unavailable.exp            | 18 -----------------
 gdb/testsuite/lib/trace-support.exp                | 23 ++++++++++++++++++++++
 12 files changed, 44 insertions(+), 134 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index dfc9b77..442f271 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,20 @@
+2015-11-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* gdb.trace/backtrace.exp: Use global fpreg/spreg definition, add $
+	in front.
+	* gdb.trace/change-loc.exp: Use global pcreg definition.
+	* gdb.trace/collection.exp: Use global pcreg/fpreg/spreg definition.
+	* gdb.trace/entry-values.exp: Use global spreg definition, add $
+	in front.
+	* gdb.trace/mi-trace-frame-collected.exp: Use global pcreg definition.
+	* gdb.trace/pending.exp: Likewise.
+	* gdb.trace/report.exp: Use global pcreg/fpreg/spreg definition.
+	* gdb.trace/trace-break.exp: Likewise.
+	* gdb.trace/trace-condition.exp: Use global pcreg definition, add $
+	in front.
+	* gdb.trace/unavailable.exp: Use global pcreg/fpreg/spreg definition.
+	* lib/trace-support.exp: Define fpreg, spreg, pcreg variables.
+
 2015-11-09  Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.ada/fin_fun_out: New testcase.
diff --git a/gdb/testsuite/gdb.trace/backtrace.exp b/gdb/testsuite/gdb.trace/backtrace.exp
index f69089b..c172029 100644
--- a/gdb/testsuite/gdb.trace/backtrace.exp
+++ b/gdb/testsuite/gdb.trace/backtrace.exp
@@ -140,23 +140,9 @@ gdb_trace_setactions "8.6: setup TP to collect regs, args, and locals" \
 	"$tdp4" \
 	"collect \$regs, \$args, \$locs" "^$"
 
-if [is_amd64_regs_target] {
-    set fpreg "\$rbp"
-    set spreg "\$rsp"
-} elseif [is_x86_like_target] {
-    set fpreg "\$ebp"
-    set spreg "\$esp"
-} elseif [is_aarch64_target] {
-    set fpreg "\$x29"
-    set spreg "\$sp"
-} else {
-    set fpreg "\$fp"
-    set spreg "\$sp"
-}
-
 gdb_trace_setactions "8.6: setup TP to collect stack mem cast expr" \
        "$tdp6" \
-       "collect $fpreg, \(\*\(void \*\*\) \($spreg\)\) @ 64" "^$"
+       "collect \$$fpreg, \(\*\(void \*\*\) \(\$$spreg\)\) @ 64" "^$"
 
 gdb_test_no_output "tstart" ""
 
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index a417ef9..97c604e 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -58,14 +58,6 @@ if { ![gdb_target_supports_trace] } then {
     return -1
 }
 
-if [is_amd64_regs_target] {
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set pcreg "eip"
-} else {
-    set pcreg "pc"
-}
-
 
 # Set tracepoint during tracing experiment.
 
diff --git a/gdb/testsuite/gdb.trace/collection.exp b/gdb/testsuite/gdb.trace/collection.exp
index 69ad6ee..987fb6f 100644
--- a/gdb/testsuite/gdb.trace/collection.exp
+++ b/gdb/testsuite/gdb.trace/collection.exp
@@ -36,24 +36,6 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug nowarnings}]} {
 set ws "\[\r\n\t \]+"
 set cr "\[\r\n\]+"
 
-if [is_amd64_regs_target] {
-    set fpreg "rbp"
-    set spreg "rsp"
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set fpreg "ebp"
-    set spreg "esp"
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set fpreg "x29"
-    set spreg "sp"
-    set pcreg "pc"
-} else {
-    set fpreg "fp"
-    set spreg "sp"
-    set pcreg "pc"
-}
-
 #
 # Utility procs
 #
diff --git a/gdb/testsuite/gdb.trace/entry-values.exp b/gdb/testsuite/gdb.trace/entry-values.exp
index 0cf5615..07aaa22 100644
--- a/gdb/testsuite/gdb.trace/entry-values.exp
+++ b/gdb/testsuite/gdb.trace/entry-values.exp
@@ -214,20 +214,12 @@ if ![gdb_target_supports_trace] {
 
 gdb_test "trace foo" "Tracepoint $decimal at .*"
 
-if [is_amd64_regs_target] {
-    set spreg "\$rsp"
-} elseif [is_x86_like_target] {
-    set spreg "\$esp"
-} else {
-    set spreg "\$sp"
-}
-
 # Collect arguments i and j.  Collect 'global1' which is entry value
 # of argument i.  Don't collect 'global2' to test the entry value of
 # argument j.
 
 gdb_trace_setactions "set action for tracepoint 1" "" \
-    "collect i, j, global1, \(\*\(void \*\*\) \($spreg\)\) @ 64" "^$"
+    "collect i, j, global1, \(\*\(void \*\*\) \(\$$spreg\)\) @ 64" "^$"
 
 gdb_test_no_output "tstart"
 
diff --git a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
index a7bed0b..932a548 100644
--- a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
+++ b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
@@ -51,20 +51,6 @@ mi_gdb_test "-break-insert -a gdb_recursion_test" \
 mi_gdb_test "-trace-define-variable \$tsv 1" {.*\^done} \
     "-trace-define-variable"
 
-set pcreg ""
-if [is_amd64_regs_target] {
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set pcreg "pc"
-} else {
-    # Other ports that support tracepoints should set the name of pc
-    # register here.
-    fail "set the name of pc register"
-    return -1
-}
-
 mi_gdb_test "-break-commands 3 \"collect gdb_char_test\" \"collect gdb_union1_test\" \"collect gdb_struct1_test.l\" \"collect gdb_arr_test\[0\]\" \"collect $${pcreg}\" \"teval \$tsv += 1\" \"collect \$tsv\" \"end\" " \
     {\^done} "set action"
 
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index 9938c5a..75e954a 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -421,6 +421,7 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
     global srcfile
     global lib_sl1
     global gdb_prompt
+    global pcreg
 
     # Start with a fresh gdb.
     clean_restart $executable
@@ -436,13 +437,6 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
 	}
     }
 
-    set pcreg "pc"
-    if [is_amd64_regs_target] {
-	set pcreg "rip"
-    } elseif [is_x86_like_target] {
-	set pcreg "eip"
-    }
-
     gdb_trace_setactions "set action for pending tracepoint" "" \
 	"collect \$$pcreg" "^$"
 
diff --git a/gdb/testsuite/gdb.trace/report.exp b/gdb/testsuite/gdb.trace/report.exp
index b3e9000..53ea943 100644
--- a/gdb/testsuite/gdb.trace/report.exp
+++ b/gdb/testsuite/gdb.trace/report.exp
@@ -150,24 +150,6 @@ gdb_trace_setactions "9.x: setup TP to collect locals" \
 	"$tdp4" \
 	"collect \$locs" "^$"
 
-if [is_amd64_regs_target] {
-    set fpreg "rbp"
-    set spreg "rsp"
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set fpreg "ebp"
-    set spreg "esp"
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set fpreg "x29"
-    set spreg "sp"
-    set pcreg "pc"
-} else {
-    set fpreg "fp"
-    set spreg "sp"
-    set pcreg "pc"
-}
-
 gdb_trace_setactions "9.x: setup TP to collect stack memory" \
 	"$tdp5" \
 	"collect \$$fpreg, \*\(void \*\*\) \$$spreg @ 64" "^$"
diff --git a/gdb/testsuite/gdb.trace/trace-break.exp b/gdb/testsuite/gdb.trace/trace-break.exp
index 1f57601..4329f3a 100644
--- a/gdb/testsuite/gdb.trace/trace-break.exp
+++ b/gdb/testsuite/gdb.trace/trace-break.exp
@@ -37,22 +37,6 @@ if ![gdb_target_supports_trace] {
     return -1
 }
 
-set fpreg "fp"
-set spreg "sp"
-set pcreg "pc"
-
-if [is_amd64_regs_target] {
-    set fpreg "rbp"
-    set spreg "rsp"
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set fpreg "ebp"
-    set spreg "esp"
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set fpreg "x29"
-}
-
 # Set breakpoint and tracepoint at the same address.
 
 proc break_trace_same_addr_1 { trace_type option } \
diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index aec0401..15efb68 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -21,16 +21,6 @@ set expfile $testfile.exp
 # Some targets have leading underscores on assembly symbols.
 set additional_flags [gdb_target_symbol_prefix_flags]
 
-if [is_amd64_regs_target] {
-    set pcreg "\$rip"
-} elseif [is_x86_like_target] {
-    set pcreg "\$eip"
-} elseif [is_aarch64_target] {
-    set pcreg "\$pc"
-} else {
-    set pcreg "\$pc"
-}
-
 if [prepare_for_testing $expfile $executable $srcfile \
 	[list debug $additional_flags]] {
     untested "failed to prepare for trace tests"
@@ -139,7 +129,7 @@ proc 18955_i386_failure { trace_command } {
 foreach trace_command { "trace" "ftrace" } {
     # This condition is always true as the PC should be set to the tracepoint
     # address when hit.
-    test_tracepoints $trace_command "$pcreg == *set_point" 10
+    test_tracepoints $trace_command "\$$pcreg == *set_point" 10
 
     # Can we read local variables?
     test_tracepoints $trace_command "anarg == 100 || anarg == 200" 2 18955_x86_64_failure
diff --git a/gdb/testsuite/gdb.trace/unavailable.exp b/gdb/testsuite/gdb.trace/unavailable.exp
index 910c1dd..5f70fc5 100644
--- a/gdb/testsuite/gdb.trace/unavailable.exp
+++ b/gdb/testsuite/gdb.trace/unavailable.exp
@@ -26,24 +26,6 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile \
 set ws "\[\r\n\t \]+"
 set cr "\[\r\n\]+"
 
-if [is_amd64_regs_target] {
-    set fpreg "rbp"
-    set spreg "rsp"
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set fpreg "ebp"
-    set spreg "esp"
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set fpreg "x29"
-    set spreg "sp"
-    set pcreg "pc"
-} else {
-    set fpreg "fp"
-    set spreg "sp"
-    set pcreg "pc"
-}
-
 #
 # Utility procs
 #
diff --git a/gdb/testsuite/lib/trace-support.exp b/gdb/testsuite/lib/trace-support.exp
index 0af73b4..4ec8cac 100644
--- a/gdb/testsuite/lib/trace-support.exp
+++ b/gdb/testsuite/lib/trace-support.exp
@@ -20,6 +20,29 @@
 
 
 #
+# Program counter / stack pointer / frame pointer for supported targets.
+# Used in many tests, kept here to avoid duplication.
+#
+
+if [is_amd64_regs_target] {
+    set fpreg "rbp"
+    set spreg "rsp"
+    set pcreg "rip"
+} elseif [is_x86_like_target] {
+    set fpreg "ebp"
+    set spreg "esp"
+    set pcreg "eip"
+} elseif [is_aarch64_target] {
+    set fpreg "x29"
+    set spreg "sp"
+    set pcreg "pc"
+} else {
+    set fpreg "fp"
+    set spreg "sp"
+    set pcreg "pc"
+}
+
+#
 # Procedure: gdb_target_supports_trace
 # Returns true if GDB is connected to a target that supports tracing.
 # Allows tests to abort early if not running on a trace-aware target.
-- 
2.6.2

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

* [PATCH 2/2] gdb/testsuite/gdb.trace: Deduplicate set_point assembly.
  2015-11-10 11:21 [PATCH 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg Marcin Kościelnicki
@ 2015-11-10 11:21 ` Marcin Kościelnicki
  2015-11-11  8:42   ` Yao Qi
  2015-11-10 11:35 ` [PATCH v2 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg Marcin Kościelnicki
  1 sibling, 1 reply; 7+ messages in thread
From: Marcin Kościelnicki @ 2015-11-10 11:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Marcin Kościelnicki

The assembly code for emitting the proper tracepointable instruction
was duplicated in many places.  Keep it in one place, to reduce work
needed for new targets.

gdb/testsuite/ChangeLog:

	* gdb.trace/change-loc.h: include "trace-common.h", remove SYMBOL
	macro.
	(func5): Removed.
	(func4): Use TRACEPOINT_ASM.
	* gdb.trace/ftrace-lock.c: include "trace-common.h", remove SYMBOL
	macro.
	(func): Removed.
	(thread_function): Use TRACEPOINT_ASM.
	* gdb.trace/ftrace.c: include "trace-common.h", remove SYMBOL macro.
	(func): Remove.
	(marker): Use TRACEPOINT_ASM.
	* gdb.trace/pendshr1.c: include "trace-common.h", remove SYMBOL macro.
	(pendfunc1): Remove.
	(pendfunc): Use TRACEPOINT_ASM.
	* gdb.trace/pendshr2.c: include "trace-common.h", remove SYMBOL macro.
	(foo): Remove.
	(pendfunc2): Use TRACEPOINT_ASM.
	* gdb.trace/trace-break.c: include "trace-common.h", remove SYMBOL
	macro.
	(func): Remove.
	(marker): Use TRACEPOINT_ASM.
	* gdb.trace/trace-common.h: New header.
	* gdb.trace/trace-condition.c: include "trace-common.h", remove SYMBOL
	macro.
	(func): Remove.
	(marker): Use TRACEPOINT_ASM.
	* gdb.trace/trace-mt.c: include "trace-common.h", remove SYMBOL macro.
	(func): Remove.
	(thread_function): Use TRACEPOINT_ASM.
---
Preparation for s390 trace support.

 gdb/testsuite/ChangeLog                   | 32 ++++++++++++++++++
 gdb/testsuite/gdb.trace/change-loc.h      | 24 ++------------
 gdb/testsuite/gdb.trace/ftrace-lock.c     | 25 ++------------
 gdb/testsuite/gdb.trace/ftrace.c          | 23 ++-----------
 gdb/testsuite/gdb.trace/pendshr1.c        | 25 ++------------
 gdb/testsuite/gdb.trace/pendshr2.c        | 22 ++-----------
 gdb/testsuite/gdb.trace/trace-break.c     | 32 ++----------------
 gdb/testsuite/gdb.trace/trace-common.h    | 55 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.trace/trace-condition.c | 24 ++------------
 gdb/testsuite/gdb.trace/trace-mt.c        | 23 ++-----------
 10 files changed, 104 insertions(+), 181 deletions(-)
 create mode 100644 gdb/testsuite/gdb.trace/trace-common.h

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 442f271..bc58d81 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,37 @@
 2015-11-10  Marcin Kościelnicki  <koriakin@0x04.net>
 
+	* gdb.trace/change-loc.h: include "trace-common.h", remove SYMBOL
+	macro.
+	(func5): Removed.
+	(func4): Use TRACEPOINT_ASM.
+	* gdb.trace/ftrace-lock.c: include "trace-common.h", remove SYMBOL
+	macro.
+	(func): Removed.
+	(thread_function): Use TRACEPOINT_ASM.
+	* gdb.trace/ftrace.c: include "trace-common.h", remove SYMBOL macro.
+	(func): Remove.
+	(marker): Use TRACEPOINT_ASM.
+	* gdb.trace/pendshr1.c: include "trace-common.h", remove SYMBOL macro.
+	(pendfunc1): Remove.
+	(pendfunc): Use TRACEPOINT_ASM.
+	* gdb.trace/pendshr2.c: include "trace-common.h", remove SYMBOL macro.
+	(foo): Remove.
+	(pendfunc2): Use TRACEPOINT_ASM.
+	* gdb.trace/trace-break.c: include "trace-common.h", remove SYMBOL
+	macro.
+	(func): Remove.
+	(marker): Use TRACEPOINT_ASM.
+	* gdb.trace/trace-common.h: New header.
+	* gdb.trace/trace-condition.c: include "trace-common.h", remove SYMBOL
+	macro.
+	(func): Remove.
+	(marker): Use TRACEPOINT_ASM.
+	* gdb.trace/trace-mt.c: include "trace-common.h", remove SYMBOL macro.
+	(func): Remove.
+	(thread_function): Use TRACEPOINT_ASM.
+
+2015-11-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
 	* gdb.trace/backtrace.exp: Use global fpreg/spreg definition, add $
 	in front.
 	* gdb.trace/change-loc.exp: Use global pcreg definition.
diff --git a/gdb/testsuite/gdb.trace/change-loc.h b/gdb/testsuite/gdb.trace/change-loc.h
index 8201455..aaf823a 100644
--- a/gdb/testsuite/gdb.trace/change-loc.h
+++ b/gdb/testsuite/gdb.trace/change-loc.h
@@ -15,30 +15,10 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef SYMBOL_PREFIX
-#define SYMBOL(str)     SYMBOL_PREFIX #str
-#else
-#define SYMBOL(str)     #str
-#endif
-
-/* Called from asm.  */
-static void __attribute__((used))
-func5 (void)
-{}
+#include "trace-common.h"
 
 static void
 func4 (void)
 {
-  /* `set_tracepoint' is the label where we'll set multiple tracepoints and
-     breakpoints at.  The insn at the label must the large enough to
-     fit a fast tracepoint jump.  */
-  asm ("    .global " SYMBOL(set_tracepoint) "\n"
-       SYMBOL(set_tracepoint) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL(func5) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
-
+  TRACEPOINT_ASM(set_tracepoint);
 }
diff --git a/gdb/testsuite/gdb.trace/ftrace-lock.c b/gdb/testsuite/gdb.trace/ftrace-lock.c
index 8ed45f4..e5e1a16 100644
--- a/gdb/testsuite/gdb.trace/ftrace-lock.c
+++ b/gdb/testsuite/gdb.trace/ftrace-lock.c
@@ -16,12 +16,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <pthread.h>
-
-#ifdef SYMBOL_PREFIX
-#define SYMBOL(str)     SYMBOL_PREFIX #str
-#else
-#define SYMBOL(str)     #str
-#endif
+#include "trace-common.h"
 
 /* Called if the testcase failed.  */
 static void
@@ -61,26 +56,10 @@ gdb_agent_gdb_collect (void *tpoint, unsigned char *regs)
     }
 }
 
-/* Called from asm.  */
-static void __attribute__((used))
-func (void)
-{
-}
-
 static void *
 thread_function (void *arg)
 {
-  /* `set_point' is the label at which to set a fast tracepoint.  The
-     insn at the label must be large enough to fit a fast tracepoint
-     jump.  */
-  asm ("    .global " SYMBOL (set_point) "\n"
-       SYMBOL (set_point) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL (func) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
+  TRACEPOINT_ASM(set_point);
 }
 
 static void
diff --git a/gdb/testsuite/gdb.trace/ftrace.c b/gdb/testsuite/gdb.trace/ftrace.c
index 7373d66..5869af0 100644
--- a/gdb/testsuite/gdb.trace/ftrace.c
+++ b/gdb/testsuite/gdb.trace/ftrace.c
@@ -15,11 +15,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef SYMBOL_PREFIX
-#define SYMBOL(str)     SYMBOL_PREFIX #str
-#else
-#define SYMBOL(str)     #str
-#endif
+#include "trace-common.h"
 
 int globvar;
 
@@ -27,25 +23,10 @@ static void
 begin (void)
 {}
 
-/* Called from asm.  */
-static void __attribute__((used))
-func (void)
-{}
-
 static void
 marker (int anarg)
 {
-  /* `set_point' is the label at which to set a fast tracepoint.  The
-     insn at the label must be large enough to fit a fast tracepoint
-     jump.  */
-  asm ("    .global " SYMBOL(set_point) "\n"
-       SYMBOL(set_point) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL(func) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
+  TRACEPOINT_ASM(set_point);
 
   ++anarg;
 
diff --git a/gdb/testsuite/gdb.trace/pendshr1.c b/gdb/testsuite/gdb.trace/pendshr1.c
index f08fb91..065227f 100644
--- a/gdb/testsuite/gdb.trace/pendshr1.c
+++ b/gdb/testsuite/gdb.trace/pendshr1.c
@@ -15,31 +15,10 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef SYMBOL_PREFIX
-#define SYMBOL(str)     SYMBOL_PREFIX #str
-#else
-#define SYMBOL(str)     #str
-#endif
-
-static void
-pendfunc1 (void)
-{
-  int x = 0;
-  int y = x + 4;
-}
+#include "trace-common.h"
 
 void
 pendfunc (int x)
 {
-  /* `set_point1' is the label where we'll set multiple tracepoints and
-     breakpoints at.  The insn at the label must the large enough to
-     fit a fast tracepoint jump.  */
-  asm ("    .global " SYMBOL(set_point1) "\n"
-       SYMBOL(set_point1) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL(pendfunc1) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
+  TRACEPOINT_ASM(set_point1);
 }
diff --git a/gdb/testsuite/gdb.trace/pendshr2.c b/gdb/testsuite/gdb.trace/pendshr2.c
index f7ec733..71d5220 100644
--- a/gdb/testsuite/gdb.trace/pendshr2.c
+++ b/gdb/testsuite/gdb.trace/pendshr2.c
@@ -15,28 +15,10 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef SYMBOL_PREFIX
-#define SYMBOL(str)     SYMBOL_PREFIX #str
-#else
-#define SYMBOL(str)     #str
-#endif
-
-static void
-foo ()
-{}
+#include "trace-common.h"
 
 void
 pendfunc2 (int x)
 {
-  /* `set_point2' is the label where we'll set multiple tracepoints and
-     breakpoints at.  The insn at the label must the large enough to
-     fit a fast tracepoint jump.  */
-  asm ("    .global " SYMBOL(set_point2) "\n"
-       SYMBOL(set_point2) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL(foo) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
+  TRACEPOINT_ASM(set_point2);
 }
diff --git a/gdb/testsuite/gdb.trace/trace-break.c b/gdb/testsuite/gdb.trace/trace-break.c
index 66bbe53..a1e80cf 100644
--- a/gdb/testsuite/gdb.trace/trace-break.c
+++ b/gdb/testsuite/gdb.trace/trace-break.c
@@ -15,16 +15,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef SYMBOL_PREFIX
-#define SYMBOL(str)     SYMBOL_PREFIX #str
-#else
-#define SYMBOL(str)     #str
-#endif
-
-/* Called from asm.  */
-static void __attribute__((used))
-func (void)
-{}
+#include "trace-common.h"
 
 static void
 marker (void)
@@ -34,26 +25,9 @@ marker (void)
   int a = 0;
   int b = a;
 
-  /* `set_point' is the label where we'll set multiple tracepoints and
-     breakpoints at.  The insn at the label must the large enough to
-     fit a fast tracepoint jump.  */
-  asm ("    .global " SYMBOL(set_point) "\n"
-       SYMBOL(set_point) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL(func) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
+  TRACEPOINT_ASM(set_point);
 
-  asm ("    .global " SYMBOL(after_set_point) "\n"
-       SYMBOL(after_set_point) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL(func) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
+  TRACEPOINT_ASM(after_set_point);
 }
 
 static void
diff --git a/gdb/testsuite/gdb.trace/trace-common.h b/gdb/testsuite/gdb.trace/trace-common.h
new file mode 100644
index 0000000..5f5bd6f
--- /dev/null
+++ b/gdb/testsuite/gdb.trace/trace-common.h
@@ -0,0 +1,55 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011-2015 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/>.  */
+
+#ifdef SYMBOL_PREFIX
+#define SYMBOL(str)     SYMBOL_PREFIX #str
+#else
+#define SYMBOL(str)     #str
+#endif
+
+/* TRACEPOINT_ASM expands to an assembly instruction large enough to fit
+   a fast tracepoint jump.  The parameter is the label where we'll set
+   tracepoints and breakpoints.  */
+
+#if (defined __x86_64__ || defined __i386__)
+
+static void
+x86_trace_dummy ()
+{
+  int x = 0;
+  int y = x + 4;
+}
+
+#define TRACEPOINT_ASM(name) \
+  asm ("    .global " SYMBOL(name) "\n" \
+       SYMBOL(name) ":\n" \
+       "    call " SYMBOL(x86_trace_dummy) "\n" \
+       )
+
+#elif (defined __aarch64__)
+
+#define TRACEPOINT_ASM(name) \
+  asm ("    .global " SYMBOL(name) "\n" \
+       SYMBOL(name) ":\n" \
+       "    nop\n" \
+       )
+
+#else
+
+#error "unsupported architecture for trace tests"
+
+#endif
diff --git a/gdb/testsuite/gdb.trace/trace-condition.c b/gdb/testsuite/gdb.trace/trace-condition.c
index d988d76..6cca8be 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.c
+++ b/gdb/testsuite/gdb.trace/trace-condition.c
@@ -15,11 +15,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-#ifdef SYMBOL_PREFIX
-#define SYMBOL(str)     SYMBOL_PREFIX #str
-#else
-#define SYMBOL(str)     #str
-#endif
+#include "trace-common.h"
 
 int globvar;
 
@@ -28,26 +24,10 @@ begin (void)
 {
 }
 
-/* Called from asm.  */
-static void __attribute__((used))
-func (void)
-{
-}
-
 static void
 marker (int anarg)
 {
-  /* `set_point' is the label at which to set a fast tracepoint.  The
-     insn at the label must be large enough to fit a fast tracepoint
-     jump.  */
-  asm ("    .global " SYMBOL (set_point) "\n"
-       SYMBOL (set_point) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL (func) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
+  TRACEPOINT_ASM(set_point);
 }
 
 static void
diff --git a/gdb/testsuite/gdb.trace/trace-mt.c b/gdb/testsuite/gdb.trace/trace-mt.c
index 7ae0305..a8adb0d 100644
--- a/gdb/testsuite/gdb.trace/trace-mt.c
+++ b/gdb/testsuite/gdb.trace/trace-mt.c
@@ -16,31 +16,12 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include <pthread.h>
-
-#ifdef SYMBOL_PREFIX
-#define SYMBOL(str)     SYMBOL_PREFIX #str
-#else
-#define SYMBOL(str)     #str
-#endif
-/* Called from asm.  */
-static void __attribute__((used))
-func (void)
-{}
+#include "trace-common.h"
 
 static void *
 thread_function(void *arg)
 {
-  /* `set_point1' is the label at which to set a fast tracepoint.  The
-     insn at the label must be large enough to fit a fast tracepoint
-     jump.  */
-  asm ("    .global " SYMBOL(set_point1) "\n"
-       SYMBOL(set_point1) ":\n"
-#if (defined __x86_64__ || defined __i386__)
-       "    call " SYMBOL(func) "\n"
-#elif (defined __aarch64__)
-       "    nop\n"
-#endif
-       );
+  TRACEPOINT_ASM(set_point1);
 }
 
 static void
-- 
2.6.2

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

* [PATCH v2 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg.
  2015-11-10 11:21 [PATCH 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg Marcin Kościelnicki
  2015-11-10 11:21 ` [PATCH 2/2] gdb/testsuite/gdb.trace: Deduplicate set_point assembly Marcin Kościelnicki
@ 2015-11-10 11:35 ` Marcin Kościelnicki
  2015-11-10 17:55   ` Yao Qi
  1 sibling, 1 reply; 7+ messages in thread
From: Marcin Kościelnicki @ 2015-11-10 11:35 UTC (permalink / raw)
  To: gdb-patches; +Cc: Marcin Kościelnicki

These variables were used in many gdb.trace tests.  Keep them in one place,
to reduce work needed for new targets.

gdb/testsuite/ChangeLog:

	* gdb.trace/backtrace.exp: Use global fpreg/spreg definition, add $
	in front.
	* gdb.trace/change-loc.exp: Use global pcreg definition.
	* gdb.trace/collection.exp: Use global pcreg/fpreg/spreg definition.
	* gdb.trace/entry-values.exp: Use global spreg definition, add $
	in front.
	* gdb.trace/mi-trace-frame-collected.exp: Use global pcreg definition.
	* gdb.trace/pending.exp: Likewise.
	* gdb.trace/report.exp: Use global pcreg/fpreg/spreg definition.
	* gdb.trace/trace-break.exp: Likewise.
	* gdb.trace/trace-condition.exp: Use global pcreg definition, add $
	in front.
	* gdb.trace/unavailable.exp: Use global pcreg/fpreg/spreg definition.
	* gdb.trace/while-dyn.exp: Use global fpreg definition, add $
	in front.
	* lib/trace-support.exp: Define fpreg, spreg, pcreg variables.
---
Whoops, missed the one in while-dyn.exp.

 gdb/testsuite/ChangeLog                            | 19 ++++++++++++++++++
 gdb/testsuite/gdb.trace/backtrace.exp              | 16 +--------------
 gdb/testsuite/gdb.trace/change-loc.exp             |  8 --------
 gdb/testsuite/gdb.trace/collection.exp             | 18 -----------------
 gdb/testsuite/gdb.trace/entry-values.exp           | 10 +---------
 .../gdb.trace/mi-trace-frame-collected.exp         | 14 -------------
 gdb/testsuite/gdb.trace/pending.exp                |  8 +-------
 gdb/testsuite/gdb.trace/report.exp                 | 18 -----------------
 gdb/testsuite/gdb.trace/trace-break.exp            | 16 ---------------
 gdb/testsuite/gdb.trace/trace-condition.exp        | 12 +----------
 gdb/testsuite/gdb.trace/unavailable.exp            | 18 -----------------
 gdb/testsuite/gdb.trace/while-dyn.exp              | 12 +----------
 gdb/testsuite/lib/trace-support.exp                | 23 ++++++++++++++++++++++
 13 files changed, 47 insertions(+), 145 deletions(-)

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index dfc9b77..0f04109 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,22 @@
+2015-11-10  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* gdb.trace/backtrace.exp: Use global fpreg/spreg definition, add $
+	in front.
+	* gdb.trace/change-loc.exp: Use global pcreg definition.
+	* gdb.trace/collection.exp: Use global pcreg/fpreg/spreg definition.
+	* gdb.trace/entry-values.exp: Use global spreg definition, add $
+	in front.
+	* gdb.trace/mi-trace-frame-collected.exp: Use global pcreg definition.
+	* gdb.trace/pending.exp: Likewise.
+	* gdb.trace/report.exp: Use global pcreg/fpreg/spreg definition.
+	* gdb.trace/trace-break.exp: Likewise.
+	* gdb.trace/trace-condition.exp: Use global pcreg definition, add $
+	in front.
+	* gdb.trace/unavailable.exp: Use global pcreg/fpreg/spreg definition.
+	* gdb.trace/while-dyn.exp: Use global fpreg definition, add $
+	in front.
+	* lib/trace-support.exp: Define fpreg, spreg, pcreg variables.
+
 2015-11-09  Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.ada/fin_fun_out: New testcase.
diff --git a/gdb/testsuite/gdb.trace/backtrace.exp b/gdb/testsuite/gdb.trace/backtrace.exp
index f69089b..c172029 100644
--- a/gdb/testsuite/gdb.trace/backtrace.exp
+++ b/gdb/testsuite/gdb.trace/backtrace.exp
@@ -140,23 +140,9 @@ gdb_trace_setactions "8.6: setup TP to collect regs, args, and locals" \
 	"$tdp4" \
 	"collect \$regs, \$args, \$locs" "^$"
 
-if [is_amd64_regs_target] {
-    set fpreg "\$rbp"
-    set spreg "\$rsp"
-} elseif [is_x86_like_target] {
-    set fpreg "\$ebp"
-    set spreg "\$esp"
-} elseif [is_aarch64_target] {
-    set fpreg "\$x29"
-    set spreg "\$sp"
-} else {
-    set fpreg "\$fp"
-    set spreg "\$sp"
-}
-
 gdb_trace_setactions "8.6: setup TP to collect stack mem cast expr" \
        "$tdp6" \
-       "collect $fpreg, \(\*\(void \*\*\) \($spreg\)\) @ 64" "^$"
+       "collect \$$fpreg, \(\*\(void \*\*\) \(\$$spreg\)\) @ 64" "^$"
 
 gdb_test_no_output "tstart" ""
 
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index a417ef9..97c604e 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -58,14 +58,6 @@ if { ![gdb_target_supports_trace] } then {
     return -1
 }
 
-if [is_amd64_regs_target] {
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set pcreg "eip"
-} else {
-    set pcreg "pc"
-}
-
 
 # Set tracepoint during tracing experiment.
 
diff --git a/gdb/testsuite/gdb.trace/collection.exp b/gdb/testsuite/gdb.trace/collection.exp
index 69ad6ee..987fb6f 100644
--- a/gdb/testsuite/gdb.trace/collection.exp
+++ b/gdb/testsuite/gdb.trace/collection.exp
@@ -36,24 +36,6 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile {debug nowarnings}]} {
 set ws "\[\r\n\t \]+"
 set cr "\[\r\n\]+"
 
-if [is_amd64_regs_target] {
-    set fpreg "rbp"
-    set spreg "rsp"
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set fpreg "ebp"
-    set spreg "esp"
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set fpreg "x29"
-    set spreg "sp"
-    set pcreg "pc"
-} else {
-    set fpreg "fp"
-    set spreg "sp"
-    set pcreg "pc"
-}
-
 #
 # Utility procs
 #
diff --git a/gdb/testsuite/gdb.trace/entry-values.exp b/gdb/testsuite/gdb.trace/entry-values.exp
index 0cf5615..07aaa22 100644
--- a/gdb/testsuite/gdb.trace/entry-values.exp
+++ b/gdb/testsuite/gdb.trace/entry-values.exp
@@ -214,20 +214,12 @@ if ![gdb_target_supports_trace] {
 
 gdb_test "trace foo" "Tracepoint $decimal at .*"
 
-if [is_amd64_regs_target] {
-    set spreg "\$rsp"
-} elseif [is_x86_like_target] {
-    set spreg "\$esp"
-} else {
-    set spreg "\$sp"
-}
-
 # Collect arguments i and j.  Collect 'global1' which is entry value
 # of argument i.  Don't collect 'global2' to test the entry value of
 # argument j.
 
 gdb_trace_setactions "set action for tracepoint 1" "" \
-    "collect i, j, global1, \(\*\(void \*\*\) \($spreg\)\) @ 64" "^$"
+    "collect i, j, global1, \(\*\(void \*\*\) \(\$$spreg\)\) @ 64" "^$"
 
 gdb_test_no_output "tstart"
 
diff --git a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
index a7bed0b..932a548 100644
--- a/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
+++ b/gdb/testsuite/gdb.trace/mi-trace-frame-collected.exp
@@ -51,20 +51,6 @@ mi_gdb_test "-break-insert -a gdb_recursion_test" \
 mi_gdb_test "-trace-define-variable \$tsv 1" {.*\^done} \
     "-trace-define-variable"
 
-set pcreg ""
-if [is_amd64_regs_target] {
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set pcreg "pc"
-} else {
-    # Other ports that support tracepoints should set the name of pc
-    # register here.
-    fail "set the name of pc register"
-    return -1
-}
-
 mi_gdb_test "-break-commands 3 \"collect gdb_char_test\" \"collect gdb_union1_test\" \"collect gdb_struct1_test.l\" \"collect gdb_arr_test\[0\]\" \"collect $${pcreg}\" \"teval \$tsv += 1\" \"collect \$tsv\" \"end\" " \
     {\^done} "set action"
 
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index 9938c5a..75e954a 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -421,6 +421,7 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
     global srcfile
     global lib_sl1
     global gdb_prompt
+    global pcreg
 
     # Start with a fresh gdb.
     clean_restart $executable
@@ -436,13 +437,6 @@ proc pending_tracepoint_with_action_resolved { trace_type } \
 	}
     }
 
-    set pcreg "pc"
-    if [is_amd64_regs_target] {
-	set pcreg "rip"
-    } elseif [is_x86_like_target] {
-	set pcreg "eip"
-    }
-
     gdb_trace_setactions "set action for pending tracepoint" "" \
 	"collect \$$pcreg" "^$"
 
diff --git a/gdb/testsuite/gdb.trace/report.exp b/gdb/testsuite/gdb.trace/report.exp
index b3e9000..53ea943 100644
--- a/gdb/testsuite/gdb.trace/report.exp
+++ b/gdb/testsuite/gdb.trace/report.exp
@@ -150,24 +150,6 @@ gdb_trace_setactions "9.x: setup TP to collect locals" \
 	"$tdp4" \
 	"collect \$locs" "^$"
 
-if [is_amd64_regs_target] {
-    set fpreg "rbp"
-    set spreg "rsp"
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set fpreg "ebp"
-    set spreg "esp"
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set fpreg "x29"
-    set spreg "sp"
-    set pcreg "pc"
-} else {
-    set fpreg "fp"
-    set spreg "sp"
-    set pcreg "pc"
-}
-
 gdb_trace_setactions "9.x: setup TP to collect stack memory" \
 	"$tdp5" \
 	"collect \$$fpreg, \*\(void \*\*\) \$$spreg @ 64" "^$"
diff --git a/gdb/testsuite/gdb.trace/trace-break.exp b/gdb/testsuite/gdb.trace/trace-break.exp
index 1f57601..4329f3a 100644
--- a/gdb/testsuite/gdb.trace/trace-break.exp
+++ b/gdb/testsuite/gdb.trace/trace-break.exp
@@ -37,22 +37,6 @@ if ![gdb_target_supports_trace] {
     return -1
 }
 
-set fpreg "fp"
-set spreg "sp"
-set pcreg "pc"
-
-if [is_amd64_regs_target] {
-    set fpreg "rbp"
-    set spreg "rsp"
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set fpreg "ebp"
-    set spreg "esp"
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set fpreg "x29"
-}
-
 # Set breakpoint and tracepoint at the same address.
 
 proc break_trace_same_addr_1 { trace_type option } \
diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index aec0401..15efb68 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -21,16 +21,6 @@ set expfile $testfile.exp
 # Some targets have leading underscores on assembly symbols.
 set additional_flags [gdb_target_symbol_prefix_flags]
 
-if [is_amd64_regs_target] {
-    set pcreg "\$rip"
-} elseif [is_x86_like_target] {
-    set pcreg "\$eip"
-} elseif [is_aarch64_target] {
-    set pcreg "\$pc"
-} else {
-    set pcreg "\$pc"
-}
-
 if [prepare_for_testing $expfile $executable $srcfile \
 	[list debug $additional_flags]] {
     untested "failed to prepare for trace tests"
@@ -139,7 +129,7 @@ proc 18955_i386_failure { trace_command } {
 foreach trace_command { "trace" "ftrace" } {
     # This condition is always true as the PC should be set to the tracepoint
     # address when hit.
-    test_tracepoints $trace_command "$pcreg == *set_point" 10
+    test_tracepoints $trace_command "\$$pcreg == *set_point" 10
 
     # Can we read local variables?
     test_tracepoints $trace_command "anarg == 100 || anarg == 200" 2 18955_x86_64_failure
diff --git a/gdb/testsuite/gdb.trace/unavailable.exp b/gdb/testsuite/gdb.trace/unavailable.exp
index 910c1dd..5f70fc5 100644
--- a/gdb/testsuite/gdb.trace/unavailable.exp
+++ b/gdb/testsuite/gdb.trace/unavailable.exp
@@ -26,24 +26,6 @@ if {[prepare_for_testing $testfile.exp $testfile $srcfile \
 set ws "\[\r\n\t \]+"
 set cr "\[\r\n\]+"
 
-if [is_amd64_regs_target] {
-    set fpreg "rbp"
-    set spreg "rsp"
-    set pcreg "rip"
-} elseif [is_x86_like_target] {
-    set fpreg "ebp"
-    set spreg "esp"
-    set pcreg "eip"
-} elseif [is_aarch64_target] {
-    set fpreg "x29"
-    set spreg "sp"
-    set pcreg "pc"
-} else {
-    set fpreg "fp"
-    set spreg "sp"
-    set pcreg "pc"
-}
-
 #
 # Utility procs
 #
diff --git a/gdb/testsuite/gdb.trace/while-dyn.exp b/gdb/testsuite/gdb.trace/while-dyn.exp
index fe4535e..765edfe 100644
--- a/gdb/testsuite/gdb.trace/while-dyn.exp
+++ b/gdb/testsuite/gdb.trace/while-dyn.exp
@@ -43,16 +43,6 @@ if { ![gdb_target_supports_trace] } then {
 # test while-stepping dynamically (live target)
 #
 
-if [is_amd64_regs_target] {
-    set fpreg "\$rbp"
-} elseif [is_x86_like_target] {
-    set fpreg "\$ebp"
-} elseif [is_aarch64_target] {
-    set fpreg "\$x29"
-} else {
-    set fpreg "\$fp"
-}
-
 proc test_while_stepping { while_stepping } {
     global fpreg
     global decimal
@@ -74,7 +64,7 @@ proc test_while_stepping { while_stepping } {
 
     gdb_trace_setactions "5.12: define $while_stepping <stepcount>" \
 	"" \
-	"collect $fpreg" "^$" \
+	"collect \$$fpreg" "^$" \
 	"$while_stepping 5" "^$" \
 	"collect p" "^$" \
 	"end" "^$" \
diff --git a/gdb/testsuite/lib/trace-support.exp b/gdb/testsuite/lib/trace-support.exp
index 0af73b4..4ec8cac 100644
--- a/gdb/testsuite/lib/trace-support.exp
+++ b/gdb/testsuite/lib/trace-support.exp
@@ -20,6 +20,29 @@
 
 
 #
+# Program counter / stack pointer / frame pointer for supported targets.
+# Used in many tests, kept here to avoid duplication.
+#
+
+if [is_amd64_regs_target] {
+    set fpreg "rbp"
+    set spreg "rsp"
+    set pcreg "rip"
+} elseif [is_x86_like_target] {
+    set fpreg "ebp"
+    set spreg "esp"
+    set pcreg "eip"
+} elseif [is_aarch64_target] {
+    set fpreg "x29"
+    set spreg "sp"
+    set pcreg "pc"
+} else {
+    set fpreg "fp"
+    set spreg "sp"
+    set pcreg "pc"
+}
+
+#
 # Procedure: gdb_target_supports_trace
 # Returns true if GDB is connected to a target that supports tracing.
 # Allows tests to abort early if not running on a trace-aware target.
-- 
2.6.2

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

* Re: [PATCH v2 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg.
  2015-11-10 11:35 ` [PATCH v2 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg Marcin Kościelnicki
@ 2015-11-10 17:55   ` Yao Qi
  2015-11-10 19:08     ` Marcin Kościelnicki
  0 siblings, 1 reply; 7+ messages in thread
From: Yao Qi @ 2015-11-10 17:55 UTC (permalink / raw)
  To: Marcin Kościelnicki; +Cc: gdb-patches

Marcin Kościelnicki <koriakin@0x04.net> writes:

> gdb/testsuite/ChangeLog:
>
> 	* gdb.trace/backtrace.exp: Use global fpreg/spreg definition, add $
> 	in front.
> 	* gdb.trace/change-loc.exp: Use global pcreg definition.
> 	* gdb.trace/collection.exp: Use global pcreg/fpreg/spreg definition.
> 	* gdb.trace/entry-values.exp: Use global spreg definition, add $
> 	in front.
> 	* gdb.trace/mi-trace-frame-collected.exp: Use global pcreg definition.
> 	* gdb.trace/pending.exp: Likewise.
> 	* gdb.trace/report.exp: Use global pcreg/fpreg/spreg definition.
> 	* gdb.trace/trace-break.exp: Likewise.
> 	* gdb.trace/trace-condition.exp: Use global pcreg definition, add $
> 	in front.
> 	* gdb.trace/unavailable.exp: Use global pcreg/fpreg/spreg definition.
> 	* gdb.trace/while-dyn.exp: Use global fpreg definition, add $
> 	in front.
> 	* lib/trace-support.exp: Define fpreg, spreg, pcreg variables.

Looks good to me.  Thanks.

-- 
Yao (齐尧)

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

* Re: [PATCH v2 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg.
  2015-11-10 17:55   ` Yao Qi
@ 2015-11-10 19:08     ` Marcin Kościelnicki
  0 siblings, 0 replies; 7+ messages in thread
From: Marcin Kościelnicki @ 2015-11-10 19:08 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 10/11/15 18:55, Yao Qi wrote:
> Marcin Kościelnicki <koriakin@0x04.net> writes:
>
>> gdb/testsuite/ChangeLog:
>>
>> 	* gdb.trace/backtrace.exp: Use global fpreg/spreg definition, add $
>> 	in front.
>> 	* gdb.trace/change-loc.exp: Use global pcreg definition.
>> 	* gdb.trace/collection.exp: Use global pcreg/fpreg/spreg definition.
>> 	* gdb.trace/entry-values.exp: Use global spreg definition, add $
>> 	in front.
>> 	* gdb.trace/mi-trace-frame-collected.exp: Use global pcreg definition.
>> 	* gdb.trace/pending.exp: Likewise.
>> 	* gdb.trace/report.exp: Use global pcreg/fpreg/spreg definition.
>> 	* gdb.trace/trace-break.exp: Likewise.
>> 	* gdb.trace/trace-condition.exp: Use global pcreg definition, add $
>> 	in front.
>> 	* gdb.trace/unavailable.exp: Use global pcreg/fpreg/spreg definition.
>> 	* gdb.trace/while-dyn.exp: Use global fpreg definition, add $
>> 	in front.
>> 	* lib/trace-support.exp: Define fpreg, spreg, pcreg variables.
>
> Looks good to me.  Thanks.
>

Thanks, pushed.  Any feedback on the second patch?

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

* Re: [PATCH 2/2] gdb/testsuite/gdb.trace: Deduplicate set_point assembly.
  2015-11-10 11:21 ` [PATCH 2/2] gdb/testsuite/gdb.trace: Deduplicate set_point assembly Marcin Kościelnicki
@ 2015-11-11  8:42   ` Yao Qi
  2015-11-11 12:42     ` Marcin Kościelnicki
  0 siblings, 1 reply; 7+ messages in thread
From: Yao Qi @ 2015-11-11  8:42 UTC (permalink / raw)
  To: Marcin Kościelnicki; +Cc: gdb-patches

Marcin Kościelnicki <koriakin@0x04.net> writes:

> +/* TRACEPOINT_ASM expands to an assembly instruction large enough to fit
> +   a fast tracepoint jump.  The parameter is the label where we'll set
> +   tracepoints and breakpoints.  */
> +
> +#if (defined __x86_64__ || defined __i386__)
> +
> +static void
> +x86_trace_dummy ()
> +{
> +  int x = 0;
> +  int y = x + 4;
> +}
> +
> +#define TRACEPOINT_ASM(name) \
> +  asm ("    .global " SYMBOL(name) "\n" \
> +       SYMBOL(name) ":\n" \
> +       "    call " SYMBOL(x86_trace_dummy) "\n" \
> +       )
> +
> +#elif (defined __aarch64__)
> +
> +#define TRACEPOINT_ASM(name) \
> +  asm ("    .global " SYMBOL(name) "\n" \
> +       SYMBOL(name) ":\n" \
> +       "    nop\n" \
> +       )
> +
> +#else

TRACEPOINT_ASM isn't a good name to me.  How about
FAST_TRACEPOINT_LABEL?

Otherwise the patch looks good to me.

-- 
Yao (齐尧)

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

* Re: [PATCH 2/2] gdb/testsuite/gdb.trace: Deduplicate set_point assembly.
  2015-11-11  8:42   ` Yao Qi
@ 2015-11-11 12:42     ` Marcin Kościelnicki
  0 siblings, 0 replies; 7+ messages in thread
From: Marcin Kościelnicki @ 2015-11-11 12:42 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 11/11/15 09:42, Yao Qi wrote:
> Marcin Kościelnicki <koriakin@0x04.net> writes:
>
>> +/* TRACEPOINT_ASM expands to an assembly instruction large enough to fit
>> +   a fast tracepoint jump.  The parameter is the label where we'll set
>> +   tracepoints and breakpoints.  */
>> +
>> +#if (defined __x86_64__ || defined __i386__)
>> +
>> +static void
>> +x86_trace_dummy ()
>> +{
>> +  int x = 0;
>> +  int y = x + 4;
>> +}
>> +
>> +#define TRACEPOINT_ASM(name) \
>> +  asm ("    .global " SYMBOL(name) "\n" \
>> +       SYMBOL(name) ":\n" \
>> +       "    call " SYMBOL(x86_trace_dummy) "\n" \
>> +       )
>> +
>> +#elif (defined __aarch64__)
>> +
>> +#define TRACEPOINT_ASM(name) \
>> +  asm ("    .global " SYMBOL(name) "\n" \
>> +       SYMBOL(name) ":\n" \
>> +       "    nop\n" \
>> +       )
>> +
>> +#else
>
> TRACEPOINT_ASM isn't a good name to me.  How about
> FAST_TRACEPOINT_LABEL?
>
> Otherwise the patch looks good to me.
>

Yeah, that sounds better, will push with FAST_TRACEPOINT_LABEL.

Thanks!

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

end of thread, other threads:[~2015-11-11 12:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-10 11:21 [PATCH 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg Marcin Kościelnicki
2015-11-10 11:21 ` [PATCH 2/2] gdb/testsuite/gdb.trace: Deduplicate set_point assembly Marcin Kościelnicki
2015-11-11  8:42   ` Yao Qi
2015-11-11 12:42     ` Marcin Kościelnicki
2015-11-10 11:35 ` [PATCH v2 1/2] gdb/testsuite/gdb.trace: Deduplicate pcreg/spreg/fpreg Marcin Kościelnicki
2015-11-10 17:55   ` Yao Qi
2015-11-10 19:08     ` Marcin Kościelnicki

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