Re: [PATCH 14/16] remote, btrace: add branch trace remote ops

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Wed, 23 May 2012 13:22:29 +0200, markus.t.metzger@intel.com wrote:
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -66,6 +66,7 @@
>  #include "ax.h"
>  #include "ax-gdb.h"
>  #include "agent.h"
> +#include "btrace.h"
>  
>  /* Temp hacks for tracepoint encoding migration.  */
>  static char *target_buf;
> @@ -1284,6 +1285,9 @@ enum {
>    PACKET_qXfer_fdpic,
>    PACKET_QDisableRandomization,
>    PACKET_QAgent,
> +  PACKET_qbtrace,
> +  PACKET_Qbtrace,
> +  PACKET_qXfer_btrace,
>    PACKET_MAX
>  };
>  
> @@ -3925,6 +3929,10 @@ static struct protocol_feature remote_protocol_features[] = {
>    { "QAgent", PACKET_DISABLE, remote_supported_packet, PACKET_QAgent},
>    { "tracenz", PACKET_DISABLE,
>      remote_string_tracing_feature, -1 },
> +  { "qbtrace", PACKET_DISABLE, remote_supported_packet, PACKET_qbtrace },
> +  { "Qbtrace", PACKET_DISABLE, remote_supported_packet, PACKET_Qbtrace },
> +  { "qXfer:btrace:read", PACKET_DISABLE, remote_supported_packet,
> +    PACKET_qXfer_btrace }
>  };
>  
>  static char *remote_support_xml;
> @@ -8588,6 +8596,10 @@ remote_xfer_partial (struct target_ops *ops, enum target_object object,
>        return remote_read_qxfer (ops, "uib", annex, readbuf, offset, len,
>  				&remote_protocol_packets[PACKET_qXfer_uib]);
>  
> +    case TARGET_OBJECT_BTRACE:
> +      return remote_read_qxfer (ops, "btrace", annex, readbuf, offset, len,
> +        &remote_protocol_packets[PACKET_qXfer_btrace]);
> +
>      default:
>        return -1;
>      }
> @@ -10901,6 +10913,188 @@ remote_can_use_agent (void)
>    return (remote_protocol_packets[PACKET_QAgent].support != PACKET_DISABLE);
>  }
>  
> +struct btrace_target_info
> +{
> +  /* The ptid of the traced thread.  */
> +  ptid_t ptid;
> +};
> +
> +static int
> +remote_supports_btrace (void)
> +{
> +  if (remote_protocol_packets[PACKET_qbtrace].support != PACKET_ENABLE)
> +    return 0;
> +  if (remote_protocol_packets[PACKET_Qbtrace].support != PACKET_ENABLE)
> +    return 0;
> +  if (remote_protocol_packets[PACKET_qXfer_btrace].support != PACKET_ENABLE)
> +    return 0;
> +
> +  return 1;
> +}
> +
> +static struct btrace_target_info *
> +remote_enable_btrace (ptid_t ptid)
> +{
> +  struct packet_config *packet = &remote_protocol_packets[PACKET_Qbtrace];
> +  struct remote_state *rs = get_remote_state ();
> +  struct btrace_target_info *tinfo = NULL;
> +  char *buf = rs->buf;
> +  char *endbuf = rs->buf + get_remote_packet_size ();
> +
> +  if (packet->support != PACKET_ENABLE)
> +    {
> +      errno = ENOSYS;

errno is not used for intra-GDB/gdbserver communication.  Everywhere.


> +      return NULL;
> +    }
> +
> +  buf += xsnprintf (buf, endbuf - buf, "%s", packet->name);
> +  buf += xsnprintf (buf, endbuf - buf, ":on:");
> +  buf = write_ptid (buf, endbuf, ptid);
> +  putpkt (rs->buf);
> +  getpkt (&rs->buf, &rs->buf_size, 0);
> +
> +  switch (packet_ok (rs->buf, packet))
> +    {
> +    case PACKET_OK:
> +      tinfo = xzalloc (sizeof (*tinfo));
> +      if (tinfo)

Remove, xzalloc can never fail.

> +        tinfo->ptid = ptid;
> +      else
> +        errno = ENOMEM;
> +      break;
> +    case PACKET_ERROR:
> +      {
> +        int pid = ptid_get_lwp (ptid);
> +        if (!pid)
> +          pid = ptid_get_pid (ptid);
> +
> +        error (_("Couldn't enable btrace for %d: %s"),
> +               pid, rs->buf);

Use target_pid_to_str.  Everywhere.


> +      }
> +    default:
> +      errno = ENOSYS;
> +      break;
> +    }
> +
> +  return tinfo;
> +}
> +
> +static int
> +remote_disable_btrace (struct btrace_target_info *tinfo)
> +{
> +  struct packet_config *packet = &remote_protocol_packets[PACKET_Qbtrace];
> +  struct remote_state *rs = get_remote_state ();
> +  char *buf = rs->buf;
> +  char *endbuf = rs->buf + get_remote_packet_size ();
> +
> +  if (!tinfo)
> +    return EINVAL;
> +
> +  if (packet->support != PACKET_ENABLE)
> +    return ENOSYS;
> +
> +  buf += xsnprintf (buf, endbuf - buf, "%s", packet->name);
> +  buf += xsnprintf (buf, endbuf - buf, ":off:");
> +  buf = write_ptid (buf, endbuf, tinfo->ptid);
> +  putpkt (rs->buf);
> +  getpkt (&rs->buf, &rs->buf_size, 0);
> +
> +  switch (packet_ok (rs->buf, packet))
> +    {
> +    case PACKET_OK:
> +      xfree (tinfo);
> +      return 0;
> +
> +    case PACKET_ERROR:
> +      {
> +        int pid = ptid_get_lwp (tinfo->ptid);
> +        if (!pid)
> +          pid = ptid_get_pid (tinfo->ptid);
> +
> +        error (_("Couldn't disable btrace for %d: %s"),
> +               pid, rs->buf);
> +      }
> +    default:
> +      return ENOSYS;
> +    }
> +}
> +
> +static int
> +remote_btrace_has_changed (struct btrace_target_info *tinfo)
> +{
> +  struct packet_config *packet = &remote_protocol_packets[PACKET_qbtrace];
> +  struct remote_state *rs = get_remote_state ();
> +  char *buf = rs->buf;
> +  char *endbuf = rs->buf + get_remote_packet_size ();
> +
> +  if (!tinfo)
> +    {
> +      errno = EINVAL;
> +      return 0;
> +    }
> +  if (packet->support != PACKET_ENABLE)
> +    {
> +      errno = ENOSYS;
> +      return 0;
> +    }
> +
> +  buf += xsnprintf (buf, endbuf - buf, "%s", packet->name);
> +  buf += xsnprintf (buf, endbuf - buf, ":");
> +  buf = write_ptid (buf, endbuf, tinfo->ptid);
> +  putpkt (rs->buf);
> +  getpkt (&rs->buf, &rs->buf_size, 0);
> +
> +  switch (packet_ok (rs->buf, packet))
> +    {
> +    case PACKET_OK:
> +      return (strcmp (rs->buf, "yes") == 0);
> +    case PACKET_ERROR:
> +      error (_("%s"), rs->buf);
> +    default:
> +      errno = ENOSYS;
> +      return 0;
> +    }
> +}
> +
> +static VEC (btrace_block_s) *
> +remote_read_btrace (struct btrace_target_info *tinfo)
> +{
> +  struct packet_config *packet = &remote_protocol_packets[PACKET_qXfer_btrace];
> +  struct remote_state *rs = get_remote_state ();
> +  char *xml, annex[64], *pend;
> +  VEC (btrace_block_s) *btrace = NULL;
> +
> +  if (!tinfo)
> +    {
> +      errno = EINVAL;
> +      return NULL;
> +    }
> +  if (packet->support != PACKET_ENABLE)
> +    {
> +      errno = ENOSYS;
> +      return NULL;
> +    }
> +
> +#if !defined(HAVE_LIBEXPAT)
> +  errno = ENOSYS;
> +  return NULL;
> +#endif
> +
> +  memset (annex, 0, sizeof (annex));
> +  pend = write_ptid (annex, annex + sizeof (annex), tinfo->ptid);
> +
> +  xml = target_read_stralloc (&current_target,
> +                              TARGET_OBJECT_BTRACE, annex);
> +  if (xml)
> +    {
> +      struct cleanup *cleanup = make_cleanup (xfree, xml);

Empty line after declarations.

> +      btrace = parse_xml_btrace (xml);
> +      do_cleanups (cleanup);
> +    }
> +
> +  return btrace;
> +}
> +
>  static void
>  init_remote_ops (void)
>  {
> @@ -11016,6 +11210,11 @@ Specify the serial device it is connected to\n\
>    remote_ops.to_traceframe_info = remote_traceframe_info;
>    remote_ops.to_use_agent = remote_use_agent;
>    remote_ops.to_can_use_agent = remote_can_use_agent;
> +  remote_ops.to_supports_btrace = remote_supports_btrace;
> +  remote_ops.to_enable_btrace = remote_enable_btrace;
> +  remote_ops.to_disable_btrace = remote_disable_btrace;
> +  remote_ops.to_btrace_has_changed = remote_btrace_has_changed;
> +  remote_ops.to_read_btrace = remote_read_btrace;
>  }
>  
>  /* Set up the extended remote vector by making a copy of the standard
> @@ -11538,6 +11737,15 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL,
>    add_packet_config_cmd (&remote_protocol_packets[PACKET_QAgent],
>  			 "QAgent", "agent", 0);
>  
> +  add_packet_config_cmd (&remote_protocol_packets[PACKET_qbtrace],
> +       "qbtrace", "query-btrace", 0);
> +
> +  add_packet_config_cmd (&remote_protocol_packets[PACKET_Qbtrace],
> +       "Qbtrace", "enable-btrace", 0);
> +
> +  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_btrace],
> +       "qXfer:btrace", "read-btrace", 0);
> +
>    /* Keep the old ``set remote Z-packet ...'' working.  Each individual
>       Z sub-packet has its own set and show commands, but users may
>       have sets to this variable in their .gdbinit files (or in their
> diff --git a/gdb/target.h b/gdb/target.h
> index 86513f7..3730863 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -283,7 +283,9 @@ enum target_object
>    /* Darwin dynamic linker info data.  */
>    TARGET_OBJECT_DARWIN_DYLD_INFO,
>    /* OpenVMS Unwind Information Block.  */
> -  TARGET_OBJECT_OPENVMS_UIB
> +  TARGET_OBJECT_OPENVMS_UIB,
> +  /* Branch trace data, in XML format.  */
> +  TARGET_OBJECT_BTRACE
>    /* Possible future objects: TARGET_OBJECT_FILE, ...  */
>  };
>  
> -- 
> 1.7.1


Thanks,
Jan