Re: [PATCH v3 03/15] Read CTF by the ctf target

[Doug Evans <dje at google dot com>]

Yao Qi writes:
 > On 03/30/2013 12:27 AM, Doug Evans wrote:
 > > One last thing.  Apologies for not bringing this up earlier.
 > > 
 > > I'm not sure we have any conventions explicitly saying to use CORE_ADDR
 > > instead of {,U}LONGEST for addresses, but tp->base.loc->address is a CORE_ADDR.
 > > It feels like everywhere you're using ULONGEST for addresses you need to
 > > use CORE_ADDR instead.
 > > 
 > > Or maybe there's a specific reason to use ULONGEST?
 > 
 > Doug, good point.  We should use CORE_ADDR here.  This patch
 > <http://sourceware.org/ml/gdb-patches/2013-03/msg01126.html> has
 > converted ULONGEST in to_trace_find method of target_ops to CORE_ADDR.
 > CTF stuffs start to use CORE_ADDR in the updated patch below.  Is it OK?

Hi.  Just a few nits, inline.

 > 2013-04-08  Hui Zhu  <hui@codesourcery.com>
 > 	    Yao Qi  <yao@codesourcery.com>
 > 
 > 	* configure.ac: Check libbabeltrace is installed.
 > 	* config.in: Regenerate.
 > 	* configure: Regenerate.
 > 	* Makefile.in (LIBBABELTRACE): New.
 > 	(CLIBS): Add LIBBABELTRACE.
 > 	* ctf.c (ctx, ctf_iter, trace_dirname): New.
 > 	(ctf_destroy, ctf_open_dir, ctf_open): New.
 > 	(ctf_close, ctf_files_info): New.
 > 	(ctf_fetch_registers, ctf_xfer_partial): New.
 > 	(ctf_get_trace_state_variable_value): New.
 > 	(ctf_get_tpnum_from_frame_event): New.
 > 	(ctf_get_traceframe_address): New.
 > 	(ctf_trace_find, ctf_has_stack): New.
 > 	(ctf_has_registers, ctf_traceframe_info, init_ctf_ops): New.
 > 	(_initialize_ctf): New.
 > 	* tracepoint.c (get_tracepoint_number): New
 >  	(struct traceframe_info, trace_regblock_size): Move it to ...
 > 	* tracepoint.h: ... here.
 > 	(get_tracepoint_number): Declare it.
 > ---
 >  gdb/Makefile.in  |    6 +-
 >  gdb/config.in    |    3 +
 >  gdb/configure    |  517 +++++++++++++++++++++++++++++++++++++
 >  gdb/configure.ac |   41 +++
 >  gdb/ctf.c        |  750 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 >  gdb/tracepoint.c |   16 +-
 >  gdb/tracepoint.h |   13 +
 >  7 files changed, 1335 insertions(+), 11 deletions(-)
 > 
 > diff --git a/gdb/Makefile.in b/gdb/Makefile.in
 > index 498d42a..0814bcb 100644
 > --- a/gdb/Makefile.in
 > +++ b/gdb/Makefile.in
 > @@ -154,6 +154,10 @@ LIBEXPAT = @LIBEXPAT@
 >  # Where is lzma?  This will be empty if lzma was not available.
 >  LIBLZMA = @LIBLZMA@
 >  
 > +# Where is libbabeltrace? This will be empty if lbabeltrace was not
 > +# available.
 > +LIBBABELTRACE = @LIBBABELTRACE@
 > +
 >  WARN_CFLAGS = @WARN_CFLAGS@
 >  WERROR_CFLAGS = @WERROR_CFLAGS@
 >  GDB_WARN_CFLAGS = $(WARN_CFLAGS)
 > @@ -475,7 +479,7 @@ INTERNAL_LDFLAGS = $(CFLAGS) $(GLOBAL_CFLAGS) $(MH_LDFLAGS) $(LDFLAGS) $(CONFIG_
 >  # LIBIBERTY appears twice on purpose.
 >  CLIBS = $(SIM) $(READLINE) $(OPCODES) $(BFD) $(INTL) $(LIBIBERTY) $(LIBDECNUMBER) \
 >  	$(XM_CLIBS) $(NAT_CLIBS) $(GDBTKLIBS) @LIBS@ @PYTHON_LIBS@ \
 > -	$(LIBEXPAT) $(LIBLZMA) \
 > +	$(LIBEXPAT) $(LIBLZMA) $(LIBBABELTRACE) \
 >  	$(LIBIBERTY) $(WIN32LIBS) $(LIBGNU)
 >  CDEPS = $(XM_CDEPS) $(NAT_CDEPS) $(SIM) $(BFD) $(READLINE_DEPS) \
 >  	$(OPCODES) $(INTL_DEPS) $(LIBIBERTY) $(CONFIG_DEPS) $(LIBGNU)
 > diff --git a/gdb/config.in b/gdb/config.in
 > index 9e21325..c4e8eaa 100644
 > --- a/gdb/config.in
 > +++ b/gdb/config.in
 > @@ -180,6 +180,9 @@
 >  /* Define if your <locale.h> file defines LC_MESSAGES. */
 >  #undef HAVE_LC_MESSAGES
 >  
 > +/* Define if libbabeltrace is available */
 > +#undef HAVE_LIBBABELTRACE
 > +
 >  /* Define to 1 if you have the `dl' library (-ldl). */
 >  #undef HAVE_LIBDL
 >  
 > [...]
 >
 > diff --git a/gdb/configure.ac b/gdb/configure.ac
 > index c17f587..149ac37 100644
 > --- a/gdb/configure.ac
 > +++ b/gdb/configure.ac
 > @@ -2321,6 +2321,47 @@ if test "$enable_gdbserver" = "yes" -a "$gdbserver_build_enabled" != "yes"; then
 >    AC_MSG_ERROR(Automatic gdbserver build is not supported for this configuration)
 >  fi
 >  
 > +# Check for babeltrace and babeltrace-ctf
 > +AC_ARG_WITH(babeltrace,
 > +  AC_HELP_STRING([--with-babeltrace], [include babeltrace support (auto/yes/no)]),
 > +  [], [with_babeltrace=auto])
 > +AC_MSG_CHECKING([whether to use babeltrace])
 > +AC_MSG_RESULT([$with_babeltrace])
 > +
 > +if test "x$with_babeltrace" = "xno"; then
 > +  AC_MSG_WARN([babletrace support disabled; GDB is unable to read CTF data.])
 > +else
 > +  # Append -Werror to CFLAGS so that configure can catch the warning
 > +  # "assignment from incompatible pointer type", which is related to
 > +  # the babeltrace change from 1.0.3 to 1.1.0.  Babeltrace 1.1.0 works
 > +  # in GDB, while babeltrace 1.0.3 is broken.
 > +  # AC_LIB_HAVE_LINKFLAGS may modify CPPFLAGS in it, so it should be
 > +  # safe to save and restore CFLAGS here.
 > +  saved_CFLAGS=$CFLAGS
 > +  CFLAGS="$CFLAGS -Werror"
 > +  AC_LIB_HAVE_LINKFLAGS([babeltrace], [babeltrace-ctf],
 > +			[#include <babeltrace/babeltrace.h>
 > +			 #include <babeltrace/ctf/events.h>
 > +			 #include <babeltrace/ctf/iterator.h>],
 > +			[struct bt_iter_pos *pos = bt_iter_get_pos (bt_ctf_get_iter (NULL));
 > +			struct bt_ctf_event *event = NULL;
 > +			const struct bt_definition *scope;
 > +
 > +			scope = bt_ctf_get_top_level_scope (event, 
 > +			      				   BT_STREAM_EVENT_HEADER);
 > +		        bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "id"));
 > +			])
 > +  CFLAGS=$saved_CFLAGS
 > +
 > +  if test "$HAVE_LIBBABELTRACE" != yes; then
 > +     if test "$with_babeltrace" = yes; then
 > +       AC_MSG_ERROR([babeltrace is missing or unusable])
 > +     else
 > +       AC_MSG_WARN([babeltrace is missing or unusable; GDB is unable to read CTF data.])
 > +     fi
 > +  fi
 > +fi
 > +
 >  # If nativefile (NAT_FILE) is not set in config/*/*.m[ht] files, we link
 >  # to an empty version.
 >  
 > diff --git a/gdb/ctf.c b/gdb/ctf.c
 > index de54051..53daf91 100644
 > --- a/gdb/ctf.c
 > +++ b/gdb/ctf.c
 > @@ -24,6 +24,7 @@
 >  #include "tracepoint.h"
 >  #include "regcache.h"
 >  #include "gdb_stat.h"
 > +#include "exec.h"
 >  
 >  #include <ctype.h>
 >  
 > @@ -670,3 +671,752 @@ ctf_trace_file_writer_new (void)
 >  
 >    return (struct trace_file_writer *) writer;
 >  }
 > +
 > +#if HAVE_LIBBABELTRACE
 > +/* Use libbabeltrace to read CTF data.  The libbabeltrace provides
 > +   iterator to iterate over each event in CTF data and APIs to get
 > +   details of event and packet, so it is very convenient to use
 > +   libbabeltrace to access events in CTF.  */
 > +
 > +#include <babeltrace/babeltrace.h>
 > +#include <babeltrace/ctf/events.h>
 > +#include <babeltrace/ctf/iterator.h>
 > +
 > +/* The struct pointer for current CTF directory.  */
 > +static struct bt_context *ctx = NULL;
 > +static struct bt_ctf_iter *ctf_iter = NULL;
 > +/* The name of CTF directory.  */
 > +static char *trace_dirname;
 > +
 > +static struct target_ops ctf_ops;
 > +
 > +/* Destroy ctf iterator and context.  */
 > +
 > +static void
 > +ctf_destroy (void)
 > +{
 > +  if (ctf_iter != NULL)
 > +    {
 > +      bt_ctf_iter_destroy (ctf_iter);
 > +      ctf_iter = NULL;
 > +    }
 > +  if (ctx != NULL)
 > +    {
 > +      bt_context_put (ctx);
 > +      ctx = NULL;
 > +    }
 > +}
 > +
 > +/* Open CTF trace data in DIRNAME.  */
 > +
 > +static void
 > +ctf_open_dir (char *dirname)
 > +{
 > +  int ret;
 > +  struct bt_iter_pos begin_pos;
 > +  struct bt_iter_pos *pos;
 > +
 > +  ctx = bt_context_create ();
 > +  if (ctx == NULL)
 > +    error (_("Unable to initialize libbabeltrace"));
 > +  ret = bt_context_add_trace (ctx, dirname, "ctf", NULL, NULL, NULL);
 > +  if (ret < 0)
 > +    {
 > +      ctf_destroy ();
 > +      error (_("Unable to use libbabeltrace open \"%s\""), dirname);

The wording of this error message (here and below) is awkward.
I don't know libbabeltrace enough to know if this is accurate, but
it reads better.

      error (_("Unable to use libbabeltrace on \"%s\""), dirname);

or

      error (_("Unable to use libbabeltrace on directory \"%s\""), dirname);

Also, IWBN to add a more specific reason for the failure,
but I'm not sure how easy that would be.
Is there a bt_foo function akin to strerror?

 > +    }
 > +
 > +  begin_pos.type = BT_SEEK_BEGIN;
 > +  ctf_iter = bt_ctf_iter_create (ctx, &begin_pos, NULL);
 > +  if (ctf_iter == NULL)
 > +    {
 > +      ctf_destroy ();
 > +      error (_("Unable to use libbabeltrace open \"%s\""), dirname);
 > +    }
 > +
 > +  /* Iterate over events, and look for an event for register block
 > +     to set trace_regblock_size.  */
 > +
 > +  /* Save the current position.  */
 > +  pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
 > +  gdb_assert (pos->type == BT_SEEK_RESTORE);
 > +
 > +  while (1)
 > +    {
 > +      const char *name;
 > +      struct bt_ctf_event *event;
 > +
 > +      event = bt_ctf_iter_read_event (ctf_iter);
 > +
 > +      name = bt_ctf_event_name (event);
 > +
 > +      if (name == NULL)
 > +	break;
 > +      else if (strcmp (name, "register") == 0)
 > +	{
 > +	  const struct bt_definition *scope
 > +	    = bt_ctf_get_top_level_scope (event,
 > +					  BT_EVENT_FIELDS);
 > +	  const struct bt_definition *array
 > +	    = bt_ctf_get_field (event, scope, "contents");
 > +
 > +	  trace_regblock_size
 > +	    = bt_ctf_get_array_len (bt_ctf_get_decl_from_def (array));
 > +	}
 > +
 > +      if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0)
 > +	break;
 > +    }
 > +
 > +  /* Restore the position.  */
 > +  bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
 > +

