[3/3] Convert DTrace probe interface to C++ (and perform some cleanups)

Message ID 20171113175901.25367-4-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior Nov. 13, 2017, 5:59 p.m. UTC
  This patch converts the DTrace probe
interface (gdb/dtrace-probe.[ch]) to C++, and also performs some
cleanups that were on my TODO list for a while.

The main changes were the conversion of 'struct dtrace_probe' to 'class
dtrace_probe', and a new 'class dtrace_static_probe_ops' to replace the
use of 'dtrace_probe_ops'.  Both classes implement the virtual methods
exported by their parents, 'class probe' and 'class static_probe_ops',
respectively.  I believe it's now a bit simpler to understand the
logic behind the dtrace-probe interface.

There are several helper functions used to parse parts of a dtrace
probe, and since they are generic and don't need to know about the
probe they're working on, I decided to leave them as simple static
functions (instead of e.g. converting them to class methods).

I've also converted a few uses of "VEC" to "std::vector", which makes
the code simpler and easier to maintain.  And, as usual, some cleanups
here and there.

Even though I'm sending a series of patches, they need to be tested
and committed as a single unit, because of inter-dependencies.  But it
should be easier to review in separate logical units.

I've regtested this patch on BuildBot, no regressions found.

gdb/ChangeLog:
2017-11-13  Sergio Durigan Junior  <sergiodj@redhat.com>

	* dtrace-probe.c (struct probe_ops dtrace_probe_ops): Delete.
	(dtrace_probe_arg_s): Delete type and VEC.
	(dtrace_probe_enabler_s): Likewise.
	(struct dtrace_probe): Replace by...
	(class dtrace_static_probe_ops): ...this and...
	(class dtrace_probe): ...this.
	(dtrace_probe_is_linespec): Rename to...
	(dtrace_static_probe_ops::is_linespec): ...this.  Adjust code
	to reflect change.
	(dtrace_process_dof_probe): Use 'std::vector' instead of VEC.
	Adjust code.  Create new instance of 'dtrace_probe'.
	(dtrace_build_arg_exprs): Rename to...
	(dtrace_probe::build_arg_exprs): ...this.  Adjust code to
	reflect change.
	(dtrace_get_probes): Rename to...
	(dtrace_static_probe_ops::get_probes): ...this.  Adjust code
	to reflect change.
	(dtrace_get_arg): Rename to...
	(dtrace_probe::get_arg_by_number): ...this.  Adjust code to
	reflect change.
	(dtrace_probe_is_enabled): Rename to...
	(dtrace_probe::is_enabled): ...this.  Adjust code to reflect
	change.
	(dtrace_get_probe_address): Rename to...
	(dtrace_probe::get_relocated_address): ...this.  Adjust code
	to reflect change.
	(dtrace_get_probe_argument_count): Rename to...
	(dtrace_probe::get_argument_count): ...this.  Adjust code to
	reflect change.
	(dtrace_can_evaluate_probe_arguments): Rename to...
	(dtrace_probe::can_evaluate_arguments): ...this.  Adjust code
	to reflect change.
	(dtrace_evaluate_probe_argument): Rename to...
	(dtrace_probe::evaluate_argument): ...this.  Adjust code to
	reflect change.
	(dtrace_compile_to_ax): Rename to...
	(dtrace_probe::compile_to_ax): ...this.  Adjust code to
	reflect change.
	(dtrace_probe_destroy): Delete.
	(dtrace_type_name): Rename to...
	(dtrace_static_probe_ops::type_name): ...this.  Adjust code to
	reflect change.
	(dtrace_probe::get_static_ops): New method.
	(dtrace_gen_info_probes_table_header): Rename to...
	(dtrace_static_probe_ops::gen_info_probes_table_header):
	...this.  Adjust code to reflect change.
	(dtrace_gen_info_probes_table_values): Rename to...
	(dtrace_probe::gen_info_probes_table_values): ...this.  Adjust
	code to reflect change.
	(dtrace_enable_probe): Rename to...
	(dtrace_probe::enable_probe): ...this.  Adjust code to reflect
	change.
	(dtrace_disable_probe): Rename to...
	(dtrace_probe::disable_probe): ...this.  Adjust code to reflect
	change.
	(struct probe_ops dtrace_probe_ops): Delete.
	(dtrace_static_probe_ops::emit_info_probes_extra_fields): New
	method.
	(info_probes_dtrace_command): Call 'info_probes_for_spops'
	instead of 'info_probes_for_ops'.
	(_initialize_dtrace_probe): Use 'all_static_probe_ops' instead
	of 'all_probe_ops'.
---
 gdb/dtrace-probe.c | 528 +++++++++++++++++++++++++++--------------------------
 1 file changed, 268 insertions(+), 260 deletions(-)
  

Comments

Simon Marchi Nov. 15, 2017, 4:40 a.m. UTC | #1
Hi Sergio,

