Re: [patch v4 06/13] linux, i386, amd64: enable btrace for 32bit and 64bit linux native

[Pedro Alves <palves@redhat.com>]

On 11/27/2012 10:49 AM, markus.t.metzger@intel.com wrote:
> From: Markus Metzger <markus.t.metzger@intel.com>
> 
> Install the btrace target ops for i386-linux-nat and amd64-linux-nat.
> 
> 2012-11-27 Markus Metzger <markus.t.metzger@intel.com>
> 
> 	* amd64-linux-nat.c: Include btrace.h and linux-btrace.h.
> 	(_initialize_amd64_linux_nat): Initialize btrace ops.
> 	* i386-linux.nat.c: Include btrace.h and linux-btrace.h.
> 	(_initialize_i386_linux_nat): Initialize btrace ops.
> 	* config/i386/linux.mh: Add linux-btrace.o.
> 	* config/i386/linux64.mh: Add linux-btrace.o.
> 
> 
> ---
>  gdb/amd64-linux-nat.c      |   51 ++++++++++++++++++++++++++++++++++++++++++++
>  gdb/config/i386/linux.mh   |    3 +-
>  gdb/config/i386/linux64.mh |    2 +-
>  gdb/i386-linux-nat.c       |   51 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 105 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
> index f4a12db..0e38a57 100644
> --- a/gdb/amd64-linux-nat.c
> +++ b/gdb/amd64-linux-nat.c
> @@ -25,6 +25,8 @@
>  #include "regset.h"
>  #include "linux-nat.h"
>  #include "amd64-linux-tdep.h"
> +#include "linux-btrace.h"
> +#include "btrace.h"
>  
>  #include "gdb_assert.h"
>  #include "gdb_string.h"
> @@ -1079,6 +1081,48 @@ amd64_linux_read_description (struct target_ops *ops)
>      }
>  }
>  
> +/* Enable branch tracing for @ptid.  */
> +static struct btrace_target_info *
> +amd64_linux_enable_btrace (ptid_t ptid)
> +{
> +  struct btrace_target_info *tinfo;
> +
> +  errno = 0;
> +  tinfo = linux_enable_btrace (ptid);
> +
> +  if (!tinfo)
> +    error (_("Could not enable branch tracing for %s: %s."),
> +	   target_pid_to_str (ptid), safe_strerror (errno));
> +
> +  return tinfo;
> +}
> +
> +/* Disable branch tracing for @ptid.  */
> +static void
> +amd64_linux_disable_btrace (struct btrace_target_info *tinfo)
> +{
> +  int errcode = linux_disable_btrace (tinfo);
> +
> +  if (errcode)
> +    error (_("Could not disable branch tracing: %s."), safe_strerror (errcode));
> +}
> +
> +/* Read branch trace data for @tinfo.  */
> +static VEC (btrace_block_s) *
> +amd64_linux_read_btrace (struct btrace_target_info *tinfo)
> +{
> +  /* Fill in the number of bits in a pointer for this architecture.  */
> +  if (!tinfo->ptr_bits)
> +    {
> +      struct gdbarch *gdbarch = target_gdbarch ();
> +
> +      if (gdbarch)

You shouldn't ever see a NULL here.

> +	tinfo->ptr_bits = gdbarch_ptr_bit (gdbarch);

It seems like this is something that should always be able
to compute on demand, without needing to store it once
per thread.  This seems to have the same issue I pointed out
in a previous patch, with assuming TINFO is the same thread
as the current thread/inferior, and therefore that
target_gdbarch is appropriate for TINFO.

> +    }
> +
> +  return linux_read_btrace (tinfo);
> +}
> +
>  /* Provide a prototype to silence -Wmissing-prototypes.  */
>  void _initialize_amd64_linux_nat (void);
>  
> @@ -1117,6 +1161,13 @@ _initialize_amd64_linux_nat (void)
>  
>    t->to_read_description = amd64_linux_read_description;
>  
> +  /* Add btrace methods.  */
> +  t->to_supports_btrace = linux_supports_btrace;
> +  t->to_enable_btrace = amd64_linux_enable_btrace;
> +  t->to_disable_btrace = amd64_linux_disable_btrace;
> +  t->to_btrace_has_changed = linux_btrace_has_changed;
> +  t->to_read_btrace = amd64_linux_read_btrace;
> +
>    /* Register the target.  */
>    linux_nat_add_target (t);
>    linux_nat_set_new_thread (t, amd64_linux_new_thread);
> diff --git a/gdb/config/i386/linux.mh b/gdb/config/i386/linux.mh
> index 8316d87..7c64e83 100644
> --- a/gdb/config/i386/linux.mh
> +++ b/gdb/config/i386/linux.mh
> @@ -4,7 +4,8 @@ NAT_FILE= config/nm-linux.h
>  NATDEPFILES= inf-ptrace.o fork-child.o \
>  	i386-nat.o i386-linux-nat.o \
>  	proc-service.o linux-thread-db.o \
> -	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o
> +	linux-nat.o linux-osdata.o linux-fork.o linux-procfs.o linux-ptrace.o \
> +	linux-btrace.o
>  NAT_CDEPS = $(srcdir)/proc-service.list
>  
>  # The dynamically loaded libthread_db needs access to symbols in the
> diff --git a/gdb/config/i386/linux64.mh b/gdb/config/i386/linux64.mh
> index d2b95fd..8d782c1 100644
> --- a/gdb/config/i386/linux64.mh
> +++ b/gdb/config/i386/linux64.mh
> @@ -3,7 +3,7 @@ NATDEPFILES= inf-ptrace.o fork-child.o \
>  	i386-nat.o amd64-nat.o amd64-linux-nat.o \
>  	linux-nat.o linux-osdata.o \
>  	proc-service.o linux-thread-db.o linux-fork.o \
> -	linux-procfs.o linux-ptrace.o
> +	linux-procfs.o linux-ptrace.o linux-btrace.o
>  NAT_FILE= config/nm-linux.h
>  NAT_CDEPS = $(srcdir)/proc-service.list
>  
> diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
> index 5a65ef6..c12d94a 100644
> --- a/gdb/i386-linux-nat.c
> +++ b/gdb/i386-linux-nat.c
> @@ -25,6 +25,8 @@
>  #include "regset.h"
>  #include "target.h"
>  #include "linux-nat.h"
> +#include "linux-btrace.h"
> +#include "btrace.h"
>  
>  #include "gdb_assert.h"
>  #include "gdb_string.h"
> @@ -1008,6 +1010,48 @@ i386_linux_read_description (struct target_ops *ops)
>      return tdesc_i386_linux;
>  }
>  
> +/* Enable branch tracing for @ptid.  */
> +static struct btrace_target_info *
> +i386_linux_enable_btrace (ptid_t ptid)
> +{
> +  struct btrace_target_info *tinfo;
> +
> +  errno = 0;
> +  tinfo = linux_enable_btrace (ptid);
> +
> +  if (!tinfo)
> +    error (_("Could not enable branch tracing for %s: %s."),
> +	   target_pid_to_str (ptid), safe_strerror (errno));
> +
> +  return tinfo;
> +}
> +
> +/* Disable branch tracing for @ptid.  */
> +static void
> +i386_linux_disable_btrace (struct btrace_target_info *tinfo)
> +{
> +  int errcode = linux_disable_btrace (tinfo);
> +
> +  if (errcode)
> +    error (_("Could not disable branch tracing: %s."), safe_strerror (errcode));
> +}
> +
> +/* Read branch trace data for @tinfo.  */
> +static VEC (btrace_block_s) *
> +i386_linux_read_btrace (struct btrace_target_info *tinfo)
> +{
> +  /* Fill in the number of bits in a pointer for this architecture.  */
> +  if (!tinfo->ptr_bits)
> +    {
> +      struct gdbarch *gdbarch = target_gdbarch ();
> +
> +      if (gdbarch)
> +	tinfo->ptr_bits = gdbarch_ptr_bit (gdbarch);
> +    }
> +
> +  return linux_read_btrace (tinfo);
> +}
> +
>  /* -Wmissing-prototypes */
>  extern initialize_file_ftype _initialize_i386_linux_nat;
>  
> @@ -1041,6 +1085,13 @@ _initialize_i386_linux_nat (void)
>  
>    t->to_read_description = i386_linux_read_description;
>  
> +  /* Add btrace methods.  */
> +  t->to_supports_btrace = linux_supports_btrace;
> +  t->to_enable_btrace = i386_linux_enable_btrace;
> +  t->to_disable_btrace = i386_linux_disable_btrace;
> +  t->to_btrace_has_changed = linux_btrace_has_changed;
> +  t->to_read_btrace = i386_linux_read_btrace;
> +
>    /* Register the target.  */
>    linux_nat_add_target (t);
>    linux_nat_set_new_thread (t, i386_linux_new_thread);
> -- 1.7.1


-- 
Pedro Alves