[PATCH 3/6] Modify internalvar mechanism

[PATCH 3/6] Modify internalvar mechanism

[Sergio Durigan Junior]

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.

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);
 }

Re: [PATCH 3/6] Modify internalvar mechanism

[Jan Kratochvil]

On Mon, 04 Apr 2011 05:08:34 +0200, Sergio Durigan Junior wrote:
> --- a/gdb/thread.c
> +++ b/gdb/thread.c
> @@ -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 =
        ^^^ const

> +{
> +  thread_id_make_value,
> +  NULL,
> +  NULL
> +};


Thanks,
Jan

Re: [PATCH 3/6] Modify internalvar mechanism

[Pedro Alves]

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

Re: [PATCH 3/6] Modify internalvar mechanism

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

>> 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.

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

A programmer can pass arguments to a SystemTap probe.  In GDB these are
represented as convenience variables.  We also wanted to be able to
collect these probe arguments from tracepoints set at probe points.

Rather than write custom code to recognize the variables by name, I
thought it would be better to provide a general way to define variables
which can be compiled to AX.

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

Sorry about that.

Tom

Re: [PATCH 3/6] Modify internalvar mechanism

[Sergio Durigan Junior]

Pedro Alves <pedro@codesourcery.com> writes:

> 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?

A SystemTap probe can have up to 10 arguments.  In our original plan,
the idea is that the user should be able to collect those arguments
using the tracepoint mechanism.  Since we already have the necessary
code to evaluate an argument (which is treated as an internal varibale
inside GDB), we thought it would be easier to extend this code in order
to compile it to AX.

You could do something like:

    (gdb) trace probe:test
    ..
    (gdb) actions
    ..
    > collect $_probe_arg0
    > end
    ..

In order to collect the probe's first argument, and so on.

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

My fault, sorry.

Regards,

Sergio.

Re: [PATCH 3/6] Modify internalvar mechanism

[Pedro Alves]

On Tuesday 12 April 2011 23:33:05, Sergio Durigan Junior wrote:
> Pedro Alves <pedro@codesourcery.com> writes:
> 
> > 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?
> 
> A SystemTap probe can have up to 10 arguments.  In our original plan,
> the idea is that the user should be able to collect those arguments
> using the tracepoint mechanism.  Since we already have the necessary
> code to evaluate an argument (which is treated as an internal varibale
> inside GDB), we thought it would be easier to extend this code in order
> to compile it to AX.
> 
> You could do something like:
> 
>     (gdb) trace probe:test
>     ..
>     (gdb) actions
>     ..
>     > collect $_probe_arg0
>     > end
>     ..

Do I understand correctly that this still creates
a regular trap tracepoint at the probe location?
A fast tracepoint ("ftrace") at that spec would
create a fast tracepoint at the probe's location, and 
things would magically work, because $_probe_argN is just
sugar for collecting memory and registers, right?

I noticed that patch 4 does some changes to 
start_tracing to tweak the probes' semaphores, if any.
What are these semaphores?  How do other stap tools
handle them?  I ask because that bends a bit the definition
of "trace" being a regular tracepoint at a given
location, so I'd like to understand it.

> In order to collect the probe's first argument, and so on.

I see.  Sounds like a good approach.

(I'm reading patch 4 piecemeal, but it's taking a
bit to grok it all).

-- 
Pedro Alves

Re: [PATCH 3/6] Modify internalvar mechanism

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:

Pedro> Do I understand correctly that this still creates
Pedro> a regular trap tracepoint at the probe location?
Pedro> A fast tracepoint ("ftrace") at that spec would
Pedro> create a fast tracepoint at the probe's location, and 
Pedro> things would magically work, because $_probe_argN is just
Pedro> sugar for collecting memory and registers, right?

I believe it should all work, but I did not think to try fast
tracepoints.  I don't know whether Sergio did.

Yes, the probe arguments are just a wrapper around some kind of
assembly-level operand.  These are known statically to gdb; the operands
are encoded as strings in the appropriate section.  A good part of the
main patch is devoted to parsing these.

It might be nice if we had a way to print the operand strings.  This
isn't always useful, but maybe we could add some optional flag to "info
probe".  Or, maybe just another flag to readelf.

Pedro> I noticed that patch 4 does some changes to 
Pedro> start_tracing to tweak the probes' semaphores, if any.
Pedro> What are these semaphores?  How do other stap tools
Pedro> handle them?  I ask because that bends a bit the definition
Pedro> of "trace" being a regular tracepoint at a given
Pedro> location, so I'd like to understand it.

A probe can optionally have an associated "semaphore".  A semaphore in
the sdt.h sense is just a global variable (of type unsigned short) which
can be used to gate the probe.  Typically these are only generated by
code using the dtrace compatibility wrappers.

Without a semaphore a probe may look like:

  STAB_PROBE (provider, name, arg);

But with a semaphore it would more typically be written (though I think
in practice there may be macros expanding to the semaphore name):

  if (provider_name_semaphore)
    STAB_PROBE (provider, name, arg);

Aside from providing some source compatibility with dtrace probes, this
can also be used in a situation where computing the probe arguments is
expensive.  In Fedora 15, at least Python takes this approach.  But,
OTOH, libgcc and glibc do not.

The only other tool I know of that deals with these probe points is
SystemTap.  It treats the semaphore as a counter.  I don't know the
specifics, but basically it is incremented by stap at "attach" and
decremented at "detach".  We made gdb work the same way, for
compatibility.  (I have not done it but I believe it is possible to use
stap and gdb to probe the same process at the same time.)

So, since setting the semaphore is necessary to make the probe even
trigger, it seemed to make sense to have gdb set it, and to leave it set
during the trace experiment.  One reason we decided to make `probe:'
linespecs special with regard to semaphores is because it makes it clear
that the user has asked for some additional behavioral change on top of
the breakpoint.  (We could alternatively have made `break *0xaddress'
magically work at a probe point, the way we did for probe arguments...)

Tom

Re: [PATCH 3/6] Modify internalvar mechanism

[Tom Tromey]

Pedro> I noticed that patch 4 does some changes to 
Pedro> start_tracing to tweak the probes' semaphores, if any.
Pedro> What are these semaphores?  How do other stap tools
Pedro> handle them?  I ask because that bends a bit the definition
Pedro> of "trace" being a regular tracepoint at a given
Pedro> location, so I'd like to understand it.

While developing this, Sergio and I talked extensively to Roland and the
other SystemTap guys about the sdt.h implementation.  Today I finally
got around to writing up our notes:

http://sourceware.org/systemtap/wiki/UserSpaceProbeImplementation

There is already some documentation about adding probes:

http://sourceware.org/systemtap/wiki/AddingUserSpaceProbingToApps

.. though it seems to focus a bit too much (IMO) on the dtrace
compatibility aspect.  You can see a simpler approach in the test suite
part of patch #4 -- you don't actually need anything more than the
#include.

Tom