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


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

Re: [PATCH 2/3] Implement new features needed for handling SystemTap probes


Hi Sergio,

On Fri, 09 Mar 2012 21:33:21 +0100, Sergio Durigan Junior wrote:
[...]
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -60,6 +60,8 @@
>  #include "jit.h"
>  #include "xml-syscall.h"
>  #include "parser-defs.h"
> +#include "gdb_regex.h"
> +#include "stap-probe.h"

I believe there was intended some abstraction, more talked about it elsewhere,
breakpoint.c should only include some (nonexistent) "probe.h", not
"stap-probe.h".


>  #include "cli/cli-utils.h"
>  #include "continuations.h"
>  #include "stack.h"
> @@ -290,6 +292,9 @@ static struct breakpoint_ops momentary_breakpoint_ops;
>     breakpoints.  */
>  struct breakpoint_ops bkpt_breakpoint_ops;
>  
> +/* Breakpoints set on SystemTap probes.  */
> +static struct breakpoint_ops bkpt_stap_probe_breakpoint_ops;
> +
>  /* A reference-counted struct command_line.  This lets multiple
>     breakpoints share a single command list.  */
>  struct counted_command_line
> @@ -1926,6 +1931,40 @@ unduplicated_should_be_inserted (struct bp_location *bl)
>    return result;
>  }
>  
> +/* See the comment in breakpoint.h.  */
> +
> +void
> +modify_semaphore (struct bp_location *loc, int set)
> +{
> +  struct gdbarch *arch = loc->gdbarch;
> +  gdb_byte bytes[sizeof (LONGEST)];
> +  /* The ABI specifies "unsigned short".  */
> +  struct type *type = builtin_type (arch)->builtin_unsigned_short;
> +  CORE_ADDR address = loc->semaphore;
> +  ULONGEST value;
> +
> +  if (address == 0)
> +    return;
> +
> +  /* Swallow errors.  */

Could here be a reason why it is valid?


> +  if (target_read_memory (address, bytes, TYPE_LENGTH (type)) != 0)
> +    return;
> +
> +  value = extract_unsigned_integer (bytes, TYPE_LENGTH (type),
> +				    gdbarch_byte_order (arch));
> +  /* Note that we explicitly don't worry about overflow or
> +     underflow.  */
> +  if (set)
> +    ++value;
> +  else
> +    --value;
> +
> +  store_unsigned_integer (bytes, TYPE_LENGTH (type),
> +			  gdbarch_byte_order (arch), value);
> +

> +  target_write_memory (address, bytes, TYPE_LENGTH (type));

And here again, why not to check errors?


> +}
> +
>  /* Parses a conditional described by an expression COND into an
>     agent expression bytecode suitable for evaluation
>     by the bytecode interpreter.  Return NULL if there was
[...]
> --- a/gdb/cli/cli-utils.c
> +++ b/gdb/cli/cli-utils.c
[...]
> @@ -245,3 +257,32 @@ remove_trailing_whitespace (const char *start, char *s)
>  
>    return s;
>  }
> +
> +/* See documentation in cli-utils.h.  */
> +
> +char *
> +extract_arg (char **arg)
> +{
> +  char *result, *copy;
> +
> +  if (!*arg)
> +    return NULL;
> +
> +  /* Find the start of the argument.  */
> +  *arg = skip_spaces (*arg);
> +  if (! **arg)

Excessive space according to GDB Coding Style.


> +    return NULL;
> +  result = *arg;
> +
> +  /* Find the end of the argument.  */
> +  *arg = skip_to_space (*arg + 1);
> +
> +  if (result == *arg)
> +    return NULL;
> +
> +  copy = xmalloc (*arg - result + 1);
> +  memcpy (copy, result, *arg - result);
> +  copy[*arg - result] = '\0';
> +
> +  return copy;
> +}
[...]
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -36,6 +36,8 @@
>  #include "demangle.h"
>  #include "psympriv.h"
>  #include "filenames.h"
> +#include "stap-probe.h"
> +#include "arch-utils.h"
>  #include "gdbtypes.h"
>  #include "value.h"
>  #include "infcall.h"
> @@ -60,6 +62,21 @@ struct elfinfo
>      asection *mdebugsect;	/* Section pointer for .mdebug section */
>    };
>  
> +/* Per-objfile data for SystemTap probe info.  */
> +
> +static const struct objfile_data *stap_probe_key = NULL;
> +
> +/* Per-objfile data about SystemTap probes.  */
> +
> +struct stap_probe_per_objfile
> +  {
> +    /* The number of probes in this objfile.  */
> +    int stap_num_probes;
> +
> +    /* The probes themselves.  */

nitpick:
/* Pointer to array of STAP_NUM_PROBES elements with the probes.  */


> +    struct stap_probe *probes;
> +  };
> +
>  static void free_elfinfo (void *);
>  
>  /* Minimal symbols located at the GOT entries for .plt - that is the real
> @@ -1579,7 +1596,273 @@ elfstab_offset_sections (struct objfile *objfile, struct partial_symtab *pst)
>      complaint (&symfile_complaints,
>  	       _("elf/stab section information missing for %s"), filename);
>  }
> +
> +/* Helper function that parses the information contained in a
> +   SystemTap's probe.  Basically, the information consists in:
> +
> +   - Probe's PC address;
> +   - Link-time section address of `.stapsdt.base' section;
> +   - Link-time address of the semaphore variable, or ZERO if the
> +     probe doesn't have an associated semaphore;
> +   - Probe's provider name;
> +   - Probe's name;
> +   - Probe's argument format.  */
> +
> +static void
> +handle_probe (struct objfile *objfile, struct sdt_note *el,
> +	      struct stap_probe *ret, CORE_ADDR base)
> +{
> +  bfd *abfd = objfile->obfd;
> +  int size = bfd_get_arch_size (abfd) / 8;
> +  struct gdbarch *gdbarch = get_objfile_arch (objfile);
> +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +  struct type *ptr_type = builtin_type (gdbarch)->builtin_data_ptr;
> +  CORE_ADDR base_ref;
> +
> +  ret->gdbarch = gdbarch;
> +
> +  /* Provider and the name of the probe.  */
> +  ret->provider = &el->data[3 * size];
> +  ret->name = memchr (ret->provider, '\0',
> +		      (char *) el->data + el->size - ret->provider);
> +  /* Making sure there is a name.  */
> +  if (!ret->name)
> +    {
> +      complaint (&symfile_complaints, _("corrupt probe when reading `%s'"),
> +		 objfile->name);
> +      ret->provider = NULL;
> +      ret->name = NULL;
> +    }
> +  else
> +    ++ret->name;
> +
> +  /* Retrieving the probe's address.  */
> +  ret->address = extract_typed_address (&el->data[0], ptr_type);

Empty line before comment.

> +  /* Link-time sh_addr of `.stapsdt.base' section.  */
> +  base_ref = extract_typed_address (&el->data[size], ptr_type);

Empty line before comment.

> +  /* Semaphore address.  */
> +  ret->sem_addr = extract_typed_address (&el->data[2 * size], ptr_type);
> +
> +  ret->address += (ANOFFSET (objfile->section_offsets,
> +			     SECT_OFF_TEXT (objfile))
> +		   + base - base_ref);
> +  if (ret->sem_addr)
> +    ret->sem_addr += (ANOFFSET (objfile->section_offsets,
> +				SECT_OFF_DATA (objfile))
> +		      + base - base_ref);
> +
> +  /* Arguments.  We can only extract the argument format if there is a valid
> +     name for this probe.  */
> +  if (ret->name)
> +    {
> +      ret->args = memchr (ret->name, '\0',
> +			  (char *) el->data + el->size - ret->name);
> +
> +      if (ret->args != NULL)
> +	++ret->args;
> +      if (ret->args == NULL
> +	  || (memchr (ret->args, '\0',
> +		      (char *) el->data + el->size - ret->name)
> +	      != el->data + el->size - 1))

Here would be missing '(char *) ' for el->data.  But it is no longer needed as
you made DATA gdb_byte[] now.  But in such case please drop the no longer
needed '(char *) ' casts above.


> +	{
> +	  complaint (&symfile_complaints, _("corrupt probe when reading `%s'"),
> +		     objfile->name);
> +	  ret->args = NULL;
> +	}
> +    }
> +  else
> +    ret->args = NULL;
> +}
> +
> +/* The name of the SystemTap section where we will find information about
> +   the probes.  */
> +
> +#define STAP_BASE_SECTION_NAME ".stapsdt.base"

Such #defines are usually at the top of file (under #includes).