Remove blank line.

 > +}
 > +
 > +/* This is the implementation of target_ops method to_open.  Open CTF
 > +   trace data, read trace status, trace state variables and tracepoint
 > +   definitions from the first packet.  Set the start position at the
 > +   second packet which contains events on trace blocks.  */
 > +
 > +static void
 > +ctf_open (char *dirname, int from_tty)
 > +{
 > +  if (!dirname)
 > +    error (_("No CTF directory specified."));
 > +
 > +  ctf_open_dir (dirname);
 > +
 > +  target_preopen (from_tty);
 > +  trace_dirname = xstrdup (dirname);
 > +  push_target (&ctf_ops);
 > +}
 > +
 > +/* This is the implementation of target_ops method to_close.  Destroy
 > +   CTF iterator and context.  */
 > +
 > +static void
 > +ctf_close (void)
 > +{
 > +  ctf_destroy ();
 > +  xfree (trace_dirname);
 > +  trace_dirname = NULL;
 > +}
 > +
 > +/* This is the implementation of target_ops method to_files_info.
 > +   Print the directory name of CTF trace data.  */
 > +
 > +static void
 > +ctf_files_info (struct target_ops *t)
 > +{
 > +  printf_filtered ("\t`%s'\n", trace_dirname);
 > +}
 > +
 > +/* This is the implementation of target_ops method to_fetch_registers.
 > +   Iterate over events whose name is "register" in current frame,
 > +   extract contents from events, and set REGCACHE with the contents.
 > +   If no matched events are found, mark registers unavailable.  */
 > +
 > +static void
 > +ctf_fetch_registers (struct target_ops *ops,
 > +		     struct regcache *regcache, int regno)
 > +{
 > +  struct gdbarch *gdbarch = get_regcache_arch (regcache);
 > +  int offset, regn, regsize, pc_regno;
 > +  char *regs = NULL;
 > +  struct bt_ctf_event *event = NULL;
 > +  struct bt_iter_pos *pos;
 > +
 > +  /* An uninitialized reg size says we're not going to be
 > +     successful at getting register blocks.  */
 > +  if (trace_regblock_size == 0)
 > +    return;
 > +
 > +  gdb_assert (ctf_iter != NULL);
 > +  /* Save the current position.  */
 > +  pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
 > +  gdb_assert (pos->type == BT_SEEK_RESTORE);
 > +
 > +  while (1)
 > +    {
 > +      const char *name;
 > +      struct bt_ctf_event *event1;
 > +
 > +      event1 = bt_ctf_iter_read_event (ctf_iter);
 > +
 > +      name = bt_ctf_event_name (event1);
 > +
 > +      if (name == NULL || strcmp (name, "frame") == 0)
 > +	break;
 > +      else if (strcmp (name, "register") == 0)
 > +	{
 > +	  event = event1;
 > +	  break;
 > +	}
 > +
 > +      if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0)
 > +	break;
 > +    }
 > +
 > +  /* Restore the position.  */
 > +  bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
 > +
 > +  if (event != NULL)
 > +    {
 > +      const struct bt_definition *scope
 > +	= bt_ctf_get_top_level_scope (event,
 > +				      BT_EVENT_FIELDS);
 > +      const struct bt_definition *array
 > +	= bt_ctf_get_field (event, scope, "contents");
 > +
 > +      regs = bt_ctf_get_char_array (array);
 > +      /* Assume the block is laid out in GDB register number order,
 > +	 each register with the size that it has in GDB.  */
 > +      offset = 0;
 > +      for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++)
 > +	{
 > +	  regsize = register_size (gdbarch, regn);
 > +	  /* Make sure we stay within block bounds.  */
 > +	  if (offset + regsize >= trace_regblock_size)
 > +	    break;
 > +	  if (regcache_register_status (regcache, regn) == REG_UNKNOWN)
 > +	    {
 > +	      if (regno == regn)
 > +		{
 > +		  regcache_raw_supply (regcache, regno, regs + offset);
 > +		  break;
 > +		}
 > +	      else if (regno == -1)
 > +		{
 > +		  regcache_raw_supply (regcache, regn, regs + offset);
 > +		}
 > +	    }
 > +	  offset += regsize;
 > +	}
 > +      return;
 > +    }
 > +
 > +  regs = alloca (trace_regblock_size);
 > +
 > +  /* We get here if no register data has been found.  Mark registers
 > +     as unavailable.  */
 > +  for (regn = 0; regn < gdbarch_num_regs (gdbarch); regn++)
 > +    regcache_raw_supply (regcache, regn, NULL);
 > +
 > +  /* We can often usefully guess that the PC is going to be the same
 > +     as the address of the tracepoint.  */
 > +  pc_regno = gdbarch_pc_regnum (gdbarch);
 > +  if (pc_regno >= 0 && (regno == -1 || regno == pc_regno))
 > +    {
 > +      struct tracepoint *tp = get_tracepoint (get_tracepoint_number ());
 > +
 > +      if (tp != NULL && tp->base.loc)
 > +	{
 > +	  /* But don't try to guess if tracepoint is multi-location...  */
 > +	  if (tp->base.loc->next != NULL)
 > +	    {
 > +	      warning (_("Tracepoint %d has multiple "
 > +			 "locations, cannot infer $pc"),
 > +		       tp->base.number);
 > +	      return;
 > +	    }
 > +	  /* ... or does while-stepping.  */
 > +	  if (tp->step_count > 0)
 > +	    {
 > +	      warning (_("Tracepoint %d does while-stepping, "
 > +			 "cannot infer $pc"),
 > +		       tp->base.number);
 > +	      return;
 > +	    }
 > +
 > +	  store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
 > +				  gdbarch_byte_order (gdbarch),
 > +				  tp->base.loc->address);
 > +	  regcache_raw_supply (regcache, pc_regno, regs);
 > +	}
 > +    }
 > +}
 > +
 > +/* This is the implementation of target_ops method to_xfer_partial.
 > +   Iterate over events whose name is "memory" in
 > +   current frame, extract the address and length from events.  If
 > +   OFFSET is within the range, read the contents from events to
 > +   READBUF.  */
 > +
 > +static LONGEST
 > +ctf_xfer_partial (struct target_ops *ops, enum target_object object,
 > +		  const char *annex, gdb_byte *readbuf,
 > +		  const gdb_byte *writebuf, ULONGEST offset,
 > +		  LONGEST len)
 > +{
 > +  /* We're only doing regular memory for now.  */
 > +  if (object != TARGET_OBJECT_MEMORY)
 > +    return -1;
 > +
 > +  if (readbuf == NULL)
 > +    error (_("ctf_xfer_partial: trace file is read-only"));
 > +
 > +  if (get_traceframe_number () != -1)
 > +    {
 > +      struct bt_iter_pos *pos;
 > +      int i = 0;
 > +
 > +      gdb_assert (ctf_iter != NULL);
 > +      /* Save the current position.  */
 > +      pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
 > +      gdb_assert (pos->type == BT_SEEK_RESTORE);
 > +
 > +      /* Iterate through the traceframe's blocks, looking for
 > +	 memory.  */
 > +      while (1)
 > +	{
 > +	  ULONGEST amt;
 > +	  uint64_t maddr;
 > +	  uint16_t mlen;
 > +	  enum bfd_endian byte_order
 > +	    = gdbarch_byte_order (target_gdbarch ());
 > +	  const struct bt_definition *scope;
 > +	  const struct bt_definition *def;
 > +	  struct bt_ctf_event *event
 > +	    = bt_ctf_iter_read_event (ctf_iter);
 > +	  const char *name = bt_ctf_event_name (event);
 > +
 > +	  if (strcmp (name, "frame") == 0)
 > +	    break;
 > +	  else if (strcmp (name, "memory") != 0)
 > +	    {
 > +	      if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0)
 > +		break;
 > +
 > +	      continue;
 > +	    }
 > +
 > +	  scope = bt_ctf_get_top_level_scope (event,
 > +					      BT_EVENT_FIELDS);
 > +
 > +	  def = bt_ctf_get_field (event, scope, "address");
 > +	  maddr = bt_ctf_get_uint64 (def);
 > +	  def = bt_ctf_get_field (event, scope, "length");
 > +	  mlen = (uint16_t) bt_ctf_get_uint64 (def);

I don't know libbabeltrace, I'm assuming length can be at most 16 bits.

 > +
 > +	  /* If the block includes the first part of the desired
 > +	     range, return as much it has; GDB will re-request the
 > +	     remainder, which might be in a different block of this
 > +	     trace frame.  */
 > +	  if (maddr <= offset && offset < (maddr + mlen))
 > +	    {
 > +	      const struct bt_definition *array
 > +		= bt_ctf_get_field (event, scope, "contents");
 > +	      const struct bt_declaration *decl
 > +		= bt_ctf_get_decl_from_def (array);
 > +	      gdb_byte *contents;
 > +	      int k;
 > +
 > +	      contents = xmalloc (mlen);
 > +
 > +	      for (k = 0; k < mlen; k++)
 > +		{
 > +		  const struct bt_definition *element
 > +		    = bt_ctf_get_index (event, array, k);
 > +
 > +		  contents[k] = (gdb_byte) bt_ctf_get_uint64 (element);
 > +		}
 > +
 > +	      amt = (maddr + mlen) - offset;
 > +	      if (amt > len)
 > +		amt = len;
 > +
 > +	      memcpy (readbuf, &contents[offset - maddr], amt);
 > +
 > +	      xfree (contents);
 > +
 > +	      /* Restore the position.  */
 > +	      bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
 > +
 > +	      return amt;
 > +	    }
 > +
 > +	  if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0)
 > +	    break;
 > +	}
 > +
 > +      /* Restore the position.  */
 > +      bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
 > +    }
 > +
 > +  /* It's unduly pedantic to refuse to look at the executable for
 > +     read-only pieces; so do the equivalent of readonly regions aka
 > +     QTro packet.  */
 > +  if (exec_bfd != NULL)
 > +    {
 > +      asection *s;
 > +      bfd_size_type size;
 > +      bfd_vma vma;
 > +
 > +      for (s = exec_bfd->sections; s; s = s->next)
 > +	{
 > +	  if ((s->flags & SEC_LOAD) == 0
 > +	      || (s->flags & SEC_READONLY) == 0)
 > +	    continue;
 > +
 > +	  vma = s->vma;
 > +	  size = bfd_get_section_size (s);
 > +	  if (vma <= offset && offset < (vma + size))
 > +	    {
 > +	      ULONGEST amt;
 > +
 > +	      amt = (vma + size) - offset;
 > +	      if (amt > len)
 > +		amt = len;
 > +
 > +	      amt = bfd_get_section_contents (exec_bfd, s,
 > +					      readbuf, offset - vma, amt);
 > +	      return amt;
 > +	    }
 > +	}
 > +    }
 > +
 > +  /* Indicate failure to find the requested memory block.  */
 > +  return -1;
 > +}
 > +
 > +/* This is the implementation of target_ops method
 > +   to_get_trace_state_variable_value.
 > +   Iterate over events whose name is "tsv" in current frame.  When the
 > +   trace variable is found, set the value of it to *VAL and return
 > +   true, otherwise return false.  */
 > +
 > +static int
 > +ctf_get_trace_state_variable_value (int tsvnum, LONGEST *val)
 > +{
 > +  struct bt_iter_pos *pos;
 > +  int found = 0;
 > +
 > +  gdb_assert (ctf_iter != NULL);
 > +  /* Save the current position.  */
 > +  pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
 > +  gdb_assert (pos->type == BT_SEEK_RESTORE);
 > +
 > +  /* Iterate through the traceframe's blocks, looking for 'V'
 > +     block.  */
 > +  while (1)
 > +    {
 > +      struct bt_ctf_event *event
 > +	= bt_ctf_iter_read_event (ctf_iter);
 > +      const char *name = bt_ctf_event_name (event);
 > +
 > +      if (name == NULL || strcmp (name, "frame") == 0)
 > +	break;
 > +      else if (strcmp (name, "tsv") == 0)
 > +	{
 > +	  const struct bt_definition *scope;
 > +	  const struct bt_definition *def;
 > +
 > +	  scope = bt_ctf_get_top_level_scope (event,
 > +					      BT_EVENT_FIELDS);
 > +
 > +	  def = bt_ctf_get_field (event, scope, "num");
 > +	  if (tsvnum == (int32_t) bt_ctf_get_uint64 (def))
 > +	    {
 > +	      def = bt_ctf_get_field (event, scope, "val");
 > +	      *val = bt_ctf_get_uint64 (def);
 > +
 > +	      found = 1;
 > +	    }
 > +	}
 > +
 > +      if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0)
 > +	break;
 > +    }
 > +
 > +  /* Restore the position.  */
 > +  bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
 > +
 > +  return found;
 > +}
 > +
 > +/* Return the tracepoint number in "frame" event.  */
 > +
 > +static int
 > +ctf_get_tpnum_from_frame_event (struct bt_ctf_event *event)
 > +{
 > +  /* The packet context of events has a field "tpnum".  */
 > +  const struct bt_definition *scope
 > +    = bt_ctf_get_top_level_scope (event, BT_STREAM_PACKET_CONTEXT);
 > +  uint64_t tpnum
 > +    = bt_ctf_get_uint64 (bt_ctf_get_field (event, scope, "tpnum"));
 > +
 > +  return (int) tpnum;
 > +}
 > +
 > +/* Return the address at which the current frame was collected.  */
 > +
 > +static CORE_ADDR
 > +ctf_get_traceframe_address (void)
 > +{
 > +  struct bt_ctf_event *event = NULL;
 > +  struct bt_iter_pos *pos;
 > +  CORE_ADDR addr = 0;
 > +
 > +  gdb_assert (ctf_iter != NULL);
 > +  pos  = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
 > +  gdb_assert (pos->type == BT_SEEK_RESTORE);
 > +
 > +  while (1)
 > +    {
 > +      const char *name;
 > +      struct bt_ctf_event *event1;
 > +
 > +      event1 = bt_ctf_iter_read_event (ctf_iter);
 > +
 > +      name = bt_ctf_event_name (event1);
 > +
 > +      if (name == NULL)
 > +	break;
 > +      else if (strcmp (name, "frame") == 0)
 > +	{
 > +	  event = event1;
 > +	  break;
 > +	}
 > +
 > +      if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0)
 > +	break;
 > +    }
 > +
 > +  if (event != NULL)
 > +    {
 > +      int tpnum = ctf_get_tpnum_from_frame_event (event);
 > +      struct tracepoint *tp
 > +	= get_tracepoint_by_number_on_target (tpnum);
 > +
 > +      if (tp && tp->base.loc)
 > +	addr = tp->base.loc->address;
 > +    }
 > +
 > +  /* Restore the position.  */
 > +  bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
 > +
 > +  return addr;
 > +}
 > +
 > +/* This is the implementation of target_ops method to_trace_find.
 > +   Iterate the events whose name is "frame", extract the tracepoint
 > +   number in it.  Return traceframe number when matched.  */
 > +
 > +static int
 > +ctf_trace_find (enum trace_find_type type, int num,
 > +		CORE_ADDR addr1, CORE_ADDR addr2, int *tpp)
 > +{
 > +  int ret = -1;
 > +  int tfnum = 0;
 > +  int found = 0;
 > +  struct bt_iter_pos pos;
 > +
 > +  if (num == -1)
 > +    {
 > +      if (tpp != NULL)
 > +	*tpp = -1;
 > +      return -1;
 > +    }
 > +
 > +  gdb_assert (ctf_iter != NULL);
 > +  /* Set iterator back to the beginning.  */
 > +  pos.type = BT_SEEK_BEGIN;
 > +  bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), &pos);
 > +
 > +  while (1)
 > +    {
 > +      int id;
 > +      struct bt_ctf_event *event;
 > +      const char *name;
 > +
 > +      event = bt_ctf_iter_read_event (ctf_iter);
 > +
 > +      name = bt_ctf_event_name (event);
 > +
 > +      if (event == NULL || name == NULL)
 > +	break;
 > +
 > +      if (strcmp (name, "frame") == 0)
 > +	{
 > +	  CORE_ADDR tfaddr;
 > +
 > +	  if (type == tfind_number)
 > +	    {
 > +	      /* Looking for a specific trace frame.  */
 > +	      if (tfnum == num)
 > +		found = 1;
 > +	    }
 > +	  else
 > +	    {
 > +	      /* Start from the _next_ trace frame.  */
 > +	      if (tfnum > get_traceframe_number ())
 > +		{
 > +		  switch (type)
 > +		    {
 > +		    case tfind_tp:
 > +		      {
 > +			struct tracepoint *tp = get_tracepoint (num);
 > +
 > +			if (tp != NULL
 > +			    && (tp->number_on_target
 > +				== ctf_get_tpnum_from_frame_event (event)))
 > +			  found = 1;
 > +			break;
 > +		      }
 > +		    case tfind_pc:
 > +		      tfaddr = ctf_get_traceframe_address ();
 > +		      if (tfaddr == addr1)
 > +			found = 1;
 > +		      break;
 > +		    case tfind_range:
 > +		      tfaddr = ctf_get_traceframe_address ();
 > +		      if (addr1 <= tfaddr && tfaddr <= addr2)
 > +			found = 1;
 > +		      break;
 > +		    case tfind_outside:
 > +		      tfaddr = ctf_get_traceframe_address ();
 > +		      if (!(addr1 <= tfaddr && tfaddr <= addr2))
 > +			found = 1;
 > +		      break;
 > +		    default:
 > +		      internal_error (__FILE__, __LINE__, _("unknown tfind type"));
 > +		    }
 > +		}
 > +	    }
 > +	  if (found)
 > +	    {
 > +	      if (tpp != NULL)
 > +		*tpp = ctf_get_tpnum_from_frame_event (event);
 > +
 > +	      /* Skip the event "frame".  */
 > +	      bt_iter_next (bt_ctf_get_iter (ctf_iter));
 > +
 > +	      return tfnum;
 > +	    }
 > +	  tfnum++;
 > +	}
 > +
 > +      if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0)
 > +	break;
 > +    }
 > +
 > +  return -1;
 > +}
 > +
 > +/* This is the implementation of target_ops method to_has_stack.
 > +   The target has a stack when GDB has already selected one trace
 > +   frame.  */
 > +
 > +static int
 > +ctf_has_stack (struct target_ops *ops)
 > +{
 > +  return get_traceframe_number () != -1;
 > +}
 > +
 > +/* This is the implementation of target_ops method to_has_registers.
 > +   The target has registers when GDB has already selected one trace
 > +   frame.  */
 > +
 > +static int
 > +ctf_has_registers (struct target_ops *ops)
 > +{
 > +  return get_traceframe_number () != -1;
 > +}
 > +
 > +/* This is the implementation of target_ops method to_traceframe_info.
 > +   Iterate the events whose name is "memory", in current
 > +   frame, extract memory range information, and return them in
 > +   traceframe_info.  */
 > +
 > +static struct traceframe_info *
 > +ctf_traceframe_info (void)
 > +{
 > +  struct traceframe_info *info = XCNEW (struct traceframe_info);
 > +  const char *name;
 > +  struct bt_iter_pos *pos;
 > +
 > +  gdb_assert (ctf_iter != NULL);
 > +  /* Save the current position.  */
 > +  pos = bt_iter_get_pos (bt_ctf_get_iter (ctf_iter));
 > +  gdb_assert (pos->type == BT_SEEK_RESTORE);
 > +
 > +  do
 > +    {
 > +      struct bt_ctf_event *event
 > +	= bt_ctf_iter_read_event (ctf_iter);
 > +
 > +      name = bt_ctf_event_name (event);
 > +
 > +      if (name == NULL || strcmp (name, "register") == 0
 > +	  || strcmp (name, "frame") == 0)
 > +	;
 > +      else if (strcmp (name, "memory") == 0)
 > +	{
 > +	  const struct bt_definition *scope
 > +	    = bt_ctf_get_top_level_scope (event,
 > +					  BT_EVENT_FIELDS);
 > +	  const struct bt_definition *def;
 > +	  struct mem_range *r;
 > +
 > +	  r = VEC_safe_push (mem_range_s, info->memory, NULL);
 > +	  def = bt_ctf_get_field (event, scope, "address");
 > +	  r->start = bt_ctf_get_uint64 (def);
 > +
 > +	  def = bt_ctf_get_field (event, scope, "length");
 > +	  r->length = (uint16_t) bt_ctf_get_uint64 (def);
 > +	}
 > +      else
 > +	warning (_("Unhandled trace block type (%s) "
 > +		   "while building trace frame info."),
 > +		 name);

These days we prefer multi-line single statements for if/else/etc.
to be wrapped in braces.  i.e.,

	{
	  warning (_("Unhandled trace block type (%s) "
		   "while building trace frame info."),
		   name);
	}

 > +
 > +      if (bt_iter_next (bt_ctf_get_iter (ctf_iter)) < 0)
 > +	break;
 > +    }
 > +  while (name != NULL && strcmp (name, "frame") != 0);
 > +
 > +  /* Restore the position.  */
 > +  bt_iter_set_pos (bt_ctf_get_iter (ctf_iter), pos);
 > +
 > +  return info;
 > +}
 > +
 > +static void
 > +init_ctf_ops (void)
 > +{
 > +  memset (&ctf_ops, 0, sizeof (ctf_ops));
 > +
 > +  ctf_ops.to_shortname = "ctf";
 > +  ctf_ops.to_longname = "CTF file";
 > +  ctf_ops.to_doc = "Use a CTF directory as a target.\n\
 > +Specify the filename of the CTF directory.";
 > +  ctf_ops.to_open = ctf_open;
 > +  ctf_ops.to_close = ctf_close;
 > +  ctf_ops.to_fetch_registers = ctf_fetch_registers;
 > +  ctf_ops.to_xfer_partial = ctf_xfer_partial;
 > +  ctf_ops.to_files_info = ctf_files_info;
 > +  ctf_ops.to_trace_find = ctf_trace_find;
 > +  ctf_ops.to_get_trace_state_variable_value
 > +    = ctf_get_trace_state_variable_value;
 > +  ctf_ops.to_stratum = process_stratum;
 > +  ctf_ops.to_has_stack = ctf_has_stack;
 > +  ctf_ops.to_has_registers = ctf_has_registers;
 > +  ctf_ops.to_traceframe_info = ctf_traceframe_info;
 > +  ctf_ops.to_magic = OPS_MAGIC;
 > +}
 > +
 > +#endif
 > +
 > +/* -Wmissing-prototypes */
 > +extern initialize_file_ftype _initialize_ctf;
 > +
 > +/* module initialization */

