| Message ID | 20161103143300.24934-6-antoine.tremblay@ericsson.com |
|---|---|
| State | New |
| Headers | show |
> 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.
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.
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 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.
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.
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"
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(-)