> +
> +/* Helper function which tries to find the base address of the SystemTap
> +   base section named STAP_BASE_SECTION_NAME.  */
> +
> +static void
> +get_base_address_1 (bfd *abfd, asection *sect, void *obj)
> +{
> +  bfd_vma *base = (bfd_vma *) obj;
> +
> +  if (*base == (bfd_vma) -1
> +      && (sect->flags & (SEC_DATA | SEC_ALLOC | SEC_HAS_CONTENTS))
> +      && sect->name && !strcmp (sect->name, STAP_BASE_SECTION_NAME))
> +    *base = sect->vma;
> +}
> +
> +/* Helper function which iterates over every section in the BFD file,
> +   trying to find the base address of the SystemTap base section.
> +   Returns the section address if found, or -1 otherwise.  */
> +
> +static bfd_vma
> +get_base_address (bfd *obfd)
> +{
> +  bfd_vma base = (bfd_vma) -1;

(1) Special values (such as 0xfff...fff are discouraged).
(2) I do not see a reason here for it, there will be just one
    STAP_BASE_SECTION_NAME section anyway, if there are multiple such sections
    it is already some undefined state and it does not matter if one chooses
    the first one or the last one and finally if the secion is not found the
    code mishandles the 0xfff...fff value anyway (which it should not).

I would put some error/warning there and making turning that `void *' into
'asection *' may handle the special not-found value for error/warning even
easy without new parameter struct.


> +
> +  bfd_map_over_sections (obfd, get_base_address_1, (void *) &base);
> +
> +  return base;
> +}
> +
> +/* Implementation of `sym_get_probes', as documented in symfile.h.  */
> +
> +static struct stap_probe *
> +elf_get_probes (struct objfile *objfile, int *num_probes)
> +{
> +  struct stap_probe *ret = NULL;
> +  struct stap_probe_per_objfile *probes_per_objfile;
> +
> +  /* Initially, no probes.  */
> +  *num_probes = 0;
> +
> +  /* Have we parsed this objfile's probes already?  */
> +  probes_per_objfile
> +    = (struct stap_probe_per_objfile *) objfile_data (objfile,
> +						      stap_probe_key);

Excessive cast.


> +
> +  if (!probes_per_objfile)
> +    {
> +      /* If we are here, then this is the first time we are parsing the
> +	 probe's information.  We basically have to count how many probes
> +	 the objfile has, and then fill in the necessary information
> +	 for each one.  */
> +
> +      bfd *obfd = objfile->obfd;
> +      bfd_vma base = get_base_address (obfd);
> +      struct sdt_note *iter;
> +      int i;
> +      int n = 0;
> +
> +      if (! elf_tdata (obfd)->sdt_note_head)
> +	/* There isn't any probe here.  */
> +	return NULL;

These multiple lines should be in { block } according to new GDB Coding Style rule.


> +
> +      /* Allocating space for probe info.  */
> +      for (iter = elf_tdata (obfd)->sdt_note_head;
> +	   iter;
> +	   iter = iter->next, ++n);
> +
> +      ret = xcalloc (n, sizeof (struct stap_probe));
> +
> +      /* Parsing each probe's information.  */
> +      for (iter = elf_tdata (obfd)->sdt_note_head, i = 0;
> +	   iter;
> +	   iter = iter->next, i++)
> +	/* We first have to handle all the information about the
> +	   probe which is present in the section.  */
> +	handle_probe (objfile, iter, &ret[i], base);
> +
> +      /* Creating a cache for these probes in the objfile's registry.  */
> +      probes_per_objfile = xmalloc (sizeof (struct stap_probe_per_objfile));
> +
> +      probes_per_objfile->stap_num_probes = n;
> +      probes_per_objfile->probes = ret;
> +
> +      set_objfile_data (objfile, stap_probe_key, probes_per_objfile);
> +    }
> +  else
> +    ret = probes_per_objfile->probes;
> +
> +  *num_probes = probes_per_objfile->stap_num_probes;
> +
> +  return ret;
> +}
> +
> +/* Implementation of `sym_get_probe_argument_count', as documented in
> +   symfile.h.  */
> +
> +static int
> +elf_get_probe_argument_count (struct objfile *objfile,
> +			      struct stap_probe *probe)
> +{
> +  const char *pargs = probe->args;
> +
> +  if (!pargs || !*pargs || *pargs == ':')
> +    /* No arguments.  */
> +    return 0;

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +
> +  return stap_get_probe_argument_count (probe);
> +}
> +
> +/* Implementation of `sym_evaluate_probe_argument', as documented in
> +   symfile.h.  */
> +
> +static struct value *
> +elf_evaluate_probe_argument (struct objfile *objfile,
> +			     struct stap_probe *probe,
> +			     struct frame_info *frame,
> +			     int n)
> +{
> +  return stap_evaluate_probe_argument (objfile, probe, frame, n);
> +}
> +
> +/* Implementation of `sym_compile_to_ax', as documented in symfile.h.  */
> +
> +static void
> +elf_compile_to_ax (struct objfile *objfile,
> +		   struct stap_probe *probe,
> +		   struct agent_expr *expr,
> +		   struct axs_value *value,
> +		   int n)
> +{

Some comment that current GDB can parse only the SystemTap kind of probes from
ELF, this is why this function is so trivial.

But more about the abstraction I write elsewhere.


> +  stap_compile_to_ax (objfile, probe, expr, value, n);
> +}
> +
> +/* Implementation of `sym_relocate_probe', as documented in symfile.h.  */
> +
> +static void
> +elf_symfile_relocate_probe (struct objfile *objfile,
> +			    struct section_offsets *new_offsets,
> +			    struct section_offsets *delta)
> +{
> +  int i;
> +  struct stap_probe_per_objfile *p
> +    = (struct stap_probe_per_objfile *) objfile_data (objfile,
> +						      stap_probe_key);
> +
> +  if (!p)
> +    /* No probe to relocate.  */
> +    return;

These multiple lines should be in { block } according to new GDB Coding Style rule.


> +
> +  for (i = 0; i < p->stap_num_probes; i++)
> +    {
> +      p->probes[i].address += ANOFFSET (delta, SECT_OFF_TEXT (objfile));
> +      if (p->probes[i].sem_addr)
> +	p->probes[i].sem_addr += ANOFFSET (delta, SECT_OFF_DATA (objfile));
> +    }
> +}
> +
> +/* Helper function used to free the space allocated for storing SystemTap
> +   probe information.  */
> +
> +static void
> +stap_probe_key_free (struct objfile *objfile, void *d)
> +{
> +  int i;
> +  struct stap_probe_per_objfile *data = (struct stap_probe_per_objfile *) d;

Excessive cast.


> +
> +  for (i = 0; i < data->stap_num_probes; i++)
> +    stap_free_parsed_args (data->probes[i].parsed_args);
> +  xfree (data->probes);
> +  xfree (data);
> +}
> +
>  
> +
> +/* Implementation `sym_probe_fns', as documented in symfile.h.  */
> +
> +static const struct sym_probe_fns elf_probe_fns =
> +{
> +  elf_get_probes,		/* sym_get_probes */
> +  elf_get_probe_argument_count,	/* sym_get_probe_argument_count */
> +  elf_evaluate_probe_argument,	/* sym_evaluate_probe_argument */
> +  elf_compile_to_ax,		/* sym_compile_to_ax */
> +  elf_symfile_relocate_probe,	/* sym_relocate_probe */
> +};
> +
>  /* Register that we are able to handle ELF object file formats.  */
>  
>  static const struct sym_fns elf_sym_fns =
[...]
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -58,8 +58,14 @@
>  #include "features/i386/i386-avx.c"
>  #include "features/i386/i386-mmx.c"
>  
> +#include "stap-probe.h"
>  #include "ax.h"
>  #include "ax-gdb.h"
> +#include "user-regs.h"
> +#include "cli/cli-utils.h"
> +#include "expression.h"
> +#include "parser-defs.h"
> +#include <ctype.h>
>  
>  /* Register names.  */
>  
> @@ -7251,6 +7257,312 @@ i386_validate_tdesc_p (struct gdbarch_tdep *tdep,
>    return valid_p;
>  }
>  
> +int
> +i386_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)

Missing function comment.

> +{
> +  return (*s == '$' /* Literal number.  */
> +	  || (isdigit (*s) && s[1] == '(' && s[2] == '%') /* Displacement.  */
> +	  || (*s == '(' && s[1] == '%') /* Register indirection.  */
> +	  || (*s == '%' && isalpha (s[1]))); /* Register access.  */
> +}
> +
> +int
> +i386_stap_parse_special_token (struct gdbarch *gdbarch,
> +			       struct stap_parse_info *p)

Missing function comment.