blank line

 > +void
 > +_initialize_ctf (void)
 > +{
 > +#if HAVE_LIBBABELTRACE
 > +  init_ctf_ops ();
 > +
 > +  add_target (&ctf_ops);
 > +#endif
 > +}
 > diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
 > index 3d8b131..962e632 100644
 > --- a/gdb/tracepoint.c
 > +++ b/gdb/tracepoint.c
 > @@ -127,14 +127,6 @@ extern void (*deprecated_readline_end_hook) (void);
 >  typedef struct trace_state_variable tsv_s;
 >  DEF_VEC_O(tsv_s);
 >  
 > -/* An object describing the contents of a traceframe.  */
 > -
 > -struct traceframe_info
 > -{
 > -  /* Collected memory.  */
 > -  VEC(mem_range_s) *memory;
 > -};
 > -
 >  static VEC(tsv_s) *tvariables;
 >  
 >  /* The next integer to assign to a variable.  */
 > @@ -3303,8 +3295,6 @@ static const struct trace_file_write_ops tfile_write_ops =
 >  #define TRACE_WRITE_V_BLOCK(writer, num, val)	\
 >    writer->ops->frame_ops->write_v_block ((writer), (num), (val))
 >  
 > -extern int trace_regblock_size;
 > -
 >  /* Save tracepoint data to file named FILENAME through WRITER.  WRITER
 >     determines the trace file format.  If TARGET_DOES_SAVE is non-zero,
 >     the save is performed on the target, otherwise GDB obtains all trace
 > @@ -3741,6 +3731,12 @@ get_traceframe_number (void)
 >    return traceframe_number;
 >  }
 >  
 > +int
 > +get_tracepoint_number (void)
 > +{
 > +  return tracepoint_number;
 > +}
 > +
 >  /* Make the traceframe NUM be the current trace frame.  Does nothing
 >     if NUM is already current.  */
 >  
 > diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
 > index 8df906f..378824e 100644
 > --- a/gdb/tracepoint.h
 > +++ b/gdb/tracepoint.h
 > @@ -24,6 +24,14 @@
 >  #include "memrange.h"
 >  #include "gdb_vecs.h"
 >  
 > +/* An object describing the contents of a traceframe.  */
 > +
 > +struct traceframe_info
 > +{
 > +  /* Collected memory.  */
 > +  VEC(mem_range_s) *memory;
 > +};
 > +
 >  /* A trace state variable is a value managed by a target being
 >     traced.  A trace state variable (or tsv for short) can be accessed
 >     and assigned to by tracepoint actions and conditionals, but is not
 > @@ -142,6 +150,8 @@ struct trace_status *current_trace_status (void);
 >  
 >  extern char *default_collect;
 >  
 > +extern int trace_regblock_size;
 > +
 >  /* Struct to collect random info about tracepoints on the target.  */
 >  
 >  struct uploaded_tp
 > @@ -324,6 +334,9 @@ extern void (*deprecated_trace_start_stop_hook) (int start, int from_tty);
 >  /* Returns the current traceframe number.  */
 >  extern int get_traceframe_number (void);
 >  
 > +/* Returns the tracepoint number for current traceframe.  */
 > +extern int get_tracepoint_number (void);
 > +
 >  /* Make the traceframe NUM be the current GDB trace frame number, and
 >     do nothing more.  In particular, this does not flush the
 >     register/frame caches or notify the target about the trace frame
 > -- 
 > 1.7.7.6