This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/4] Support tracepoints for ARM linux in GDBServer




On 01/07/2016 12:44 PM, Antoine Tremblay wrote:
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.

A note about tests :

New tests passing: All of gdb.trace except below tests.

Failing tests:

new FAIL: gdb.trace/unavailable.exp: unavailable locals:
register locals: tfile: info locals
new FAIL: gdb.trace/unavailable.exp: unavailable locals:
register locals: tfile: print locd
new FAIL: gdb.trace/unavailable.exp: unavailable locals:
register locals: tfile: print locf

These tests are failing since we would need the proper gdbarch containing
the vfp registers when trying to read pseudo-registers.  However, we would
need to have an inferior running to have this and since we don't, the tests
fail.

Should these be set as expected fail ?


Note that these tests no longer fail after this patch series :
https://sourceware.org/ml/gdb-patches/2016-02/msg00161.html

Thanks for that work Marcin KoÅcielnicki!

So I think we could leave them failing if this patch set goes in before, since they will get fixed...or I'll adapt the commit log if Marcin's patch set is first to be merged.

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                    | 22 +++++++++++++----
  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 | 10 +++++++-
  gdb/testsuite/lib/trace-support.exp    |  4 ++++
  11 files changed, 131 insertions(+), 15 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 484d98d..28110cc 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,8 @@

  *** Changes since GDB 7.10

+* Support for tracepoints on arm-linux was added in GDBServer.
+
  * Record btrace now supports non-stop mode.

  * Support for tracepoints on aarch64-linux was added in GDBserver.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0778383..cffad48 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -36633,6 +36633,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:
@@ -36851,6 +36856,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::
@@ -37361,7 +37369,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

@@ -37441,7 +37450,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
@@ -37452,9 +37461,12 @@ 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 length.  E.g., the arm and mips
+can insert either a 2 or 4 byte breakpoint. Some architectures have
+additional meanings for kind 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 d967e58..76d0e32 100644
--- a/gdb/gdbserver/linux-arm-low.c
+++ b/gdb/gdbserver/linux-arm-low.c
@@ -1005,6 +1005,14 @@ arm_regs_info (void)
      return &regs_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,
@@ -1031,7 +1039,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 2e220b8..3a9a816 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -791,6 +791,19 @@ set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
  			 &err_ignored);
  }

+/* 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 4d9a76c..02a1038 100644
--- a/gdb/gdbserver/mem-break.h
+++ b/gdb/gdbserver/mem-break.h
@@ -146,6 +146,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 fe7195d..059a373 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -2269,6 +2269,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 40d0da9..3a50f47 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -754,6 +754,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.  */
@@ -1820,6 +1825,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
@@ -2495,6 +2501,7 @@ cmd_qtdp (char *own_buf)
    ULONGEST num;
    ULONGEST addr;
    ULONGEST count;
+  ULONGEST kind;
    struct tracepoint *tpoint;
    char *actparm;
    char *packet = own_buf;
@@ -2561,6 +2568,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')
@@ -2575,11 +2588,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);
@@ -3153,9 +3168,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)
      {
@@ -3248,8 +3271,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 528d863..17581e2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -241,6 +241,8 @@ static int stop_reply_queue_length (void);

  static void readahead_cache_invalidate (void);

+static int remote_supports_tracepoint_kinds (void);
+
  /* For "remote".  */

  static struct cmd_list_element *remote_cmdlist;
@@ -1491,6 +1493,9 @@ enum {
    /* Support TARGET_WAITKIND_NO_RESUMED.  */
    PACKET_no_resumed,

+  /* Support target dependant tracepoint kinds.  */
+  PACKET_TracepointKinds,
+
    PACKET_MAX
  };

@@ -4534,6 +4539,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;
@@ -11692,6 +11699,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)
  {
@@ -11780,6 +11793,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,
@@ -11788,6 +11802,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 */
@@ -11862,6 +11880,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);
@@ -13767,6 +13790,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 eceb182..4f05423 100644
--- a/gdb/testsuite/gdb.trace/trace-common.h
+++ b/gdb/testsuite/gdb.trace/trace-common.h
@@ -40,7 +40,7 @@ x86_trace_dummy ()
         "    call " SYMBOL(x86_trace_dummy) "\n" \
         )

-#elif (defined __aarch64__)
+#elif (defined __aarch64__ || (defined __arm__ && !defined __thumb__))

  #define FAST_TRACEPOINT_LABEL(name) \
    asm ("    .global " SYMBOL(name) "\n" \
@@ -48,6 +48,14 @@ x86_trace_dummy ()
         "    nop\n" \
         )

+#elif (defined __arm__ && defined __thumb2__)
+
+#define FAST_TRACEPOINT_LABEL(name) \
+  asm ("    .global " SYMBOL(name) "\n" \
+       SYMBOL(name) ":\n" \
+       "    nop.w\n" \
+       )
+
  #else

  #error "unsupported architecture for trace tests"
diff --git a/gdb/testsuite/lib/trace-support.exp b/gdb/testsuite/lib/trace-support.exp
index f593c43..ef63f7a 100644
--- a/gdb/testsuite/lib/trace-support.exp
+++ b/gdb/testsuite/lib/trace-support.exp
@@ -36,6 +36,10 @@ if [is_amd64_regs_target] {
      set fpreg "x29"
      set spreg "sp"
      set pcreg "pc"
+} elseif [is_aarch32_target] {
+    set fpreg "sp"
+    set spreg "sp"
+    set pcreg "pc"
  } else {
      set fpreg "fp"
      set spreg "sp"



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]