> +{
> +  const char *s = p->arg;
> +
> +  /* In order to parse special tokens, we use a state-machine that go
> +     through every known token and try to get a match.  */
> +  enum
> +    {
> +      TRIPLET,
> +      THREE_ARG_DISPLACEMENT,
> +      DONE
> +    } current_state;
> +
> +  current_state = TRIPLET;
> +
> +  /* The special tokens to be parsed here are:
> +
> +     - `register base + (register index * size) + offset', as represented
> +     in `(%rcx,%rax,8)', or `[OFFSET](BASE_REG,INDEX_REG[,SIZE])'.
> +
> +     - Operands of the form `-8+3+1(%rbp)', which must be interpreted as
> +     `*(-8 + 3 - 1 + (void *) $eax)'.  */
> +
> +  while (current_state != DONE)
> +    {
> +      const char *s = p->arg;
> +
> +      switch (current_state)
> +	{
> +	case TRIPLET:
> +	    {
> +	      if (isdigit (*s) || *s == '-' || *s == '+')
> +		{
> +		  int got_minus[3];
> +		  int i;
> +		  long displacements[3];
> +		  const char *start;
> +		  char *regname;
> +		  int len;
> +		  struct stoken str;
> +
> +		  got_minus[0] = 0;
> +		  if (*s == '+')
> +		    ++s;
> +		  else if (*s == '-')
> +		    {
> +		      ++s;
> +		      got_minus[0] = 1;
> +		    }
> +
> +		  displacements[0] = strtol (s, (char **) &s, 10);
> +
> +		  if (*s != '+' && *s != '-')
> +		    /* We are not dealing with a triplet.  */
> +		    break;
> +
> +		  got_minus[1] = 0;
> +		  if (*s == '+')
> +		    ++s;
> +		  else
> +		    {
> +		      ++s;
> +		      got_minus[1] = 1;
> +		    }
> +
> +		  displacements[1] = strtol (s, (char **) &s, 10);
> +
> +		  if (*s != '+' && *s != '-')
> +		    /* We are not dealing with a triplet.  */
> +		    break;

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +
> +		  got_minus[2] = 0;
> +		  if (*s == '+')
> +		    ++s;
> +		  else
> +		    {
> +		      ++s;
> +		      got_minus[2] = 1;
> +		    }
> +
> +		  displacements[2] = strtol (s, (char **) &s, 10);
> +
> +		  if (*s != '(' || s[1] != '%')
> +		    break;
> +
> +		  s += 2;
> +		  start = s;
> +
> +		  while (isalnum (*s))
> +		    ++s;
> +
> +		  if (*s++ != ')')
> +		    break;
> +
> +		  len = s - start;
> +		  regname = alloca (len + 1);
> +
> +		  strncpy (regname, start, len);
> +		  regname[len] = '\0';
> +
> +		  if (user_reg_map_name_to_regnum (gdbarch,
> +						   regname, len) == -1)
> +		    error (_("Invalid register name `%s' "
> +			     "on expression `%s'."),
> +			   regname, p->saved_arg);
> +
> +		  for (i = 0; i < 3; i++)
> +		    {
> +		      write_exp_elt_opcode (OP_LONG);
> +		      write_exp_elt_type
> +			(builtin_type (gdbarch)->builtin_long);
> +		      write_exp_elt_longcst (displacements[i]);
> +		      write_exp_elt_opcode (OP_LONG);
> +		      if (got_minus[i])
> +			write_exp_elt_opcode (UNOP_NEG);
> +		    }
> +
> +		  write_exp_elt_opcode (OP_REGISTER);
> +		  str.ptr = regname;
> +		  str.length = len;
> +		  write_exp_string (str);
> +		  write_exp_elt_opcode (OP_REGISTER);
> +
> +		  write_exp_elt_opcode (UNOP_CAST);
> +		  write_exp_elt_type (builtin_type (gdbarch)->builtin_data_ptr);
> +		  write_exp_elt_opcode (UNOP_CAST);
> +
> +		  write_exp_elt_opcode (BINOP_ADD);
> +		  write_exp_elt_opcode (BINOP_ADD);
> +		  write_exp_elt_opcode (BINOP_ADD);
> +
> +		  write_exp_elt_opcode (UNOP_CAST);
> +		  write_exp_elt_type (lookup_pointer_type (p->arg_type));
> +		  write_exp_elt_opcode (UNOP_CAST);
> +
> +		  write_exp_elt_opcode (UNOP_IND);
> +
> +		  p->arg = s;
> +
> +		  return 1;
> +		}
> +	      break;
> +	    }
[...]
> --- a/gdb/ppc-linux-tdep.c
> +++ b/gdb/ppc-linux-tdep.c
[...]
> @@ -1276,6 +1284,65 @@ ppc_linux_core_read_description (struct gdbarch *gdbarch,
>      }
>  }
>  
> +static int
> +ppc_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)

Missing function comment.

> +{
> +  return (*s == 'i' /* Literal number.  */
> +	  || (isdigit (*s) && s[1] == '('
> +	      && isdigit (s[2])) /* Displacement.  */
> +	  || (*s == '(' && isdigit (s[1])) /* Register indirection.  */
> +	  || isdigit (*s)); /* Register value.  */
> +}
> +
> +static int
> +ppc_stap_parse_special_token (struct gdbarch *gdbarch,
> +			      struct stap_parse_info *p)

Missing function comment.

> +{
> +  if (isdigit (*p->arg))
> +    {
> +      /* This temporary pointer is needed because we have to do a lookahead.
> +	  We could be dealing with a register displacement, and in such case
> +	  we would not need to do anything.  */
> +      const char *s = p->arg;
> +      char *regname;
> +      int len;
> +      struct stoken str;
> +
> +      while (isdigit (*s))
> +	++s;
> +
> +      if (*s == '(')
> +	/* It is a register displacement indeed.  Returning 0 means we are
> +	   deferring the treatment of this case to the generic parser.  */
> +	return 0;

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +
> +      len = s - p->arg;
> +      regname = alloca (len + 2);
> +      regname[0] = 'r';
> +
> +      strncpy (regname + 1, p->arg, len);
> +      ++len;
> +      regname[len] = '\0';
> +
> +      if (user_reg_map_name_to_regnum (gdbarch, regname, len) == -1)
> +	error (_("Invalid register name `%s' on expression `%s'."),
> +	       regname, p->saved_arg);
> +
> +      write_exp_elt_opcode (OP_REGISTER);
> +      str.ptr = regname;
> +      str.length = len;
> +      write_exp_string (str);
> +      write_exp_elt_opcode (OP_REGISTER);
> +
> +      p->arg = s;
> +    }
> +  else
> +    /* All the other tokens should be handled correctly by the generic
> +       parser.  */
> +    return 0;

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +
> +  return 1;
> +}
>  
>  /* Cell/B.E. active SPE context tracking support.  */
>  
> @@ -1593,6 +1660,15 @@ ppc_linux_init_abi (struct gdbarch_info info,
>    /* Get the syscall number from the arch's register.  */
>    set_gdbarch_get_syscall_number (gdbarch, ppc_linux_get_syscall_number);
>  
> +  /* SystemTap functions.  */
> +  set_gdbarch_stap_integer_prefix (gdbarch, "i");
> +  set_gdbarch_stap_register_indirection_prefix (gdbarch, "(");
> +  set_gdbarch_stap_register_indirection_suffix (gdbarch, ")");
> +  set_gdbarch_stap_gdb_register_prefix (gdbarch, "r");
> +  set_gdbarch_stap_is_single_operand (gdbarch, ppc_stap_is_single_operand);
> +  set_gdbarch_stap_parse_special_token (gdbarch,
> +					ppc_stap_parse_special_token);
> +
>    if (tdep->wordsize == 4)
>      {
>        /* Until November 2001, gcc did not comply with the 32 bit SysV
> diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
> index ac0c526..661d90c 100644
> --- a/gdb/s390-tdep.c
> +++ b/gdb/s390-tdep.c
> @@ -55,6 +55,12 @@
>  #include "features/s390x-linux64v1.c"
>  #include "features/s390x-linux64v2.c"
>  
> +#include "stap-probe.h"
> +#include "ax.h"
> +#include "ax-gdb.h"
> +#include "user-regs.h"
> +#include "cli/cli-utils.h"
> +#include <ctype.h>
>  
>  /* The tdep structure.  */
>  
> @@ -2953,6 +2959,15 @@ s390_address_class_name_to_type_flags (struct gdbarch *gdbarch,
>      return 0;
>  }
>  
> +static int
> +s390_stap_is_single_operand (struct gdbarch *gdbarch, const char *s)

Missing function comment.


> +{
> +  return ((isdigit (*s) && s[1] == '(' && s[2] == '%') /* Displacement
> +							  or indirection.  */
> +	  || *s == '%' /* Register access.  */
> +	  || isdigit (*s)); /* Literal number.  */
> +}
> +
>  /* Set up gdbarch struct.  */
>  
>  static struct gdbarch *
[...]
> --- /dev/null
> +++ b/gdb/stap-probe.c
> @@ -0,0 +1,1689 @@
> +/* SystemTap probe support for GDB.
> +
> +   Copyright (C) 2011 Free Software Foundation, Inc.

Update the year.


> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
[...]
> +    default:
> +      internal_error (__FILE__, __LINE__,
> +		      _("Undefined bitness for probe."));
> +      break;
> +    }
> +}
> +
> +static void
> +stap_parse_register_operand (struct stap_parse_info *p)

Missing function comment.

> +{
> +  /* Simple flag to indicate whether we have seen a minus signal before
> +     certain number.  */
> +  int got_minus = 0;

Empty line before comment.

> +  /* Flags to indicate whether this register access is being displaced and/or
> +     indirected.  */
> +  int disp_p = 0, indirect_p = 0;
> +  struct gdbarch *gdbarch = p->gdbarch;

Empty line before comment.

> +  /* Needed to generate the register name as a part of an expression.  */
> +  struct stoken str;

Empty line before comment.

> +  /* Variables used to extract the register name from the probe's
> +     argument.  */
> +  const char *start;
> +  char *regname;
> +  int len;
> +
> +  /* Prefixes for the parser.  */
> +  const char *reg_prefix = gdbarch_stap_register_prefix (gdbarch);
> +  const char *reg_ind_prefix
> +    = gdbarch_stap_register_indirection_prefix (gdbarch);
> +  const char *gdb_reg_prefix = gdbarch_stap_gdb_register_prefix (gdbarch);
> +  int reg_prefix_len = reg_prefix ? strlen (reg_prefix) : 0;
> +  int reg_ind_prefix_len = reg_ind_prefix ? strlen (reg_ind_prefix) : 0;
> +  int gdb_reg_prefix_len = gdb_reg_prefix ? strlen (gdb_reg_prefix) : 0;
> +
> +  /* Suffixes for the parser.  */
> +  const char *reg_suffix = gdbarch_stap_register_suffix (gdbarch);
> +  const char *reg_ind_suffix
> +    = gdbarch_stap_register_indirection_suffix (gdbarch);
> +  const char *gdb_reg_suffix = gdbarch_stap_gdb_register_suffix (gdbarch);
> +  int reg_suffix_len = reg_suffix ? strlen (reg_suffix) : 0;
> +  int reg_ind_suffix_len = reg_ind_suffix ? strlen (reg_ind_suffix) : 0;
> +  int gdb_reg_suffix_len = gdb_reg_suffix ? strlen (gdb_reg_suffix) : 0;
> +
> +  /* Checking for a displacement argument.  */
> +  if (*p->arg == '+')
> +    /* If it's a plus sign, we don't need to do anything, just advance the
> +       pointer.  */
> +    ++p->arg;

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +
> +  if (*p->arg == '-')
> +    {
> +      got_minus = 1;
> +      ++p->arg;
> +    }
> +
> +  if (isdigit (*p->arg))
> +    {
> +      /* The value of the displacement.  */
> +      long displacement;
> +
> +      disp_p = 1;
> +      displacement = strtol (p->arg, (char **) &p->arg, 10);
> +
> +      /* Generating the expression for the displacement.  */
> +      write_exp_elt_opcode (OP_LONG);
> +      write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> +      write_exp_elt_longcst (displacement);
> +      write_exp_elt_opcode (OP_LONG);
> +      if (got_minus)
> +	write_exp_elt_opcode (UNOP_NEG);
> +    }
> +
> +  /* Getting rid of register indirection prefix.  */
> +  if (reg_ind_prefix
> +      && strncmp (p->arg, reg_ind_prefix, reg_ind_prefix_len) == 0)
> +    {
> +      indirect_p = 1;
> +      p->arg += reg_ind_prefix_len;
> +    }
> +
> +  if (disp_p && !indirect_p)
> +    error (_("Invalid register displacement syntax on expression `%s'."),
> +	   p->saved_arg);
> +
> +  /* Getting rid of register prefix.  */
> +  if (reg_prefix && strncmp (p->arg, reg_prefix, reg_prefix_len) == 0)
> +    p->arg += reg_prefix_len;
> +
> +  /* Now we should have only the register name.  Let's extract it and get
> +     the associated number.  */
> +  start = p->arg;
> +
> +  /* We assume the register name is composed by letters and numbers.  */
> +  while (isalnum (*p->arg))
> +    ++p->arg;
> +
> +  len = p->arg - start;
> +
> +  regname = alloca (len + gdb_reg_prefix_len + gdb_reg_suffix_len + 1);
> +  regname[0] = '\0';
> +
> +  /* We only add the GDB's register prefix/suffix if we are dealing with
> +     a numeric register.  */
> +  if (gdb_reg_prefix && isdigit (*start))
> +    {
> +      strncpy (regname, gdb_reg_prefix, gdb_reg_prefix_len);
> +      strncpy (regname + gdb_reg_prefix_len, start, len);
> +
> +      if (gdb_reg_suffix)
> +	strncpy (regname + gdb_reg_prefix_len + len,
> +		 gdb_reg_suffix, gdb_reg_suffix_len);
> +
> +      len += gdb_reg_prefix_len + gdb_reg_suffix_len;
> +    }
> +  else
> +    strncpy (regname, start, len);
> +
> +  regname[len] = '\0';

Empty line before comment.

> +  /* Is this a valid register name?  */
> +  if (user_reg_map_name_to_regnum (gdbarch, regname, len) == -1)
> +    error (_("Invalid register name `%s' on expression `%s'."),
> +	   regname, p->saved_arg);
> +
> +  write_exp_elt_opcode (OP_REGISTER);
> +  str.ptr = regname;
> +  str.length = len;
> +  write_exp_string (str);
> +  write_exp_elt_opcode (OP_REGISTER);
> +
> +  if (indirect_p)
> +    {
> +      if (disp_p)
> +	write_exp_elt_opcode (BINOP_ADD);
> +
> +      /* Casting to the expected type.  */
> +      write_exp_elt_opcode (UNOP_CAST);
> +      write_exp_elt_type (lookup_pointer_type (p->arg_type));
> +      write_exp_elt_opcode (UNOP_CAST);
> +
> +      write_exp_elt_opcode (UNOP_IND);
> +    }
> +
> +  /* Getting rid of the register name suffix.  */
> +  if (reg_suffix)
> +    {
> +      if (strncmp (p->arg, reg_suffix, reg_suffix_len) != 0)
> +	error (_("Missing register name suffix `%s' on expression `%s'."),
> +	       reg_suffix, p->saved_arg);
> +
> +      p->arg += reg_suffix_len;
> +    }
> +
> +  /* Getting rid of the register indirection suffix.  */
> +  if (indirect_p && reg_ind_suffix)
> +    {
> +      if (strncmp (p->arg, reg_ind_suffix, reg_ind_suffix_len) != 0)
> +	error (_("Missing indirection suffix `%s' on expression `%s'."),
> +	       reg_ind_suffix, p->saved_arg);
> +
> +      p->arg += reg_ind_suffix_len;
> +    }
> +}
> +
> +static void
> +stap_parse_single_operand (struct stap_parse_info *p)

Missing function comment.

> +{
> +  struct gdbarch *gdbarch = p->gdbarch;

Empty line before comment.

> +  /* Prefixes for the parser.  */
> +  const char *const_prefix = gdbarch_stap_integer_prefix (gdbarch);
> +  const char *reg_prefix = gdbarch_stap_register_prefix (gdbarch);
> +  const char *reg_ind_prefix
> +    = gdbarch_stap_register_indirection_prefix (gdbarch);
> +  int const_prefix_len = const_prefix ? strlen (const_prefix) : 0;
> +  int reg_prefix_len = reg_prefix ? strlen (reg_prefix) : 0;
> +  int reg_ind_prefix_len = reg_ind_prefix ? strlen (reg_ind_prefix) : 0;
> +
> +  /* Suffixes for the parser.  */
> +  const char *const_suffix = gdbarch_stap_integer_suffix (gdbarch);
> +  const char *reg_suffix = gdbarch_stap_register_suffix (gdbarch);
> +  const char *reg_ind_suffix
> +    = gdbarch_stap_register_indirection_suffix (gdbarch);
> +  int const_suffix_len = const_suffix ? strlen (const_suffix) : 0;
> +  int reg_suffix_len = reg_suffix ? strlen (reg_suffix) : 0;
> +  int reg_ind_suffix_len = reg_ind_suffix ? strlen (reg_ind_suffix) : 0;
> +
> +  /* We first try to parse this token as a "special token".  */
> +  if (gdbarch_stap_parse_special_token_p (gdbarch))
> +    {
> +      int ret = gdbarch_stap_parse_special_token (gdbarch, p);
> +
> +      if (ret)
> +	/* If the return value of the above function is not zero,
> +	   it means it successfully parsed the special token.
> +
> +	   If it is NULL, we try to parse it using our method.  */
> +	return;

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +    }
> +
> +  if (*p->arg == '-' || *p->arg == '~' || *p->arg == '+')
> +    {
> +      char c = *p->arg;

Empty line before comment.

> +      /* We use this variable to do a lookahead.  */
> +      const char *tmp = p->arg;
> +
> +      ++tmp;
> +
> +      /* This is an unary operation.  Here is a list of allowed tokens
> +	 here:
> +
> +	 - numeric literal;
> +	 - number (from register displacement)
> +	 - subexpression (beginning with `(')
> +
> +	 We handle the register displacement here, and the other cases
> +	 recursively.  */
> +      if (isdigit (*tmp))
> +	{
> +	  int number = strtol (tmp, (char **) &tmp, 10);
> +
> +	  if (p->inside_paren_p)
> +	    tmp = skip_spaces_const (tmp);
> +	  if (!reg_ind_prefix
> +	      || strncmp (tmp, reg_ind_prefix, reg_ind_prefix_len) != 0)
> +	    goto not_displacement;

Please no goto.  You can invert the condition and change 'else' below into new
'if' with some check for 'tmp == p->arg + 1' or even a new helper variable,
but no goto.


> +
> +	  /* If we are here, it means it is a displacement.  The only
> +	     operations allowed here are `-' and `+'.  */
> +	  if (c == '~')
> +	    error (_("Invalid operator `%c' for register displacement "
> +		     "on expression `%s'."), c, p->saved_arg);
> +
> +	  stap_parse_register_operand (p);
> +	}
> +      else
> +not_displacement:
> +	{
> +	  p->arg = tmp;
> +	  stap_parse_argument_conditionally (p);
> +	  if (c == '-')
> +	    write_exp_elt_opcode (UNOP_NEG);
> +	  else if (c == '~')
> +	    write_exp_elt_opcode (UNOP_COMPLEMENT);
> +	}
> +    }
> +  else if (isdigit (*p->arg))
> +    {
> +      /* A temporary variable, needed for lookahead.  */
> +      const char *tmp = p->arg;
> +      long number;
> +
> +      /* We can be dealing with a numeric constant (if `const_prefix' is
> +	 NULL), or with a register displacement.  */
> +      number = strtol (tmp, (char **) &tmp, 10);
> +
> +      if (p->inside_paren_p)
> +	tmp = skip_spaces_const (tmp);
> +      if (!const_prefix && reg_ind_prefix
> +	  && strncmp (tmp, reg_ind_prefix, reg_ind_prefix_len) != 0)
> +	{
> +	  /* We are dealing with a numeric constant.  */
> +	  write_exp_elt_opcode (OP_LONG);
> +	  write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> +	  write_exp_elt_longcst (number);
> +	  write_exp_elt_opcode (OP_LONG);
> +
> +	  p->arg = tmp;
> +
> +	  if (const_suffix)
> +	    {
> +	      if (strncmp (p->arg, const_suffix, const_suffix_len) == 0)
> +		p->arg += const_suffix_len;
> +	      else
> +		error (_("Invalid constant suffix on expression `%s'."),
> +		       p->saved_arg);
> +	    }
> +	}
> +      else if (reg_ind_prefix
> +	       && strncmp (tmp, reg_ind_prefix, reg_ind_prefix_len) == 0)
> +	stap_parse_register_operand (p);
> +      else
> +	error (_("Unknown numeric token on expression `%s'."),
> +	       p->saved_arg);
> +    }
> +  else if (const_prefix
> +	   && strncmp (p->arg, const_prefix, const_prefix_len) == 0)
> +    {
> +      /* We are dealing with a numeric constant.  */
> +      long number;
> +
> +      p->arg += const_prefix_len;
> +      number = strtol (p->arg, (char **) &p->arg, 10);
> +
> +      write_exp_elt_opcode (OP_LONG);
> +      write_exp_elt_type (builtin_type (gdbarch)->builtin_long);
> +      write_exp_elt_longcst (number);
> +      write_exp_elt_opcode (OP_LONG);
> +
> +      if (const_suffix)
> +	{
> +	  if (strncmp (p->arg, const_suffix, const_suffix_len) == 0)
> +	    p->arg += const_suffix_len;
> +	  else
> +	    error (_("Invalid constant suffix on expression `%s'."),
> +		   p->saved_arg);
> +	}
> +    }
> +  else if ((reg_prefix
> +	    && strncmp (p->arg, reg_prefix, reg_prefix_len) == 0)
> +	   || (reg_ind_prefix
> +	       && strncmp (p->arg, reg_ind_prefix, reg_ind_prefix_len) == 0))
> +    stap_parse_register_operand (p);
> +  else
> +    error (_("Operator `%c' not recognized on expression `%s'."),
> +	   *p->arg, p->saved_arg);
> +}
> +
> +static void
> +stap_parse_argument_conditionally (struct stap_parse_info *p)

Missing function comment.

> +{
> +  if (*p->arg == '-' || *p->arg == '~' || *p->arg == '+' /* Unary.  */
> +      || isdigit (*p->arg)
> +      || gdbarch_stap_is_single_operand (p->gdbarch, p->arg))
> +    stap_parse_single_operand (p);
> +  else if (*p->arg == '(')
> +    {
> +      /* We are dealing with a parenthesized operand.  It means we
> +	 have to parse it as it was a separate expression, without
> +	 left-side or precedence.  */
> +      ++p->arg;
> +      p->arg = skip_spaces_const (p->arg);
> +      ++p->inside_paren_p;
> +
> +      stap_parse_argument_1 (p, 0, STAP_OPERAND_PREC_NONE);
> +
> +      --p->inside_paren_p;
> +      if (*p->arg != ')')
> +	error (_("Missign close-paren on expression `%s'."),
> +	       p->saved_arg);
> +
> +      ++p->arg;
> +      if (p->inside_paren_p)
> +	p->arg = skip_spaces_const (p->arg);
> +    }
> +  else
> +    error (_("Cannot parse expression `%s'."), p->saved_arg);
> +}
> +
> +static void
> +stap_parse_argument_1 (struct stap_parse_info *p, int has_lhs,
> +		       enum stap_operand_prec prec)

Missing function comment.

> +{
> +  /* This is an operator-precedence parser.
> +
> +     We work with left- and right-sides of expressions, and
> +     parse them depending on the precedence of the operators
> +     we find.  */
> +
> +  if (p->inside_paren_p)
> +    p->arg = skip_spaces_const (p->arg);
> +
> +  if (!has_lhs)
> +    /* We were called without a left-side, either because this is the
> +       first call, or because we were called to parse a parenthesized
> +       expression.  It doesn't really matter; we have to parse the
> +       left-side in order to continue the process.  */
> +    stap_parse_argument_conditionally (p);

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +
> +  /* Start to parse the right-side, and to "join" left and right sides
> +     depending on the operation specified.
> +
> +     This loop shall continue until we run out of characters in the input,
> +     or until we find a close-parenthesis, which means that we've reached
> +     the end of a sub-expression.  */
> +  while (p->arg && *p->arg && *p->arg != ')' && !isspace (*p->arg))
> +    {
> +      const char *tmp_exp_buf;
> +      enum exp_opcode opcode;
> +      enum stap_operand_prec cur_prec;
> +
> +      if (!stap_is_operator (*p->arg))
> +	error (_("Invalid operator `%c' on expression `%s'."), *p->arg,
> +	       p->saved_arg);
> +
> +      /* We have to save the current value of the expression buffer because
> +	 the `stap_get_opcode' modifies it in order to get the current
> +	 operator.  If this operator's precedence is lower than PREC, we
> +	 should return and not advance the expression buffer pointer.  */
> +      tmp_exp_buf = p->arg;
> +      stap_get_opcode (&tmp_exp_buf, &opcode);
> +
> +      cur_prec = stap_get_operator_prec (opcode);
> +      if (cur_prec < prec)
> +	/* If the precedence of the operator that we are seeing now is
> +	   lower than the precedence of the first operator seen before
> +	   this parsing process began, it means we should stop parsing
> +	   and return.  */
> +	break;

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +
> +      p->arg = tmp_exp_buf;
> +      if (p->inside_paren_p)
> +	p->arg = skip_spaces_const (p->arg);
> +
> +      /* Parse the right-side of the expression.  */
> +      stap_parse_argument_conditionally (p);
> +
> +      /* While we still have operators, try to parse another
> +	 right-side, but using the current right-side as a left-side.  */
> +      while (*p->arg && stap_is_operator (*p->arg))
> +	{
> +	  enum exp_opcode lookahead_opcode;
> +	  enum stap_operand_prec lookahead_prec;
> +
> +	  /* Saving the current expression buffer position.  The explanation
> +	     is the same as above.  */
> +	  tmp_exp_buf = p->arg;
> +	  stap_get_opcode (&tmp_exp_buf, &lookahead_opcode);
> +	  lookahead_prec = stap_get_operator_prec (lookahead_opcode);
> +
> +	  if (lookahead_prec <= prec)
> +	    /* If we are dealing with an operator whose precedence is lower
> +	       than the first one, just abandon the attempt.  */
> +	    break;

These multiple lines should be in { block } according to new GDB Coding Style rule.

> +
> +	  /* Parse the right-side of the expression, but since we already
> +	     have a left-side at this point, set `has_lhs' to 1.  */
> +	  stap_parse_argument_1 (p, 1, lookahead_prec);
> +	}
> +
> +      write_exp_elt_opcode (opcode);
> +    }
> +}
> +
> +/* Parse a probe's argument.
> +
> +   Assuming that:
> +
> +   LP = literal integer prefix
> +   LS = literal integer suffix
> +
> +   RP = register prefix
> +   RS = register suffix
> +
> +   RIP = register indirection prefix
> +   RIS = register indirection suffix
> +
> +   This routine assumes that arguments' tokens are of the form:
> +
> +   - [LP] NUMBER [LS]
> +   - [RP] REGISTER [RS]
> +   - [RIP] [RP] REGISTER [RS] [RIS]
> +   - If we find a number without LP, we try to parse it as a literal integer
> +   constant (if LP == NULL), or as a register displacement.
> +   - We count parenthesis, and only skip whitespaces if we are inside them.
> +   - If we find an operator, we skip it.
> +
> +   This function can also call a special function that will try to match
> +   unknown tokens.  It will return 1 if the argument has been parsed
> +   successfully, or zero otherwise.  */
> +
> +static int
> +stap_parse_argument (const char **arg, struct type *atype,
> +		     struct gdbarch *gdbarch)
> +{
> +  struct stap_parse_info p;
> +  volatile struct gdb_exception e;
> +
> +  /* We need to initialize the expression buffer, in order to begin
> +     our parsing efforts.  The language here does not matter, since we
> +     are using our own parser.  */
> +  initialize_expout (10, current_language, gdbarch);

Put back_to = make_cleanup (free_current_contents, &expout); here.

> +
> +  p.saved_arg = *arg;
> +  p.arg = *arg;
> +  p.arg_type = atype;
> +  p.gdbarch = gdbarch;
> +  p.inside_paren_p = 0;
> +
> +  TRY_CATCH (e, RETURN_MASK_ERROR)
> +    {
> +      stap_parse_argument_1 (&p, 0, STAP_OPERAND_PREC_NONE);
> +    }
> +  if (e.reason < 0)
> +    {
> +      xfree (expout);
> +      return 0;
> +    }

And drop TRY_CATCH magic here with just discard_cleanups (back_to);
here (therefore if stap_parse_argument_1 does not throw).

Make possible this function throws.  Make the function return always
non-NULL 'struct expression *' so that the callers no longer have to deal with
the global variable mess underneath.

Then the caller will have TRY_CATCH and it can display by complaint that
e.message which happened.  In general if we have exceptions they are fine to
use IMO.  Hopefully the exception system will be more sane in the future.

Maybe this is more a personal preference of this function redesign, feel free
to discuss it more.


> +
> +  gdb_assert (p.inside_paren_p == 0);
> +
> +  /* Casting the final expression to the appropriate type.  */
> +  write_exp_elt_opcode (UNOP_CAST);
> +  write_exp_elt_type (atype);
> +  write_exp_elt_opcode (UNOP_CAST);
> +
> +  reallocate_expout ();
> +
> +  p.arg = skip_spaces_const (p.arg);
> +  *arg = p.arg;
> +
> +  return 1;
> +}
> +
> +/* Helper function which is responsible for freeing the space allocated to
> +   hold information about a probe's arguments.  */
> +
> +static void
> +stap_free_args_info (void *args_info_ptr)
> +{
> +  struct stap_args_info *a = (struct stap_args_info *) args_info_ptr;
> +  int i;
> +
> +  for (i = 0; i < a->n_args; i++)
> +    xfree (a->args[i].aexpr);
> +
> +  xfree (a->args);
> +  xfree (a);
> +}
> +
> +/* Function which parses an argument string from PROBE, correctly splitting
> +   the arguments and storing their information in properly ways.
> +
> +   Consider the following argument string (x86 syntax):
> +
> +   `4@%eax 4@$10'
> +
> +   We have two arguments, `%eax' and `$10', both with 32-bit unsigned bitness.
> +   This function basically handles them, properly filling some structures with
> +   this information.  */
> +
> +static void
> +stap_parse_probe_arguments (struct stap_probe *probe)
> +{
> +  struct stap_args_info *args_info;
> +  struct cleanup *back_to;
> +  const char *cur = probe->args;
> +  int current_arg = -1;

Empty line before comment.

> +  /* This is a state-machine parser, which means we will always be
> +     in a known state when parsing an argument.  The state could be
> +     either `NEW_ARG' if we are parsing a new argument, `BITNESS' if
> +     we are parsing the bitness-definition part (i.e., `4@'), or
> +     `PARSE_ARG' if we are actually parsing the argument part.  */
> +  enum
> +    {
> +      NEW_ARG,
> +      BITNESS,
> +      PARSE_ARG,
> +    } current_state;
> +
> +  /* For now, we assume everything is not going to work.  */
> +  probe->parsed_args = &dummy_stap_args_info;
> +
> +  if (!cur || !*cur || *cur == ':')
> +    return;
> +
> +  args_info = xmalloc (sizeof (struct stap_args_info));
> +  args_info->n_args = 0;
> +  back_to = make_cleanup (stap_free_args_info, args_info);
> +  args_info->args = xcalloc (STAP_MAX_ARGS, sizeof (struct stap_probe_arg));
> +
> +  /* Ok, let's start.  */

Excessive comment.

> +  current_state = NEW_ARG;
> +
> +  while (*cur)
> +    {
> +      switch (current_state)
> +	{
> +	case NEW_ARG:
> +	  ++current_arg;
> +
> +	  if (current_arg >= STAP_MAX_ARGS)
> +	    {
> +	      complaint (&symfile_complaints,
> +			 _("probe `%s' has more arguments than the maximum "
> +			   "allowed"), probe->name);
> +	      do_cleanups (back_to);
> +	      return;
> +	    }
> +
> +	  current_state = BITNESS;
> +	  break;
> +
> +	case BITNESS:
> +	    {
> +	      enum stap_arg_bitness b;
> +	      int got_minus = 0;
> +
> +	      /* We expect to find something like:
> +
> +		 N@OP
> +
> +		 Where `N' can be [+,-][4,8].  This is not mandatory, so
> +		 we check it here.  If we don't find it, go to the next
> +		 state.  */
> +	      if ((*cur == '-' && cur[1] && cur[2] != '@')
> +		  && cur[1] != '@')
> +		{
> +		  current_state = PARSE_ARG;
> +		  args_info->args[current_arg].bitness
> +		    = STAP_ARG_BITNESS_UNDEFINED;
> +		  break;
> +		}
> +
> +	      if (*cur == '-')
> +		{
> +		  /* Discard the `-'.  */
> +		  ++cur;
> +		  got_minus = 1;
> +		}
> +
> +	      if (*cur == '4')
> +		b = got_minus ? STAP_ARG_BITNESS_32BIT_SIGNED
> +		  : STAP_ARG_BITNESS_32BIT_UNSIGNED;

Here should be parentheses according to GNU Coding Style.

> +	      else if (*cur == '8')
> +		b = got_minus ? STAP_ARG_BITNESS_64BIT_SIGNED
> +		  : STAP_ARG_BITNESS_64BIT_UNSIGNED;

Here should be parentheses according to GNU Coding Style.

> +	      else
> +		{
> +		  /* We have an error, because we don't expect anything
> +		     except 4 and 8.  */
> +		  complaint (&symfile_complaints,
> +			     _("unrecognized bitness `%c' for probe `%s'"),
> +			     *cur, probe->name);
> +		  do_cleanups (back_to);
> +		  return;
> +		}
> +
> +	      args_info->args[current_arg].bitness = b;
> +	      args_info->args[current_arg].atype
> +		= stap_get_expected_argument_type (probe->gdbarch, b);

Empty line before comment.

> +	      /* Discard the number and the `@' sign.  */
> +	      cur += 2;

Empty line before comment.

> +	      /* Move on.  */
> +	      current_state = PARSE_ARG;
> +	    }
> +	  break;
> +
> +	case PARSE_ARG:
> +	    {
> +	      if (!stap_parse_argument (&cur,
> +					args_info->args[current_arg].atype,
> +					probe->gdbarch))
> +		{
> +		  /* We have tried to parse this argument, but it's
> +		     malformed.  This is an error.  */
> +		  complaint (&symfile_complaints,
> +			     _("malformed argument for probe `%s'"),
> +			     probe->name);
> +		  do_cleanups (back_to);
> +		  return;
> +		}
> +
> +	      if (stap_expression_debug)
> +		dump_raw_expression (expout, gdb_stdlog,
> +				     "before conversion to prefix form");
> +
> +	      prefixify_expression (expout);
> +
> +	      if (stap_expression_debug)
> +		dump_prefix_expression (expout, gdb_stdlog);
> +
> +	      args_info->args[current_arg].aexpr = expout;
> +	      expout = NULL;
> +
> +	      ++args_info->n_args;

Empty line before comment.

> +	      /* Start it over again.  */
> +	      cur = skip_spaces_const (cur);
> +	      current_state = NEW_ARG;
> +	    }
> +	  break;
> +	}
> +
> +      if (!*cur && current_state != NEW_ARG)
> +	{
> +	  /* We reached the end of the argument string, but we're
> +	     still in the middle of the process of parsing an argument.
> +	     It means the argument string is malformed.  */
> +	  complaint (&symfile_complaints,
> +		     _("malformed argument for probe `%s'"),
> +		     probe->name);
> +	  do_cleanups (back_to);
> +	  return;
> +	}
> +    }
> +
> +  args_info->args = xrealloc (args_info->args,
> +			      args_info->n_args
> +			      * sizeof (struct stap_probe_arg));
> +  args_info->probe = probe;
> +
> +  probe->parsed_args = args_info;
> +
> +  discard_cleanups (back_to);
> +}
[...]
> +}
> +
> +struct value *
> +stap_safe_evaluate_at_pc (struct frame_info *frame, int n)

Missing function comment.

> +{
> +  struct stap_probe *probe;
> +  struct objfile *objfile;
> +  int n_probes;
> +
> +  probe = find_probe_by_pc (get_frame_pc (frame), &objfile);
> +  if (!probe)
> +    return NULL;
> +  gdb_assert (objfile->sf && objfile->sf->sym_probe_fns);
> +
> +  n_probes
> +    = objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> +								probe);
> +  if (n >= n_probes)
> +    return NULL;
> +
> +  return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
> +								  probe,
> +								  frame,
> +								  n);
> +}
> +
> +/* This function frees the space allocated to hold information about
> +   the probe's parsed arguments.  */
> +
> +void
> +stap_free_parsed_args (struct stap_args_info *parsed_args)
> +{
> +  int i;
> +
> +  if (!parsed_args
> +      || parsed_args == &dummy_stap_args_info
> +      || parsed_args->n_args == 0)
> +    return;
> +
> +  for (i = 0; i < parsed_args->n_args; i++)
> +    xfree (parsed_args->args[i].aexpr);
> +
> +  xfree (parsed_args->args);
> +  xfree (parsed_args);
> +}
> +
> +/* A utility structure.  A VEC of these is built when handling "info
> +   probes".  */
> +
> +struct stap_probe_and_objfile
> +{
> +  /* The probe.  */
> +  struct stap_probe *probe;

Empty line before comment.

> +  /* The probe's objfile.  */
> +  struct objfile *objfile;
> +};
> +
> +typedef struct stap_probe_and_objfile stap_entry;
> +DEF_VEC_O (stap_entry);
> +
> +/* A helper function for collect_probes that compiles a regexp and
> +   throws an exception on error.  This installs a cleanup to free the
> +   resulting pattern on success.  If RX is NULL, this does nothing.  */
> +
> +static void
> +compile_rx_or_error (regex_t *pattern, const char *rx, const char *message)
> +{
> +  int code;
> +
> +  if (!rx)
> +    return;
> +
> +  code = regcomp (pattern, rx, REG_NOSUB);
> +  if (code == 0)
> +    make_regfree_cleanup (pattern);
> +  else
> +    {
> +      char *err = get_regcomp_error (code, pattern);
> +
> +      make_cleanup (xfree, err);
> +      error (_("%s: %s"), message, err);

Excessive localization _().

> +    }
> +}
> +
> +/* Make a vector of probes matching OBJNAME, PROVIDER, and PROBE.
> +   Each argument is a regexp, or NULL, which matches anything.  */
> +
> +static VEC (stap_entry) *
> +collect_probes (char *objname, char *provider, char *probe)
> +{
> +  struct objfile *objfile;
> +  VEC (stap_entry) *result = NULL;
> +  struct cleanup *cleanup;
> +  regex_t obj_pat, prov_pat, probe_pat;
> +
> +  cleanup = make_cleanup (VEC_cleanup (stap_entry), &result);
> +
> +  compile_rx_or_error (&prov_pat, provider, _("Invalid provider regexp"));
> +  compile_rx_or_error (&probe_pat, probe, _("Invalid probe regexp"));
> +  compile_rx_or_error (&obj_pat, objname, _("Invalid object file regexp"));
> +
> +  ALL_OBJFILES (objfile)
> +  {
> +    struct stap_probe *probes;
> +    int i, num_probes;
> +
> +    if (! objfile->sf || ! objfile->sf->sym_probe_fns)
> +      continue;
> +
> +    if (objname)
> +      {
> +	if (regexec (&obj_pat, objfile->name, 0, NULL, 0) != 0)
> +	  continue;
> +      }
> +
> +    probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile, &num_probes);
> +    for (i = 0; i < num_probes; ++i)
> +      {
> +	stap_entry entry;
> +
> +	if (provider)
> +	  {
> +	    if (regexec (&prov_pat, probes[i].provider, 0, NULL, 0) != 0)
> +	      continue;
> +	  }
> +
> +	if (probe)
> +	  {
> +	    if (regexec (&probe_pat, probes[i].name, 0, NULL, 0) != 0)
> +	      continue;
> +	  }
> +
> +	entry.probe = &probes[i];
> +	entry.objfile = objfile;
> +	VEC_safe_push (stap_entry, result, &entry);
> +      }
> +  }
> +
> +  discard_cleanups (cleanup);