On 2017-11-13 12:59 PM, Sergio Durigan Junior wrote:
> This patch converts the DTrace probe
> interface (gdb/dtrace-probe.[ch]) to C++, and also performs some
> cleanups that were on my TODO list for a while.
> 
> The main changes were the conversion of 'struct dtrace_probe' to 'class
> dtrace_probe', and a new 'class dtrace_static_probe_ops' to replace the
> use of 'dtrace_probe_ops'.  Both classes implement the virtual methods
> exported by their parents, 'class probe' and 'class static_probe_ops',
> respectively.  I believe it's now a bit simpler to understand the
> logic behind the dtrace-probe interface.
> 
> There are several helper functions used to parse parts of a dtrace
> probe, and since they are generic and don't need to know about the
> probe they're working on, I decided to leave them as simple static
> functions (instead of e.g. converting them to class methods).
> 
> I've also converted a few uses of "VEC" to "std::vector", which makes
> the code simpler and easier to maintain.  And, as usual, some cleanups
> here and there.
> 
> Even though I'm sending a series of patches, they need to be tested
> and committed as a single unit, because of inter-dependencies.  But it
> should be easier to review in separate logical units.
> 
> I've regtested this patch on BuildBot, no regressions found.
> 
> gdb/ChangeLog:
> 2017-11-13  Sergio Durigan Junior  <sergiodj@redhat.com>
> 
> 	* dtrace-probe.c (struct probe_ops dtrace_probe_ops): Delete.
> 	(dtrace_probe_arg_s): Delete type and VEC.
> 	(dtrace_probe_enabler_s): Likewise.
> 	(struct dtrace_probe): Replace by...
> 	(class dtrace_static_probe_ops): ...this and...
> 	(class dtrace_probe): ...this.
> 	(dtrace_probe_is_linespec): Rename to...
> 	(dtrace_static_probe_ops::is_linespec): ...this.  Adjust code
> 	to reflect change.
> 	(dtrace_process_dof_probe): Use 'std::vector' instead of VEC.
> 	Adjust code.  Create new instance of 'dtrace_probe'.
> 	(dtrace_build_arg_exprs): Rename to...
> 	(dtrace_probe::build_arg_exprs): ...this.  Adjust code to
> 	reflect change.
> 	(dtrace_get_probes): Rename to...
> 	(dtrace_static_probe_ops::get_probes): ...this.  Adjust code
> 	to reflect change.
> 	(dtrace_get_arg): Rename to...
> 	(dtrace_probe::get_arg_by_number): ...this.  Adjust code to
> 	reflect change.
> 	(dtrace_probe_is_enabled): Rename to...
> 	(dtrace_probe::is_enabled): ...this.  Adjust code to reflect
> 	change.
> 	(dtrace_get_probe_address): Rename to...
> 	(dtrace_probe::get_relocated_address): ...this.  Adjust code
> 	to reflect change.
> 	(dtrace_get_probe_argument_count): Rename to...
> 	(dtrace_probe::get_argument_count): ...this.  Adjust code to
> 	reflect change.
> 	(dtrace_can_evaluate_probe_arguments): Rename to...
> 	(dtrace_probe::can_evaluate_arguments): ...this.  Adjust code
> 	to reflect change.
> 	(dtrace_evaluate_probe_argument): Rename to...
> 	(dtrace_probe::evaluate_argument): ...this.  Adjust code to
> 	reflect change.
> 	(dtrace_compile_to_ax): Rename to...
> 	(dtrace_probe::compile_to_ax): ...this.  Adjust code to
> 	reflect change.
> 	(dtrace_probe_destroy): Delete.
> 	(dtrace_type_name): Rename to...
> 	(dtrace_static_probe_ops::type_name): ...this.  Adjust code to
> 	reflect change.
> 	(dtrace_probe::get_static_ops): New method.
> 	(dtrace_gen_info_probes_table_header): Rename to...
> 	(dtrace_static_probe_ops::gen_info_probes_table_header):
> 	...this.  Adjust code to reflect change.
> 	(dtrace_gen_info_probes_table_values): Rename to...
> 	(dtrace_probe::gen_info_probes_table_values): ...this.  Adjust
> 	code to reflect change.
> 	(dtrace_enable_probe): Rename to...
> 	(dtrace_probe::enable_probe): ...this.  Adjust code to reflect
> 	change.
> 	(dtrace_disable_probe): Rename to...
> 	(dtrace_probe::disable_probe): ...this.  Adjust code to reflect
> 	change.
> 	(struct probe_ops dtrace_probe_ops): Delete.
> 	(dtrace_static_probe_ops::emit_info_probes_extra_fields): New
> 	method.
> 	(info_probes_dtrace_command): Call 'info_probes_for_spops'
> 	instead of 'info_probes_for_ops'.
> 	(_initialize_dtrace_probe): Use 'all_static_probe_ops' instead
> 	of 'all_probe_ops'.
> ---
>  gdb/dtrace-probe.c | 528 +++++++++++++++++++++++++++--------------------------
>  1 file changed, 268 insertions(+), 260 deletions(-)
> 
> diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
> index 2bbe03e4b3..b53ff7ac6c 100644
> --- a/gdb/dtrace-probe.c
> +++ b/gdb/dtrace-probe.c
> @@ -41,10 +41,6 @@
>  # define SHT_SUNW_dof	0x6ffffff4
>  #endif
>  
> -/* Forward declaration.  */
> -
> -extern const struct probe_ops dtrace_probe_ops;
> -
>  /* The following structure represents a single argument for the
>     probe.  */
>  
> @@ -60,9 +56,6 @@ struct dtrace_probe_arg
>    struct expression *expr;
>  };
>  
> -typedef struct dtrace_probe_arg dtrace_probe_arg_s;
> -DEF_VEC_O (dtrace_probe_arg_s);
> -
>  /* The following structure represents an enabler for a probe.  */
>  
>  struct dtrace_probe_enabler
> @@ -73,39 +66,121 @@ struct dtrace_probe_enabler
>    CORE_ADDR address;
>  };
>  
> -typedef struct dtrace_probe_enabler dtrace_probe_enabler_s;
> -DEF_VEC_O (dtrace_probe_enabler_s);
> +/* Class that implements the static probe methods for "stap" probes.  */
> +
> +class dtrace_static_probe_ops : public static_probe_ops
> +{
> +public:
> +  /* See probe.h.  */
> +  bool is_linespec (const char **linespecp) const override;
> +
> +  /* See probe.h.  */
> +  void get_probes (std::vector<probe *> *probesp,
> +		   struct objfile *objfile) const override;
> +
> +  /* See probe.h.  */
> +  const char *type_name () const override;
> +
> +  /* See probe.h.  */
> +  bool emit_info_probes_extra_fields () const override;
> +
> +  /* See probe.h.  */
> +  void gen_info_probes_table_header
> +    (std::vector<struct info_probe_column> *heads) const override;
> +};
> +
> +/* DTrace static_probe_ops.  */
> +
> +const dtrace_static_probe_ops dtrace_static_probe_ops;
>  
>  /* The following structure represents a dtrace probe.  */
>  
> -struct dtrace_probe
> +class dtrace_probe : public probe
>  {
> -  /* Generic information about the probe.  This must be the first
> -     element of this struct, in order to maintain binary compatibility
> -     with the `struct probe' and be able to fully abstract it.  */
> -  struct probe p;
> +public:
> +  /* Constructor for dtrace_probe.  */
> +  dtrace_probe (std::string &&name_, std::string &&provider_, CORE_ADDR address_,
> +		struct gdbarch *arch_,
> +		std::vector<struct dtrace_probe_arg> &args_,
> +		std::vector<struct dtrace_probe_enabler> &enablers_)

I am wondering why you don't use && for the vectors, as you do with the strings.
If the constructor is "stealing" from the vectors passed as parameter, I think it
would make sense if the parameters were &&, to indicate to the caller that they'll
be moved from (and force it to use std::move).

> +    : probe (std::move (name_), std::move (provider_), address_, arch_),
> +      m_args (std::move (args_)),
> +      m_enablers (std::move (enablers_)),
> +      m_args_expr_built (false)
> +  {}
> +
> +  /* Destructor for dtrace_probe.  */
> +  ~dtrace_probe ()
> +  {
> +    for (struct dtrace_probe_arg arg : m_args)
> +      {
> +	xfree (arg.type_str);
> +	xfree (arg.expr);
> +      }

As for stap_probe, I think defining a destructor in dtrace_probe_arg would
be clearer.

> +  }
> +
> +  /* See probe.h.  */
> +  CORE_ADDR get_relocated_address (struct objfile *objfile) override;
> +
> +  /* See probe.h.  */
> +  unsigned get_argument_count (struct frame_info *frame) override;
> +
> +  /* See probe.h.  */
> +  int can_evaluate_arguments () const override;
> +
> +  /* See probe.h.  */
> +  struct value *evaluate_argument (unsigned n,
> +				   struct frame_info *frame) override;
> +
> +  /* See probe.h.  */
> +  void compile_to_ax (struct agent_expr *aexpr,
> +		      struct axs_value *axs_value,
> +		      unsigned n) override;
> +
> +  /* See probe.h.  */
> +  const static_probe_ops *get_static_ops () const override;
> +
> +  /* See probe.h.  */
> +  void gen_info_probes_table_values
> +    (std::vector<const char *> *values) const override;
> +
> +  /* See probe.h.  */
> +  bool can_enable () const override
> +  {
> +    return true;
> +  }
> +
> +  /* See probe.h.  */
> +  void enable () override;
>  
> +  /* See probe.h.  */
> +  void disable () override;
> +
> +  /* Return the Nth argument of the probe.  */
> +  struct dtrace_probe_arg *get_arg_by_number (unsigned n,
> +					      struct gdbarch *gdbarch);
> +
> +  /* Build the GDB internal expressiosn that, once evaluated, will
> +     calculate the values of the arguments of the probe.  */
> +  void build_arg_exprs (struct gdbarch *gdbarch);
> +
> +  /* Determine whether the probe is "enabled" or "disabled".  A
> +     disabled probe is a probe in which one or more enablers are
> +     disabled.  */
> +  bool is_enabled () const;
> +
> +private:
>    /* A probe can have zero or more arguments.  */
> -  int probe_argc;
> -  VEC (dtrace_probe_arg_s) *args;
> +  int m_argc;

It seems like this field is never set, but used at least once.  Should we use args->size() instead?

> +  std::vector<struct dtrace_probe_arg> m_args;
>  
>    /* A probe can have zero or more "enablers" associated with it.  */
> -  VEC (dtrace_probe_enabler_s) *enablers;
> +  std::vector<struct dtrace_probe_enabler> m_enablers;
>  
>    /* Whether the expressions for the arguments have been built.  */
> -  unsigned int args_expr_built : 1;
> +  bool m_args_expr_built;
>  };
>  
> -/* Implementation of the probe_is_linespec method.  */
> -
> -static int
> -dtrace_probe_is_linespec (const char **linespecp)
> -{
> -  static const char *const keywords[] = { "-pdtrace", "-probe-dtrace", NULL };
> -
> -  return probe_is_linespec_by_keyword (linespecp, keywords);
> -}
> -
>  /* DOF programs can contain an arbitrary number of sections of 26
>     different types.  In order to support DTrace USDT probes we only
>     need to handle a subset of these section types, fortunately.  These
> @@ -322,8 +397,6 @@ dtrace_process_dof_probe (struct objfile *objfile,
>  			  char *argtab, uint64_t strtab_size)
>  {
>    int i, j, num_probes, num_enablers;
> -  struct cleanup *cleanup;
> -  VEC (dtrace_probe_enabler_s) *enablers;
>    char *p;
>  
>    /* Each probe section can define zero or more probes of two
> @@ -368,9 +441,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
>  
>    /* Build the list of enablers for the probes defined in this Probe
>       DOF section.  */
> -  enablers = NULL;
> -  cleanup
> -    = make_cleanup (VEC_cleanup (dtrace_probe_enabler_s), &enablers);
> +  std::vector<struct dtrace_probe_enabler> enablers;
>    num_enablers = DOF_UINT (dof, probe->dofpr_nenoffs);
>    for (i = 0; i < num_enablers; i++)
>      {
> @@ -380,38 +451,32 @@ dtrace_process_dof_probe (struct objfile *objfile,
>  
>        enabler.address = DOF_UINT (dof, probe->dofpr_addr)
>  	+ DOF_UINT (dof, enabler_offset);
> -      VEC_safe_push (dtrace_probe_enabler_s, enablers, &enabler);
> +      enablers.push_back (enabler);
>      }
>  
>    for (i = 0; i < num_probes; i++)
>      {
>        uint32_t probe_offset
>  	= ((uint32_t *) offtab)[DOF_UINT (dof, probe->dofpr_offidx) + i];
> -      struct dtrace_probe *ret =
> -	XOBNEW (&objfile->per_bfd->storage_obstack, struct dtrace_probe);
> -
> -      ret->p.pops = &dtrace_probe_ops;
> -      ret->p.arch = gdbarch;
> -      ret->args_expr_built = 0;
>  
>        /* Set the provider and the name of the probe.  */
> -      ret->p.provider
> -	= xstrdup (strtab + DOF_UINT (dof, provider->dofpv_name));
> -      ret->p.name = xstrdup (strtab + DOF_UINT (dof, probe->dofpr_name));
> +      const char *probe_provider
> +	= strtab + DOF_UINT (dof, provider->dofpv_name);
> +      const char *name = strtab + DOF_UINT (dof, probe->dofpr_name);
>  
>        /* The probe address.  */
> -      ret->p.address
> +      CORE_ADDR address
>  	= DOF_UINT (dof, probe->dofpr_addr) + DOF_UINT (dof, probe_offset);
>  
>        /* Number of arguments in the probe.  */
> -      ret->probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
> +      int probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
>  
>        /* Store argument type descriptions.  A description of the type
>           of the argument is in the (J+1)th null-terminated string
>           starting at 'strtab' + 'probe->dofpr_nargv'.  */
> -      ret->args = NULL;
> +      std::vector<struct dtrace_probe_arg> args;
>        p = strtab + DOF_UINT (dof, probe->dofpr_nargv);
> -      for (j = 0; j < ret->probe_argc; j++)
> +      for (j = 0; j < probe_argc; j++)
>  	{
>  	  struct dtrace_probe_arg arg;
>  	  expression_up expr;
> @@ -442,17 +507,18 @@ dtrace_process_dof_probe (struct objfile *objfile,
>  	  if (expr != NULL && expr->elts[0].opcode == OP_TYPE)
>  	    arg.type = expr->elts[1].type;
>  
> -	  VEC_safe_push (dtrace_probe_arg_s, ret->args, &arg);
> +	  args.push_back (arg);
>  	}
>  
> -      /* Add the vector of enablers to this probe, if any.  */
> -      ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers);
> +      std::vector<struct dtrace_probe_enabler> enablers_copy = enablers;

Why is this copy necessary?  What happens with the original vector?

> +      dtrace_probe *ret = new dtrace_probe (std::string (name),
> +					    std::string (probe_provider),
> +					    address, gdbarch,
> +					    args, enablers_copy);
>  
>        /* Successfully created probe.  */
> -      probesp->push_back ((struct probe *) ret);
> +      probesp->push_back (ret);
>      }
> -
> -  do_cleanups (cleanup);
>  }
>  
>  /* Helper function to collect the probes described in the DOF program
> @@ -555,28 +621,23 @@ dtrace_process_dof (asection *sect, struct objfile *objfile,
>  	     sect->name);
>  }
>  
> -/* Helper function to build the GDB internal expressiosn that, once
> -   evaluated, will calculate the values of the arguments of a given
> -   PROBE.  */
> +/* Implementation of 'build_arg_exprs' method.  */
>  
> -static void
> -dtrace_build_arg_exprs (struct dtrace_probe *probe,
> -			struct gdbarch *gdbarch)
> +void
> +dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch)
>  {
> -  struct parser_state pstate;
> -  struct dtrace_probe_arg *arg;
> -  int i;
> -
> -  probe->args_expr_built = 1;
> +  m_args_expr_built = true;
>  
>    /* Iterate over the arguments in the probe and build the
>       corresponding GDB internal expression that will generate the
>       value of the argument when executed at the PC of the probe.  */
> -  for (i = 0; i < probe->probe_argc; i++)
> +  for (int i = 0; i < m_argc; i++)
>      {
> +      struct dtrace_probe_arg *arg;
>        struct cleanup *back_to;
> +      struct parser_state pstate;
>  
> -      arg = VEC_index (dtrace_probe_arg_s, probe->args, i);
> +      arg = &m_args[i];
>  
>        /* Initialize the expression buffer in the parser state.  The
>  	 language does not matter, since we are using our own
> @@ -606,138 +667,82 @@ dtrace_build_arg_exprs (struct dtrace_probe *probe,
>      }
>  }
>  
> -/* Helper function to return the Nth argument of a given PROBE.  */
> -
> -static struct dtrace_probe_arg *
> -dtrace_get_arg (struct dtrace_probe *probe, unsigned n,
> -		struct gdbarch *gdbarch)
> -{
> -  if (!probe->args_expr_built)
> -    dtrace_build_arg_exprs (probe, gdbarch);
> -
> -  return VEC_index (dtrace_probe_arg_s, probe->args, n);
> -}
> -
> -/* Implementation of the get_probes method.  */
> +/* Implementation of 'get_arg_by_number' method.  */
>  
> -static void
> -dtrace_get_probes (std::vector<probe *> *probesp, struct objfile *objfile)
> +struct dtrace_probe_arg *
> +dtrace_probe::get_arg_by_number (unsigned n, struct gdbarch *gdbarch)
>  {
> -  bfd *abfd = objfile->obfd;
> -  asection *sect = NULL;
> -
> -  /* Do nothing in case this is a .debug file, instead of the objfile
> -     itself.  */
> -  if (objfile->separate_debug_objfile_backlink != NULL)
> -    return;
> +  if (!m_args_expr_built)
> +    this->build_arg_exprs (gdbarch);
>  
> -  /* Iterate over the sections in OBJFILE looking for DTrace
> -     information.  */
> -  for (sect = abfd->sections; sect != NULL; sect = sect->next)
> -    {
> -      if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
> -	{
> -	  bfd_byte *dof;
> -
> -	  /* Read the contents of the DOF section and then process it to
> -	     extract the information of any probe defined into it.  */
> -	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
> -	    complaint (&symfile_complaints,
> -		       _("could not obtain the contents of"
> -			 "section '%s' in objfile `%s'."),
> -		       sect->name, abfd->filename);
> -      
> -	  dtrace_process_dof (sect, objfile, probesp,
> -			      (struct dtrace_dof_hdr *) dof);
> -	  xfree (dof);
> -	}
> -    }
> +  return &m_args[n];

It would be nice to add a gdb_assert (n < m_args.size ()) here.

Thanks,

Simon
  
Sergio Durigan Junior Nov. 16, 2017, 4:11 a.m. UTC | #2
On Tuesday, November 14 2017, Simon Marchi wrote:

> Hi Sergio,

Hey Simon,

Thanks for the review.

> On 2017-11-13 12:59 PM, Sergio Durigan Junior wrote:
>> This patch converts the DTrace probe
>> interface (gdb/dtrace-probe.[ch]) to C++, and also performs some
>> cleanups that were on my TODO list for a while.
>> 
>> The main changes were the conversion of 'struct dtrace_probe' to 'class
>> dtrace_probe', and a new 'class dtrace_static_probe_ops' to replace the
>> use of 'dtrace_probe_ops'.  Both classes implement the virtual methods
>> exported by their parents, 'class probe' and 'class static_probe_ops',
>> respectively.  I believe it's now a bit simpler to understand the
>> logic behind the dtrace-probe interface.
>> 
>> There are several helper functions used to parse parts of a dtrace
>> probe, and since they are generic and don't need to know about the
>> probe they're working on, I decided to leave them as simple static
>> functions (instead of e.g. converting them to class methods).
>> 
>> I've also converted a few uses of "VEC" to "std::vector", which makes
>> the code simpler and easier to maintain.  And, as usual, some cleanups
>> here and there.
>> 
>> Even though I'm sending a series of patches, they need to be tested
>> and committed as a single unit, because of inter-dependencies.  But it
>> should be easier to review in separate logical units.
>> 
>> I've regtested this patch on BuildBot, no regressions found.
>> 
>> gdb/ChangeLog:
>> 2017-11-13  Sergio Durigan Junior  <sergiodj@redhat.com>
>> 
>> 	* dtrace-probe.c (struct probe_ops dtrace_probe_ops): Delete.
>> 	(dtrace_probe_arg_s): Delete type and VEC.
>> 	(dtrace_probe_enabler_s): Likewise.
>> 	(struct dtrace_probe): Replace by...
>> 	(class dtrace_static_probe_ops): ...this and...
>> 	(class dtrace_probe): ...this.
>> 	(dtrace_probe_is_linespec): Rename to...
>> 	(dtrace_static_probe_ops::is_linespec): ...this.  Adjust code
>> 	to reflect change.
>> 	(dtrace_process_dof_probe): Use 'std::vector' instead of VEC.
>> 	Adjust code.  Create new instance of 'dtrace_probe'.
>> 	(dtrace_build_arg_exprs): Rename to...
>> 	(dtrace_probe::build_arg_exprs): ...this.  Adjust code to
>> 	reflect change.
>> 	(dtrace_get_probes): Rename to...
>> 	(dtrace_static_probe_ops::get_probes): ...this.  Adjust code
>> 	to reflect change.
>> 	(dtrace_get_arg): Rename to...
>> 	(dtrace_probe::get_arg_by_number): ...this.  Adjust code to
>> 	reflect change.
>> 	(dtrace_probe_is_enabled): Rename to...
>> 	(dtrace_probe::is_enabled): ...this.  Adjust code to reflect
>> 	change.
>> 	(dtrace_get_probe_address): Rename to...
>> 	(dtrace_probe::get_relocated_address): ...this.  Adjust code
>> 	to reflect change.
>> 	(dtrace_get_probe_argument_count): Rename to...
>> 	(dtrace_probe::get_argument_count): ...this.  Adjust code to
>> 	reflect change.
>> 	(dtrace_can_evaluate_probe_arguments): Rename to...
>> 	(dtrace_probe::can_evaluate_arguments): ...this.  Adjust code
>> 	to reflect change.
>> 	(dtrace_evaluate_probe_argument): Rename to...
>> 	(dtrace_probe::evaluate_argument): ...this.  Adjust code to
>> 	reflect change.
>> 	(dtrace_compile_to_ax): Rename to...
>> 	(dtrace_probe::compile_to_ax): ...this.  Adjust code to
>> 	reflect change.
>> 	(dtrace_probe_destroy): Delete.
>> 	(dtrace_type_name): Rename to...
>> 	(dtrace_static_probe_ops::type_name): ...this.  Adjust code to
>> 	reflect change.
>> 	(dtrace_probe::get_static_ops): New method.
>> 	(dtrace_gen_info_probes_table_header): Rename to...
>> 	(dtrace_static_probe_ops::gen_info_probes_table_header):
>> 	...this.  Adjust code to reflect change.
>> 	(dtrace_gen_info_probes_table_values): Rename to...
>> 	(dtrace_probe::gen_info_probes_table_values): ...this.  Adjust
>> 	code to reflect change.
>> 	(dtrace_enable_probe): Rename to...
>> 	(dtrace_probe::enable_probe): ...this.  Adjust code to reflect
>> 	change.
>> 	(dtrace_disable_probe): Rename to...
>> 	(dtrace_probe::disable_probe): ...this.  Adjust code to reflect
>> 	change.
>> 	(struct probe_ops dtrace_probe_ops): Delete.
>> 	(dtrace_static_probe_ops::emit_info_probes_extra_fields): New
>> 	method.
>> 	(info_probes_dtrace_command): Call 'info_probes_for_spops'
>> 	instead of 'info_probes_for_ops'.
>> 	(_initialize_dtrace_probe): Use 'all_static_probe_ops' instead
>> 	of 'all_probe_ops'.
>> ---
>>  gdb/dtrace-probe.c | 528 +++++++++++++++++++++++++++--------------------------
>>  1 file changed, 268 insertions(+), 260 deletions(-)
>> 
>> diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
>> index 2bbe03e4b3..b53ff7ac6c 100644
>> --- a/gdb/dtrace-probe.c
>> +++ b/gdb/dtrace-probe.c
>> @@ -41,10 +41,6 @@
>>  # define SHT_SUNW_dof	0x6ffffff4
>>  #endif
>>  
>> -/* Forward declaration.  */
>> -
>> -extern const struct probe_ops dtrace_probe_ops;
>> -
>>  /* The following structure represents a single argument for the
>>     probe.  */
>>  
>> @@ -60,9 +56,6 @@ struct dtrace_probe_arg
>>    struct expression *expr;
>>  };
>>  
>> -typedef struct dtrace_probe_arg dtrace_probe_arg_s;
>> -DEF_VEC_O (dtrace_probe_arg_s);
>> -
>>  /* The following structure represents an enabler for a probe.  */
>>  
>>  struct dtrace_probe_enabler
>> @@ -73,39 +66,121 @@ struct dtrace_probe_enabler
>>    CORE_ADDR address;
>>  };
>>  
>> -typedef struct dtrace_probe_enabler dtrace_probe_enabler_s;
>> -DEF_VEC_O (dtrace_probe_enabler_s);
>> +/* Class that implements the static probe methods for "stap" probes.  */
>> +
>> +class dtrace_static_probe_ops : public static_probe_ops
>> +{
>> +public:
>> +  /* See probe.h.  */
>> +  bool is_linespec (const char **linespecp) const override;
>> +
>> +  /* See probe.h.  */
>> +  void get_probes (std::vector<probe *> *probesp,
>> +		   struct objfile *objfile) const override;
>> +
>> +  /* See probe.h.  */
>> +  const char *type_name () const override;
>> +
>> +  /* See probe.h.  */
>> +  bool emit_info_probes_extra_fields () const override;
>> +
>> +  /* See probe.h.  */
>> +  void gen_info_probes_table_header
>> +    (std::vector<struct info_probe_column> *heads) const override;
>> +};
>> +
>> +/* DTrace static_probe_ops.  */
>> +
>> +const dtrace_static_probe_ops dtrace_static_probe_ops;
>>  
>>  /* The following structure represents a dtrace probe.  */
>>  
>> -struct dtrace_probe
>> +class dtrace_probe : public probe
>>  {
>> -  /* Generic information about the probe.  This must be the first
>> -     element of this struct, in order to maintain binary compatibility
>> -     with the `struct probe' and be able to fully abstract it.  */
>> -  struct probe p;
>> +public:
>> +  /* Constructor for dtrace_probe.  */
>> +  dtrace_probe (std::string &&name_, std::string &&provider_, CORE_ADDR address_,
>> +		struct gdbarch *arch_,
>> +		std::vector<struct dtrace_probe_arg> &args_,
>> +		std::vector<struct dtrace_probe_enabler> &enablers_)
>
> I am wondering why you don't use && for the vectors, as you do with the strings.
> If the constructor is "stealing" from the vectors passed as parameter, I think it
> would make sense if the parameters were &&, to indicate to the caller that they'll
> be moved from (and force it to use std::move).

That's true.  I think I was misunderstanding the relationship between
rvalues, lvalues and &&.  Fixed.

>> +    : probe (std::move (name_), std::move (provider_), address_, arch_),
>> +      m_args (std::move (args_)),
>> +      m_enablers (std::move (enablers_)),
>> +      m_args_expr_built (false)
>> +  {}
>> +
>> +  /* Destructor for dtrace_probe.  */
>> +  ~dtrace_probe ()
>> +  {
>> +    for (struct dtrace_probe_arg arg : m_args)
>> +      {
>> +	xfree (arg.type_str);
>> +	xfree (arg.expr);
>> +      }
>
> As for stap_probe, I think defining a destructor in dtrace_probe_arg would
> be clearer.

I used the same approach as in stap-probe.c: expression_up,
emplace_back, a constructor that takes care of std::move'ing things, and
no destructor.  I also converted 'type_str' to std::string.

>> +  }
>> +
>> +  /* See probe.h.  */
>> +  CORE_ADDR get_relocated_address (struct objfile *objfile) override;
>> +
>> +  /* See probe.h.  */
>> +  unsigned get_argument_count (struct frame_info *frame) override;
>> +
>> +  /* See probe.h.  */
>> +  int can_evaluate_arguments () const override;
>> +
>> +  /* See probe.h.  */
>> +  struct value *evaluate_argument (unsigned n,
>> +				   struct frame_info *frame) override;
>> +
>> +  /* See probe.h.  */
>> +  void compile_to_ax (struct agent_expr *aexpr,
>> +		      struct axs_value *axs_value,
>> +		      unsigned n) override;
>> +
>> +  /* See probe.h.  */
>> +  const static_probe_ops *get_static_ops () const override;
>> +
>> +  /* See probe.h.  */
>> +  void gen_info_probes_table_values
>> +    (std::vector<const char *> *values) const override;
>> +
>> +  /* See probe.h.  */
>> +  bool can_enable () const override
>> +  {
>> +    return true;
>> +  }
>> +
>> +  /* See probe.h.  */
>> +  void enable () override;
>>  
>> +  /* See probe.h.  */
>> +  void disable () override;
>> +
>> +  /* Return the Nth argument of the probe.  */
>> +  struct dtrace_probe_arg *get_arg_by_number (unsigned n,
>> +					      struct gdbarch *gdbarch);
>> +
>> +  /* Build the GDB internal expressiosn that, once evaluated, will
>> +     calculate the values of the arguments of the probe.  */
>> +  void build_arg_exprs (struct gdbarch *gdbarch);
>> +
>> +  /* Determine whether the probe is "enabled" or "disabled".  A
>> +     disabled probe is a probe in which one or more enablers are
>> +     disabled.  */
>> +  bool is_enabled () const;
>> +
>> +private:
>>    /* A probe can have zero or more arguments.  */
>> -  int probe_argc;
>> -  VEC (dtrace_probe_arg_s) *args;
>> +  int m_argc;
>
> It seems like this field is never set, but used at least once.  Should we use args->size() instead?

OK, no problem.  Done.

>> +  std::vector<struct dtrace_probe_arg> m_args;
>>  
>>    /* A probe can have zero or more "enablers" associated with it.  */
>> -  VEC (dtrace_probe_enabler_s) *enablers;
>> +  std::vector<struct dtrace_probe_enabler> m_enablers;
>>  
>>    /* Whether the expressions for the arguments have been built.  */
>> -  unsigned int args_expr_built : 1;
>> +  bool m_args_expr_built;
>>  };
>>  
>> -/* Implementation of the probe_is_linespec method.  */
>> -
>> -static int
>> -dtrace_probe_is_linespec (const char **linespecp)
>> -{
>> -  static const char *const keywords[] = { "-pdtrace", "-probe-dtrace", NULL };
>> -
>> -  return probe_is_linespec_by_keyword (linespecp, keywords);
>> -}
>> -
>>  /* DOF programs can contain an arbitrary number of sections of 26
>>     different types.  In order to support DTrace USDT probes we only
>>     need to handle a subset of these section types, fortunately.  These
>> @@ -322,8 +397,6 @@ dtrace_process_dof_probe (struct objfile *objfile,
>>  			  char *argtab, uint64_t strtab_size)
>>  {
>>    int i, j, num_probes, num_enablers;
>> -  struct cleanup *cleanup;
>> -  VEC (dtrace_probe_enabler_s) *enablers;
>>    char *p;
>>  
>>    /* Each probe section can define zero or more probes of two
>> @@ -368,9 +441,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
>>  
>>    /* Build the list of enablers for the probes defined in this Probe
>>       DOF section.  */
>> -  enablers = NULL;
>> -  cleanup
>> -    = make_cleanup (VEC_cleanup (dtrace_probe_enabler_s), &enablers);
>> +  std::vector<struct dtrace_probe_enabler> enablers;
>>    num_enablers = DOF_UINT (dof, probe->dofpr_nenoffs);
>>    for (i = 0; i < num_enablers; i++)
>>      {
>> @@ -380,38 +451,32 @@ dtrace_process_dof_probe (struct objfile *objfile,
>>  
>>        enabler.address = DOF_UINT (dof, probe->dofpr_addr)
>>  	+ DOF_UINT (dof, enabler_offset);
>> -      VEC_safe_push (dtrace_probe_enabler_s, enablers, &enabler);
>> +      enablers.push_back (enabler);
>>      }
>>  
>>    for (i = 0; i < num_probes; i++)
>>      {
>>        uint32_t probe_offset
>>  	= ((uint32_t *) offtab)[DOF_UINT (dof, probe->dofpr_offidx) + i];
>> -      struct dtrace_probe *ret =
>> -	XOBNEW (&objfile->per_bfd->storage_obstack, struct dtrace_probe);
>> -
>> -      ret->p.pops = &dtrace_probe_ops;
>> -      ret->p.arch = gdbarch;
>> -      ret->args_expr_built = 0;
>>  
>>        /* Set the provider and the name of the probe.  */
>> -      ret->p.provider
>> -	= xstrdup (strtab + DOF_UINT (dof, provider->dofpv_name));
>> -      ret->p.name = xstrdup (strtab + DOF_UINT (dof, probe->dofpr_name));
>> +      const char *probe_provider
>> +	= strtab + DOF_UINT (dof, provider->dofpv_name);
>> +      const char *name = strtab + DOF_UINT (dof, probe->dofpr_name);
>>  
>>        /* The probe address.  */
>> -      ret->p.address
>> +      CORE_ADDR address
>>  	= DOF_UINT (dof, probe->dofpr_addr) + DOF_UINT (dof, probe_offset);
>>  
>>        /* Number of arguments in the probe.  */
>> -      ret->probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
>> +      int probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
>>  
>>        /* Store argument type descriptions.  A description of the type
>>           of the argument is in the (J+1)th null-terminated string
>>           starting at 'strtab' + 'probe->dofpr_nargv'.  */
>> -      ret->args = NULL;
>> +      std::vector<struct dtrace_probe_arg> args;
>>        p = strtab + DOF_UINT (dof, probe->dofpr_nargv);
>> -      for (j = 0; j < ret->probe_argc; j++)
>> +      for (j = 0; j < probe_argc; j++)
>>  	{
>>  	  struct dtrace_probe_arg arg;
>>  	  expression_up expr;
>> @@ -442,17 +507,18 @@ dtrace_process_dof_probe (struct objfile *objfile,
>>  	  if (expr != NULL && expr->elts[0].opcode == OP_TYPE)
>>  	    arg.type = expr->elts[1].type;
>>  
>> -	  VEC_safe_push (dtrace_probe_arg_s, ret->args, &arg);
>> +	  args.push_back (arg);
>>  	}
>>  
>> -      /* Add the vector of enablers to this probe, if any.  */
>> -      ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers);
>> +      std::vector<struct dtrace_probe_enabler> enablers_copy = enablers;
>
> Why is this copy necessary?  What happens with the original vector?

This copy is necessary because the original vector should be intact on
every iteration, but dtrace_probe's constructor performs a 'std::move'
on it.  I could have passed the original vector as an argument (by
value), and not perform the std::move in the constructor, if you prefer.

>> +      dtrace_probe *ret = new dtrace_probe (std::string (name),
>> +					    std::string (probe_provider),
>> +					    address, gdbarch,
>> +					    args, enablers_copy);
>>  
>>        /* Successfully created probe.  */
>> -      probesp->push_back ((struct probe *) ret);
>> +      probesp->push_back (ret);
>>      }
>> -
>> -  do_cleanups (cleanup);
>>  }
>>  
>>  /* Helper function to collect the probes described in the DOF program
>> @@ -555,28 +621,23 @@ dtrace_process_dof (asection *sect, struct objfile *objfile,
>>  	     sect->name);
>>  }
>>  
>> -/* Helper function to build the GDB internal expressiosn that, once
>> -   evaluated, will calculate the values of the arguments of a given
>> -   PROBE.  */
>> +/* Implementation of 'build_arg_exprs' method.  */
>>  
>> -static void
>> -dtrace_build_arg_exprs (struct dtrace_probe *probe,
>> -			struct gdbarch *gdbarch)
>> +void
>> +dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch)
>>  {
>> -  struct parser_state pstate;
>> -  struct dtrace_probe_arg *arg;
>> -  int i;
>> -
>> -  probe->args_expr_built = 1;
>> +  m_args_expr_built = true;
>>  
>>    /* Iterate over the arguments in the probe and build the
>>       corresponding GDB internal expression that will generate the
>>       value of the argument when executed at the PC of the probe.  */
>> -  for (i = 0; i < probe->probe_argc; i++)
>> +  for (int i = 0; i < m_argc; i++)
>>      {
>> +      struct dtrace_probe_arg *arg;
>>        struct cleanup *back_to;
>> +      struct parser_state pstate;
>>  
>> -      arg = VEC_index (dtrace_probe_arg_s, probe->args, i);
>> +      arg = &m_args[i];
>>  
>>        /* Initialize the expression buffer in the parser state.  The
>>  	 language does not matter, since we are using our own
>> @@ -606,138 +667,82 @@ dtrace_build_arg_exprs (struct dtrace_probe *probe,
>>      }
>>  }
>>  
>> -/* Helper function to return the Nth argument of a given PROBE.  */
>> -
>> -static struct dtrace_probe_arg *
>> -dtrace_get_arg (struct dtrace_probe *probe, unsigned n,
>> -		struct gdbarch *gdbarch)
>> -{
>> -  if (!probe->args_expr_built)
>> -    dtrace_build_arg_exprs (probe, gdbarch);
>> -
>> -  return VEC_index (dtrace_probe_arg_s, probe->args, n);
>> -}
>> -
>> -/* Implementation of the get_probes method.  */
>> +/* Implementation of 'get_arg_by_number' method.  */
>>  
>> -static void
>> -dtrace_get_probes (std::vector<probe *> *probesp, struct objfile *objfile)
>> +struct dtrace_probe_arg *
>> +dtrace_probe::get_arg_by_number (unsigned n, struct gdbarch *gdbarch)
>>  {
>> -  bfd *abfd = objfile->obfd;
>> -  asection *sect = NULL;
>> -
>> -  /* Do nothing in case this is a .debug file, instead of the objfile
>> -     itself.  */
>> -  if (objfile->separate_debug_objfile_backlink != NULL)
>> -    return;
>> +  if (!m_args_expr_built)
>> +    this->build_arg_exprs (gdbarch);
>>  
>> -  /* Iterate over the sections in OBJFILE looking for DTrace
>> -     information.  */
>> -  for (sect = abfd->sections; sect != NULL; sect = sect->next)
>> -    {
>> -      if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
>> -	{
>> -	  bfd_byte *dof;
>> -
>> -	  /* Read the contents of the DOF section and then process it to
>> -	     extract the information of any probe defined into it.  */
>> -	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
>> -	    complaint (&symfile_complaints,
>> -		       _("could not obtain the contents of"
>> -			 "section '%s' in objfile `%s'."),
>> -		       sect->name, abfd->filename);
>> -      
>> -	  dtrace_process_dof (sect, objfile, probesp,
>> -			      (struct dtrace_dof_hdr *) dof);
>> -	  xfree (dof);
>> -	}
>> -    }
>> +  return &m_args[n];
>
> It would be nice to add a gdb_assert (n < m_args.size ()) here.

I've added a bound check and an internal_error, just like I did for
stap-probe.

Thanks,
  

Patch

diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index 2bbe03e4b3..b53ff7ac6c 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -41,10 +41,6 @@ 
 # define SHT_SUNW_dof	0x6ffffff4
 #endif
 
-/* Forward declaration.  */
-
-extern const struct probe_ops dtrace_probe_ops;
-
 /* The following structure represents a single argument for the
    probe.  */
 
@@ -60,9 +56,6 @@  struct dtrace_probe_arg
   struct expression *expr;
 };
 
-typedef struct dtrace_probe_arg dtrace_probe_arg_s;
-DEF_VEC_O (dtrace_probe_arg_s);
-
 /* The following structure represents an enabler for a probe.  */
 
 struct dtrace_probe_enabler
@@ -73,39 +66,121 @@  struct dtrace_probe_enabler
   CORE_ADDR address;
 };
 
-typedef struct dtrace_probe_enabler dtrace_probe_enabler_s;
-DEF_VEC_O (dtrace_probe_enabler_s);
+/* Class that implements the static probe methods for "stap" probes.  */
+
+class dtrace_static_probe_ops : public static_probe_ops
+{
+public:
+  /* See probe.h.  */
+  bool is_linespec (const char **linespecp) const override;
+
+  /* See probe.h.  */
+  void get_probes (std::vector<probe *> *probesp,
+		   struct objfile *objfile) const override;
+
+  /* See probe.h.  */
+  const char *type_name () const override;
+
+  /* See probe.h.  */
+  bool emit_info_probes_extra_fields () const override;
+
+  /* See probe.h.  */
+  void gen_info_probes_table_header
+    (std::vector<struct info_probe_column> *heads) const override;
+};
+
+/* DTrace static_probe_ops.  */
+
+const dtrace_static_probe_ops dtrace_static_probe_ops;
 
 /* The following structure represents a dtrace probe.  */
 
-struct dtrace_probe
+class dtrace_probe : public probe
 {
-  /* Generic information about the probe.  This must be the first
-     element of this struct, in order to maintain binary compatibility
-     with the `struct probe' and be able to fully abstract it.  */
-  struct probe p;
+public:
+  /* Constructor for dtrace_probe.  */
+  dtrace_probe (std::string &&name_, std::string &&provider_, CORE_ADDR address_,
+		struct gdbarch *arch_,
+		std::vector<struct dtrace_probe_arg> &args_,
+		std::vector<struct dtrace_probe_enabler> &enablers_)
+    : probe (std::move (name_), std::move (provider_), address_, arch_),
+      m_args (std::move (args_)),
+      m_enablers (std::move (enablers_)),
+      m_args_expr_built (false)
+  {}
+
+  /* Destructor for dtrace_probe.  */
+  ~dtrace_probe ()
+  {
+    for (struct dtrace_probe_arg arg : m_args)
+      {
+	xfree (arg.type_str);
+	xfree (arg.expr);
+      }
+  }
+
+  /* See probe.h.  */
+  CORE_ADDR get_relocated_address (struct objfile *objfile) override;
+
+  /* See probe.h.  */
+  unsigned get_argument_count (struct frame_info *frame) override;
+
+  /* See probe.h.  */
+  int can_evaluate_arguments () const override;
+
+  /* See probe.h.  */
+  struct value *evaluate_argument (unsigned n,
+				   struct frame_info *frame) override;
+
+  /* See probe.h.  */
+  void compile_to_ax (struct agent_expr *aexpr,
+		      struct axs_value *axs_value,
+		      unsigned n) override;
+
+  /* See probe.h.  */
+  const static_probe_ops *get_static_ops () const override;
+
+  /* See probe.h.  */
+  void gen_info_probes_table_values
+    (std::vector<const char *> *values) const override;
+
+  /* See probe.h.  */
+  bool can_enable () const override
+  {
+    return true;
+  }
+
+  /* See probe.h.  */
+  void enable () override;
 
+  /* See probe.h.  */
+  void disable () override;
+
+  /* Return the Nth argument of the probe.  */
+  struct dtrace_probe_arg *get_arg_by_number (unsigned n,
+					      struct gdbarch *gdbarch);
+
+  /* Build the GDB internal expressiosn that, once evaluated, will
+     calculate the values of the arguments of the probe.  */
+  void build_arg_exprs (struct gdbarch *gdbarch);
+
+  /* Determine whether the probe is "enabled" or "disabled".  A
+     disabled probe is a probe in which one or more enablers are
+     disabled.  */
+  bool is_enabled () const;
+
+private:
   /* A probe can have zero or more arguments.  */
-  int probe_argc;
-  VEC (dtrace_probe_arg_s) *args;
+  int m_argc;
+  std::vector<struct dtrace_probe_arg> m_args;
 
   /* A probe can have zero or more "enablers" associated with it.  */
-  VEC (dtrace_probe_enabler_s) *enablers;
+  std::vector<struct dtrace_probe_enabler> m_enablers;
 
   /* Whether the expressions for the arguments have been built.  */
-  unsigned int args_expr_built : 1;
+  bool m_args_expr_built;
 };
 
-/* Implementation of the probe_is_linespec method.  */
-
-static int
-dtrace_probe_is_linespec (const char **linespecp)
-{
-  static const char *const keywords[] = { "-pdtrace", "-probe-dtrace", NULL };
-
-  return probe_is_linespec_by_keyword (linespecp, keywords);
-}
-
 /* DOF programs can contain an arbitrary number of sections of 26
    different types.  In order to support DTrace USDT probes we only
    need to handle a subset of these section types, fortunately.  These
@@ -322,8 +397,6 @@  dtrace_process_dof_probe (struct objfile *objfile,
 			  char *argtab, uint64_t strtab_size)
 {
   int i, j, num_probes, num_enablers;
-  struct cleanup *cleanup;
-  VEC (dtrace_probe_enabler_s) *enablers;
   char *p;
 
   /* Each probe section can define zero or more probes of two
@@ -368,9 +441,7 @@  dtrace_process_dof_probe (struct objfile *objfile,
 
   /* Build the list of enablers for the probes defined in this Probe
      DOF section.  */
-  enablers = NULL;
-  cleanup
-    = make_cleanup (VEC_cleanup (dtrace_probe_enabler_s), &enablers);
+  std::vector<struct dtrace_probe_enabler> enablers;
   num_enablers = DOF_UINT (dof, probe->dofpr_nenoffs);
   for (i = 0; i < num_enablers; i++)
     {
@@ -380,38 +451,32 @@  dtrace_process_dof_probe (struct objfile *objfile,
 
       enabler.address = DOF_UINT (dof, probe->dofpr_addr)
 	+ DOF_UINT (dof, enabler_offset);
-      VEC_safe_push (dtrace_probe_enabler_s, enablers, &enabler);
+      enablers.push_back (enabler);
     }
 
   for (i = 0; i < num_probes; i++)
     {
       uint32_t probe_offset
 	= ((uint32_t *) offtab)[DOF_UINT (dof, probe->dofpr_offidx) + i];
-      struct dtrace_probe *ret =
-	XOBNEW (&objfile->per_bfd->storage_obstack, struct dtrace_probe);
-
-      ret->p.pops = &dtrace_probe_ops;
-      ret->p.arch = gdbarch;
-      ret->args_expr_built = 0;
 
       /* Set the provider and the name of the probe.  */
-      ret->p.provider
-	= xstrdup (strtab + DOF_UINT (dof, provider->dofpv_name));
-      ret->p.name = xstrdup (strtab + DOF_UINT (dof, probe->dofpr_name));
+      const char *probe_provider
+	= strtab + DOF_UINT (dof, provider->dofpv_name);
+      const char *name = strtab + DOF_UINT (dof, probe->dofpr_name);
 
       /* The probe address.  */
-      ret->p.address
+      CORE_ADDR address
 	= DOF_UINT (dof, probe->dofpr_addr) + DOF_UINT (dof, probe_offset);
 
       /* Number of arguments in the probe.  */
-      ret->probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
+      int probe_argc = DOF_UINT (dof, probe->dofpr_nargc);
 
       /* Store argument type descriptions.  A description of the type
          of the argument is in the (J+1)th null-terminated string
          starting at 'strtab' + 'probe->dofpr_nargv'.  */
-      ret->args = NULL;
+      std::vector<struct dtrace_probe_arg> args;
       p = strtab + DOF_UINT (dof, probe->dofpr_nargv);
-      for (j = 0; j < ret->probe_argc; j++)
+      for (j = 0; j < probe_argc; j++)
 	{
 	  struct dtrace_probe_arg arg;
 	  expression_up expr;
@@ -442,17 +507,18 @@  dtrace_process_dof_probe (struct objfile *objfile,
 	  if (expr != NULL && expr->elts[0].opcode == OP_TYPE)
 	    arg.type = expr->elts[1].type;
 
-	  VEC_safe_push (dtrace_probe_arg_s, ret->args, &arg);
+	  args.push_back (arg);
 	}
 
-      /* Add the vector of enablers to this probe, if any.  */
-      ret->enablers = VEC_copy (dtrace_probe_enabler_s, enablers);
+      std::vector<struct dtrace_probe_enabler> enablers_copy = enablers;
+      dtrace_probe *ret = new dtrace_probe (std::string (name),
+					    std::string (probe_provider),
+					    address, gdbarch,
+					    args, enablers_copy);
 
       /* Successfully created probe.  */
-      probesp->push_back ((struct probe *) ret);
+      probesp->push_back (ret);
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Helper function to collect the probes described in the DOF program
@@ -555,28 +621,23 @@  dtrace_process_dof (asection *sect, struct objfile *objfile,
 	     sect->name);
 }
 
-/* Helper function to build the GDB internal expressiosn that, once
-   evaluated, will calculate the values of the arguments of a given
-   PROBE.  */
+/* Implementation of 'build_arg_exprs' method.  */
 
-static void
-dtrace_build_arg_exprs (struct dtrace_probe *probe,
-			struct gdbarch *gdbarch)
+void
+dtrace_probe::build_arg_exprs (struct gdbarch *gdbarch)
 {
-  struct parser_state pstate;
-  struct dtrace_probe_arg *arg;
-  int i;
-
-  probe->args_expr_built = 1;
+  m_args_expr_built = true;
 
   /* Iterate over the arguments in the probe and build the
      corresponding GDB internal expression that will generate the
      value of the argument when executed at the PC of the probe.  */
-  for (i = 0; i < probe->probe_argc; i++)
+  for (int i = 0; i < m_argc; i++)
     {
+      struct dtrace_probe_arg *arg;
       struct cleanup *back_to;
+      struct parser_state pstate;
 
-      arg = VEC_index (dtrace_probe_arg_s, probe->args, i);
+      arg = &m_args[i];
 
       /* Initialize the expression buffer in the parser state.  The
 	 language does not matter, since we are using our own
@@ -606,138 +667,82 @@  dtrace_build_arg_exprs (struct dtrace_probe *probe,
     }
 }
 
-/* Helper function to return the Nth argument of a given PROBE.  */
-
-static struct dtrace_probe_arg *
-dtrace_get_arg (struct dtrace_probe *probe, unsigned n,
-		struct gdbarch *gdbarch)
-{
-  if (!probe->args_expr_built)
-    dtrace_build_arg_exprs (probe, gdbarch);
-
-  return VEC_index (dtrace_probe_arg_s, probe->args, n);
-}
-
-/* Implementation of the get_probes method.  */
+/* Implementation of 'get_arg_by_number' method.  */
 
-static void
-dtrace_get_probes (std::vector<probe *> *probesp, struct objfile *objfile)
+struct dtrace_probe_arg *
+dtrace_probe::get_arg_by_number (unsigned n, struct gdbarch *gdbarch)
 {
-  bfd *abfd = objfile->obfd;
-  asection *sect = NULL;
-
-  /* Do nothing in case this is a .debug file, instead of the objfile
-     itself.  */
-  if (objfile->separate_debug_objfile_backlink != NULL)
-    return;
+  if (!m_args_expr_built)
+    this->build_arg_exprs (gdbarch);
 
-  /* Iterate over the sections in OBJFILE looking for DTrace
-     information.  */
-  for (sect = abfd->sections; sect != NULL; sect = sect->next)
-    {
-      if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
-	{
-	  bfd_byte *dof;
-
-	  /* Read the contents of the DOF section and then process it to
-	     extract the information of any probe defined into it.  */
-	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
-	    complaint (&symfile_complaints,
-		       _("could not obtain the contents of"
-			 "section '%s' in objfile `%s'."),
-		       sect->name, abfd->filename);
-      
-	  dtrace_process_dof (sect, objfile, probesp,
-			      (struct dtrace_dof_hdr *) dof);
-	  xfree (dof);
-	}
-    }
+  return &m_args[n];
 }
 
-/* Helper function to determine whether a given probe is "enabled" or
-   "disabled".  A disabled probe is a probe in which one or more
-   enablers are disabled.  */
+/* Implementation of the probe is_enabled method.  */
 
-static int
-dtrace_probe_is_enabled (struct dtrace_probe *probe)
+bool
+dtrace_probe::is_enabled () const
 {
-  int i;
-  struct gdbarch *gdbarch = probe->p.arch;
-  struct dtrace_probe_enabler *enabler;
+  struct gdbarch *gdbarch = this->get_gdbarch ();
 
-  for (i = 0;
-       VEC_iterate (dtrace_probe_enabler_s, probe->enablers, i, enabler);
-       i++)
-    if (!gdbarch_dtrace_probe_is_enabled (gdbarch, enabler->address))
-      return 0;
+  for (struct dtrace_probe_enabler enabler : m_enablers)
+    if (!gdbarch_dtrace_probe_is_enabled (gdbarch, enabler.address))
+      return false;
 
-  return 1;
+  return true;
 }
 
 /* Implementation of the get_probe_address method.  */
 
-static CORE_ADDR
-dtrace_get_probe_address (struct probe *probe, struct objfile *objfile)
+CORE_ADDR
+dtrace_probe::get_relocated_address (struct objfile *objfile)
 {
-  gdb_assert (probe->pops == &dtrace_probe_ops);
-  return probe->address + ANOFFSET (objfile->section_offsets,
-				    SECT_OFF_DATA (objfile));
+  return this->get_address () + ANOFFSET (objfile->section_offsets,
+					  SECT_OFF_DATA (objfile));
 }
 
-/* Implementation of the get_probe_argument_count method.  */
+/* Implementation of the get_argument_count method.  */
 
-static unsigned
-dtrace_get_probe_argument_count (struct probe *probe_generic,
-				 struct frame_info *frame)
+unsigned
+dtrace_probe::get_argument_count (struct frame_info *frame)
 {
-  struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
-
-  gdb_assert (probe_generic->pops == &dtrace_probe_ops);
-
-  return dtrace_probe->probe_argc;
+  return m_argc;
 }
 
-/* Implementation of the can_evaluate_probe_arguments method.  */
+/* Implementation of the can_evaluate_arguments method.  */
 
-static int
-dtrace_can_evaluate_probe_arguments (struct probe *probe_generic)
+int
+dtrace_probe::can_evaluate_arguments () const
 {
-  struct gdbarch *gdbarch = probe_generic->arch;
+  struct gdbarch *gdbarch = this->get_gdbarch ();
 
-  gdb_assert (probe_generic->pops == &dtrace_probe_ops);
   return gdbarch_dtrace_parse_probe_argument_p (gdbarch);
 }
 
-/* Implementation of the evaluate_probe_argument method.  */
+/* Implementation of the evaluate_argument method.  */
 
-static struct value *
-dtrace_evaluate_probe_argument (struct probe *probe_generic, unsigned n,
-				struct frame_info *frame)
+struct value *
+dtrace_probe::evaluate_argument (unsigned n,
+				 struct frame_info *frame)
 {
-  struct gdbarch *gdbarch = probe_generic->arch;
-  struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
+  struct gdbarch *gdbarch = this->get_gdbarch ();
   struct dtrace_probe_arg *arg;
   int pos = 0;
 
-  gdb_assert (probe_generic->pops == &dtrace_probe_ops);
-
-  arg = dtrace_get_arg (dtrace_probe, n, gdbarch);
+  arg = this->get_arg_by_number (n, gdbarch);
   return evaluate_subexp_standard (arg->type, arg->expr, &pos, EVAL_NORMAL);
 }
 
 /* Implementation of the compile_to_ax method.  */
 
-static void
-dtrace_compile_to_ax (struct probe *probe_generic, struct agent_expr *expr,
-		      struct axs_value *value, unsigned n)
+void
+dtrace_probe::compile_to_ax (struct agent_expr *expr, struct axs_value *value,
+			     unsigned n)
 {
-  struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe_generic;
   struct dtrace_probe_arg *arg;
   union exp_element *pc;
 
-  gdb_assert (probe_generic->pops == &dtrace_probe_ops);
-
-  arg = dtrace_get_arg (dtrace_probe, n, expr->gdbarch);
+  arg = this->get_arg_by_number (n, expr->gdbarch);
 
   pc = arg->expr->elts;
   gen_expr (arg->expr, &pc, expr, value);
@@ -746,83 +751,40 @@  dtrace_compile_to_ax (struct probe *probe_generic, struct agent_expr *expr,
   value->type = arg->type;
 }
 
-/* Implementation of the probe_destroy method.  */
-
-static void
-dtrace_probe_destroy (struct probe *probe_generic)
-{
-  struct dtrace_probe *probe = (struct dtrace_probe *) probe_generic;
-  struct dtrace_probe_arg *arg;
-  int i;
-
-  gdb_assert (probe_generic->pops == &dtrace_probe_ops);
-
-  for (i = 0; VEC_iterate (dtrace_probe_arg_s, probe->args, i, arg); i++)
-    {
-      xfree (arg->type_str);
-      xfree (arg->expr);
-    }
-
-  VEC_free (dtrace_probe_enabler_s, probe->enablers);
-  VEC_free (dtrace_probe_arg_s, probe->args);
-}
-
-/* Implementation of the type_name method.  */
+/* Implementation of the 'get_static_ops' method.  */
 
-static const char *
-dtrace_type_name (struct probe *probe_generic)
+const static_probe_ops *
+dtrace_probe::get_static_ops () const
 {
-  gdb_assert (probe_generic->pops == &dtrace_probe_ops);
-  return "dtrace";
-}
-
-/* Implementation of the gen_info_probes_table_header method.  */
-
-static void
-dtrace_gen_info_probes_table_header (VEC (info_probe_column_s) **heads)
-{
-  info_probe_column_s dtrace_probe_column;
-
-  dtrace_probe_column.field_name = "enabled";
-  dtrace_probe_column.print_name = _("Enabled");
-
-  VEC_safe_push (info_probe_column_s, *heads, &dtrace_probe_column);
+  return &dtrace_static_probe_ops;
 }
 
 /* Implementation of the gen_info_probes_table_values method.  */
 
-static void
-dtrace_gen_info_probes_table_values (struct probe *probe_generic,
-				     VEC (const_char_ptr) **ret)
+void
+dtrace_probe::gen_info_probes_table_values
+  (std::vector<const char *> *values) const
 {
-  struct dtrace_probe *probe = (struct dtrace_probe *) probe_generic;
   const char *val = NULL;
 
-  gdb_assert (probe_generic->pops == &dtrace_probe_ops);
-
-  if (VEC_empty (dtrace_probe_enabler_s, probe->enablers))
+  if (m_enablers.empty ())
     val = "always";
-  else if (!gdbarch_dtrace_probe_is_enabled_p (probe_generic->arch))
+  else if (!gdbarch_dtrace_probe_is_enabled_p (this->get_gdbarch ()))
     val = "unknown";
-  else if (dtrace_probe_is_enabled (probe))
+  else if (this->is_enabled ())
     val = "yes";
   else
     val = "no";
 
-  VEC_safe_push (const_char_ptr, *ret, val);
+  values->push_back (val);
 }
 
-/* Implementation of the enable_probe method.  */
+/* Implementation of the enable method.  */
 
-static void
-dtrace_enable_probe (struct probe *probe)
+void
+dtrace_probe::enable ()
 {
-  struct gdbarch *gdbarch = probe->arch;
-  struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe;
-  struct dtrace_probe_enabler *enabler;
-  int i;
-
-  gdb_assert (probe->pops == &dtrace_probe_ops);
+  struct gdbarch *gdbarch = this->get_gdbarch ();
 
   /* Enabling a dtrace probe implies patching the text section of the
      running process, so make sure the inferior is indeed running.  */
@@ -830,31 +792,23 @@  dtrace_enable_probe (struct probe *probe)
     error (_("No inferior running"));
 
   /* Fast path.  */
-  if (dtrace_probe_is_enabled (dtrace_probe))
+  if (this->is_enabled ())
     return;
 
   /* Iterate over all defined enabler in the given probe and enable
      them all using the corresponding gdbarch hook.  */
-
-  for (i = 0;
-       VEC_iterate (dtrace_probe_enabler_s, dtrace_probe->enablers, i, enabler);
-       i++)
+  for (struct dtrace_probe_enabler enabler : m_enablers)
     if (gdbarch_dtrace_enable_probe_p (gdbarch))
-      gdbarch_dtrace_enable_probe (gdbarch, enabler->address);
+      gdbarch_dtrace_enable_probe (gdbarch, enabler.address);
 }
 
 
 /* Implementation of the disable_probe method.  */
 
-static void
-dtrace_disable_probe (struct probe *probe)
+void
+dtrace_probe::disable ()
 {
-  struct gdbarch *gdbarch = probe->arch;
-  struct dtrace_probe *dtrace_probe = (struct dtrace_probe *) probe;
-  struct dtrace_probe_enabler *enabler;
-  int i;
-
-  gdb_assert (probe->pops == &dtrace_probe_ops);
+  struct gdbarch *gdbarch = this->get_gdbarch ();
 
   /* Disabling a dtrace probe implies patching the text section of the
      running process, so make sure the inferior is indeed running.  */
@@ -862,57 +816,111 @@  dtrace_disable_probe (struct probe *probe)
     error (_("No inferior running"));
 
   /* Fast path.  */
-  if (!dtrace_probe_is_enabled (dtrace_probe))
+  if (!this->is_enabled ())
     return;
 
   /* Are we trying to disable a probe that does not have any enabler
      associated?  */
-  if (VEC_empty (dtrace_probe_enabler_s, dtrace_probe->enablers))
-    error (_("Probe %s:%s cannot be disabled: no enablers."), probe->provider, probe->name);
+  if (m_enablers.empty ())
+    error (_("Probe %s:%s cannot be disabled: no enablers."),
+	   this->get_provider (), this->get_name ());
 
   /* Iterate over all defined enabler in the given probe and disable
      them all using the corresponding gdbarch hook.  */
-
-  for (i = 0;
-       VEC_iterate (dtrace_probe_enabler_s, dtrace_probe->enablers, i, enabler);
-       i++)
+  for (struct dtrace_probe_enabler enabler : m_enablers)
     if (gdbarch_dtrace_disable_probe_p (gdbarch))
-      gdbarch_dtrace_disable_probe (gdbarch, enabler->address);
+      gdbarch_dtrace_disable_probe (gdbarch, enabler.address);
 }
 
-/* DTrace probe_ops.  */
-
-const struct probe_ops dtrace_probe_ops =
-{
-  dtrace_probe_is_linespec,
-  dtrace_get_probes,
-  dtrace_get_probe_address,
-  dtrace_get_probe_argument_count,
-  dtrace_can_evaluate_probe_arguments,
-  dtrace_evaluate_probe_argument,
-  dtrace_compile_to_ax,
-  NULL, /* set_semaphore  */
-  NULL, /* clear_semaphore  */
-  dtrace_probe_destroy,
-  dtrace_type_name,
-  dtrace_gen_info_probes_table_header,
-  dtrace_gen_info_probes_table_values,
-  dtrace_enable_probe,
-  dtrace_disable_probe
-};
+/* Implementation of the is_linespec method.  */
+
+bool
+dtrace_static_probe_ops::is_linespec (const char **linespecp) const
+{
+  static const char *const keywords[] = { "-pdtrace", "-probe-dtrace", NULL };
+
+  return probe_is_linespec_by_keyword (linespecp, keywords);
+}
+
+/* Implementation of the get_probes method.  */
+
+void
+dtrace_static_probe_ops::get_probes (std::vector<probe *> *probesp,
+				     struct objfile *objfile) const
+{
+  bfd *abfd = objfile->obfd;
+  asection *sect = NULL;
+
+  /* Do nothing in case this is a .debug file, instead of the objfile
+     itself.  */
+  if (objfile->separate_debug_objfile_backlink != NULL)
+    return;
+
+  /* Iterate over the sections in OBJFILE looking for DTrace
+     information.  */
+  for (sect = abfd->sections; sect != NULL; sect = sect->next)
+    {
+      if (elf_section_data (sect)->this_hdr.sh_type == SHT_SUNW_dof)
+	{
+	  bfd_byte *dof;
+
+	  /* Read the contents of the DOF section and then process it to
+	     extract the information of any probe defined into it.  */
+	  if (!bfd_malloc_and_get_section (abfd, sect, &dof))
+	    complaint (&symfile_complaints,
+		       _("could not obtain the contents of"
+			 "section '%s' in objfile `%s'."),
+		       sect->name, abfd->filename);
+      
+	  dtrace_process_dof (sect, objfile, probesp,
+			      (struct dtrace_dof_hdr *) dof);
+	  xfree (dof);
+	}
+    }
+}
+
+/* Implementation of the type_name method.  */
+
+const char *
+dtrace_static_probe_ops::type_name () const
+{
+  return "dtrace";
+}
+
+/* Implementation of the 'emit_info_probes_extra_fields' method.  */
+
+bool
+dtrace_static_probe_ops::emit_info_probes_extra_fields () const
+{
+  return true;
+}
+
+/* Implementation of the gen_info_probes_table_header method.  */
+
+void
+dtrace_static_probe_ops::gen_info_probes_table_header
+ (std::vector<struct info_probe_column> *heads) const
+{
+  struct info_probe_column dtrace_probe_column;
+
+  dtrace_probe_column.field_name = "enabled";
+  dtrace_probe_column.print_name = _("Enabled");
+
+  heads->push_back(dtrace_probe_column);
+}
 
 /* Implementation of the `info probes dtrace' command.  */
 
 static void
 info_probes_dtrace_command (const char *arg, int from_tty)
 {
-  info_probes_for_ops (arg, from_tty, &dtrace_probe_ops);
+  info_probes_for_spops (arg, from_tty, &dtrace_static_probe_ops);
 }
 
 void
 _initialize_dtrace_probe (void)
 {
-  all_probe_ops.push_back (&dtrace_probe_ops);
+  all_static_probe_ops.push_back (&dtrace_static_probe_ops);
 
   add_cmd ("dtrace", class_info, info_probes_dtrace_command,
 	   _("\