* [PATCH V2 2/5] Enable tracing of pseudo-registers on ARM
2016-11-03 14:33 [PATCH V2 0/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
@ 2016-11-03 14:33 ` Antoine Tremblay
2016-11-03 14:33 ` [PATCH V2 4/5] Use FAST_TRACEPOINT_LABEL in range-stepping.exp Antoine Tremblay
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Antoine Tremblay @ 2016-11-03 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
This patch implements the ax_pseudo_register_push_stack and
ax_pseudo_register_collect gdbarch functions so that a pseudo-register can
be traced.
No regressions, tested on ubuntu 14.04 ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }
gdb/ChangeLog:
* arm-tdep.c (arm_pseudo_register_to_register): New function.
(arm_ax_pseudo_register_collect): New function.
(arm_ax_pseudo_register_push_stack): New function.
(arm_gdbarch_init): Set
gdbarch_ax_pseudo_register_{collect,push_stack} functions.
gdb/testsuite/ChangeLog:
* gdb.trace/tracefile-pseudo-reg.c: Include arm_neon.h if on arm.
(main): Add a register variable and a tracepoint label.
* gdb.trace/tracefile-pseudo-reg.exp: Add arm pseudo register
tracing test with s5 pseudo register.
---
gdb/arm-tdep.c | 68 ++++++++++++++++++++++++
gdb/testsuite/gdb.trace/tracefile-pseudo-reg.c | 16 ++++--
gdb/testsuite/gdb.trace/tracefile-pseudo-reg.exp | 35 +++++++++---
3 files changed, 108 insertions(+), 11 deletions(-)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 75343dd..6c00dac 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8876,6 +8876,70 @@ arm_pseudo_write (struct gdbarch *gdbarch, struct regcache *regcache,
}
}
+/* Map the pseudo register number REG to the proper register number. */
+
+static int
+arm_pseudo_register_to_register (struct gdbarch *gdbarch, int reg)
+{
+ int double_regnum = 0;
+ int num_regs = gdbarch_num_regs (gdbarch);
+ char name_buf[4];
+
+ /* Single precision pseudo registers. s0-s31. */
+ if (reg >= num_regs && reg < num_regs + 32)
+ {
+ xsnprintf (name_buf, sizeof (name_buf), "d%d", (reg - num_regs) / 2);
+ double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
+ strlen (name_buf));
+ }
+ /* Quadruple precision pseudo regisers. q0-q15. */
+ else if (reg >= num_regs + 32 && reg < num_regs + 32 + 16)
+ {
+ xsnprintf (name_buf, sizeof (name_buf), "d%d", (reg - num_regs - 32) * 2);
+ double_regnum = user_reg_map_name_to_regnum (gdbarch, name_buf,
+ strlen (name_buf));
+ }
+ /* Error bad register number. */
+ else
+ return -1;
+
+ return double_regnum;
+}
+
+/* Implementation of the ax_pseudo_register_collect gdbarch function. */
+
+static int
+arm_ax_pseudo_register_collect (struct gdbarch *gdbarch,
+ struct agent_expr *ax, int reg)
+{
+ int rawnum = arm_pseudo_register_to_register (gdbarch, reg);
+
+ /* Error. */
+ if (rawnum < 0)
+ return 1;
+
+ ax_reg_mask (ax, rawnum);
+
+ return 0;
+}
+
+/* Implementation of the ax_pseudo_register_push_stack gdbarch function. */
+
+static int
+arm_ax_pseudo_register_push_stack (struct gdbarch *gdbarch,
+ struct agent_expr *ax, int reg)
+{
+ int rawnum = arm_pseudo_register_to_register (gdbarch, reg);
+
+ /* Error. */
+ if (rawnum < 0)
+ return 1;
+
+ ax_reg (ax, rawnum);
+
+ return 0;
+}
+
static struct value *
value_of_arm_user_reg (struct frame_info *frame, const void *baton)
{
@@ -9557,6 +9621,10 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
set_gdbarch_num_pseudo_regs (gdbarch, num_pseudos);
set_gdbarch_pseudo_register_read (gdbarch, arm_pseudo_read);
set_gdbarch_pseudo_register_write (gdbarch, arm_pseudo_write);
+ set_gdbarch_ax_pseudo_register_push_stack
+ (gdbarch, arm_ax_pseudo_register_push_stack);
+ set_gdbarch_ax_pseudo_register_collect
+ (gdbarch, arm_ax_pseudo_register_collect);
}
if (tdesc_data)
diff --git a/gdb/testsuite/gdb.trace/tracefile-pseudo-reg.c b/gdb/testsuite/gdb.trace/tracefile-pseudo-reg.c
index 3cc3ec0..1a751ee 100644
--- a/gdb/testsuite/gdb.trace/tracefile-pseudo-reg.c
+++ b/gdb/testsuite/gdb.trace/tracefile-pseudo-reg.c
@@ -15,12 +15,14 @@
You should have received a copy of the GNU General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>. */
-/*
- * Test program for reading target description from tfile: collects AVX
- * registers on x86_64.
- */
+/* Test program for reading target description from tfile: collects pseudo
+ registers on the target. */
+#if (defined __x86_64__)
#include <immintrin.h>
+#elif (defined __arm__)
+#include <stdint.h>
+#endif
void
dummy (void)
@@ -35,6 +37,7 @@ end (void)
int
main (void)
{
+#if (defined __x86_64__)
/* Strictly speaking, it should be ymm15 (xmm15 is 128-bit), but gcc older
than 4.9 doesn't recognize "ymm15" as a valid register name. */
register __v8si a asm("xmm15") = {
@@ -48,6 +51,11 @@ main (void)
0x12340008,
};
asm volatile ("traceme: call dummy" : : "x" (a));
+#elif (defined __arm__)
+ register uint32_t a asm("s5") = 0x3f800000; /* 1. */
+ asm volatile ("traceme: bl dummy" : : "x" (a));
+#endif
+
end ();
return 0;
}
diff --git a/gdb/testsuite/gdb.trace/tracefile-pseudo-reg.exp b/gdb/testsuite/gdb.trace/tracefile-pseudo-reg.exp
index 4c52c64..6125c23 100644
--- a/gdb/testsuite/gdb.trace/tracefile-pseudo-reg.exp
+++ b/gdb/testsuite/gdb.trace/tracefile-pseudo-reg.exp
@@ -12,8 +12,8 @@
# 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 { ! [is_amd64_regs_target] } {
- verbose "Skipping tfile AVX test (target is not x86_64)."
+if { ! [is_amd64_regs_target] && ! [istarget "arm*-*-*"] } {
+ verbose "Skipping tracefile pseudo register tests, target is not supported."
return
}
@@ -21,8 +21,14 @@ load_lib "trace-support.exp"
standard_testfile
+set add_flags ""
+
+if { [is_amd64_regs_target] } {
+ set add_flags "-mavx"
+}
+
if {[prepare_for_testing $testfile.exp $testfile $srcfile \
- [list debug additional_flags=-mavx]]} {
+ [list debug additional_flags=$add_flags]]} {
return -1
}
@@ -36,20 +42,31 @@ if ![gdb_target_supports_trace] {
return -1
}
-gdb_test_multiple "print \$ymm15" "check for AVX support" {
+if { [is_amd64_regs_target] } {
+ set reg "\$ymm15"
+} elseif { [istarget "arm*-*-*"] } {
+ set reg "\$s5"
+}
+
+set reg_message "check for register $reg"
+
+gdb_test_multiple "print $reg" $reg_message {
-re " = void.*$gdb_prompt $" {
- verbose "Skipping tfile AVX test (target doesn't support AVX)."
+ verbose "Skipping tracefile pseudo register tests, target is not supported."
return
}
-re " = \\{.*}.*$gdb_prompt $" {
# All is well.
}
+ -re " = 0.*$gdb_prompt $" {
+ # All is well.
+ }
}
gdb_test "trace traceme" ".*"
gdb_trace_setactions "set actions for tracepoint" "" \
- "collect \$ymm15" "^$"
+ "collect $reg" "^$"
gdb_breakpoint "end"
@@ -70,4 +87,8 @@ gdb_test "target tfile ${tracefile}.tf" "" "change to tfile target" \
gdb_test "tfind 0" "Found trace frame 0, tracepoint .*"
-gdb_test "print/x \$ymm15.v8_int32" " = \\{0x12340001, .*, 0x12340008}"
+if { [is_amd64_regs_target] } {
+ gdb_test "print/x \$ymm15.v8_int32" " = \\{0x12340001, .*, 0x12340008}"
+} elseif { [istarget "arm*-*-*"] } {
+ gdb_test "print \$s5" "1"
+}
--
2.9.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH V2 4/5] Use FAST_TRACEPOINT_LABEL in range-stepping.exp
2016-11-03 14:33 [PATCH V2 0/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
2016-11-03 14:33 ` [PATCH V2 2/5] Enable tracing of pseudo-registers on ARM Antoine Tremblay
@ 2016-11-03 14:33 ` Antoine Tremblay
2016-11-03 14:33 ` [PATCH V2 3/5] Improve tests to allow for targets that support trace but not ftrace Antoine Tremblay
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Antoine Tremblay @ 2016-11-03 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
This patch uses FAST_TRACEPOINT_LABEL for the fast tracepoint label rather
than the local version of that same code.
gdb/testsuite/ChangeLog:
* gdb.trace/range-stepping.c: Use FAST_TRACEPOINT_LABEL
---
gdb/testsuite/gdb.trace/range-stepping.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/gdb/testsuite/gdb.trace/range-stepping.c b/gdb/testsuite/gdb.trace/range-stepping.c
index 46ddcf7..401cee7 100644
--- a/gdb/testsuite/gdb.trace/range-stepping.c
+++ b/gdb/testsuite/gdb.trace/range-stepping.c
@@ -15,22 +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
-
-/* `set_point' further below is the label where we'll set tracepoints
- at. The insn at the label must the large enough to fit a fast
- tracepoint jump. */
-#if (defined __x86_64__ || defined __i386__)
-# define NOP " .byte 0xe9,0x00,0x00,0x00,0x00\n" /* jmp $+5 (5-byte nop) */
-#elif (defined __aarch64__)
-# define NOP " nop\n"
-#else
-# define NOP "" /* port me */
-#endif
+#include "trace-common.h"
int
main(void)
@@ -45,10 +30,7 @@ main(void)
#define LINE_WITH_FAST_TRACEPOINT \
do { \
i = 1; \
- asm (" .global " SYMBOL (set_point) "\n" \
- SYMBOL (set_point) ":\n" \
- NOP \
- ); \
+ FAST_TRACEPOINT_LABEL(set_point); \
i = 2; \
} while (0)
--
2.9.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH V2 3/5] Improve tests to allow for targets that support trace but not ftrace
2016-11-03 14:33 [PATCH V2 0/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
2016-11-03 14:33 ` [PATCH V2 2/5] Enable tracing of pseudo-registers on ARM Antoine Tremblay
2016-11-03 14:33 ` [PATCH V2 4/5] Use FAST_TRACEPOINT_LABEL in range-stepping.exp Antoine Tremblay
@ 2016-11-03 14:33 ` Antoine Tremblay
2016-11-03 14:33 ` [PATCH V2 1/5] Teach arm unwinders to terminate gracefully Antoine Tremblay
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Antoine Tremblay @ 2016-11-03 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
This patch is in preparation for ARM tracepoints support in GDBServer.
Previously targets that supported tracing also supported fast tracing and
the tests could be somewhat merged without issue. With the introduction of
ARM tracepoints, without fast tracepoints support this changes.
This patch enables the trace tests to be run even if the
target does not support fast tracepoints.
gdb/testsuite/ChangeLog:
* gdb.trace/change-loc.exp: Catch non existing IPA lib case and
set the test to untested.
* gdb.trace/ftrace-lock.exp: Likewise.
* gdb.trace/ftrace.exp: Likewise.
* gdb.trace/pending.exp: Likewise.
* gdb.trace/range-stepping.exp: Likewise.
* gdb.trace/trace-break.exp: Likewise.
* gdb.trace/trace-condition.exp: Move ftrace tests after testing
for the IPA availability.
(test_trace_command): New function.
* gdb.trace/trace-enable-disable.exp: Move ftrace tests after
testing for the IPA availability.
* gdb.trace/trace-mt.exp (foreach): Catch non existing IPA lib case and
set the test to untested.
---
gdb/testsuite/gdb.trace/change-loc.exp | 5 +-
gdb/testsuite/gdb.trace/ftrace-lock.exp | 6 ++-
gdb/testsuite/gdb.trace/ftrace.exp | 6 ++-
gdb/testsuite/gdb.trace/pending.exp | 5 +-
gdb/testsuite/gdb.trace/range-stepping.exp | 6 ++-
gdb/testsuite/gdb.trace/trace-break.exp | 6 ++-
gdb/testsuite/gdb.trace/trace-condition.exp | 64 ++++++++++++++----------
gdb/testsuite/gdb.trace/trace-enable-disable.exp | 27 ++++++----
gdb/testsuite/gdb.trace/trace-mt.exp | 5 +-
9 files changed, 86 insertions(+), 44 deletions(-)
diff --git a/gdb/testsuite/gdb.trace/change-loc.exp b/gdb/testsuite/gdb.trace/change-loc.exp
index 9fef3f0..1cd5ce7 100644
--- a/gdb/testsuite/gdb.trace/change-loc.exp
+++ b/gdb/testsuite/gdb.trace/change-loc.exp
@@ -354,7 +354,10 @@ tracepoint_install_in_trace_disabled "trace"
# Re-compile test case with IPA.
set libipa [get_in_proc_agent]
-gdb_load_shlib $libipa
+if { [catch {gdb_load_shlib $libipa}] } {
+ untested "Failed to load $libipa"
+ return -1
+}
if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable \
[list debug nowarnings shlib=$libipa shlib=$lib_sl1 shlib_load] ] != "" } {
diff --git a/gdb/testsuite/gdb.trace/ftrace-lock.exp b/gdb/testsuite/gdb.trace/ftrace-lock.exp
index 0b12c8d..179cd0e 100644
--- a/gdb/testsuite/gdb.trace/ftrace-lock.exp
+++ b/gdb/testsuite/gdb.trace/ftrace-lock.exp
@@ -48,7 +48,11 @@ if ![gdb_target_supports_trace] {
# Compile the test case with the in-process agent library.
set libipa [get_in_proc_agent]
-set remote_libipa [gdb_load_shlib $libipa]
+
+if { [catch {gdb_load_shlib $libipa} remote_libipa] } {
+ untested "Failed to load $libipa"
+ return -1
+}
lappend options shlib=$libipa
diff --git a/gdb/testsuite/gdb.trace/ftrace.exp b/gdb/testsuite/gdb.trace/ftrace.exp
index e90485c..8cb0ac6 100644
--- a/gdb/testsuite/gdb.trace/ftrace.exp
+++ b/gdb/testsuite/gdb.trace/ftrace.exp
@@ -38,7 +38,11 @@ if ![gdb_target_supports_trace] {
}
set libipa [get_in_proc_agent]
-set remote_libipa [gdb_load_shlib $libipa]
+
+if { [catch {gdb_load_shlib $libipa} remote_libipa] } {
+ untested "Failed to load $libipa"
+ return -1
+}
# Can't use prepare_for_testing, because that splits compiling into
# building objects and then linking, and we'd fail with "linker input
diff --git a/gdb/testsuite/gdb.trace/pending.exp b/gdb/testsuite/gdb.trace/pending.exp
index f7905fb..1ba0eec 100644
--- a/gdb/testsuite/gdb.trace/pending.exp
+++ b/gdb/testsuite/gdb.trace/pending.exp
@@ -503,7 +503,10 @@ pending_tracepoint_installed_during_trace "trace"
# Re-compile test case with IPA.
set libipa [get_in_proc_agent]
-gdb_load_shlib $libipa
+if { [catch gdb_load_shlib $libipa] } {
+ untested "Failed to load $libipa"
+ return -1
+}
lappend exec_opts "shlib=$libipa"
diff --git a/gdb/testsuite/gdb.trace/range-stepping.exp b/gdb/testsuite/gdb.trace/range-stepping.exp
index ba8c3d2..a606bd4 100644
--- a/gdb/testsuite/gdb.trace/range-stepping.exp
+++ b/gdb/testsuite/gdb.trace/range-stepping.exp
@@ -67,7 +67,11 @@ proc range_stepping_with_tracepoint { type } {
range_stepping_with_tracepoint "trace"
set libipa [get_in_proc_agent]
-set remote_libipa [gdb_load_shlib $libipa]
+
+if { [catch {gdb_load_shlib $libipa} remote_libipa] } {
+ untested "Failed to load $libipa"
+ return -1
+}
if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
executable [list debug nowarnings shlib=$libipa] ] != "" } {
diff --git a/gdb/testsuite/gdb.trace/trace-break.exp b/gdb/testsuite/gdb.trace/trace-break.exp
index a90d02d..9756d38 100644
--- a/gdb/testsuite/gdb.trace/trace-break.exp
+++ b/gdb/testsuite/gdb.trace/trace-break.exp
@@ -349,7 +349,11 @@ break_trace_same_addr_6 "trace" "enable" "trace" "disable"
break_trace_same_addr_6 "trace" "disable" "trace" "enable"
set libipa [get_in_proc_agent]
-set remote_libipa [gdb_load_shlib $libipa]
+
+if { [catch {gdb_load_shlib $libipa} remote_libipa] } {
+ untested "Failed to load $libipa"
+ return -1
+}
# Can't use prepare_for_testing, because that splits compiling into
# building objects and then linking, and we'd fail with "linker input
diff --git a/gdb/testsuite/gdb.trace/trace-condition.exp b/gdb/testsuite/gdb.trace/trace-condition.exp
index e36dba4..b5f5798 100644
--- a/gdb/testsuite/gdb.trace/trace-condition.exp
+++ b/gdb/testsuite/gdb.trace/trace-condition.exp
@@ -37,31 +37,6 @@ if ![gdb_target_supports_trace] {
return -1
}
-set libipa [get_in_proc_agent]
-set remote_libipa [gdb_load_shlib $libipa]
-
-# Can't use prepare_for_testing, because that splits compiling into
-# building objects and then linking, and we'd fail with "linker input
-# file unused because linking not done" when building the object.
-
-if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
- executable [list debug $additional_flags shlib=$libipa] ] != "" } {
- untested "failed to compile ftrace tests"
- return -1
-}
-
-clean_restart ${executable}
-
-if ![runto_main] {
- fail "Can't run to main for ftrace tests"
- return 0
-}
-
-if { [gdb_test "info sharedlibrary" ".*${remote_libipa}.*" "IPA loaded"] != 0 } {
- untested "Could not find IPA lib loaded"
- return 1
-}
-
proc test_tracepoints { trace_command condition num_frames { kfail_proc 0 } } {
global executable gdb_prompt
@@ -126,7 +101,10 @@ proc 18955_i386_failure { trace_command } {
}
}
-foreach trace_command { "trace" "ftrace" } {
+proc test_trace_command { trace_command } {
+
+ global pcreg
+
# 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
@@ -303,3 +281,37 @@ foreach trace_command { "trace" "ftrace" } {
test_tracepoints $trace_command "(0x0aaaaaaaaaaaaaaa > 0x09999999bbbbbbbb ? 1 : 0) == 1" 10
test_tracepoints $trace_command "(0x00088888ccaaaaaa > 0x09999999bbbbbbbb ? 1 : 0) == 1" 0
}
+
+
+test_trace_command "trace"
+
+set libipa [get_in_proc_agent]
+
+if { [catch {gdb_load_shlib $libipa} remote_libipa] } {
+ untested "Failed to load $libipa"
+ return -1
+}
+
+# Can't use prepare_for_testing, because that splits compiling into
+# building objects and then linking, and we'd fail with "linker input
+# file unused because linking not done" when building the object.
+
+if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
+ executable [list debug $additional_flags shlib=$libipa] ] != "" } {
+ untested "failed to compile ftrace tests"
+ return -1
+}
+
+clean_restart ${executable}
+
+if ![runto_main] {
+ fail "Can't run to main for ftrace tests"
+ return 0
+}
+
+if { [gdb_test "info sharedlibrary" ".*${remote_libipa}.*" "IPA loaded"] != 0 } {
+ untested "Could not find IPA lib loaded"
+ return 1
+}
+
+test_trace_command "ftrace"
diff --git a/gdb/testsuite/gdb.trace/trace-enable-disable.exp b/gdb/testsuite/gdb.trace/trace-enable-disable.exp
index 0c35c92..c3d1b7b 100644
--- a/gdb/testsuite/gdb.trace/trace-enable-disable.exp
+++ b/gdb/testsuite/gdb.trace/trace-enable-disable.exp
@@ -39,17 +39,6 @@ if ![gdb_target_supports_trace] {
return -1
}
-# Compile the test case with the in-process agent library.
-set libipa [get_in_proc_agent]
-gdb_load_shlib $libipa
-
-lappend options shlib=$libipa
-
-if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable $options] != "" } {
- untested "Couldn't compile test program with in-process agent library"
- return -1
-}
-
# This test makes sure that disabling and enabling tracepoints works
# correctly. TRACEPOINT_CMD is the command used to set tracepoints
# (e.g. trace or ftrace).
@@ -125,4 +114,20 @@ proc test_tracepoint_enable_disable { tracepoint_cmd } {
}
test_tracepoint_enable_disable trace
+
+# Compile the test case with the in-process agent library.
+set libipa [get_in_proc_agent]
+
+if { [catch {gdb_load_shlib $libipa}] } {
+ untested "Failed to load $libipa"
+ return -1
+}
+
+lappend options shlib=$libipa
+
+if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable $options] != "" } {
+ untested "Couldn't compile test program with in-process agent library"
+ return -1
+}
+
test_tracepoint_enable_disable ftrace
diff --git a/gdb/testsuite/gdb.trace/trace-mt.exp b/gdb/testsuite/gdb.trace/trace-mt.exp
index b580344..15f6c63 100644
--- a/gdb/testsuite/gdb.trace/trace-mt.exp
+++ b/gdb/testsuite/gdb.trace/trace-mt.exp
@@ -107,7 +107,10 @@ foreach break_always_inserted { "on" "off" } {
step_over_tracepoint "trace"
set libipa [get_in_proc_agent]
-set remote_libipa [gdb_load_shlib $libipa]
+if { [catch {gdb_load_shlib $libipa} remote_libipa] } {
+ untested "Failed to load $libipa"
+ return -1
+}
# Compile test case again with IPA.
if { [gdb_compile_pthreads "$srcdir/$subdir/$srcfile" $binfile \
--
2.9.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH V2 1/5] Teach arm unwinders to terminate gracefully
2016-11-03 14:33 [PATCH V2 0/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
` (2 preceding siblings ...)
2016-11-03 14:33 ` [PATCH V2 3/5] Improve tests to allow for targets that support trace but not ftrace Antoine Tremblay
@ 2016-11-03 14:33 ` Antoine Tremblay
2016-11-03 14:33 ` [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
2016-11-07 9:25 ` [PATCH V2 0/5] " Yao Qi
5 siblings, 0 replies; 15+ messages in thread
From: Antoine Tremblay @ 2016-11-03 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
When examining a trace buffer we have the following issue:
~~~
tfind start
Register 13 is not available
Found trace frame 0, tracepoint 2
#-1 0x40123556 in pendfunc2
^^^
~~~
The reason for this is that the target's stack pointer is unavailable
when examining the trace buffer. What we are seeing is due to the
'tfind' command creating a sentinel frame and unwinding it. If an
exception is thrown, we are left with the sentinel frame being displayed
at level #-1. The exception is thrown when the prologue unwinder tries
to read the stack pointer to construct an ID for the frame.
This patch fixes this and similar issues by making all the arm unwinders
catch NOT_AVAILABLE_ERROR exceptions when either register or memory is
unreadable and report back to the frame core code with UNWIND_UNAVAILABLE.
Note this commit log adapted from 7dfa3edc033c443036d9f2a3e01120f7fb54f498
which fixed a similar issue for aarch64.
No regressions, tested on ubuntu 14.04 ARMv7 and x86.
With gdbserver-{native,extended} / { -marm -mthumb }
gdb/ChangeLog:
* arm-tdep.c (struct arm_prologue_cache) <available_p>: New field.
(arm_make_prologue_cache): Swallow NOT_AVAIABLE_ERROR or set
available_p.
(arm_prologue_unwind_stop_reason): Return UNWIND_UNAVAILABLE if
available_p is not set.
(arm_prologue_this_id): Call frame_id_build_unavailable_stack if
available_p is not set.
(arm_make_stub_cache): Swallow NOT_AVAIABLE_ERROR or set
available_p.
(arm_stub_this_id): Call frame_id_build_unavailable_stack if
available_p is not set.
(arm_m_exception_cache): Swallow NOT_AVAIABLE_ERROR or set
available_p.
(arm_m_exception_this_id): Call frame_id_build_unavailable_stack if
available_p is not set.
---
gdb/arm-tdep.c | 142 ++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 95 insertions(+), 47 deletions(-)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 645825f..75343dd 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -258,6 +258,9 @@ struct arm_prologue_cache
to identify this frame. */
CORE_ADDR prev_sp;
+ /* Is the target available to read from ? */
+ int available_p;
+
/* The frame base for this frame is just prev_sp - frame size.
FRAMESIZE is the distance from the frame pointer to the
initial stack pointer. */
@@ -1847,19 +1850,29 @@ arm_make_prologue_cache (struct frame_info *this_frame)
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
- arm_scan_prologue (this_frame, cache);
+ TRY
+ {
+ arm_scan_prologue (this_frame, cache);
+ unwound_fp = get_frame_register_unsigned (this_frame, cache->framereg);
+ if (unwound_fp == 0)
+ return cache;
- unwound_fp = get_frame_register_unsigned (this_frame, cache->framereg);
- if (unwound_fp == 0)
- return cache;
+ cache->prev_sp = unwound_fp + cache->framesize;
- cache->prev_sp = unwound_fp + cache->framesize;
+ /* Calculate actual addresses of saved registers using offsets
+ determined by arm_scan_prologue. */
+ for (reg = 0; reg < gdbarch_num_regs (get_frame_arch (this_frame)); reg++)
+ if (trad_frame_addr_p (cache->saved_regs, reg))
+ cache->saved_regs[reg].addr += cache->prev_sp;
- /* Calculate actual addresses of saved registers using offsets
- determined by arm_scan_prologue. */
- for (reg = 0; reg < gdbarch_num_regs (get_frame_arch (this_frame)); reg++)
- if (trad_frame_addr_p (cache->saved_regs, reg))
- cache->saved_regs[reg].addr += cache->prev_sp;
+ cache->available_p = 1;
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (ex.error != NOT_AVAILABLE_ERROR)
+ throw_exception (ex);
+ }
+ END_CATCH
return cache;
}
@@ -1877,6 +1890,9 @@ arm_prologue_unwind_stop_reason (struct frame_info *this_frame,
*this_cache = arm_make_prologue_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
+ if (!cache->available_p)
+ return UNWIND_UNAVAILABLE;
+
/* This is meant to halt the backtrace at "_start". */
pc = get_frame_pc (this_frame);
if (pc <= gdbarch_tdep (get_frame_arch (this_frame))->lowest_pc)
@@ -1905,16 +1921,23 @@ arm_prologue_this_id (struct frame_info *this_frame,
*this_cache = arm_make_prologue_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
- /* Use function start address as part of the frame ID. If we cannot
- identify the start address (due to missing symbol information),
- fall back to just using the current PC. */
- pc = get_frame_pc (this_frame);
- func = get_frame_func (this_frame);
- if (!func)
- func = pc;
+ if (!cache->available_p)
+ {
+ *this_id = frame_id_build_unavailable_stack (cache->prev_sp);
+ }
+ else
+ {
+ /* Use function start address as part of the frame ID. If we cannot
+ identify the start address (due to missing symbol information),
+ fall back to just using the current PC. */
+ pc = get_frame_pc (this_frame);
+ func = get_frame_func (this_frame);
+ if (!func)
+ func = pc;
- id = frame_id_build (cache->prev_sp, func);
- *this_id = id;
+ id = frame_id_build (cache->prev_sp, func);
+ *this_id = id;
+ }
}
static struct value *
@@ -2894,7 +2917,17 @@ arm_make_stub_cache (struct frame_info *this_frame)
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
- cache->prev_sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+ TRY
+ {
+ cache->prev_sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+ cache->available_p = 1;
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (ex.error != NOT_AVAILABLE_ERROR)
+ throw_exception (ex);
+ }
+ END_CATCH
return cache;
}
@@ -2912,7 +2945,10 @@ arm_stub_this_id (struct frame_info *this_frame,
*this_cache = arm_make_stub_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
- *this_id = frame_id_build (cache->prev_sp, get_frame_pc (this_frame));
+ if (!cache->available_p)
+ *this_id = frame_id_build_unavailable_stack (cache->prev_sp);
+ else
+ *this_id = frame_id_build (cache->prev_sp, get_frame_pc (this_frame));
}
static int
@@ -2965,29 +3001,38 @@ arm_m_exception_cache (struct frame_info *this_frame)
cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
cache->saved_regs = trad_frame_alloc_saved_regs (this_frame);
- unwound_sp = get_frame_register_unsigned (this_frame,
- ARM_SP_REGNUM);
-
- /* The hardware saves eight 32-bit words, comprising xPSR,
- ReturnAddress, LR (R14), R12, R3, R2, R1, R0. See details in
- "B1.5.6 Exception entry behavior" in
- "ARMv7-M Architecture Reference Manual". */
- cache->saved_regs[0].addr = unwound_sp;
- cache->saved_regs[1].addr = unwound_sp + 4;
- cache->saved_regs[2].addr = unwound_sp + 8;
- cache->saved_regs[3].addr = unwound_sp + 12;
- cache->saved_regs[12].addr = unwound_sp + 16;
- cache->saved_regs[14].addr = unwound_sp + 20;
- cache->saved_regs[15].addr = unwound_sp + 24;
- cache->saved_regs[ARM_PS_REGNUM].addr = unwound_sp + 28;
-
- /* If bit 9 of the saved xPSR is set, then there is a four-byte
- aligner between the top of the 32-byte stack frame and the
- previous context's stack pointer. */
- cache->prev_sp = unwound_sp + 32;
- if (safe_read_memory_integer (unwound_sp + 28, 4, byte_order, &xpsr)
- && (xpsr & (1 << 9)) != 0)
- cache->prev_sp += 4;
+ TRY
+ {
+ unwound_sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
+ /* The hardware saves eight 32-bit words, comprising xPSR,
+ ReturnAddress, LR (R14), R12, R3, R2, R1, R0. See details in
+ "B1.5.6 Exception entry behavior" in
+ "ARMv7-M Architecture Reference Manual". */
+ cache->saved_regs[0].addr = unwound_sp;
+ cache->saved_regs[1].addr = unwound_sp + 4;
+ cache->saved_regs[2].addr = unwound_sp + 8;
+ cache->saved_regs[3].addr = unwound_sp + 12;
+ cache->saved_regs[12].addr = unwound_sp + 16;
+ cache->saved_regs[14].addr = unwound_sp + 20;
+ cache->saved_regs[15].addr = unwound_sp + 24;
+ cache->saved_regs[ARM_PS_REGNUM].addr = unwound_sp + 28;
+
+ /* If bit 9 of the saved xPSR is set, then there is a four-byte
+ aligner between the top of the 32-byte stack frame and the
+ previous context's stack pointer. */
+ cache->prev_sp = unwound_sp + 32;
+ if (safe_read_memory_integer (unwound_sp + 28, 4, byte_order, &xpsr)
+ && (xpsr & (1 << 9)) != 0)
+ cache->prev_sp += 4;
+
+ cache->available_p = 1;
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ if (ex.error != NOT_AVAILABLE_ERROR)
+ throw_exception (ex);
+ }
+ END_CATCH
return cache;
}
@@ -3006,9 +3051,12 @@ arm_m_exception_this_id (struct frame_info *this_frame,
*this_cache = arm_m_exception_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
- /* Our frame ID for a stub frame is the current SP and LR. */
- *this_id = frame_id_build (cache->prev_sp,
- get_frame_pc (this_frame));
+ if (!cache->available_p)
+ *this_id = frame_id_build_unavailable_stack (cache->prev_sp);
+ else
+ /* Our frame ID for a stub frame is the current SP and LR. */
+ *this_id = frame_id_build (cache->prev_sp,
+ get_frame_pc (this_frame));
}
/* Implementation of function hook 'prev_register' in
--
2.9.2
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer
2016-11-03 14:33 [PATCH V2 0/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
` (3 preceding siblings ...)
2016-11-03 14:33 ` [PATCH V2 1/5] Teach arm unwinders to terminate gracefully Antoine Tremblay
@ 2016-11-03 14:33 ` Antoine Tremblay
2016-11-03 17:51 ` Eli Zaretskii
2016-11-10 14:01 ` Yao Qi
2016-11-07 9:25 ` [PATCH V2 0/5] " Yao Qi
5 siblings, 2 replies; 15+ messages in thread
From: Antoine Tremblay @ 2016-11-03 14:33 UTC (permalink / raw)
To: gdb-patches; +Cc: Antoine Tremblay
This patch adds support for tracepoints for ARM linux in GDBServer.
To enable this, this patch introduces a new :K (kind) field in the
QTDP packet to encode the breakpoint kind, this is the same kind as a z0
packet.
This is the new qSupported feature: TracepointKinds
This field is decoded by sw_breakpoint_from_kind target ops in linux-low.
Tested on Ubuntu 14.04 ARMv7 and x86 with no regression.
With gdbserver-{native,extended} / { -marm -mthumb }
gdb/ChangeLog:
* NEWS: Add news for tracepoins on ARM.
gdb/doc/ChangeLog:
* gdb.texinfo (General Query Packets): Add TracepointKinds packet.
(ARM Breakpoint Kinds): Add QTDP reference.
(Tracepoint Packets): Add kind parameter to QTDP packet.
gdb/gdbserver/ChangeLog:
* linux-arm-low.c (arm_supports_tracepoints): New function.
(struct linux_target_ops) <supports_tracepoints>: Initialize.
* mem-break.c (set_breakpoint_at_with_kind): New function.
* mem-break.h (set_breakpoint_at_with_kind): New function declaration.
* server.c (handle_query): Add TracepointsKinds feature.
* tracepoint.c (struct tracepoint) <kind>: New field.
(add_tracepoint): Initialize kind field.
(cmd_qtdp): Handle kind field 'K'.
(install_tracepoint): Use set_breakpoint_at_with_kind when kind is
present.
(cmd_qtstart): Likewise.
gdb/ChangeLog:
* remote.c (remote_supports_tracepoint_kinds): New function declaration.
(PACKET_TracepointKinds): New enum field.
(remote_protocol_features[]): New TracepointKinds element.
(remote_supports_tracepoint_kinds): New function.
(remote_download_tracepoint): Fetch the breakpoint kind and send
it as K parameter to QTDP packet.
(_initialize_remote): Add TracepointKinds packet_config_cmd.
gdb/testsuite/ChangeLog:
* gdb.trace/collection.exp (gdb_collect_return_test): Set test
unsupported for arm/aarch32 targets as it's not supported by the
arch.
* gdb.trace/trace-common.h: Add ARM fast tracepoint label to allow
tracepoints tests.
* lib/trace-support.exp: Add arm/aarch32 target support.
---
gdb/NEWS | 2 ++
gdb/doc/gdb.texinfo | 23 ++++++++++++++----
gdb/gdbserver/linux-arm-low.c | 10 +++++++-
gdb/gdbserver/mem-break.c | 13 ++++++++++
gdb/gdbserver/mem-break.h | 7 ++++++
gdb/gdbserver/server.c | 1 +
gdb/gdbserver/tracepoint.c | 43 ++++++++++++++++++++++++++++------
gdb/remote.c | 27 +++++++++++++++++++++
gdb/testsuite/gdb.trace/collection.exp | 7 +++++-
gdb/testsuite/gdb.trace/trace-common.h | 12 ++++++++--
gdb/testsuite/lib/trace-support.exp | 3 +++
11 files changed, 132 insertions(+), 16 deletions(-)
diff --git a/gdb/NEWS b/gdb/NEWS
index a6b1282..233f11e 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,8 @@
*** Changes since GDB 7.12
+* Support for tracepoints on arm-linux was added in GDBServer.
+
* Building GDB and GDBserver now requires a C++11 compiler.
For example, GCC 4.8 or later.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index df548dc..7df63ee 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -37088,6 +37088,11 @@ These are the currently defined stub features and their properties:
@tab @samp{-}
@tab No
+@item @samp{TracepointKinds}
+@tab No
+@tab @samp{-}
+@tab No
+
@end multitable
These are the currently defined stub features, in more detail:
@@ -37310,6 +37315,9 @@ The remote stub understands the @samp{QThreadEvents} packet.
@item no-resumed
The remote stub reports the @samp{N} stop reply.
+@item TracepointKinds
+The remote stub reports the @samp{:K} kind parameter for @samp{QTDP} packets.
+
@end table
@item qSymbol::
@@ -37820,7 +37828,8 @@ details of XML target descriptions for each architecture.
@subsubsection @acronym{ARM} Breakpoint Kinds
@cindex breakpoint kinds, @acronym{ARM}
-These breakpoint kinds are defined for the @samp{Z0} and @samp{Z1} packets.
+These breakpoint kinds are defined for the @samp{Z0}, @samp{Z1}
+and @samp{QTDP} packets.
@table @r
@@ -37900,7 +37909,7 @@ tracepoints (@pxref{Tracepoints}).
@table @samp
-@item QTDP:@var{n}:@var{addr}:@var{ena}:@var{step}:@var{pass}[:F@var{flen}][:X@var{len},@var{bytes}]@r{[}-@r{]}
+@item QTDP:@var{n}:@var{addr}:@var{ena}:@var{step}:@var{pass}[:F@var{flen}][:X@var{len},@var{bytes}][:K@var{kind}]@r{[}-@r{]}
@cindex @samp{QTDP} packet
Create a new tracepoint, number @var{n}, at @var{addr}. If @var{ena}
is @samp{E}, then the tracepoint is enabled; if it is @samp{D}, then
@@ -37911,9 +37920,13 @@ the number of bytes that the target should copy elsewhere to make room
for the tracepoint. If an @samp{X} is present, it introduces a
tracepoint condition, which consists of a hexadecimal length, followed
by a comma and hex-encoded bytes, in a manner similar to action
-encodings as described below. If the trailing @samp{-} is present,
-further @samp{QTDP} packets will follow to specify this tracepoint's
-actions.
+encodings as described below. If a @samp{K} is present, it
+indicates a target specific breakpoint kind. The kind can be the
+length of the breakpoint. E.g., the arm and mips can insert either a
+2 or 4 byte breakpoint or have additional meaning see
+@ref{Architecture-Specific Protocol Details}. If the trailing @samp{-}
+is present, further @samp{QTDP} packets will follow to specify this
+tracepoint's actions.
Replies:
@table @samp
diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
index ed9b356..a1ca9b9 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -1032,6 +1032,14 @@ arm_regs_info (void)
return ®s_info_arm;
}
+/* Implementation of the linux_target_ops method "support_tracepoints". */
+
+static int
+arm_supports_tracepoints (void)
+{
+ return 1;
+}
+
struct linux_target_ops the_low_target = {
arm_arch_setup,
arm_regs_info,
@@ -1058,7 +1066,7 @@ struct linux_target_ops the_low_target = {
arm_new_fork,
arm_prepare_to_resume,
NULL, /* process_qsupported */
- NULL, /* supports_tracepoints */
+ arm_supports_tracepoints,
NULL, /* get_thread_area */
NULL, /* install_fast_tracepoint_jump_pad */
NULL, /* emit_ops */
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index bee9c30..c8fb7c9 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -870,6 +870,19 @@ set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
return set_breakpoint_type_at (other_breakpoint, where, handler);
}
+/* See mem-break.h */
+
+struct breakpoint *
+set_breakpoint_at_with_kind (CORE_ADDR where,
+ int (*handler) (CORE_ADDR),
+ int kind)
+{
+ int err_ignored;
+
+ return set_breakpoint (other_breakpoint, raw_bkpt_type_sw,
+ where, kind, handler,
+ &err_ignored);
+}
static int
delete_raw_breakpoint (struct process_info *proc, struct raw_breakpoint *todel)
diff --git a/gdb/gdbserver/mem-break.h b/gdb/gdbserver/mem-break.h
index 9e7ee93..07c6894 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -148,6 +148,13 @@ int gdb_breakpoint_here (CORE_ADDR where);
struct breakpoint *set_breakpoint_at (CORE_ADDR where,
int (*handler) (CORE_ADDR));
+/* Same as set_breakpoint_at but allow the kind to be specified */
+
+struct breakpoint *set_breakpoint_at_with_kind (CORE_ADDR where,
+ int (*handler)(CORE_ADDR),
+ int kind);
+
+
/* Delete a breakpoint. */
int delete_breakpoint (struct breakpoint *bkpt);
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 3f9ff2b..3b3c371 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2350,6 +2350,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
strcat (own_buf, ";EnableDisableTracepoints+");
strcat (own_buf, ";QTBuffer:size+");
strcat (own_buf, ";tracenz+");
+ strcat (own_buf, ";TracepointKinds+");
}
if (target_supports_hardware_single_step ()
diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 7700ad1..cdb2c1d 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -747,6 +747,11 @@ struct tracepoint
/* Link to the next tracepoint in the list. */
struct tracepoint *next;
+ /* Optional kind of the breakpoint to be used. Note this can mean
+ different things for different archs as z0 breakpoint command.
+ Value is -1 if not persent. */
+ int32_t kind;
+
#ifndef IN_PROCESS_AGENT
/* The list of actions to take when the tracepoint triggers, in
string/packet form. */
@@ -1813,6 +1818,7 @@ add_tracepoint (int num, CORE_ADDR addr)
tpoint->compiled_cond = 0;
tpoint->handle = NULL;
tpoint->next = NULL;
+ tpoint->kind = -1;
/* Find a place to insert this tracepoint into list in order to keep
the tracepoint list still in the ascending order. There may be
@@ -2484,6 +2490,7 @@ cmd_qtdp (char *own_buf)
ULONGEST num;
ULONGEST addr;
ULONGEST count;
+ ULONGEST kind;
struct tracepoint *tpoint;
char *actparm;
char *packet = own_buf;
@@ -2550,6 +2557,12 @@ cmd_qtdp (char *own_buf)
tpoint->cond = gdb_parse_agent_expr (&actparm);
packet = actparm;
}
+ else if (*packet == 'K')
+ {
+ ++packet;
+ packet = unpack_varlen_hex (packet, &kind);
+ tpoint->kind = kind;
+ }
else if (*packet == '-')
break;
else if (*packet == '\0')
@@ -2564,11 +2577,13 @@ cmd_qtdp (char *own_buf)
}
trace_debug ("Defined %stracepoint %d at 0x%s, "
- "enabled %d step %" PRIu64 " pass %" PRIu64,
+ "enabled %d step %" PRIu64 " pass %" PRIu64
+ " kind %" PRId32,
tpoint->type == fast_tracepoint ? "fast "
: tpoint->type == static_tracepoint ? "static " : "",
tpoint->number, paddress (tpoint->address), tpoint->enabled,
- tpoint->step_count, tpoint->pass_count);
+ tpoint->step_count, tpoint->pass_count,
+ tpoint->kind);
}
else if (tpoint)
add_tracepoint_action (tpoint, packet);
@@ -3150,9 +3165,17 @@ install_tracepoint (struct tracepoint *tpoint, char *own_buf)
/* Tracepoints are installed as memory breakpoints. Just go
ahead and install the trap. The breakpoints module
handles duplicated breakpoints, and the memory read
- routine handles un-patching traps from memory reads. */
- tpoint->handle = set_breakpoint_at (tpoint->address,
- tracepoint_handler);
+ routine handles un-patching traps from memory reads.
+ If tracepoint kind is not set, use the default values
+ otherwise what was set from the gdb client will be used. */
+ if (tpoint->kind == -1)
+ tpoint->handle = set_breakpoint_at (tpoint->address,
+ tracepoint_handler);
+ else
+ tpoint->handle =
+ set_breakpoint_at_with_kind (tpoint->address,
+ tracepoint_handler,
+ tpoint->kind);
}
else if (tpoint->type == fast_tracepoint || tpoint->type == static_tracepoint)
{
@@ -3253,8 +3276,14 @@ cmd_qtstart (char *packet)
ahead and install the trap. The breakpoints module
handles duplicated breakpoints, and the memory read
routine handles un-patching traps from memory reads. */
- tpoint->handle = set_breakpoint_at (tpoint->address,
- tracepoint_handler);
+ if (tpoint->kind == -1)
+ tpoint->handle = set_breakpoint_at (tpoint->address,
+ tracepoint_handler);
+ else
+ tpoint->handle =
+ set_breakpoint_at_with_kind (tpoint->address,
+ tracepoint_handler,
+ tpoint->kind);
}
else if (tpoint->type == fast_tracepoint
|| tpoint->type == static_tracepoint)
diff --git a/gdb/remote.c b/gdb/remote.c
index 517e36d..377a6da 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -241,6 +241,8 @@ static void readahead_cache_invalidate (void);
static void remote_unpush_and_throw (void);
+static int remote_supports_tracepoint_kinds (void);
+
/* For "remote". */
static struct cmd_list_element *remote_cmdlist;
@@ -1521,6 +1523,9 @@ enum {
/* Support TARGET_WAITKIND_NO_RESUMED. */
PACKET_no_resumed,
+ /* Support target dependant tracepoint kinds. */
+ PACKET_TracepointKinds,
+
PACKET_MAX
};
@@ -4693,6 +4698,8 @@ static const struct protocol_feature remote_protocol_features[] = {
{ "vContSupported", PACKET_DISABLE, remote_supported_packet, PACKET_vContSupported },
{ "QThreadEvents", PACKET_DISABLE, remote_supported_packet, PACKET_QThreadEvents },
{ "no-resumed", PACKET_DISABLE, remote_supported_packet, PACKET_no_resumed },
+ { "TracepointKinds", PACKET_DISABLE, remote_supported_packet,
+ PACKET_TracepointKinds }
};
static char *remote_support_xml;
@@ -12197,6 +12204,12 @@ remote_can_run_breakpoint_commands (struct target_ops *self)
return packet_support (PACKET_BreakpointCommands) == PACKET_ENABLE;
}
+static int
+remote_supports_tracepoint_kinds (void)
+{
+ return packet_support (PACKET_TracepointKinds) == PACKET_ENABLE;
+}
+
static void
remote_trace_init (struct target_ops *self)
{
@@ -12285,6 +12298,7 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
char *pkt;
struct breakpoint *b = loc->owner;
struct tracepoint *t = (struct tracepoint *) b;
+ int kind;
encode_actions_rsp (loc, &tdp_actions, &stepping_actions);
old_chain = make_cleanup (free_actions_list_cleanup_wrapper,
@@ -12293,6 +12307,10 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
stepping_actions);
tpaddr = loc->address;
+
+ /* Fetch the proper tracepoint kind. */
+ gdbarch_remote_breakpoint_from_pc (target_gdbarch (), &tpaddr, &kind);
+
sprintf_vma (addrbuf, tpaddr);
xsnprintf (buf, BUF_SIZE, "QTDP:%x:%s:%c:%lx:%x", b->number,
addrbuf, /* address */
@@ -12367,6 +12385,11 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
"ignoring tp %d cond"), b->number);
}
+ /* Tracepoint Kinds are modeled after the breakpoint Z0 kind packet.
+ Send the tracepoint kind if we support it. */
+ if (remote_supports_tracepoint_kinds ())
+ xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":K%x", kind);
+
if (b->commands || *default_collect)
strcat (buf, "-");
putpkt (buf);
@@ -14333,6 +14356,10 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
add_packet_config_cmd (&remote_protocol_packets[PACKET_no_resumed],
"N stop reply", "no-resumed-stop-reply", 0);
+ add_packet_config_cmd (&remote_protocol_packets[PACKET_TracepointKinds],
+ "TracepointKinds",
+ "tracepoint-kinds", 0);
+
/* Assert that we've registered "set remote foo-packet" commands
for all packet configs. */
{
diff --git a/gdb/testsuite/gdb.trace/collection.exp b/gdb/testsuite/gdb.trace/collection.exp
index f225429..a30234f 100644
--- a/gdb/testsuite/gdb.trace/collection.exp
+++ b/gdb/testsuite/gdb.trace/collection.exp
@@ -764,7 +764,12 @@ proc gdb_trace_collection_test {} {
gdb_collect_expression_test globals_test_func \
"globalarr\[\(l6, l7\)\]" "7" "a\[\(b, c\)\]"
- gdb_collect_return_test
+ #This architecture has no method to collect a return address.
+ if { [is_aarch32_target] } {
+ unsupported "collect \$_ret: This architecture has no method to collect a return address"
+ } else {
+ gdb_collect_return_test
+ }
gdb_collect_strings_test strings_test_func "locstr" "abcdef" "" \
"local string"
diff --git a/gdb/testsuite/gdb.trace/trace-common.h b/gdb/testsuite/gdb.trace/trace-common.h
index 60cf9e8..9d607f7 100644
--- a/gdb/testsuite/gdb.trace/trace-common.h
+++ b/gdb/testsuite/gdb.trace/trace-common.h
@@ -40,7 +40,8 @@ x86_trace_dummy ()
" call " SYMBOL(x86_trace_dummy) "\n" \
)
-#elif (defined __aarch64__) || (defined __powerpc__)
+#elif (defined __aarch64__) || (defined __powerpc__) \
+ || (defined __arm__ && !defined __thumb__)
#define FAST_TRACEPOINT_LABEL(name) \
asm (" .global " SYMBOL(name) "\n" \
@@ -48,11 +49,18 @@ x86_trace_dummy ()
" nop\n" \
)
-#elif (defined __s390__)
+#elif (defined __arm__ && defined __thumb2__)
#define FAST_TRACEPOINT_LABEL(name) \
asm (" .global " SYMBOL(name) "\n" \
SYMBOL(name) ":\n" \
+ " nop.w\n" \
+ )
+
+#elif (defined __s390__)
+#define FAST_TRACEPOINT_LABEL(name) \
+ asm (" .global " SYMBOL(name) "\n" \
+ SYMBOL(name) ":\n" \
" mvc 0(8, %r15), 0(%r15)\n" \
)
diff --git a/gdb/testsuite/lib/trace-support.exp b/gdb/testsuite/lib/trace-support.exp
index b307f3f..df0fda0 100644
--- a/gdb/testsuite/lib/trace-support.exp
+++ b/gdb/testsuite/lib/trace-support.exp
@@ -43,6 +43,9 @@ if [is_amd64_regs_target] {
} elseif { [istarget "s390*-*-*"] } {
set fpreg "r11"
set spreg "r15"
+} elseif [is_aarch32_target] {
+ set fpreg "sp"
+ set spreg "sp"
set pcreg "pc"
} else {
set fpreg "fp"
--
2.9.2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer
2016-11-03 14:33 ` [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
@ 2016-11-03 17:51 ` Eli Zaretskii
2016-11-03 18:12 ` Antoine Tremblay
2016-11-10 14:01 ` Yao Qi
1 sibling, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-11-03 17:51 UTC (permalink / raw)
To: Antoine Tremblay; +Cc: gdb-patches
> From: Antoine Tremblay <antoine.tremblay@ericsson.com>
> CC: Antoine Tremblay <antoine.tremblay@ericsson.com>
> Date: Thu, 3 Nov 2016 10:33:00 -0400
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index a6b1282..233f11e 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,8 @@
>
> *** Changes since GDB 7.12
>
> +* Support for tracepoints on arm-linux was added in GDBServer.
> +
> * Building GDB and GDBserver now requires a C++11 compiler.
>
> For example, GCC 4.8 or later.
This part is OK.
> +encodings as described below. If a @samp{K} is present, it
> +indicates a target specific breakpoint kind. The kind can be the
Please use @var{kind} here, in reference to the packet parameter.
> +length of the breakpoint. E.g., the arm and mips can insert either a
> +2 or 4 byte breakpoint or have additional meaning see
> +@ref{Architecture-Specific Protocol Details}. If the trailing @samp{-}
> +is present, further @samp{QTDP} packets will follow to specify this
> +tracepoint's actions.
This paragraph needs to use 2 spaces between sentences, not one.
The patch for the manual is OK with these gotchas fixed.
Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer
2016-11-03 17:51 ` Eli Zaretskii
@ 2016-11-03 18:12 ` Antoine Tremblay
0 siblings, 0 replies; 15+ messages in thread
From: Antoine Tremblay @ 2016-11-03 18:12 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Antoine Tremblay, gdb-patches
Eli Zaretskii writes:
>> From: Antoine Tremblay <antoine.tremblay@ericsson.com>
>> CC: Antoine Tremblay <antoine.tremblay@ericsson.com>
>> Date: Thu, 3 Nov 2016 10:33:00 -0400
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index a6b1282..233f11e 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -3,6 +3,8 @@
>>
>> *** Changes since GDB 7.12
>>
>> +* Support for tracepoints on arm-linux was added in GDBServer.
>> +
>> * Building GDB and GDBserver now requires a C++11 compiler.
>>
>> For example, GCC 4.8 or later.
>
> This part is OK.
>
>> +encodings as described below. If a @samp{K} is present, it
>> +indicates a target specific breakpoint kind. The kind can be the
>
> Please use @var{kind} here, in reference to the packet parameter.
>
Ooops fixed.
>> +length of the breakpoint. E.g., the arm and mips can insert either a
>> +2 or 4 byte breakpoint or have additional meaning see
>> +@ref{Architecture-Specific Protocol Details}. If the trailing @samp{-}
>> +is present, further @samp{QTDP} packets will follow to specify this
>> +tracepoint's actions.
>
> This paragraph needs to use 2 spaces between sentences, not one.
>
Right, fixed.
> The patch for the manual is OK with these gotchas fixed.
>
> Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer
2016-11-03 14:33 ` [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
2016-11-03 17:51 ` Eli Zaretskii
@ 2016-11-10 14:01 ` Yao Qi
2016-11-15 14:42 ` Antoine Tremblay
1 sibling, 1 reply; 15+ messages in thread
From: Yao Qi @ 2016-11-10 14:01 UTC (permalink / raw)
To: Antoine Tremblay; +Cc: gdb-patches
On Thu, Nov 03, 2016 at 10:33:00AM -0400, Antoine Tremblay wrote:
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index 3f9ff2b..3b3c371 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -2350,6 +2350,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
> strcat (own_buf, ";EnableDisableTracepoints+");
> strcat (own_buf, ";QTBuffer:size+");
> strcat (own_buf, ";tracenz+");
> + strcat (own_buf, ";TracepointKinds+");
Tracepoint "Kinds" is only useful to arm so far, and it is not needed
to other archs support tracepoint, like x86. We should only reply
";TracepointKinds+" on archs where it is useful.
> }
>
> if (target_supports_hardware_single_step ()
> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
> index 7700ad1..cdb2c1d 100644
> --- a/gdb/gdbserver/tracepoint.c
> +++ b/gdb/gdbserver/tracepoint.c
> @@ -747,6 +747,11 @@ struct tracepoint
> /* Link to the next tracepoint in the list. */
> struct tracepoint *next;
>
> + /* Optional kind of the breakpoint to be used. Note this can mean
> + different things for different archs as z0 breakpoint command.
> + Value is -1 if not persent. */
> + int32_t kind;
This field is only useful to trap-based tracepoint. It signals that we
need to create a sub-class trap_based_tracepoint of struct tracepoint.
> +
> #ifndef IN_PROCESS_AGENT
> /* The list of actions to take when the tracepoint triggers, in
> string/packet form. */
> @@ -1813,6 +1818,7 @@ add_tracepoint (int num, CORE_ADDR addr)
> tpoint->compiled_cond = 0;
> tpoint->handle = NULL;
> tpoint->next = NULL;
> + tpoint->kind = -1;
>
> /* Find a place to insert this tracepoint into list in order to keep
> the tracepoint list still in the ascending order. There may be
> @@ -2484,6 +2490,7 @@ cmd_qtdp (char *own_buf)
> ULONGEST num;
> ULONGEST addr;
> ULONGEST count;
> + ULONGEST kind;
> struct tracepoint *tpoint;
> char *actparm;
> char *packet = own_buf;
> @@ -2550,6 +2557,12 @@ cmd_qtdp (char *own_buf)
> tpoint->cond = gdb_parse_agent_expr (&actparm);
> packet = actparm;
> }
> + else if (*packet == 'K')
> + {
> + ++packet;
> + packet = unpack_varlen_hex (packet, &kind);
> + tpoint->kind = kind;
> + }
> else if (*packet == '-')
> break;
> else if (*packet == '\0')
> @@ -12293,6 +12307,10 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
> stepping_actions);
>
> tpaddr = loc->address;
> +
> + /* Fetch the proper tracepoint kind. */
> + gdbarch_remote_breakpoint_from_pc (target_gdbarch (), &tpaddr, &kind);
> +
This function is already removed recently.
> sprintf_vma (addrbuf, tpaddr);
> xsnprintf (buf, BUF_SIZE, "QTDP:%x:%s:%c:%lx:%x", b->number,
> addrbuf, /* address */
> @@ -12367,6 +12385,11 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
> "ignoring tp %d cond"), b->number);
> }
>
> + /* Tracepoint Kinds are modeled after the breakpoint Z0 kind packet.
What do you mean?
> diff --git a/gdb/testsuite/gdb.trace/collection.exp b/gdb/testsuite/gdb.trace/collection.exp
> index f225429..a30234f 100644
> --- a/gdb/testsuite/gdb.trace/collection.exp
> +++ b/gdb/testsuite/gdb.trace/collection.exp
> @@ -764,7 +764,12 @@ proc gdb_trace_collection_test {} {
> gdb_collect_expression_test globals_test_func \
> "globalarr\[\(l6, l7\)\]" "7" "a\[\(b, c\)\]"
>
> - gdb_collect_return_test
> + #This architecture has no method to collect a return address.
> + if { [is_aarch32_target] } {
> + unsupported "collect \$_ret: This architecture has no method to collect a return address"
> + } else {
> + gdb_collect_return_test
> + }
You need to implement arm_gen_return_address.
>
> gdb_collect_strings_test strings_test_func "locstr" "abcdef" "" \
> "local string"
> diff --git a/gdb/testsuite/gdb.trace/trace-common.h b/gdb/testsuite/gdb.trace/trace-common.h
> index 60cf9e8..9d607f7 100644
> --- a/gdb/testsuite/gdb.trace/trace-common.h
> +++ b/gdb/testsuite/gdb.trace/trace-common.h
> @@ -40,7 +40,8 @@ x86_trace_dummy ()
> " call " SYMBOL(x86_trace_dummy) "\n" \
> )
>
> -#elif (defined __aarch64__) || (defined __powerpc__)
> +#elif (defined __aarch64__) || (defined __powerpc__) \
> + || (defined __arm__ && !defined __thumb__)
>
> #define FAST_TRACEPOINT_LABEL(name) \
> asm (" .global " SYMBOL(name) "\n" \
> @@ -48,11 +49,18 @@ x86_trace_dummy ()
> " nop\n" \
> )
>
> -#elif (defined __s390__)
> +#elif (defined __arm__ && defined __thumb2__)
>
> #define FAST_TRACEPOINT_LABEL(name) \
> asm (" .global " SYMBOL(name) "\n" \
> SYMBOL(name) ":\n" \
> + " nop.w\n" \
> + )
> +
> +#elif (defined __s390__)
> +#define FAST_TRACEPOINT_LABEL(name) \
> + asm (" .global " SYMBOL(name) "\n" \
> + SYMBOL(name) ":\n" \
> " mvc 0(8, %r15), 0(%r15)\n" \
> )
>
(defined __arm__ && defined __thumb__) (thumb-1) is still not handled.
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer
2016-11-10 14:01 ` Yao Qi
@ 2016-11-15 14:42 ` Antoine Tremblay
2016-11-16 20:49 ` Yao Qi
0 siblings, 1 reply; 15+ messages in thread
From: Antoine Tremblay @ 2016-11-15 14:42 UTC (permalink / raw)
To: Yao Qi; +Cc: Antoine Tremblay, gdb-patches
Yao Qi writes:
> On Thu, Nov 03, 2016 at 10:33:00AM -0400, Antoine Tremblay wrote:
>> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
>> index 3f9ff2b..3b3c371 100644
>> --- a/gdb/gdbserver/server.c
>> +++ b/gdb/gdbserver/server.c
>> @@ -2350,6 +2350,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
>> strcat (own_buf, ";EnableDisableTracepoints+");
>> strcat (own_buf, ";QTBuffer:size+");
>> strcat (own_buf, ";tracenz+");
>> + strcat (own_buf, ";TracepointKinds+");
>
> Tracepoint "Kinds" is only useful to arm so far, and it is not needed
> to other archs support tracepoint, like x86. We should only reply
> ";TracepointKinds+" on archs where it is useful.
>
OK I'll add a target method _supports_tracepoint_kinds to avoid that.
I've also moved the kind resolution in remote.c under a check for
tracepoint support like so :
/* Send the tracepoint kind if GDBServer supports it. */
if (remote_supports_tracepoint_kinds ())
{
/* Fetch the proper tracepoint kind. */
int kind = gdbarch_breakpoint_kind_from_pc (target_gdbarch (), &tpaddr);
xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":K%x", kind);
}
>> }
>>
>> if (target_supports_hardware_single_step ()
>> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
>> index 7700ad1..cdb2c1d 100644
>> --- a/gdb/gdbserver/tracepoint.c
>> +++ b/gdb/gdbserver/tracepoint.c
>> @@ -747,6 +747,11 @@ struct tracepoint
>> /* Link to the next tracepoint in the list. */
>> struct tracepoint *next;
>>
>> + /* Optional kind of the breakpoint to be used. Note this can mean
>> + different things for different archs as z0 breakpoint command.
>> + Value is -1 if not persent. */
>> + int32_t kind;
>
> This field is only useful to trap-based tracepoint. It signals that we
> need to create a sub-class trap_based_tracepoint of struct tracepoint.
>
Currently struct tracepoint is a merged struct if you will of all the
tracepoint types, fast, static, trap.
Moving to a subclass for trap-based tracepoints, would require making a
subclass for all the others too, static, fast. It would be quite
inconsistent otherwise.
While I do not object to this change, I think it should be part of
another patch series and that this change is orthogonal to the
tracepoint support for arm.
WDYT ?
>> +
>> #ifndef IN_PROCESS_AGENT
>> /* The list of actions to take when the tracepoint triggers, in
>> string/packet form. */
>> @@ -1813,6 +1818,7 @@ add_tracepoint (int num, CORE_ADDR addr)
>> tpoint->compiled_cond = 0;
>> tpoint->handle = NULL;
>> tpoint->next = NULL;
>> + tpoint->kind = -1;
>>
>> /* Find a place to insert this tracepoint into list in order to keep
>> the tracepoint list still in the ascending order. There may be
>> @@ -2484,6 +2490,7 @@ cmd_qtdp (char *own_buf)
>> ULONGEST num;
>> ULONGEST addr;
>> ULONGEST count;
>> + ULONGEST kind;
>> struct tracepoint *tpoint;
>> char *actparm;
>> char *packet = own_buf;
>> @@ -2550,6 +2557,12 @@ cmd_qtdp (char *own_buf)
>> tpoint->cond = gdb_parse_agent_expr (&actparm);
>> packet = actparm;
>> }
>> + else if (*packet == 'K')
>> + {
>> + ++packet;
>> + packet = unpack_varlen_hex (packet, &kind);
>> + tpoint->kind = kind;
>> + }
>> else if (*packet == '-')
>> break;
>> else if (*packet == '\0')
>> @@ -12293,6 +12307,10 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
>> stepping_actions);
>>
>> tpaddr = loc->address;
>> +
>> + /* Fetch the proper tracepoint kind. */
>> + gdbarch_remote_breakpoint_from_pc (target_gdbarch (), &tpaddr, &kind);
>> +
>
> This function is already removed recently.
Fixed. Thanks.
>
>> sprintf_vma (addrbuf, tpaddr);
>> xsnprintf (buf, BUF_SIZE, "QTDP:%x:%s:%c:%lx:%x", b->number,
>> addrbuf, /* address */
>> @@ -12367,6 +12385,11 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
>> "ignoring tp %d cond"), b->number);
>> }
>>
>> + /* Tracepoint Kinds are modeled after the breakpoint Z0 kind packet.
>
> What do you mean?
I meant that the kind field in the tracepoints is the same as the kind
field for the breakpoints.
I think that comment was more confusing than anything, kinds are
described in the doc anyway so I'll forgo that comment and just write:
/* Send the tracepoint kind if GDBServer supports it. */
if (remote_supports_tracepoint_kinds ())
>
>> diff --git a/gdb/testsuite/gdb.trace/collection.exp b/gdb/testsuite/gdb.trace/collection.exp
>> index f225429..a30234f 100644
>> --- a/gdb/testsuite/gdb.trace/collection.exp
>> +++ b/gdb/testsuite/gdb.trace/collection.exp
>> @@ -764,7 +764,12 @@ proc gdb_trace_collection_test {} {
>> gdb_collect_expression_test globals_test_func \
>> "globalarr\[\(l6, l7\)\]" "7" "a\[\(b, c\)\]"
>>
>> - gdb_collect_return_test
>> + #This architecture has no method to collect a return address.
>> + if { [is_aarch32_target] } {
>> + unsupported "collect \$_ret: This architecture has no method to collect a return address"
>> + } else {
>> + gdb_collect_return_test
>> + }
>
> You need to implement arm_gen_return_address.
>
Done. Thanks.
>>
>> gdb_collect_strings_test strings_test_func "locstr" "abcdef" "" \
>> "local string"
>> diff --git a/gdb/testsuite/gdb.trace/trace-common.h b/gdb/testsuite/gdb.trace/trace-common.h
>> index 60cf9e8..9d607f7 100644
>> --- a/gdb/testsuite/gdb.trace/trace-common.h
>> +++ b/gdb/testsuite/gdb.trace/trace-common.h
>> @@ -40,7 +40,8 @@ x86_trace_dummy ()
>> " call " SYMBOL(x86_trace_dummy) "\n" \
>> )
>>
>> -#elif (defined __aarch64__) || (defined __powerpc__)
>> +#elif (defined __aarch64__) || (defined __powerpc__) \
>> + || (defined __arm__ && !defined __thumb__)
>>
>> #define FAST_TRACEPOINT_LABEL(name) \
>> asm (" .global " SYMBOL(name) "\n" \
>> @@ -48,11 +49,18 @@ x86_trace_dummy ()
>> " nop\n" \
>> )
>>
>> -#elif (defined __s390__)
>> +#elif (defined __arm__ && defined __thumb2__)
>>
>> #define FAST_TRACEPOINT_LABEL(name) \
>> asm (" .global " SYMBOL(name) "\n" \
>> SYMBOL(name) ":\n" \
>> + " nop.w\n" \
>> + )
>> +
>> +#elif (defined __s390__)
>> +#define FAST_TRACEPOINT_LABEL(name) \
>> + asm (" .global " SYMBOL(name) "\n" \
>> + SYMBOL(name) ":\n" \
>> " mvc 0(8, %r15), 0(%r15)\n" \
>> )
>>
>
> (defined __arm__ && defined __thumb__) (thumb-1) is still not handled.
thumb-1 is not supported in the future fast tracepoints thus I had not
included it here but indeed it should work with normal tracepoints.
Fast tracepoints with thumb-1 should just error out anyway.
I'll add thumb-1 in there, thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer
2016-11-15 14:42 ` Antoine Tremblay
@ 2016-11-16 20:49 ` Yao Qi
0 siblings, 0 replies; 15+ messages in thread
From: Yao Qi @ 2016-11-16 20:49 UTC (permalink / raw)
To: Antoine Tremblay; +Cc: gdb-patches
On Tue, Nov 15, 2016 at 2:36 PM, Antoine Tremblay
<antoine.tremblay@ericsson.com> wrote:
>>
>> This field is only useful to trap-based tracepoint. It signals that we
>> need to create a sub-class trap_based_tracepoint of struct tracepoint.
>>
>
> Currently struct tracepoint is a merged struct if you will of all the
> tracepoint types, fast, static, trap.
>
> Moving to a subclass for trap-based tracepoints, would require making a
> subclass for all the others too, static, fast. It would be quite
> inconsistent otherwise.
Yes, that is what we should do. Before we add something new, we need to
clean up the existing code if necessary.
>
> While I do not object to this change, I think it should be part of
> another patch series and that this change is orthogonal to the
> tracepoint support for arm.
>
> WDYT ?
>
It is not orthogonal to the tracepoint support. In contrary, we must
"sub-struct" or "sub-class" tracepoint first, and them add "kind"
field for trap-based tracepoint. Note that "struct tracepoint" is
used in IPA as well.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH V2 0/5] Support tracepoints for ARM linux in GDBServer
2016-11-03 14:33 [PATCH V2 0/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
` (4 preceding siblings ...)
2016-11-03 14:33 ` [PATCH V2 5/5] Support tracepoints for ARM linux in GDBServer Antoine Tremblay
@ 2016-11-07 9:25 ` Yao Qi
[not found] ` <wwoka8dasqta.fsf@ericsson.com>
5 siblings, 1 reply; 15+ messages in thread
From: Yao Qi @ 2016-11-07 9:25 UTC (permalink / raw)
To: Antoine Tremblay; +Cc: gdb-patches
On Thu, Nov 3, 2016 at 2:32 PM, Antoine Tremblay
<antoine.tremblay@ericsson.com> wrote:
>
> Since all the prerequisites for this series have been addressed,
> this is a V2 of https://sourceware.org/ml/gdb-patches/2016-01/msg00111.html
All "hard" prerequisites are addressed, but we still want to "teach
unwinders to terminate gracefully in an arch-independent way".
https://sourceware.org/ml/gdb-patches/2016-05/msg00060.html
I didn't follow it up closely. I hope we can make progress on this...
"Progress" here means either "it is completely wrong, let us handle
unavailable data in each arch unwinder one by one" or "it is
correct, let us remove these redundant code in each arch".
I am still testing arm-linux gdbserver without and with software
single step. I still see some intermittent regressions _with_
software single step,
+FAIL: gdb.threads/non-stop-fair-events.exp: signal_thread=8: thread 1
broke out of loop (timeout)
+FAIL: gdb.threads/schedlock.exp: schedlock=off: cmd=step: step to
increment (1) (timeout)
This reveals something wrong in software single step in GDBserver.
I don't think we should bring tracepoint in until these regressions are
fixed. I won't work on these regressions until next pre-release. If
you can reproduce them and help to fix them, that will be great.
--
Yao (齐尧)
^ permalink raw reply [flat|nested] 15+ messages in thread