Here you leak PROV_PAT, PROBE_PAT and OBJ_PAT.


> +  return result;
> +}
> +
> +/* A qsort comparison function for stap_entry objects.  */
> +
> +static int
> +compare_entries (const void *a, const void *b)
> +{
> +  const stap_entry *ea = a;
> +  const stap_entry *eb = b;
> +  int v;
> +
> +  v = strcmp (ea->probe->provider, eb->probe->provider);

handle_probe can create NULL probe->provider or NULL probe->name etc.  But
other code like this one will crash on it.

I believe such non-valid probes rather should be dropped during parsing.

Sorry if I missed it is already handled somehow?


> +  if (v)
> +    return v;
> +
> +  v = strcmp (ea->probe->name, eb->probe->name);
> +  if (v)
> +    return v;
> +
> +  if (ea->probe->address < eb->probe->address)
> +    return -1;
> +  if (ea->probe->address > eb->probe->address)
> +    return 1;
> +
> +  return strcmp (ea->objfile->name, eb->objfile->name);
> +}
> +
> +/* Implementation of the "info probes" command.  */
> +
> +static void
> +info_probes_command (char *arg, int from_tty)
> +{
> +  char *provider, *probe = NULL, *objname = NULL;
> +  struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
> +  VEC (stap_entry) *items;
> +  int i, addr_width, any_found;
> +  stap_entry *entry;
> +
> +  provider = extract_arg (&arg);
> +  if (provider)
> +    {
> +      make_cleanup (xfree, provider);
> +
> +      probe = extract_arg (&arg);
> +      if (probe)
> +	{
> +	  make_cleanup (xfree, probe);
> +
> +	  objname = extract_arg (&arg);
> +	  if (objname)
> +	    make_cleanup (xfree, objname);
> +	}
> +    }
> +
> +  items = collect_probes (objname, provider, probe);
> +  make_cleanup (VEC_cleanup (stap_entry), &items);
> +  make_cleanup_ui_out_table_begin_end (current_uiout, 5,
> +				       VEC_length (stap_entry, items),
> +				       "SystemTapProbes");
> +
> +  if (! VEC_empty (stap_entry, items))
> +    qsort (VEC_address (stap_entry, items),
> +	   VEC_length (stap_entry, items),
> +	   sizeof (stap_entry),
> +	   compare_entries);
> +
> +  addr_width = 4 + (gdbarch_ptr_bit (get_current_arch ()) / 4);
> +
> +  ui_out_table_header (current_uiout, 10, ui_left, "provider", _("Provider"));
> +  ui_out_table_header (current_uiout, 10, ui_left, "name", _("Name"));
> +  ui_out_table_header (current_uiout, addr_width - 1, ui_left, "addr", _("Where"));
> +  ui_out_table_header (current_uiout, addr_width - 1, ui_left, "semaphore",
> +		       _("Semaphore"));
> +  ui_out_table_header (current_uiout, 30, ui_left, "object", _("Object"));
> +  ui_out_table_body (current_uiout);
> +
> +  for (i = 0; VEC_iterate (stap_entry, items, i, entry); ++i)
> +    {
> +      struct cleanup *inner;
> +
> +      inner = make_cleanup_ui_out_tuple_begin_end (current_uiout, "probe");
> +
> +      ui_out_field_string (current_uiout, "provider", entry->probe->provider);
> +      ui_out_field_string (current_uiout, "name", entry->probe->name);
> +      ui_out_field_core_addr (current_uiout, "addr", get_current_arch (),
> +			      entry->probe->address);
> +      if (entry->probe->sem_addr == 0)
> +	ui_out_field_skip (current_uiout, "semaphore");
> +      else
> +      ui_out_field_core_addr (current_uiout, "semaphore", get_current_arch (),
> +			      entry->probe->sem_addr);
> +      ui_out_field_string (current_uiout, "object", entry->objfile->name);
> +      ui_out_text (current_uiout, "\n");
> +
> +      do_cleanups (inner);
> +    }
> +
> +  any_found = ! VEC_empty (stap_entry, items);

Excessive space after ! according to GDB Coding Style.


> +  do_cleanups (cleanup);
> +
> +  if (! any_found)
> +    ui_out_message (current_uiout, 0, _("No probes matched.\n"));
> +}
> +
> +
> +
> +/* See definition in stap-probe.h.  */
> +
> +VEC (stap_probe_p) *
> +find_probes_in_objfile (struct objfile *objfile,
> +			const char *provider,
> +			const char *name)
> +{
> +  struct stap_probe *probes;
> +  int i, num_probes;
> +  VEC (stap_probe_p) *result = NULL;
> +
> +  if (! objfile->sf || ! objfile->sf->sym_probe_fns)
> +    return NULL;
> +
> +  probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile, &num_probes);
> +  for (i = 0; i < num_probes; ++i)
> +    {
> +      if (strcmp (probes[i].provider, provider) != 0)
> +	continue;
> +
> +      if (strcmp (probes[i].name, name) != 0)
> +	continue;
> +
> +      VEC_safe_push (stap_probe_p, result, &probes[i]);
> +    }
> +
> +  return result;
> +}
> +
> +/* See definition in stap-probe.h.  */
> +
> +struct symtabs_and_lines
> +parse_stap_probe (char **argptr, struct linespec_result *canonical)
> +{
> +  char *arg_start, *arg_end, *arg;
> +  char *objfile_name = NULL, *provider = NULL, *name, *p;
> +  struct cleanup *cleanup;
> +  struct symtabs_and_lines result;
> +  struct objfile *objfile;
> +  struct program_space *pspace;
> +
> +  result.sals = NULL;
> +  result.nelts = 0;
> +
> +  arg_start = *argptr;

Empty line before comment.

> +  /* The caller ensured that this starts with `-p' or `-probe'.  */
> +  gdb_assert (arg_start
> +	      && ((strncmp (arg_start, "-p", 2) == 0
> +		   && isspace (arg_start[2]))
> +		  || (strncmp (arg_start, "-probe", 6) == 0
> +		      && isspace (arg_start[6]))));
> +
> +  if (strncmp (arg_start, "-probe", 6) == 0)
> +    arg_end = arg_start + 6;
> +  else
> +    arg_end = arg_start + 2;
> +  arg_end = skip_spaces (arg_end);
> +
> +  if (!*arg_end)
> +    error (_("argument to `%s' missing"),
> +	   strncmp (arg_start, "-probe", 6) == 0 ? "-probe" : "-p");
> +
> +  arg = arg_end;
> +  arg_end = skip_to_space (arg_end);
> +
> +  /* We make a copy here so we can write over parts with impunity.  */
> +  arg = savestring (arg, arg_end - arg);
> +  cleanup = make_cleanup (xfree, arg);
> +
> +  /* Extract each word from the argument, separated by ":"s.  */
> +  p = strchr (arg, ':');
> +  if (p == NULL)
> +    {
> +      /* This is `-p name'.  */
> +      name = arg;
> +    }
> +  else
> +    {
> +      char *hold = p + 1;
> +
> +      *p = '\0';
> +      p = strchr (hold, ':');
> +      if (p == NULL)
> +	{
> +	  /* This is `-p provider:name'.  */
> +	  provider = arg;
> +	  name = hold;
> +	}
> +      else
> +	{
> +	  /* This is `-p objfile:provider:name'.  */
> +	  *p = '\0';
> +	  objfile_name = arg;
> +	  provider = hold;
> +	  name = p + 1;
> +	}
> +    }
> +
> +  if (*name == '\0')
> +    error (_("no probe name specified"));
> +  if (provider && *provider == '\0')
> +    error (_("invalid provider name"));
> +  if (objfile_name && *objfile_name == '\0')
> +    error (_("invalid objfile name"));
> +
> +  ALL_PSPACES (pspace)
> +    {

I would find this { block } and its 2 space indentation excessive.


> +      ALL_PSPACE_OBJFILES (pspace, objfile)
> +	{
> +	  struct stap_probe *probes;
> +	  int i, num_probes;
> +
> +	  if (! objfile->sf || ! objfile->sf->sym_probe_fns)
> +	    continue;
> +
> +	  if (objfile_name
> +	      && FILENAME_CMP (objfile->name, objfile_name) != 0
> +	      && FILENAME_CMP (lbasename (objfile->name), objfile_name) != 0)
> +	    continue;
> +
> +	  probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile,
> +							       &num_probes);
> +	  for (i = 0; i < num_probes; ++i)
> +	    {
> +	      struct symtab_and_line *sal;
> +
> +	      if (provider && strcmp (probes[i].provider, provider) != 0)
> +		continue;
> +
> +	      if (strcmp (probes[i].name, name) != 0)
> +		continue;
> +
> +	      ++result.nelts;
> +	      result.sals = xrealloc (result.sals,
> +				      result.nelts
> +				      * sizeof (struct symtab_and_line));
> +	      sal = &result.sals[result.nelts - 1];
> +
> +	      init_sal (sal);
> +
> +	      sal->pc = probes[i].address;
> +	      sal->explicit_pc = 1;
> +	      sal->section = find_pc_overlay (sal->pc);
> +	      sal->pspace = current_program_space;

Here should be pspace;

> +	      sal->semaphore = probes[i].sem_addr;
> +	    }
> +	}
> +    }
> +
> +  if (result.nelts == 0)
> +    {
> +      throw_error (NOT_FOUND_ERROR,
> +		   _("No probe matching objfile=`%s', provider=`%s', name=`%s'"),
> +		   objfile_name ? objfile_name : _("<any>"),
> +		   provider ? provider : _("<any>"),
> +		   name);
> +    }
> +
> +  if (canonical)
> +    {
> +      canonical->special_display = 1;
> +      canonical->pre_expanded = 1;
> +      canonical->addr_string = savestring (*argptr, arg_end - *argptr);
> +    }
> +
> +  *argptr = arg_end;
> +  do_cleanups (cleanup);
> +
> +  return result;
> +}
> +
> +
> +
> +/* See definition in stap-probe.h.  */
> +
> +struct stap_probe *
> +find_probe_by_pc (CORE_ADDR pc, struct objfile **objfile_out)
> +{
> +  struct objfile *objfile;
> +
> +  ALL_OBJFILES (objfile)
> +  {
> +    struct stap_probe *probes;
> +    int i, num_probes;
> +    stap_entry entry;
> +
> +    if (! objfile->sf || ! objfile->sf->sym_probe_fns)

Excessive space after ! according to GDB Coding Style.

> +      continue;
> +
> +    /* If this proves too inefficient, we can replace with a hash.  */
> +    probes = objfile->sf->sym_probe_fns->sym_get_probes (objfile, &num_probes);
> +    for (i = 0; i < num_probes; ++i)
> +      {
> +	if (probes[i].address == pc)
> +	  {
> +	    *objfile_out = objfile;
> +	    return &probes[i];
> +	  }
> +      }
> +  }
> +
> +  return NULL;
> +}
> +
> +/* This is called to compute the value of one of the $_probe_arg*
> +   convenience variables.  */
> +
> +static struct value *
> +compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
> +		   void *data)
> +{
> +  struct frame_info *frame = get_selected_frame (_("No frame selected"));
> +  CORE_ADDR pc = get_frame_pc (frame);
> +  int sel = (int) (uintptr_t) data;
> +  struct objfile *objfile;
> +  struct stap_probe *pc_probe;
> +  int n_probes;
> +
> +  /* SEL==-1 means "_probe_argc".  */

     /* SEL == -1 ... */

> +  gdb_assert (sel >= -1 && sel <= STAP_MAX_ARGS);
> +
> +  pc_probe = find_probe_by_pc (pc, &objfile);
> +  if (pc_probe == NULL)
> +    error (_("No SystemTap probe at PC %s"), core_addr_to_string (pc));
> +
> +  n_probes
> +    = objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> +								pc_probe);
> +  if (sel == -1)
> +    return value_from_longest (builtin_type (arch)->builtin_int, n_probes);
> +
> +  if (sel >= n_probes)
> +    error (_("Invalid probe argument %d -- probe has %d arguments available"),
> +	   sel, n_probes);
> +
> +  return objfile->sf->sym_probe_fns->sym_evaluate_probe_argument (objfile,
> +								  pc_probe,
> +								  frame, sel);
> +}
> +
> +/* This is called to compile one of the $_probe_arg* convenience
> +   variables into an agent expression.  */
> +
> +static void
> +compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
> +		   struct axs_value *value, void *data)
> +{
> +  CORE_ADDR pc = expr->scope;
> +  int sel = (int) (uintptr_t) data;
> +  struct objfile *objfile;
> +  struct stap_probe *pc_probe;
> +  int n_probes;
> +
> +  /* SEL==-1 means "_probe_argc".  */
> +  gdb_assert (sel >= -1 && sel <= STAP_MAX_ARGS);
> +
> +  pc_probe = find_probe_by_pc (pc, &objfile);
> +  if (pc_probe == NULL)
> +    error (_("No SystemTap probe at PC %s"), core_addr_to_string (pc));
> +
> +  n_probes
> +    = objfile->sf->sym_probe_fns->sym_get_probe_argument_count (objfile,
> +								pc_probe);
> +  if (sel == -1)
> +    {
> +      value->kind = axs_rvalue;
> +      value->type = builtin_type (expr->gdbarch)->builtin_int;
> +      ax_const_l (expr, n_probes);
> +      return;
> +    }
> +
> +  gdb_assert (sel >= 0);
> +  if (sel >= n_probes)
> +    error (_("Invalid probe argument %d -- probe has %d arguments available"),
> +	   sel, n_probes);
> +
> +  objfile->sf->sym_probe_fns->sym_compile_to_ax (objfile, pc_probe,
> +						 expr, value, sel);
> +}
> +
> +
> +
> +/* Implementation of `$_probe_arg*' set of variables.  */
> +
> +static const struct internalvar_funcs probe_funcs =
> +{
> +  compute_probe_arg,
> +  compile_probe_arg,
> +  NULL
> +};

