Re: [PATCH 3/6] Modify internalvar mechanism

[Pedro Alves <pedro@codesourcery.com>]

On Monday 04 April 2011 04:08:34, Sergio Durigan Junior wrote:
> Hi,
> 
> This patch modifies the mechanism of internalvar.  It basically adds
> more manipulation functions to them (`compile_to_ax' and `destroy'), and
> updates the definitions and pointers of existing internalvars.

Can you explain this?  Why would you want to compile an internal var
to AX, for example?

It would be imensely helpful if patches came with a high
level explanation of why they are necessary in the first
place.  :-/


> 
> Regtested on the compile farm.
> 
> Thanks,
> 
> Sergio.
> 
> ---
>  gdb/ChangeLog      |   32 ++++++++++++++++++++++++++++
>  gdb/ax-gdb.c       |    5 ++-
>  gdb/infrun.c       |   14 ++++++++++-
>  gdb/thread.c       |   14 ++++++++++-
>  gdb/tracepoint.c   |   14 ++++++++++-
>  gdb/value.c        |   58 ++++++++++++++++++++++++++++++++++++++++++++-------
>  gdb/value.h        |   48 ++++++++++++++++++++++++++++++++++++++++--
>  gdb/windows-tdep.c |   13 +++++++++-
>  8 files changed, 177 insertions(+), 21 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index f20653b..1fa276d 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,5 +1,37 @@
>  2011-04-04  Tom Tromey  <tromey@redhat.com>
>  
> +	* ax-gdb.c (gen_expr): Clean up code to handle internal variables
> +	and to compile agent expressions.
> +	* infrun.c (siginfo_make_value): New argument `ignore'.
> +	(siginfo_funcs): New struct.
> +	(_initialize_infrun): New argument when calling
> +	`create_internalvar_type_lazy'.
> +	* thread.c (thread_id_make_value): New argument `ignore'.
> +	(thread_funcs): New struct.
> +	(_initialize_thread): New argument when calling
> +	`create_internalvar_type_lazy'.
> +	* tracepoint.c (sdata_make_value): New argument `ignore'.
> +	(sdata_funcs): New struct.
> +	(_initialize_tracepoint): New argument when calling
> +	`create_internalvar_type_lazy'.
> +	* value.c (make_value): New struct.
> +	(create_internalvar_type_lazy): New argument `data'.
> +	(compile_internalvar_to_ax): New function.
> +	(value_of_internalvar): Properly handling `make_value' case.
> +	(clear_internalvar): Likewise.
> +	(show_convenience): Adding `TRY_CATCH' block.
> +	* value.h (internalvar_make_value): Delete, replace by...
> +	(struct internalvar_funcs): ... this.
> +	(create_internalvar_type_lazy) <fun>: Delete argument.
> +	(create_internalvar_type_lazy) <funcs>, <data>: New arguments.
> +	(compile_internalvar_to_ax): New function.
> +	* windows-tdep.c (tlb_make_value): New argument `ignore'.
> +	(tlb_funcs): New struct.
> +	(_initialize_windows_tdep): New argument when calling
> +	`create_internalvar_type_lazy'.
> +
> +2011-04-04  Tom Tromey  <tromey@redhat.com>
> +
>  	* breakpoint.c (create_breakpoints_sal): Added code to handle
>  	pre-expanded sals.
>  	(create_breakpoint): Likewise.
> diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
> index d1736e1..3e54716 100644
> --- a/gdb/ax-gdb.c
> +++ b/gdb/ax-gdb.c
> @@ -2001,7 +2001,8 @@ gen_expr (struct expression *exp, union exp_element **pc,
>  
>      case OP_INTERNALVAR:
>        {
> -	const char *name = internalvar_name ((*pc)[1].internalvar);
> +	struct internalvar *var = (*pc)[1].internalvar;
> +	const char *name = internalvar_name (var);
>  	struct trace_state_variable *tsv;
>  
>  	(*pc) += 3;
> @@ -2015,7 +2016,7 @@ gen_expr (struct expression *exp, union exp_element **pc,
>  	    value->kind = axs_rvalue;
>  	    value->type = builtin_type (exp->gdbarch)->builtin_long_long;
>  	  }
> -	else
> +	else if (! compile_internalvar_to_ax (var, ax, value))
>  	  error (_("$%s is not a trace state variable; GDB agent "
>  		   "expressions cannot use convenience variables."), name);
>        }
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index 7cee7c8..77d8da8 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -6253,7 +6253,8 @@ static struct lval_funcs siginfo_value_funcs =
>     if there's no object available.  */
>  
>  static struct value *
> -siginfo_make_value (struct gdbarch *gdbarch, struct internalvar *var)
> +siginfo_make_value (struct gdbarch *gdbarch, struct internalvar *var,
> +		    void *ignore)
>  {
>    if (target_has_stack
>        && !ptid_equal (inferior_ptid, null_ptid)
> @@ -6826,6 +6827,15 @@ show_schedule_multiple (struct ui_file *file, int from_tty,
>  			    "of all processes is %s.\n"), value);
>  }
>  
> +/* Implementation of `siginfo' variable.  */
> +
> +static const struct internalvar_funcs siginfo_funcs =
> +{
> +  siginfo_make_value,
> +  NULL,
> +  NULL
> +};
> +
>  void
>  _initialize_infrun (void)
>  {
> @@ -7098,7 +7108,7 @@ Tells gdb whether to detach the child of a fork."),
>       value with a void typed value, and when we get here, gdbarch
>       isn't initialized yet.  At this point, we're quite sure there
>       isn't another convenience variable of the same name.  */
> -  create_internalvar_type_lazy ("_siginfo", siginfo_make_value);
> +  create_internalvar_type_lazy ("_siginfo", &siginfo_funcs, NULL);
>  
>    add_setshow_boolean_cmd ("observer", no_class,
>  			   &observer_mode_1, _("\
> diff --git a/gdb/thread.c b/gdb/thread.c
> index 6ad1807..aef9dce 100644
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -1438,7 +1438,8 @@ update_thread_list (void)
>     no thread is selected, or no threads exist.  */
>  
>  static struct value *
> -thread_id_make_value (struct gdbarch *gdbarch, struct internalvar *var)
> +thread_id_make_value (struct gdbarch *gdbarch, struct internalvar *var,
> +		      void *ignore)
>  {
>    struct thread_info *tp = find_thread_ptid (inferior_ptid);
>  
> @@ -1449,6 +1450,15 @@ thread_id_make_value (struct gdbarch *gdbarch, struct internalvar *var)
>  /* Commands with a prefix of `thread'.  */
>  struct cmd_list_element *thread_cmd_list = NULL;
>  
> +/* Implementation of `thread' variable.  */
> +
> +static struct internalvar_funcs thread_funcs =
> +{
> +  thread_id_make_value,
> +  NULL,
> +  NULL
> +};
> +
>  void
>  _initialize_thread (void)
>  {
> @@ -1494,5 +1504,5 @@ Show printing of thread events (such as thread start and exit)."), NULL,
>           show_print_thread_events,
>           &setprintlist, &showprintlist);
>  
> -  create_internalvar_type_lazy ("_thread", thread_id_make_value);
> +  create_internalvar_type_lazy ("_thread", &thread_funcs, NULL);
>  }
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index 3ae35d0..12e1b80 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -4497,7 +4497,8 @@ info_static_tracepoint_markers_command (char *arg, int from_tty)
>     available.  */
>  
>  static struct value *
> -sdata_make_value (struct gdbarch *gdbarch, struct internalvar *var)
> +sdata_make_value (struct gdbarch *gdbarch, struct internalvar *var,
> +		  void *ignore)
>  {
>    LONGEST size;
>    gdb_byte *buf;
> @@ -4676,6 +4677,15 @@ traceframe_available_memory (VEC(mem_range_s) **result,
>    return 0;
>  }
>  
> +/* Implementation of `sdata' variable.  */
> +
> +static const struct internalvar_funcs sdata_funcs =
> +{
> +  sdata_make_value,
> +  NULL,
> +  NULL
> +};
> +
>  /* module initialization */
>  void
>  _initialize_tracepoint (void)
> @@ -4686,7 +4696,7 @@ _initialize_tracepoint (void)
>       value with a void typed value, and when we get here, gdbarch
>       isn't initialized yet.  At this point, we're quite sure there
>       isn't another convenience variable of the same name.  */
> -  create_internalvar_type_lazy ("_sdata", sdata_make_value);
> +  create_internalvar_type_lazy ("_sdata", &sdata_funcs, NULL);
>  
>    traceframe_number = -1;
>    tracepoint_number = -1;
> diff --git a/gdb/value.c b/gdb/value.c
> index 2acb1df..f2eb01d 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -1530,7 +1530,14 @@ struct internalvar
>        struct value *value;
>  
>        /* The call-back routine used with INTERNALVAR_MAKE_VALUE.  */
> -      internalvar_make_value make_value;
> +      struct
> +        {
> +	  /* The functions to call.  */
> +	  const struct internalvar_funcs *functions;
> +
> +	  /* The function's user-data.  */
> +	  void *data;
> +        } make_value;
>  
>        /* The internal function used with INTERNALVAR_FUNCTION.  */
>        struct
> @@ -1629,18 +1636,39 @@ create_internalvar (const char *name)
>  /* Create an internal variable with name NAME and register FUN as the
>     function that value_of_internalvar uses to create a value whenever
>     this variable is referenced.  NAME should not normally include a
> -   dollar sign.  */
> +   dollar sign.  DATA is passed uninterpreted to FUN when it is
> +   called.  CLEANUP, if not NULL, is called when the internal variable
> +   is destroyed.  It is passed DATA as its only argument.  */
>  
>  struct internalvar *
> -create_internalvar_type_lazy (char *name, internalvar_make_value fun)
> +create_internalvar_type_lazy (const char *name,
> +			      const struct internalvar_funcs *funcs,
> +			      void *data)
>  {
>    struct internalvar *var = create_internalvar (name);
>  
>    var->kind = INTERNALVAR_MAKE_VALUE;
> -  var->u.make_value = fun;
> +  var->u.make_value.functions = funcs;
> +  var->u.make_value.data = data;
>    return var;
>  }
>  
> +/* See documentation in value.h.  */
> +
> +int
> +compile_internalvar_to_ax (struct internalvar *var,
> +			   struct agent_expr *expr,
> +			   struct axs_value *value)
> +{
> +  if (var->kind != INTERNALVAR_MAKE_VALUE
> +      || var->u.make_value.functions->compile_to_ax == NULL)
> +    return 0;
> +
> +  var->u.make_value.functions->compile_to_ax (var, expr, value,
> +					      var->u.make_value.data);
> +  return 1;
> +}
> +
>  /* Look up an internal variable with name NAME.  NAME should not
>     normally include a dollar sign.
>  
> @@ -1713,7 +1741,8 @@ value_of_internalvar (struct gdbarch *gdbarch, struct internalvar *var)
>        break;
>  
>      case INTERNALVAR_MAKE_VALUE:
> -      val = (*var->u.make_value) (gdbarch, var);
> +      val = (*var->u.make_value.functions->make_value) (gdbarch, var,
> +							var->u.make_value.data);
>        break;
>  
>      default:
> @@ -1909,6 +1938,11 @@ clear_internalvar (struct internalvar *var)
>        xfree (var->u.string);
>        break;
>  
> +    case INTERNALVAR_MAKE_VALUE:
> +      if (var->u.make_value.functions->destroy != NULL)
> +	var->u.make_value.functions->destroy (var->u.make_value.data);
> +      break;
> +
>      default:
>        break;
>      }
> @@ -2080,14 +2114,22 @@ show_convenience (char *ignore, int from_tty)
>    get_user_print_options (&opts);
>    for (var = internalvars; var; var = var->next)
>      {
> +      volatile struct gdb_exception e;
> +
>        if (!varseen)
>  	{
>  	  varseen = 1;
>  	}
>        printf_filtered (("$%s = "), var->name);
> -      value_print (value_of_internalvar (gdbarch, var), gdb_stdout,
> -		   &opts);
> -      printf_filtered (("\n"));
> +
> +      TRY_CATCH (e, RETURN_MASK_ERROR)
> +	{
> +	  value_print (value_of_internalvar (gdbarch, var), gdb_stdout,
> +		       &opts);
> +	  printf_filtered (("\n"));
> +	}
> +      if (e.reason < 0)
> +	printf_filtered (_("<error: %s>\n"), e.message);
>      }
>    if (!varseen)
>      printf_unfiltered (_("No debugger convenience variables now defined.\n"
> diff --git a/gdb/value.h b/gdb/value.h
> index 0889cef..71c995c 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -697,10 +697,52 @@ extern struct internalvar *lookup_only_internalvar (const char *name);
>  
>  extern struct internalvar *create_internalvar (const char *name);
>  
> -typedef struct value * (*internalvar_make_value) (struct gdbarch *,
> -						  struct internalvar *);
> +/* An internalvar can be dynamically computed by supplying a vector of
> +   function pointers to perform various operations.  */
> +
> +struct internalvar_funcs
> +{
> +  /* Compute the value of the variable.  The DATA argument passed to
> +     the function is the same argument that was passed to
> +     `create_internalvar_type_lazy'.  */
> +
> +  struct value *(*make_value) (struct gdbarch *arch,
> +			       struct internalvar *var,
> +			       void *data);
> +
> +  /* Update the agent expression EXPR with bytecode to compute the
> +     value.  VALUE is the agent value we are updating.  The DATA
> +     argument passed to this function is the same argument that was
> +     passed to `create_internalvar_type_lazy'.  If this pointer is
> +     NULL, then the internalvar cannot be compiled to an agent
> +     expression.  */
> +
> +  void (*compile_to_ax) (struct internalvar *var,
> +			 struct agent_expr *expr,
> +			 struct axs_value *value,
> +			 void *data);
> +
> +  /* If non-NULL, this is called to destroy DATA.  The DATA argument
> +     passed to this function is the same argument that was passed to
> +     `create_internalvar_type_lazy'.  */
> +
> +  void (*destroy) (void *data);
> +};
> +
>  extern struct internalvar *
> -  create_internalvar_type_lazy (char *name, internalvar_make_value fun);
> +create_internalvar_type_lazy (const char *name,
> +			      const struct internalvar_funcs *funcs,
> +			      void *data);
> +
> +/* Compile an internal variable to an agent expression.  VAR is the
> +   variable to compile; EXPR and VALUE are the agent expression we are
> +   updating.  This will return 0 if there is no known way to compile
> +   VAR, and 1 if VAR was successfully compiled.  It may also throw an
> +   exception on error.  */
> +
> +extern int compile_internalvar_to_ax (struct internalvar *var,
> +				      struct agent_expr *expr,
> +				      struct axs_value *value);
>  
>  extern struct internalvar *lookup_internalvar (const char *name);
>  
> diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
> index 31ddd14..b319250 100644
> --- a/gdb/windows-tdep.c
> +++ b/gdb/windows-tdep.c
> @@ -268,7 +268,7 @@ static struct lval_funcs tlb_value_funcs =
>     if there's no object available.  */
>  
>  static struct value *
> -tlb_make_value (struct gdbarch *gdbarch, struct internalvar *var)
> +tlb_make_value (struct gdbarch *gdbarch, struct internalvar *var, void *ignore)
>  {
>    if (target_has_stack && !ptid_equal (inferior_ptid, null_ptid))
>      {
> @@ -425,6 +425,15 @@ init_w32_command_list (void)
>      }
>  }
>  
> +/* Implementation of `tlb' variable.  */
> +
> +static const struct internalvar_funcs tlb_funcs =
> +{
> +  tlb_make_value,
> +  NULL,
> +  NULL
> +};
> +
>  void
>  _initialize_windows_tdep (void)
>  {
> @@ -451,5 +460,5 @@ even if their meaning is unknown."),
>       value with a void typed value, and when we get here, gdbarch
>       isn't initialized yet.  At this point, we're quite sure there
>       isn't another convenience variable of the same name.  */
> -  create_internalvar_type_lazy ("_tlb", tlb_make_value);
> +  create_internalvar_type_lazy ("_tlb", &tlb_funcs, NULL);
>  }
> 

-- 
Pedro Alves