My only serious concern about this patch is some missing abstraction.  These
PROBE_FUNCS seem to be SystemTap-indepdent but the backeng-independent
compile_probe_arg already deals with 'struct stap_probe'.  Any reference to
'struct stap_probe' should have been only in elf_compile_to_ax.

Moreover I would still more see to drop [patch 1/3], call just
compute_probe_arg which returns lazy lval_computed struct value * which
provides new struct lval_funcs member which can return `struct expression *'
and generic code can call gen_expr on its own.  There is no need for special
internalvar_funcs->compile_to_ax member.

internalvar_funcs->destroy can be also replaced by lval_funcs->free_closure.


> +
> +void _initialize_stap_probe (void);
> +
> +void
> +_initialize_stap_probe (void)
> +{
> +  add_info ("probes", info_probes_command, _("\
> +Show available static probes.\n\
> +Usage: info probes [PROVIDER [NAME [OBJECT]]]\n\
> +Each argument is a regular expression, used to select probes.\n\
> +PROVIDER matches probe provider names.\n\
> +NAME matches the probe names.\n\
> +OBJECT match the executable or shared library name."));
> +
> +  add_setshow_zinteger_cmd ("stap-expression", class_maintenance,
> +			    &stap_expression_debug,
> +			    _("Set SystemTap expression debugging."),
> +			    _("Show SystemTap expression debugging."),
> +			    _("When non-zero, the internal representation "
> +			      "of SystemTap expressions will be printed."),
> +			    NULL,
> +			    show_stapexpressiondebug,
> +			    &setdebuglist, &showdebuglist);
> +
> +  create_internalvar_type_lazy ("_probe_argc", &probe_funcs,
> +				(void *) (uintptr_t) -1);
> +  create_internalvar_type_lazy ("_probe_arg0", &probe_funcs,
> +				(void *) (uintptr_t) 0);
> +  create_internalvar_type_lazy ("_probe_arg1", &probe_funcs,
> +				(void *) (uintptr_t) 1);
> +  create_internalvar_type_lazy ("_probe_arg2", &probe_funcs,
> +				(void *) (uintptr_t) 2);
> +  create_internalvar_type_lazy ("_probe_arg3", &probe_funcs,
> +				(void *) (uintptr_t) 3);
> +  create_internalvar_type_lazy ("_probe_arg4", &probe_funcs,
> +				(void *) (uintptr_t) 4);
> +  create_internalvar_type_lazy ("_probe_arg5", &probe_funcs,
> +				(void *) (uintptr_t) 5);
> +  create_internalvar_type_lazy ("_probe_arg6", &probe_funcs,
> +				(void *) (uintptr_t) 6);
> +  create_internalvar_type_lazy ("_probe_arg7", &probe_funcs,
> +				(void *) (uintptr_t) 7);
> +  create_internalvar_type_lazy ("_probe_arg8", &probe_funcs,
> +				(void *) (uintptr_t) 8);
> +  create_internalvar_type_lazy ("_probe_arg9", &probe_funcs,
> +				(void *) (uintptr_t) 9);
> +  create_internalvar_type_lazy ("_probe_arg10", &probe_funcs,
> +				(void *) (uintptr_t) 10);
> +  create_internalvar_type_lazy ("_probe_arg11", &probe_funcs,
> +				(void *) (uintptr_t) 11);
> +}
> diff --git a/gdb/stap-probe.h b/gdb/stap-probe.h
> new file mode 100644
> index 0000000..9b6dc7a
> --- /dev/null
> +++ b/gdb/stap-probe.h
> @@ -0,0 +1,144 @@
> +/* SystemTap probe support for GDB.
> +
> +   Copyright (C) 2011 Free Software Foundation, Inc.

Update the year.


> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
[...]
> --- a/gdb/symfile.h
> +++ b/gdb/symfile.h
> @@ -29,6 +29,11 @@ struct objfile;
>  struct obj_section;
>  struct obstack;
>  struct block;
> +struct stap_probe;
> +struct value;
> +struct frame_info;
> +struct agent_expr;
> +struct axs_value;
>  
>  /* Comparison function for symbol look ups.  */
>  
> @@ -297,6 +302,52 @@ struct quick_symbol_functions
>  				int need_fullname);
>  };
>  
> +/* Structure of functions used for SystemTap probe support.  If one of
> +   these functions is provided, all must be.  */
> +
> +struct sym_probe_fns
> +{
> +  /* If non-NULL, return an array of SystemTap probe objects.  The
> +     number of objects is returned in *NUM_PROBES.  */

I would also say explicitly:
	The returned value does not have to be freed and it has lifetime of
	the OBJFILE.


> +  struct stap_probe *(*sym_get_probes) (struct objfile *,
> +					      int *num_probes);
> +
> +  /* Return the number of arguments available to PROBE.  PROBE will
> +     have come from a call to this objfile's sym_get_probes method.
> +     If you provide an implementation of sym_get_probes, you must
> +     implement this method as well.  */
> +  int (*sym_get_probe_argument_count) (struct objfile *objfile,
> +				       struct stap_probe *probe);
> +
[...]
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/stap-probe.c
> @@ -0,0 +1,108 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2011 Free Software Foundation, Inc.

Update the year.


> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
[...]
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/stap-probe.exp
> @@ -0,0 +1,187 @@
> +# Copyright (C) 2011 Free Software Foundation, Inc.

Update the year.


> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
[...]
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/stap-trace.c
> @@ -0,0 +1,71 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2011 Free Software Foundation, Inc.

Update the year.


> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
[...]
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/stap-trace.exp
> @@ -0,0 +1,129 @@
> +# Copyright 2011
> +# Free Software Foundation, Inc.

Update the year.  Join the lines.


> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
[...]


Thanks,
Jan


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