Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Re: variable objects and registers
@ 2006-12-06 10:57 Nick Roberts
  2006-12-19 17:59 ` Vladimir Prus
  0 siblings, 1 reply; 25+ messages in thread
From: Nick Roberts @ 2006-12-06 10:57 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: Daniel Jacobowitz, gdb-patches


Vladimir Prus writes:
> This patch adds new command -var-registers that creates and returns a list of
> variable objects for all registers gdb knows. The command takes one option --
> the frame, which is specified just like for -var-create. While not all
> registers are saved, and so gdb might not know values of some registers in
> parent frames, for some registers it's possible, and frontends might want to
> access those values.


> Here's example output:

>      -var-registers *
>      ^done,registers={
       ^done,registers=[

>		{name="var1",exp="$eax",numchild="0",value="16",type="int"},
>		............
>                {name="var10",exp="$eflags",numchild="0",value="[ SF IF ID ]",


Since this command creates new variable objects, perhaps it should be called
something like -var-create-registers.  

Alternatively, taking things further, instead of reusing some code for each
command through create_varobj_in_frame, perhaps you could have just one
command:

"-var-create" and "-var-create -r" for registers ("-var-create -m" for
memory-mapped registers).

I've not thought it through, I'm just brainstorming.

--- gdb/mi/mi-cmd-var.c	(/patches/gdb/varobj_printing/gdb_mainline)	(revision 2338)
+++ gdb/mi/mi-cmd-var.c	(/patches/gdb/varobj_for_registers/gdb_mainline)	(revision 2338)
@@ -68,16 +68,39 @@
 
 /* VAROBJ operations */
 
+static struct varobj *
+create_varobj_in_frame (char *name, char *expression, char *frame)
+{
+  CORE_ADDR frameaddr = 0;
+  struct cleanup *cleanup;
+  enum varobj_type var_type;
+
+  if (strcmp (frame, "*") == 0)
+    var_type = USE_CURRENT_FRAME;
+  else if (strcmp (frame, "@") == 0)
+    var_type = USE_SELECTED_FRAME;  
+  else
+    {
+      var_type = USE_SPECIFIED_FRAME;
+      frameaddr = string_to_core_addr (frame);
+    }
+
+  if (varobjdebug)
+    fprintf_unfiltered (gdb_stdlog,
+		    "Name=\"%s\", Frame=\"%s\" (0x%s), Expression=\"%s\"\n",
+			name, frame, paddr (frameaddr), expression);
+
+  return varobj_create (name, expression, frameaddr, var_type);
+}
+

I think the function above should go in varobj.c


...
+  numregs = NUM_REGS + NUM_PSEUDO_REGS;
+
+  make_cleanup_ui_out_tuple_begin_end (uiout, "registers");

I think this should be

make_cleanup_ui_out_list_begin_end (uiout, "registers");


+
+  for (regnum = 0; regnum < numregs; regnum++)
+    {
+      if (REGISTER_NAME (regnum) != NULL
+	  && *(REGISTER_NAME (regnum)) != '\0')
+	{
+	  char *name;
+	  char *expression;
+	  struct varobj *var;
+	  struct cleanup *cleanup_child;
+
+	  name = varobj_gen_name ();
+	  make_cleanup (free_current_contents, &name);
+
+	  expression = xstrprintf ("$%s", REGISTER_NAME (regnum));
+	  make_cleanup (xfree, expression);
+
+	  var = create_varobj_in_frame (name, expression, frame);
+
+	  cleanup_child = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+	  print_varobj (var, PRINT_ALL_VALUES, 1 /* print expression */);
+	  do_cleanups (cleanup_child);
+	}
+    }
+
+  do_cleanups (cleanup);
+  return MI_CMD_DONE;
+}
+
+
+enum mi_cmd_result
 mi_cmd_var_delete (char *command, char **argv, int argc)
 {
   char *name;


Daniel Jacobowitz writes:

> No one else had any comments, and it looks fine to me.  The patch looks
> OK too.  It'll want a testcase, naturally.

And documentation too, hopefully!

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-06 10:57 variable objects and registers Nick Roberts
@ 2006-12-19 17:59 ` Vladimir Prus
  2006-12-19 21:58   ` Nick Roberts
  0 siblings, 1 reply; 25+ messages in thread
From: Vladimir Prus @ 2006-12-19 17:59 UTC (permalink / raw)
  To: Nick Roberts, gdb-patches

Nick Roberts wrote:

> Vladimir Prus writes:
>> This patch adds new command -var-registers that creates and returns a
>> list of variable objects for all registers gdb knows. The command takes
>> one option -- the frame, which is specified just like for -var-create.
>> While not all registers are saved, and so gdb might not know values of
>> some registers in parent frames, for some registers it's possible, and
>> frontends might want to access those values.
> 
> 
>> Here's example output:
> 
>>      -var-registers *
>>      ^done,registers={
>        ^done,registers=[

Doh!

> 
>>{name="var1",exp="$eax",numchild="0",value="16",type="int"},
>>............
>>                {name="var10",exp="$eflags",numchild="0",value="[ SF IF ID
>>                {]",
> 
> 
> Since this command creates new variable objects, perhaps it should be
> called something like -var-create-registers.

I'm not sure, since the command does not creates registers, it creates
variable objects, and

        -var-create-for-registers

is a bit long. Maybe 

        -var-list-registers

I'm unsure about right naming.

> 
> Alternatively, taking things further, instead of reusing some code for
> each command through create_varobj_in_frame, perhaps you could have just
> one command:
> 
> "-var-create" and "-var-create -r" for registers ("-var-create -m" for
> memory-mapped registers).
> 
> I've not thought it through, I'm just brainstorming.

We also need a command to create varobj for all locals, because with
variable hiding, you can't create them using expression. Would that be

        -var-create -l

? Or maybe we should not mix easy "create one varobj from expression"
command with "create a bunch of varobj" command and add the following:

        -var-create-and-list --registers ...
        -var-create-and-list --locals ...
        -var-create-and-list --whatever-else ...

or just "-var-list"? On the other hand, on Apple branch the varobjs for
locals are created using

        -stack-list-locals --make-varobjs

I slightly prefer

        -var-list --locals

but again, not 100% sure.

> --- gdb/mi/mi-cmd-var.c       (/patches/gdb/varobj_printing/gdb_mainline)
> (revision 2338)
> +++ gdb/mi/mi-cmd-var.c       (/patches/gdb/varobj_for_registers/gdb_mainline)
> (revision 2338) @@ -68,16 +68,39 @@
>  
>  /* VAROBJ operations */
>  
> +static struct varobj *
> +create_varobj_in_frame (char *name, char *expression, char *frame)
> +{
> +  CORE_ADDR frameaddr = 0;
> +  struct cleanup *cleanup;
> +  enum varobj_type var_type;
> +
> +  if (strcmp (frame, "*") == 0)
> +    var_type = USE_CURRENT_FRAME;
> +  else if (strcmp (frame, "@") == 0)
> +    var_type = USE_SELECTED_FRAME;
> +  else
> +    {
> +      var_type = USE_SPECIFIED_FRAME;
> +      frameaddr = string_to_core_addr (frame);
> +    }
> +
> +  if (varobjdebug)
> +    fprintf_unfiltered (gdb_stdlog,
> +                 "Name=\"%s\", Frame=\"%s\" (0x%s), Expression=\"%s\"\n",
> +                     name, frame, paddr (frameaddr), expression);
> +
> +  return varobj_create (name, expression, frameaddr, var_type);
> +}
> +
> 
> I think the function above should go in varobj.c

Then, the varobj.c would have to expose magic "@" and "*" values in its
interface. I think it's better to keep this closer to parsing code and keep
varobj.c separated.

> 
> 
> ...
> +  numregs = NUM_REGS + NUM_PSEUDO_REGS;
> +
> +  make_cleanup_ui_out_tuple_begin_end (uiout, "registers");
> 
> I think this should be
> 
> make_cleanup_ui_out_list_begin_end (uiout, "registers");

Yeah, fixed.

> +  for (regnum = 0; regnum < numregs; regnum++)
> +    {
> +      if (REGISTER_NAME (regnum) != NULL
> +       && *(REGISTER_NAME (regnum)) != '\0')
> +     {
> +       char *name;
> +       char *expression;
> +       struct varobj *var;
> +       struct cleanup *cleanup_child;
> +
> +       name = varobj_gen_name ();
> +       make_cleanup (free_current_contents, &name);
> +
> +       expression = xstrprintf ("$%s", REGISTER_NAME (regnum));
> +       make_cleanup (xfree, expression);
> +
> +       var = create_varobj_in_frame (name, expression, frame);
> +
> +       cleanup_child = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
> +       print_varobj (var, PRINT_ALL_VALUES, 1 /* print expression */);
> +       do_cleanups (cleanup_child);
> +     }
> +    }
> +
> +  do_cleanups (cleanup);
> +  return MI_CMD_DONE;
> +}
> +
> +
> +enum mi_cmd_result
>  mi_cmd_var_delete (char *command, char **argv, int argc)
>  {
>    char *name;
> 
> 
> Daniel Jacobowitz writes:
> 
>> No one else had any comments, and it looks fine to me.  The patch looks
>> OK too.  It'll want a testcase, naturally.
> 
> And documentation too, hopefully!

Sure. We'd need to decide on command name, though.

Thanks,
Volodya



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-19 17:59 ` Vladimir Prus
@ 2006-12-19 21:58   ` Nick Roberts
  2006-12-20 18:39     ` Vladimir Prus
  0 siblings, 1 reply; 25+ messages in thread
From: Nick Roberts @ 2006-12-19 21:58 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > ? Or maybe we should not mix easy "create one varobj from expression"
 > command with "create a bunch of varobj" command and add the following:
 > 
 >         -var-create-and-list --registers ...
 >         -var-create-and-list --locals ...
 >         -var-create-and-list --whatever-else ...

A separate command does indeed seem best.

 > or just "-var-list"? On the other hand, on Apple branch the varobjs for
 > locals are created using
 > 
 >         -stack-list-locals --make-varobjs

Most of the functionality seems to come from varobj.c so, unless Apple
contribute this code to the FSF repository, I see no need to follow their
approach.

 > I slightly prefer
 > 
 >         -var-list --locals

-var-list-create --locals?  -var-create-list --locals?

Insight creates variable objects for display of locals.  The delay is quite
apparent when there are more than a couple of them, but it only updates when
the frame changes.  In Emacs, I update after every GDB command using "info
locals" which is quicker but not as good.  We need to find a way to detect when
the frame changes and report it e.g

   * NOTIFY-ASYNC-OUTPUT contains supplementary information that the
     client should handle (e.g., a new breakpoint information).  All
     notify output is prefixed by `='.

=frame-changed

I started looking at this in the thread

http://sourceware.org/ml/gdb/2006-06/msg00162.html

but didn't make good progress.

 > >...
 > > +static struct varobj *
 > > +create_varobj_in_frame (char *name, char *expression, char *frame)
 > > +{
 > > +  CORE_ADDR frameaddr = 0;
 > > +  struct cleanup *cleanup;
 > > +  enum varobj_type var_type;
 > > +
 > > +  if (strcmp (frame, "*") == 0)
 > > +    var_type = USE_CURRENT_FRAME;
 > > +  else if (strcmp (frame, "@") == 0)
 > > +    var_type = USE_SELECTED_FRAME;
 > > +  else
 > > +    {
 > > +      var_type = USE_SPECIFIED_FRAME;
 > > +      frameaddr = string_to_core_addr (frame);
 > > +    }
 > > +
 > > +  if (varobjdebug)
 > > +    fprintf_unfiltered (gdb_stdlog,
 > > +                 "Name=\"%s\", Frame=\"%s\" (0x%s), Expression=\"%s\"\n",
 > > +                     name, frame, paddr (frameaddr), expression);
 > > +
 > > +  return varobj_create (name, expression, frameaddr, var_type);
 > > +}
 > > +
 > > 
 > > I think the function above should go in varobj.c
 > 
 > Then, the varobj.c would have to expose magic "@" and "*" values in its
 > interface. I think it's better to keep this closer to parsing code and keep
 > varobj.c separated.

Yes.  OK, in that case I would call the function create_var_in_frame.  Likewise
with existing functions:

varobj_update_one    -->  var_update
print_varobj         -->  print_var

so that they are in the file you would expect to find them.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-19 21:58   ` Nick Roberts
@ 2006-12-20 18:39     ` Vladimir Prus
  2006-12-20 20:34       ` Nick Roberts
  0 siblings, 1 reply; 25+ messages in thread
From: Vladimir Prus @ 2006-12-20 18:39 UTC (permalink / raw)
  To: Nick Roberts, gdb-patches

Nick Roberts wrote:

>  > ? Or maybe we should not mix easy "create one varobj from expression"
>  > command with "create a bunch of varobj" command and add the following:
>  > 
>  >         -var-create-and-list --registers ...
>  >         -var-create-and-list --locals ...
>  >         -var-create-and-list --whatever-else ...
> 
> A separate command does indeed seem best.
> 
>  > or just "-var-list"? On the other hand, on Apple branch the varobjs for
>  > locals are created using
>  > 
>  >         -stack-list-locals --make-varobjs
> 
> Most of the functionality seems to come from varobj.c so, unless Apple
> contribute this code to the FSF repository, I see no need to follow their
> approach.

I don't think this particular interface decision is important, but I don't
follow your general principle. It does not matter if Apple ports their code
to FSF, or we do it -- if differences are introduces for no good reason,
it makes it harder to merge changes for everyone, and that's bad.

> 
>  > I slightly prefer
>  > 
>  >         -var-list --locals
> 
> -var-list-create --locals?  -var-create-list --locals?

I've settled on -var-list. I don't think 'create' clarifies much,
and this is not user command, and '-var-list-children' also creates
variable objects without having 'create' in the name.

> 
> Insight creates variable objects for display of locals.  The delay is
> quite apparent when there are more than a couple of them, but it only
> updates when
> the frame changes.  

By "updates" you mean '-var-update' or regetting new list of locals and 
creating new vars?

> In Emacs, I update after every GDB command using "info 
> locals" which is quicker 

Quicker than what? -var-update? Is this a measurable difference? If
so, we should fix it.

> but not as good.  We need to find a way to detect 
> when the frame changes 

FWIW, KDevelop uses "info frame" 

> and report it e.g 
> 
>    * NOTIFY-ASYNC-OUTPUT contains supplementary information that the
>      client should handle (e.g., a new breakpoint information).  All
>      notify output is prefixed by `='.
> 
> =frame-changed
> 
> I started looking at this in the thread
> 
> http://sourceware.org/ml/gdb/2006-06/msg00162.html
> 
> but didn't make good progress.

I think that we need more higher-level notification -- namely "new locals
appeared" and either:
        - "varobj now refers to different object"
        - or have a command that creates varobjs for all variables
        in a function.

That way, we won't need to guess how varobjs should be changes when frame id
changes.

> 
>  > >...
>  > > +static struct varobj *
>  > > +create_varobj_in_frame (char *name, char *expression, char *frame)
>  > > +{
>  > > +  CORE_ADDR frameaddr = 0;
>  > > +  struct cleanup *cleanup;
>  > > +  enum varobj_type var_type;
>  > > +
>  > > +  if (strcmp (frame, "*") == 0)
>  > > +    var_type = USE_CURRENT_FRAME;
>  > > +  else if (strcmp (frame, "@") == 0)
>  > > +    var_type = USE_SELECTED_FRAME;
>  > > +  else
>  > > +    {
>  > > +      var_type = USE_SPECIFIED_FRAME;
>  > > +      frameaddr = string_to_core_addr (frame);
>  > > +    }
>  > > +
>  > > +  if (varobjdebug)
>  > > +    fprintf_unfiltered (gdb_stdlog,
>  > > +                 "Name=\"%s\", Frame=\"%s\" (0x%s),
>  > > Expression=\"%s\"\n",
>  > > +                     name, frame, paddr (frameaddr), expression);
>  > > +
>  > > +  return varobj_create (name, expression, frameaddr, var_type);
>  > > +}
>  > > +
>  > > 
>  > > I think the function above should go in varobj.c
>  > 
>  > Then, the varobj.c would have to expose magic "@" and "*" values in its
>  > interface. I think it's better to keep this closer to parsing code and
>  > keep varobj.c separated.
> 
> Yes.  OK, in that case I would call the function create_var_in_frame. 

That function creates and returns variable object. Calling
it 'create_var_in_frame' would imply that it creates some 'var',
and that would not be accurate.

> Likewise with existing functions:
> 
> varobj_update_one    -->  var_update
> print_varobj         -->  print_var

This one, too, prints variable object.

> so that they are in the file you would expect to find them.

I'm not sure. I don't know why a name of a function should map to the
name of the file in the way you indicate.

- Volodya



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-20 18:39     ` Vladimir Prus
@ 2006-12-20 20:34       ` Nick Roberts
  2006-12-21  1:34         ` Nick Roberts
  2006-12-21  6:43         ` Vladimir Prus
  0 siblings, 2 replies; 25+ messages in thread
From: Nick Roberts @ 2006-12-20 20:34 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > >  > or just "-var-list"? On the other hand, on Apple branch the varobjs for
 > >  > locals are created using
 > >  > 
 > >  >         -stack-list-locals --make-varobjs
 > > 
 > > Most of the functionality seems to come from varobj.c so, unless Apple
 > > contribute this code to the FSF repository, I see no need to follow their
 > > approach.
 > 
 > I don't think this particular interface decision is important, but I don't
 > follow your general principle. It does not matter if Apple ports their code
 > to FSF, or we do it -- if differences are introduces for no good reason,
 > it makes it harder to merge changes for everyone, and that's bad.

It's easier for Apple to port their changes back than us, and they can always
do this if they wish.  If it's easier to put the function in mi-cmd-stack.c I
would call it "-stack-list-locals --make-varobjs", but if it's easier to put it
in mi-cmd-var.c I would call it -var-"something".
 
 > > 
 > >  > I slightly prefer
 > >  > 
 > >  >         -var-list --locals
 > > 
 > > -var-list-create --locals?  -var-create-list --locals?
 > 
 > I've settled on -var-list. I don't think 'create' clarifies much,
 > and this is not user command,

Except that it doesn't just list existing variable objects but "creates" new
ones.  Its _always_ helpful to have a descriptive name.

 >                               and '-var-list-children' also creates
 > variable objects without having 'create' in the name.

Good analogy.  I had to add the fact that it created variable objects for
the children to the manual because the original author had ommitted/forgotten
it.  Are you saying we should copy existing bad design?

 > > Insight creates variable objects for display of locals.  The delay is
 > > quite apparent when there are more than a couple of them, but it only
 > > updates when
 > > the frame changes.  
 > 
 > By "updates" you mean '-var-update' or regetting new list of locals and 
 > creating new vars?

The latter.

 > > In Emacs, I update after every GDB command using "info 
 > > locals" which is quicker 
 > 
 > Quicker than what? -var-update? Is this a measurable difference? If
 > so, we should fix it.

Quicker than regenerating variable objects for all the locals.

 > > but not as good.  We need to find a way to detect 
 > > when the frame changes 
 > 
 > FWIW, KDevelop uses "info frame" 

Does this approach know when it's not another invocation of the frame
with the same name?

 > I think that we need more higher-level notification -- namely "new locals
 > appeared" and either:
 >         - "varobj now refers to different object"
 >         - or have a command that creates varobjs for all variables
 >         in a function.

The latter is "-stack-list-locals --make-varobjs" isn't it?  Or are you talking
about variables declared within compound statements?

 >...
 > >  > Then, the varobj.c would have to expose magic "@" and "*" values in its
 > >  > interface. I think it's better to keep this closer to parsing code and
 > >  > keep varobj.c separated.
 > > 
 > > Yes.  OK, in that case I would call the function create_var_in_frame. 
 > 
 > That function creates and returns variable object. Calling
 > it 'create_var_in_frame' would imply that it creates some 'var',
 > and that would not be accurate.
 > 
 > > Likewise with existing functions:
 > > 
 > > varobj_update_one    -->  var_update
 > > print_varobj         -->  print_var
 > 
 > This one, too, prints variable object.

They're all for variable objects.

 > > so that they are in the file you would expect to find them.
 > 
 > I'm not sure. I don't know why a name of a function should map to the
 > name of the file in the way you indicate.

The distinction I'm making is that var_* functions and variables are for use in
MI and generally live in mi_cmd_var.c while varobj_* ones may be used by
Insight and live in varobj.c

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-20 20:34       ` Nick Roberts
@ 2006-12-21  1:34         ` Nick Roberts
  2006-12-21  6:34           ` Vladimir Prus
  2006-12-30 20:26           ` Daniel Jacobowitz
  2006-12-21  6:43         ` Vladimir Prus
  1 sibling, 2 replies; 25+ messages in thread
From: Nick Roberts @ 2006-12-21  1:34 UTC (permalink / raw)
  To: Vladimir Prus, gdb-patches

 >  > I think that we need more higher-level notification -- namely "new locals
 >  > appeared" and either:
 >  >         - "varobj now refers to different object"
 >  >         - or have a command that creates varobjs for all variables
 >  >         in a function.
 > 
 > The latter is "-stack-list-locals --make-varobjs" isn't it?  Or are you
 > talking about variables declared within compound statements?

I see now that Insight has two commands:

    /* This implements the tcl command gdb_get_blocks
     *
     * Returns the start and end addresses for all blocks in
     * the selected frame.
     *
     * Arguments:
     *    None
     * Tcl Result:
     *    A list of all valid blocks in the selected_frame.
     */
    static int
    gdb_get_blocks (ClientData clientData, Tcl_Interp *interp,
    		    int objc, Tcl_Obj *CONST objv[])


and


    /* This implements the tcl command gdb_block_vars.
     *
     * Returns all variables valid in the specified block.
     *
     * Arguments:
     *    The start and end addresses which identify the block.
     * Tcl Result:
     *    All variables defined in the given block.
     */
    static int
    gdb_block_vars (ClientData clientData, Tcl_Interp *interp,
		    int objc, Tcl_Obj *CONST objv[])


gdb_block_vars only gets called if gdb_get_blocks finds a new block which
then finds any variabes local to it.  That way new variable objects can be
added (and old ones deleted if a block has disappeared) while keeping
the variable objects which are still in scope.  I think we should implement
these functions in MI (perhaps Apple already have).

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-21  1:34         ` Nick Roberts
@ 2006-12-21  6:34           ` Vladimir Prus
  2006-12-21  7:57             ` Nick Roberts
  2006-12-30 20:26           ` Daniel Jacobowitz
  1 sibling, 1 reply; 25+ messages in thread
From: Vladimir Prus @ 2006-12-21  6:34 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Thursday 21 December 2006 04:29, Nick Roberts wrote:
>  >  > I think that we need more higher-level notification -- namely "new locals
>  >  > appeared" and either:
>  >  >         - "varobj now refers to different object"
>  >  >         - or have a command that creates varobjs for all variables
>  >  >         in a function.
>  > 
>  > The latter is "-stack-list-locals --make-varobjs" isn't it?  Or are you
>  > talking about variables declared within compound statements?
> 
> I see now that Insight has two commands:
> 
>     /* This implements the tcl command gdb_get_blocks
>      *
>      * Returns the start and end addresses for all blocks in
>      * the selected frame.
>      *
>      * Arguments:
>      *    None
>      * Tcl Result:
>      *    A list of all valid blocks in the selected_frame.
>      */
>     static int
>     gdb_get_blocks (ClientData clientData, Tcl_Interp *interp,
>     		    int objc, Tcl_Obj *CONST objv[])
> 
> 
> and
> 
> 
>     /* This implements the tcl command gdb_block_vars.
>      *
>      * Returns all variables valid in the specified block.
>      *
>      * Arguments:
>      *    The start and end addresses which identify the block.
>      * Tcl Result:
>      *    All variables defined in the given block.
>      */
>     static int
>     gdb_block_vars (ClientData clientData, Tcl_Interp *interp,
> 		    int objc, Tcl_Obj *CONST objv[])
> 
> 
> gdb_block_vars only gets called if gdb_get_blocks finds a new block which
> then finds any variabes local to it.  That way new variable objects can be
> added (and old ones deleted if a block has disappeared) while keeping
> the variable objects which are still in scope.  I think we should implement
> these functions in MI (perhaps Apple already have).

Again, I think we need more automated approach. Frontend should have a
single command  that:

	1. Reports which local variables are really dead now
	2. Creates and reports variable object for new locals 
	3. Reports which varobjs are out of scope

For example:

    -var-update --locals
    ^done,varobjs=[{name="v1",in_scope="false"....}{"name="v2",in_scope="true"....}]
               created=[{name="v3"....],
               gone-forever=[{name="v0"...}]


Apple creates varobjs for all variables in all blocks in a function, and use "in_scope" to
track their scope. That might be good approach too. IIRC, you've posted a patch
to consider block boundaries when computing "in_scope"? That's exactly what's
needed for this approach to work.

- Volodya


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-20 20:34       ` Nick Roberts
  2006-12-21  1:34         ` Nick Roberts
@ 2006-12-21  6:43         ` Vladimir Prus
  2006-12-21  8:34           ` Nick Roberts
  1 sibling, 1 reply; 25+ messages in thread
From: Vladimir Prus @ 2006-12-21  6:43 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Wednesday 20 December 2006 23:30, Nick Roberts wrote:
>  > >  > or just "-var-list"? On the other hand, on Apple branch the varobjs for
>  > >  > locals are created using
>  > >  > 
>  > >  >         -stack-list-locals --make-varobjs
>  > > 
>  > > Most of the functionality seems to come from varobj.c so, unless Apple
>  > > contribute this code to the FSF repository, I see no need to follow their
>  > > approach.
>  > 
>  > I don't think this particular interface decision is important, but I don't
>  > follow your general principle. It does not matter if Apple ports their code
>  > to FSF, or we do it -- if differences are introduces for no good reason,
>  > it makes it harder to merge changes for everyone, and that's bad.
> 
> It's easier for Apple to port their changes back than us, and they can always
> do this if they wish.  If it's easier to put the function in mi-cmd-stack.c I
> would call it "-stack-list-locals --make-varobjs", but if it's easier to put it
> in mi-cmd-var.c I would call it -var-"something".
>  
>  > > 
>  > >  > I slightly prefer
>  > >  > 
>  > >  >         -var-list --locals
>  > > 
>  > > -var-list-create --locals?  -var-create-list --locals?
>  > 
>  > I've settled on -var-list. I don't think 'create' clarifies much,
>  > and this is not user command,
> 
> Except that it doesn't just list existing variable objects but "creates" new
> ones.  

I fact, I'm not sure what duplicated

	-var-list --locals 

should do. Create afresh, or just return already created?

> Its _always_ helpful to have a descriptive name. 

The name itself is not useful without docs, so we need no try too hard. For example,

	-var-list-create

and
	-var-create-list

are also inaccurate, because this command does not create list. It creates 
variable objects and then lists them.

Can we have some reasonable way out of this bikeshed?

>  >                               and '-var-list-children' also creates
>  > variable objects without having 'create' in the name.
> 
> Good analogy.  I had to add the fact that it created variable objects for
> the children to the manual because the original author had ommitted/forgotten
> it.  Are you saying we should copy existing bad design?

I'm saying this does not matters much. One has to read all the docs anyway.
 
>  > > Insight creates variable objects for display of locals.  The delay is
>  > > quite apparent when there are more than a couple of them, but it only
>  > > updates when
>  > > the frame changes.  
>  > 
>  > By "updates" you mean '-var-update' or regetting new list of locals and 
>  > creating new vars?
> 
> The latter.
> 
>  > > In Emacs, I update after every GDB command using "info 
>  > > locals" which is quicker 
>  > 
>  > Quicker than what? -var-update? Is this a measurable difference? If
>  > so, we should fix it.
> 
> Quicker than regenerating variable objects for all the locals.

Yeah, not surprising given that "info locals" is a 'wholesale' operation.

>  > > but not as good.  We need to find a way to detect 
>  > > when the frame changes 
>  > 
>  > FWIW, KDevelop uses "info frame" 
> 
> Does this approach know when it's not another invocation of the frame
> with the same name?

No, but this approach is only the first check. After that the list of all
local name is obtained and then addresses of all locals are obtained and
compared with previous values.

>  > I think that we need more higher-level notification -- namely "new locals
>  > appeared" and either:
>  >         - "varobj now refers to different object"
>  >         - or have a command that creates varobjs for all variables
>  >         in a function.
> 
> The latter is "-stack-list-locals --make-varobjs" isn't it?  Or are you talking
> about variables declared within compound statements?

Both. -stack-list-locals --make-varobjs creates variable objects for all variables
in a function, including in nested blocks.

I think it might be good idea for you and I to go though Apple branch grepping
for all "APPLE LOCAL" markers so that we know everything they've added.

>  > >  > Then, the varobj.c would have to expose magic "@" and "*" values in its
>  > >  > interface. I think it's better to keep this closer to parsing code and
>  > >  > keep varobj.c separated.
>  > > 
>  > > Yes.  OK, in that case I would call the function create_var_in_frame. 
>  > 
>  > That function creates and returns variable object. Calling
>  > it 'create_var_in_frame' would imply that it creates some 'var',
>  > and that would not be accurate.
>  > 
>  > > Likewise with existing functions:
>  > > 
>  > > varobj_update_one    -->  var_update
>  > > print_varobj         -->  print_var
>  > 
>  > This one, too, prints variable object.
> 
> They're all for variable objects.
> 
>  > > so that they are in the file you would expect to find them.
>  > 
>  > I'm not sure. I don't know why a name of a function should map to the
>  > name of the file in the way you indicate.
> 
> The distinction I'm making is that var_* functions and variables are for use in
> MI and generally live in mi_cmd_var.c while varobj_* ones may be used by
> Insight and live in varobj.c

I don't think the difference between "var" and "varobj" in function
name communicates what you want, and I don't think such a coding guideline makes sense
for future.

- Volodya


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-21  6:34           ` Vladimir Prus
@ 2006-12-21  7:57             ` Nick Roberts
  2006-12-21  8:34               ` Vladimir Prus
  0 siblings, 1 reply; 25+ messages in thread
From: Nick Roberts @ 2006-12-21  7:57 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > > gdb_block_vars only gets called if gdb_get_blocks finds a new block which
 > > then finds any variabes local to it.  That way new variable objects can be
 > > added (and old ones deleted if a block has disappeared) while keeping
 > > the variable objects which are still in scope.  I think we should implement
 > > these functions in MI (perhaps Apple already have).
 > 
 > Again, I think we need more automated approach. Frontend should have a
 > single command  that:
 > 
 > 	1. Reports which local variables are really dead now
 > 	2. Creates and reports variable object for new locals 
 > 	3. Reports which varobjs are out of scope

We don't seem to be finding much agreement, except that what I have described
does 2 and 3, and I don't really understand the difference between 1 and 3
When would a variable belong to just one of those two groups?

 > For example:
 > 
 >     -var-update --locals
 >     ^done,varobjs=[{name="v1",in_scope="false"....}{"name="v2",in_scope="true"....}]
 >                created=[{name="v3"....],
 >                gone-forever=[{name="v0"...}]
 > 
 > 
 > Apple creates varobjs for all variables in all blocks in a function, and use
 > "in_scope" to track their scope. That might be good approach too. IIRC,
 > you've posted a patch to consider block boundaries when computing
 > "in_scope"? That's exactly what's needed for this approach to work.

Yes, I'm fairly sure that's what the above functions from Insight do.  That
wouldn't be a coincidence either because ChangeLog records show that Jim Ingham
worked on Insight while at Cygnus.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-21  7:57             ` Nick Roberts
@ 2006-12-21  8:34               ` Vladimir Prus
  0 siblings, 0 replies; 25+ messages in thread
From: Vladimir Prus @ 2006-12-21  8:34 UTC (permalink / raw)
  To: Nick Roberts; +Cc: gdb-patches

On Thursday 21 December 2006 10:52, Nick Roberts wrote:
>  > > gdb_block_vars only gets called if gdb_get_blocks finds a new block which
>  > > then finds any variabes local to it.  That way new variable objects can be
>  > > added (and old ones deleted if a block has disappeared) while keeping
>  > > the variable objects which are still in scope.  I think we should implement
>  > > these functions in MI (perhaps Apple already have).
>  > 
>  > Again, I think we need more automated approach. Frontend should have a
>  > single command  that:
>  > 
>  > 	1. Reports which local variables are really dead now
>  > 	2. Creates and reports variable object for new locals 
>  > 	3. Reports which varobjs are out of scope
> 
> We don't seem to be finding much agreement, except that what I have described
> does 2 and 3, and I don't really understand the difference between 1 and 3
> When would a variable belong to just one of those two groups?

If you've left a function, then all variables in that function are really dead. If you
have a variable in most-nested scope in a function (say inside loop), that variable
can go in scope and go out of scope many times as you step though the function.
When you leave a function, the frontend might want to completely remove its
data structures. When leaving local scope, it might want to just show the relevant
GUI item is "out of scope" -- gray perhaps.

>  > For example:
>  > 
>  >     -var-update --locals
>  >     ^done,varobjs=[{name="v1",in_scope="false"....}{"name="v2",in_scope="true"....}]
>  >                created=[{name="v3"....],
>  >                gone-forever=[{name="v0"...}]
>  > 
>  > 
>  > Apple creates varobjs for all variables in all blocks in a function, and use
>  > "in_scope" to track their scope. That might be good approach too. IIRC,
>  > you've posted a patch to consider block boundaries when computing
>  > "in_scope"? That's exactly what's needed for this approach to work.
> 
> Yes, I'm fairly sure that's what the above functions from Insight do.  That
> wouldn't be a coincidence either because ChangeLog records show that Jim Ingham
> worked on Insight while at Cygnus.

The first point I'm making here is that MI is not in-process interface, so we need to minimize
the amount of commands. I believe that to update local variables, frontend should emit *one*
command, that would provide all the necessary information -- including new/deleted varobjs,
in/out scope, and new values.

- Volodya



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-21  6:43         ` Vladimir Prus
@ 2006-12-21  8:34           ` Nick Roberts
       [not found]             ` <0E190425-7C4F-4C6E-B8B3-9A3FA79F6FE3@apple.com>
  2007-01-05  9:04             ` Vladimir Prus
  0 siblings, 2 replies; 25+ messages in thread
From: Nick Roberts @ 2006-12-21  8:34 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

 > >  > I've settled on -var-list. I don't think 'create' clarifies much,
 > >  > and this is not user command,
 > > 
 > > Except that it doesn't just list existing variable objects but "creates"
 > > new ones.
 > 
 > I fact, I'm not sure what duplicated
 > 
 > 	-var-list --locals 
 > 
 > should do. Create afresh, or just return already created?

Thats a fair point.  It should only create new variable objects if execution
enters a new block.  That makes -var-list a reasonable name.  In the manual I
should have said "...creates variable objects for the children if they do not
already exist.".

 > > The latter is "-stack-list-locals --make-varobjs" isn't it?  Or are you
 > > talking about variables declared within compound statements?
 > 
 > Both. -stack-list-locals --make-varobjs creates variable objects for all
 > variables in a function, including in nested blocks.

Well plain "-stack-list-locals just prints those that are in scope (as does
"info locals").  How does it differentiate between variable object for
expressions with the same name? (ISTR varobj.c hashes the name of the expression
to work out where to store it).

 > I think it might be good idea for you and I to go though Apple branch
 > grepping for all "APPLE LOCAL" markers so that we know everything they've
 > added.

Apple have made extensive changes.  I might look for specific changes if I'm
pointed towards them (as for the asynchronous event loop) but I'm not going
to look at all of it.

 > > The distinction I'm making is that var_* functions and variables are for
 > > use in MI and generally live in mi_cmd_var.c while varobj_* ones may be
 > > used by Insight and live in varobj.c
 > 
 > I don't think the difference between "var" and "varobj" in function name
 > communicates what you want, and I don't think such a coding guideline makes
 > sense for future.

Looking more closely the distinction doesn't seem to exist for variable names
which are often just var for struct varobj (presumably for brevity).  As long
as there are two separate files for variable objects (because Insight only
needs one, varobj.c), it seems a harmless and mildly useful guideline for
function names to me.



-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
       [not found]             ` <0E190425-7C4F-4C6E-B8B3-9A3FA79F6FE3@apple.com>
@ 2006-12-21 23:30               ` Nick Roberts
       [not found]                 ` <1B6B17FA-65DE-4CE2-9BE0-20DA74780EEA@apple.com>
  0 siblings, 1 reply; 25+ messages in thread
From: Nick Roberts @ 2006-12-21 23:30 UTC (permalink / raw)
  To: Jim Ingham; +Cc: Vladimir Prus, gdb-patches

 > you get something like:
 > 
 > -stack-list-locals 2 1
 > ^done,locals=[varobj= 
 > {exp="foo",value="0",name="var1",numchild="0",type="int",typecode="INT", 
 > dynamic_type="",in_scope="true",block_start_addr="0x00002c90",block_end_ 
 > addr="0x00002d10"},varobj= 
 > {exp="bar",value="0",name="var2",numchild="0",type="int",typecode="INT", 
 > dynamic_type="",in_scope="false",block_start_addr="0x00002cc4",block_end 
 > _addr="0x00002cdc"},varobj= 
 > {exp="bar",value="-1881116284",name="var3",numchild="0",type="int",typec 
 > ode="INT",dynamic_type="",in_scope="false",block_start_addr="0x00002cdc" 
 > ,block_end_addr="0x00002cf4"}]

Yes this looks exactly like what we want.

 > It would be pretty easy to add a line number for the block start &  
 > end if you wanted to show that.  We also fixed the code that reports  
 > in & out of scope pretty much along the lines that Vlad suggested a  
 > while back so you would get correct reports.  -var-update reports  
 > coming in & out of scope, but we didn't change it to report "new  
 > variables" that arise when you step into a deeper scope.  That's not  
 > necessary if you report all the blocks up-front.
 > 
 > Xcode fetches and displays ALL the locals, and then just annotates  
 > the ones that are out of scope.  But you could also imagine the UI  
 > fetching all the locals but suppressing the ones that are not in  
 > scope, and then displaying them when it gets the "in scope" message  
 > from -var-update.  We didn't do it this way because I find having the  
 > list of variables changing while stepping through code to be  
 > distracting.
 > 
 > We also didn't so a special update for locals, since Xcode keeps  
 > track of which variable objects are the locals in each frame, and  
 > updates them in batches as needed.  That coupled with the ALL_BLOCKS  
 > setting obviates the need for a special locals listing.

We (Vlad and I) could try to port this back into FSF but it would be a lot
easier (I think) for you to do it.  It seems to me that you had a good working
relationship with Andrew Cagney (perhaps from Cygnus days) and that Apple
contributed more actively to MI then.  I'm probably speaking out of order, and
maybe opening a can of worms, but I wonder if Apple could be encouraged to
contribute more actively once again if in return, say, the Apple GDB code was
given a branch on the FSF repository (see
http://sourceware.org/ml/gdb/2005-03/msg00197.html).  This would make it easier
to port changes from FSF to Apple but also Apple to FSF.

 > Another kind of useful addition along the same lines, we extended the  
 > "FRAME" argument to -var-create so you can say:
 > 
 > -var-create - +main.c:6 bar
 > 
 > to create the variable object for bar in the scope surrounding line  
 > 6.  This is necessary if you want to do variable values in tooltips  
 > using variable objects.

Yes, I see Insight uses variable objects for tooltips too.  What advantage do
they have over just using "print"?

 > For the most part, though we are a bit spotty about updating the texi  
 > docs for our MI changes, we do update the command descriptions in the  
 > mi_cmd_* source documentation (though I note there isn't much in the  
 > mi-cmd-var.c file.)

AFAICS the texi docs (gdb.texinfo) in my copy of Apple GDB
(6.3.50.20050815-cvs) are the same as in FSF.

 > If you can get your hands on a Mac OS X box for a little while, there  
 > is a way to dump the MI commands Xcode sends to gdb, and the gdb  
 > responses to a log file.  That's probably the quickest way to see  
 > what stuff we've added and how we use MI, if you are interested.

Its unlikely in the near future, but I've got OpenDarwin 7.2.1.

 > Sorry for not being active on this thread, but I've been swamped on  
 > other things, and haven't been thinking about the MI for a while now...

What about the changes that we've been making recently e.g

http://sourceware.org/ml/gdb-patches/2006-12/msg00127.html

Do they fix things for Apple too?

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
       [not found]                 ` <1B6B17FA-65DE-4CE2-9BE0-20DA74780EEA@apple.com>
@ 2006-12-22  4:47                   ` Nick Roberts
  2006-12-22  6:23                     ` Vladimir Prus
  0 siblings, 1 reply; 25+ messages in thread
From: Nick Roberts @ 2006-12-22  4:47 UTC (permalink / raw)
  To: Jim Ingham; +Cc: Vladimir Prus, gdb-patches

 > Sadly, neither Jason nor I will get a chance to do anything other  
 > that our already scheduled tasks till we're done with Leopard -  
 > sometime middle of next year or thereabouts.  We have a lot on our  
 > plates till then, and don't have any time available for this.  I  
 > can't really say what our plans will be after that.

OK, well thanks for the pointers anyway.

 > >> Another kind of useful addition along the same lines, we extended the
 > >> "FRAME" argument to -var-create so you can say:
 > >>
 > >> -var-create - +main.c:6 bar
 > >>
 > >> to create the variable object for bar in the scope surrounding line
 > >> 6.  This is necessary if you want to do variable values in tooltips
 > >> using variable objects.
 > >
 > > Yes, I see Insight uses variable objects for tooltips too.  What  
 > > advantage do
 > > they have over just using "print"?
 > 
 > The Xcode tooltips allow structure expansion & formatting the same  
 > way the locals display does.  Using varobj's for this makes the code  
 > uniform, and simpler.

Insight just prints the type as a tooltip for structures.  You seem to be
saying that in Xcode you can do this and choose to expand it.  Using "print"
in Emacs, everything is automatically expanded which I must admit is messy
with a large array or structure.  Perhaps -data-evaluate-expression could
be modified to print "--simple-values" or "--all-values" as -stack-list-locals
does.

Actually I see Insight gets it wrong, when one variable masks another with
the same name, just printing the current value.  I guess variable objects
allow such values to be correctly accessed, but they generally they seem
to be designed for tracking values rather than getting instantaneous ones
as with tooltips.

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-22  4:47                   ` Nick Roberts
@ 2006-12-22  6:23                     ` Vladimir Prus
  2006-12-22  6:51                       ` Nick Roberts
  0 siblings, 1 reply; 25+ messages in thread
From: Vladimir Prus @ 2006-12-22  6:23 UTC (permalink / raw)
  To: Nick Roberts; +Cc: Jim Ingham, gdb-patches

On Friday 22 December 2006 07:42, Nick Roberts wrote:
>  > Sadly, neither Jason nor I will get a chance to do anything other  
>  > that our already scheduled tasks till we're done with Leopard -  
>  > sometime middle of next year or thereabouts.  We have a lot on our  
>  > plates till then, and don't have any time available for this.  I  
>  > can't really say what our plans will be after that.
> 
> OK, well thanks for the pointers anyway.
> 
>  > >> Another kind of useful addition along the same lines, we extended the
>  > >> "FRAME" argument to -var-create so you can say:
>  > >>
>  > >> -var-create - +main.c:6 bar
>  > >>
>  > >> to create the variable object for bar in the scope surrounding line
>  > >> 6.  This is necessary if you want to do variable values in tooltips
>  > >> using variable objects.
>  > >
>  > > Yes, I see Insight uses variable objects for tooltips too.  What  
>  > > advantage do
>  > > they have over just using "print"?
>  > 
>  > The Xcode tooltips allow structure expansion & formatting the same  
>  > way the locals display does.  Using varobj's for this makes the code  
>  > uniform, and simpler.
> 
> Insight just prints the type as a tooltip for structures.  You seem to be
> saying that in Xcode you can do this and choose to expand it.  Using "print"
> in Emacs, everything is automatically expanded which I must admit is messy
> with a large array or structure.  Perhaps -data-evaluate-expression could
> be modified to print "--simple-values" or "--all-values" as -stack-list-locals
> does.

I think it's much better to use varobjs for tooltips. It's much better for structures.

> Actually I see Insight gets it wrong, when one variable masks another with
> the same name, just printing the current value.  I guess variable objects
> allow such values to be correctly accessed, but they generally they seem
> to be designed for tracking values rather than getting instantaneous ones
> as with tooltips.

There's no reason why -var-create cannot be extended to also specify line
at which the expression must be evaluated -- computing the right block.

- Volodya


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-22  6:23                     ` Vladimir Prus
@ 2006-12-22  6:51                       ` Nick Roberts
  2006-12-22  7:30                         ` Vladimir Prus
  0 siblings, 1 reply; 25+ messages in thread
From: Nick Roberts @ 2006-12-22  6:51 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: Jim Ingham, gdb-patches

 > > Insight just prints the type as a tooltip for structures.  You seem to be
 > > saying that in Xcode you can do this and choose to expand it.  Using
 > > "print" in Emacs, everything is automatically expanded which I must admit
 > > is messy with a large array or structure.  Perhaps
 > > -data-evaluate-expression could be modified to print "--simple-values" or
 > > "--all-values" as -stack-list-locals does.
 > 
 > I think it's much better to use varobjs for tooltips. It's much better for
 > structures.

You might well be right but could you say why and how they would work with
tooltips?

-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-22  6:51                       ` Nick Roberts
@ 2006-12-22  7:30                         ` Vladimir Prus
  0 siblings, 0 replies; 25+ messages in thread
From: Vladimir Prus @ 2006-12-22  7:30 UTC (permalink / raw)
  To: Nick Roberts; +Cc: Jim Ingham, gdb-patches

On Friday 22 December 2006 09:46, Nick Roberts wrote:
>  > > Insight just prints the type as a tooltip for structures.  You seem to be
>  > > saying that in Xcode you can do this and choose to expand it.  Using
>  > > "print" in Emacs, everything is automatically expanded which I must admit
>  > > is messy with a large array or structure.  Perhaps
>  > > -data-evaluate-expression could be modified to print "--simple-values" or
>  > > "--all-values" as -stack-list-locals does.
>  > 
>  > I think it's much better to use varobjs for tooltips. It's much better for
>  > structures.
> 
> You might well be right but could you say why and how they would work with
> tooltips?

Just as Jim said -- you can make tooltips for structure values expandable using basically the
same code that is used for regular variable display. For KDevelop4, I'm thinking that I
can put the same widget, essentially, into variable wdiget and tooltip. Except that variable
widget would show all locals and tooltip will show just one value.

- Volodya
 


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-21  1:34         ` Nick Roberts
  2006-12-21  6:34           ` Vladimir Prus
@ 2006-12-30 20:26           ` Daniel Jacobowitz
  2006-12-30 20:41             ` Vladimir Prus
  1 sibling, 1 reply; 25+ messages in thread
From: Daniel Jacobowitz @ 2006-12-30 20:26 UTC (permalink / raw)
  To: Nick Roberts; +Cc: Vladimir Prus, gdb-patches

On Thu, Dec 21, 2006 at 02:29:44PM +1300, Nick Roberts wrote:
> I see now that Insight has two commands:
> 
>     /* This implements the tcl command gdb_get_blocks
>      *
>      * Returns the start and end addresses for all blocks in
>      * the selected frame.

>     /* This implements the tcl command gdb_block_vars.
>      *
>      * Returns all variables valid in the specified block.

> gdb_block_vars only gets called if gdb_get_blocks finds a new block which
> then finds any variabes local to it.  That way new variable objects can be
> added (and old ones deleted if a block has disappeared) while keeping
> the variable objects which are still in scope.  I think we should implement
> these functions in MI (perhaps Apple already have).

Just a note: whatever approach we end up with for this problem, let's
not use start and end addresses to identify the blocks.  In modern
compilers a block is not a contiguous range of PCs; they can overlap.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-30 20:26           ` Daniel Jacobowitz
@ 2006-12-30 20:41             ` Vladimir Prus
  2006-12-30 20:50               ` Daniel Jacobowitz
  0 siblings, 1 reply; 25+ messages in thread
From: Vladimir Prus @ 2006-12-30 20:41 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Nick Roberts, gdb-patches

On Saturday 30 December 2006 23:26, Daniel Jacobowitz wrote:
> On Thu, Dec 21, 2006 at 02:29:44PM +1300, Nick Roberts wrote:
> > I see now that Insight has two commands:
> > 
> >     /* This implements the tcl command gdb_get_blocks
> >      *
> >      * Returns the start and end addresses for all blocks in
> >      * the selected frame.
> 
> >     /* This implements the tcl command gdb_block_vars.
> >      *
> >      * Returns all variables valid in the specified block.
> 
> > gdb_block_vars only gets called if gdb_get_blocks finds a new block which
> > then finds any variabes local to it.  That way new variable objects can be
> > added (and old ones deleted if a block has disappeared) while keeping
> > the variable objects which are still in scope.  I think we should implement
> > these functions in MI (perhaps Apple already have).
> 
> Just a note: whatever approach we end up with for this problem, let's
> not use start and end addresses to identify the blocks.  In modern
> compilers a block is not a contiguous range of PCs; they can overlap.

I kinda assumed we'd use 'struct block *' everywhere. We'd need to be
able to check if a PC is inside a block and for that we'd need the
comparison with block start and end addresses. Or there's some other
way to check if PC is in block?

- Volodya


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-30 20:41             ` Vladimir Prus
@ 2006-12-30 20:50               ` Daniel Jacobowitz
  2006-12-30 23:05                 ` Nick Roberts
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Jacobowitz @ 2006-12-30 20:50 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: Nick Roberts, gdb-patches

On Sat, Dec 30, 2006 at 11:40:22PM +0300, Vladimir Prus wrote:
> I kinda assumed we'd use 'struct block *' everywhere. We'd need to be
> able to check if a PC is inside a block and for that we'd need the
> comparison with block start and end addresses. Or there's some other
> way to check if PC is in block?

I meant that we shouldn't have the front end specify blocks based on
their start/end addresses.  If we want the front end to be able to
specify blocks, we can give them UIDs.

Yes, BLOCK_START and BLOCK_END are currently the right way to check
if an address is in a block.  I posted a patch a while back that
added a predicate function to check, in case the block had more than
one range of instructions.  I never finished that patch - my goal
for my own GDB development over the next year is to finish all the
stuff I've posted as prototypes and then never come back to!

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-30 20:50               ` Daniel Jacobowitz
@ 2006-12-30 23:05                 ` Nick Roberts
  0 siblings, 0 replies; 25+ messages in thread
From: Nick Roberts @ 2006-12-30 23:05 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Vladimir Prus, gdb-patches

 > Yes, BLOCK_START and BLOCK_END are currently the right way to check
 > if an address is in a block.

Yes, I think this is how it would. Let's come up with an implementation first
and then you can try shoot it down.


-- 
Nick                                           http://www.inet.net.nz/~nickrob


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-21  8:34           ` Nick Roberts
       [not found]             ` <0E190425-7C4F-4C6E-B8B3-9A3FA79F6FE3@apple.com>
@ 2007-01-05  9:04             ` Vladimir Prus
  1 sibling, 0 replies; 25+ messages in thread
From: Vladimir Prus @ 2007-01-05  9:04 UTC (permalink / raw)
  To: Nick Roberts, gdb-patches

Nick Roberts wrote:

>  > >  > I've settled on -var-list. I don't think 'create' clarifies much,
>  > >  > and this is not user command,
>  > > 
>  > > Except that it doesn't just list existing variable objects but
>  > > "creates" new ones.
>  > 
>  > I fact, I'm not sure what duplicated
>  > 
>  > -var-list --locals
>  > 
>  > should do. Create afresh, or just return already created?
> 
> Thats a fair point.  It should only create new variable objects if
> execution
> enters a new block.  That makes -var-list a reasonable name.  In the
> manual I should have said "...creates variable objects for the children if
> they do not already exist.".

Yeah. For -var-list, I've modified the manual to say that -var-list tries
to return previously-created varobjs.

For -var-list --registers I did not implement such reusing logic, because
I feel it's not very important performance-wise (yet?). For -var-list --locals,
we'll certainly have reuse of previously-created varobj, though.

- Volodya



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-06  9:25   ` Vladimir Prus
@ 2006-12-19 22:02     ` Daniel Jacobowitz
  0 siblings, 0 replies; 25+ messages in thread
From: Daniel Jacobowitz @ 2006-12-19 22:02 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

On Wed, Dec 06, 2006 at 12:19:57PM +0300, Vladimir Prus wrote:
> Ehm, note that the test for -data-list-register-names only works for Sparc.
> I must admit I don't know how to test this behaviour in a generic fashion.
> It might be possible to compare output of -data-list-register-names
> and -var-registers, but given that the latter is implemented by cloning
> some logic of the former, that's weak test.

BTW, I meant something much simpler when I asked for a testcase: test
that it creates a bunch of varobjs.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-12-05 21:16 ` Daniel Jacobowitz
@ 2006-12-06  9:25   ` Vladimir Prus
  2006-12-19 22:02     ` Daniel Jacobowitz
  0 siblings, 1 reply; 25+ messages in thread
From: Vladimir Prus @ 2006-12-06  9:25 UTC (permalink / raw)
  To: gdb-patches

Daniel Jacobowitz wrote:

> On Wed, Nov 29, 2006 at 08:20:47PM +0300, Vladimir Prus wrote:
>> 
>> I was working on implementing a new MI command that creates variable
>> objects for registers, and I have something working good enough to
>> discuss. This patch lacks docs, but I wanted to make sure the interface
>> is fine with everybody before documenting it.
> 
> No one else had any comments, and it looks fine to me.  The patch looks
> OK too.  It'll want a testcase, naturally.

Ehm, note that the test for -data-list-register-names only works for Sparc.
I must admit I don't know how to test this behaviour in a generic fashion.
It might be possible to compare output of -data-list-register-names
and -var-registers, but given that the latter is implemented by cloning
some logic of the former, that's weak test.

>> As soon as we add the code to display memory-mapped registers, there will
>> be a problem that existing frontends might wish to show the memory-mapped
>> registers, but not wish (at the moment) to modify the code for displaying
>> regular registers. I plan to address this by either adding new
>> attribute "register-kind" to the output, that can be either "core"
>> or "memory-mapped", or by adding an option to -var-registers that says
>> what registers to show. But that's for future.
> 
> Or a different command to return just those?  I'm not sure they should
> be part of -var-registers; clients probably expect "registers" to be
> "the things that should go in a registers view", which won't include
> most MMIO registers.

FWIW, in my case MMIO registers will go straight to registers view. I think
that an interface with options to -var-registers is better that adding yet
another command.

- Volodya



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: variable objects and registers
  2006-11-29 17:21 Vladimir Prus
@ 2006-12-05 21:16 ` Daniel Jacobowitz
  2006-12-06  9:25   ` Vladimir Prus
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Jacobowitz @ 2006-12-05 21:16 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

On Wed, Nov 29, 2006 at 08:20:47PM +0300, Vladimir Prus wrote:
> 
> I was working on implementing a new MI command that creates variable objects 
> for registers, and I have something working good enough to discuss. This 
> patch lacks docs, but I wanted to make sure the interface is fine with 
> everybody before documenting it.

No one else had any comments, and it looks fine to me.  The patch looks
OK too.  It'll want a testcase, naturally.

> As soon as we add the code to display memory-mapped registers, there will be a 
> problem that existing frontends might wish to show the memory-mapped 
> registers, but not wish (at the moment) to modify the code for displaying 
> regular registers. I plan to address this by either adding new 
> attribute "register-kind" to the output, that can be either "core" 
> or "memory-mapped", or by adding an option to -var-registers that says what 
> registers to show. But that's for future.

Or a different command to return just those?  I'm not sure they should
be part of -var-registers; clients probably expect "registers" to be
"the things that should go in a registers view", which won't include
most MMIO registers.


-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 25+ messages in thread

* variable objects and registers
@ 2006-11-29 17:21 Vladimir Prus
  2006-12-05 21:16 ` Daniel Jacobowitz
  0 siblings, 1 reply; 25+ messages in thread
From: Vladimir Prus @ 2006-11-29 17:21 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2314 bytes --]


I was working on implementing a new MI command that creates variable objects 
for registers, and I have something working good enough to discuss. This 
patch lacks docs, but I wanted to make sure the interface is fine with 
everybody before documenting it.

There are already some commands related to registers, for 
example, -data-list-register-names. But for registers that are not just 
integers, one has to use variable objects to property display them. As 
result, output of -data-list-register-names goes directly to a number 
of -var-create. It is more reasonable to have a command that immediately 
produces variable objects.

Further, when memory-mapped registers are involved, gdb might want to group 
them in some hierarchy. Using variable objects is a reasonable way to achieve 
that.

This patch adds new command -var-registers that creates and returns a list of 
variable objects for all registers gdb knows. The command takes one option -- 
the frame, which is specified just like for -var-create. While not all 
registers are saved, and so gdb might not know values of some registers in 
parent frames, for some registers it's possible, and frontends might want to 
access those values.

Since the command creates several variable objects, it does not accepts a name 
of varobj. Instead, it automatically generated names much like "var-create -" 
does.

Here's example output:

      -var-registers *
      ^done,registers={
		{name="var1",exp="$eax",numchild="0",value="16",type="int"},
		............
                {name="var10",exp="$eflags",numchild="0",value="[ SF IF ID ]",


As soon as we add the code to display memory-mapped registers, there will be a 
problem that existing frontends might wish to show the memory-mapped 
registers, but not wish (at the moment) to modify the code for displaying 
regular registers. I plan to address this by either adding new 
attribute "register-kind" to the output, that can be either "core" 
or "memory-mapped", or by adding an option to -var-registers that says what 
registers to show. But that's for future.

Comments?

- Volodya

	* mi/mi-cmds.h (mi_cmd_var_registers): New.
	* mi/mi-cmds.c: Register "var-registers".
	* mi/mi-cmd-var.c (create_varobj_in_frame): New function.
	(mi_cmd_var_create): Use the above.
	(mi_cmd_var_registers): New.

[-- Attachment #2: varobj_for_registers__gdb_mainline.diff --]
[-- Type: text/x-diff, Size: 5091 bytes --]

=== gdb/mi/mi-cmds.h
==================================================================
--- gdb/mi/mi-cmds.h	(/patches/gdb/varobj_printing/gdb_mainline)	(revision 2338)
+++ gdb/mi/mi-cmds.h	(/patches/gdb/varobj_for_registers/gdb_mainline)	(revision 2338)
@@ -105,6 +105,7 @@
 extern mi_cmd_argv_ftype mi_cmd_thread_select;
 extern mi_cmd_argv_ftype mi_cmd_var_assign;
 extern mi_cmd_argv_ftype mi_cmd_var_create;
+extern mi_cmd_argv_ftype mi_cmd_var_registers;
 extern mi_cmd_argv_ftype mi_cmd_var_delete;
 extern mi_cmd_argv_ftype mi_cmd_var_evaluate_expression;
 extern mi_cmd_argv_ftype mi_cmd_var_info_expression;
=== gdb/mi/mi-cmds.c
==================================================================
--- gdb/mi/mi-cmds.c	(/patches/gdb/varobj_printing/gdb_mainline)	(revision 2338)
+++ gdb/mi/mi-cmds.c	(/patches/gdb/varobj_for_registers/gdb_mainline)	(revision 2338)
@@ -153,6 +153,7 @@
   { "trace-stop", { NULL, 0 }, NULL, NULL },
   { "var-assign", { NULL, 0 }, 0, mi_cmd_var_assign},
   { "var-create", { NULL, 0 }, 0, mi_cmd_var_create},
+  { "var-registers", { NULL, 0 }, 0, mi_cmd_var_registers},
   { "var-delete", { NULL, 0 }, 0, mi_cmd_var_delete},
   { "var-evaluate-expression", { NULL, 0 }, 0, mi_cmd_var_evaluate_expression},
   { "var-info-expression", { NULL, 0 }, 0, mi_cmd_var_info_expression},
=== gdb/mi/mi-cmd-var.c
==================================================================
--- gdb/mi/mi-cmd-var.c	(/patches/gdb/varobj_printing/gdb_mainline)	(revision 2338)
+++ gdb/mi/mi-cmd-var.c	(/patches/gdb/varobj_for_registers/gdb_mainline)	(revision 2338)
@@ -68,16 +68,39 @@
 
 /* VAROBJ operations */
 
+static struct varobj *
+create_varobj_in_frame (char *name, char *expression, char *frame)
+{
+  CORE_ADDR frameaddr = 0;
+  struct cleanup *cleanup;
+  enum varobj_type var_type;
+
+  if (strcmp (frame, "*") == 0)
+    var_type = USE_CURRENT_FRAME;
+  else if (strcmp (frame, "@") == 0)
+    var_type = USE_SELECTED_FRAME;  
+  else
+    {
+      var_type = USE_SPECIFIED_FRAME;
+      frameaddr = string_to_core_addr (frame);
+    }
+
+  if (varobjdebug)
+    fprintf_unfiltered (gdb_stdlog,
+		    "Name=\"%s\", Frame=\"%s\" (0x%s), Expression=\"%s\"\n",
+			name, frame, paddr (frameaddr), expression);
+
+  return varobj_create (name, expression, frameaddr, var_type);
+}
+
 enum mi_cmd_result
 mi_cmd_var_create (char *command, char **argv, int argc)
 {
-  CORE_ADDR frameaddr = 0;
   struct varobj *var;
   char *name;
   char *frame;
   char *expr;
   struct cleanup *old_cleanups;
-  enum varobj_type var_type;
 
   if (argc != 3)
     {
@@ -104,23 +127,8 @@
   else if (!isalpha (*name))
     error (_("mi_cmd_var_create: name of object must begin with a letter"));
 
-  if (strcmp (frame, "*") == 0)
-    var_type = USE_CURRENT_FRAME;
-  else if (strcmp (frame, "@") == 0)
-    var_type = USE_SELECTED_FRAME;  
-  else
-    {
-      var_type = USE_SPECIFIED_FRAME;
-      frameaddr = string_to_core_addr (frame);
-    }
+  var = create_varobj_in_frame (name, expr, frame);
 
-  if (varobjdebug)
-    fprintf_unfiltered (gdb_stdlog,
-		    "Name=\"%s\", Frame=\"%s\" (0x%s), Expression=\"%s\"\n",
-			name, frame, paddr (frameaddr), expr);
-
-  var = varobj_create (name, expr, frameaddr, var_type);
-
   if (var == NULL)
     error (_("mi_cmd_var_create: unable to create variable object"));
 
@@ -131,6 +139,58 @@
 }
 
 enum mi_cmd_result
+mi_cmd_var_registers (char *command, char **argv, int argc)
+{
+  int regnum, numregs;
+  struct cleanup *cleanup;
+  char *frame;
+
+  if (argc != 1)
+    error (_("mi_cmd_var_registers: Usage: FRAME."));
+  
+  frame = xstrdup (argv[0]);
+  cleanup = make_cleanup (xfree, frame);
+
+  /* Note that the test for a valid register must include checking the
+     REGISTER_NAME because NUM_REGS may be allocated for the union of
+     the register sets within a family of related processors.  In this
+     case, some entries of REGISTER_NAME will change depending upon
+     the particular processor being debugged.  */
+
+  numregs = NUM_REGS + NUM_PSEUDO_REGS;
+
+  make_cleanup_ui_out_tuple_begin_end (uiout, "registers");
+
+  for (regnum = 0; regnum < numregs; regnum++)
+    {
+      if (REGISTER_NAME (regnum) != NULL
+	  && *(REGISTER_NAME (regnum)) != '\0')
+	{
+	  char *name;
+	  char *expression;
+	  struct varobj *var;
+	  struct cleanup *cleanup_child;
+
+	  name = varobj_gen_name ();
+	  make_cleanup (free_current_contents, &name);
+
+	  expression = xstrprintf ("$%s", REGISTER_NAME (regnum));
+	  make_cleanup (xfree, expression);
+
+	  var = create_varobj_in_frame (name, expression, frame);
+
+	  cleanup_child = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
+	  print_varobj (var, PRINT_ALL_VALUES, 1 /* print expression */);
+	  do_cleanups (cleanup_child);
+	}
+    }
+
+  do_cleanups (cleanup);
+  return MI_CMD_DONE;
+}
+
+
+enum mi_cmd_result
 mi_cmd_var_delete (char *command, char **argv, int argc)
 {
   char *name;

Property changes on: 
___________________________________________________________________
Name: csl:base
 +/all/patches/gdb/varobj_printing/gdb_mainline


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2007-01-05  9:04 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-12-06 10:57 variable objects and registers Nick Roberts
2006-12-19 17:59 ` Vladimir Prus
2006-12-19 21:58   ` Nick Roberts
2006-12-20 18:39     ` Vladimir Prus
2006-12-20 20:34       ` Nick Roberts
2006-12-21  1:34         ` Nick Roberts
2006-12-21  6:34           ` Vladimir Prus
2006-12-21  7:57             ` Nick Roberts
2006-12-21  8:34               ` Vladimir Prus
2006-12-30 20:26           ` Daniel Jacobowitz
2006-12-30 20:41             ` Vladimir Prus
2006-12-30 20:50               ` Daniel Jacobowitz
2006-12-30 23:05                 ` Nick Roberts
2006-12-21  6:43         ` Vladimir Prus
2006-12-21  8:34           ` Nick Roberts
     [not found]             ` <0E190425-7C4F-4C6E-B8B3-9A3FA79F6FE3@apple.com>
2006-12-21 23:30               ` Nick Roberts
     [not found]                 ` <1B6B17FA-65DE-4CE2-9BE0-20DA74780EEA@apple.com>
2006-12-22  4:47                   ` Nick Roberts
2006-12-22  6:23                     ` Vladimir Prus
2006-12-22  6:51                       ` Nick Roberts
2006-12-22  7:30                         ` Vladimir Prus
2007-01-05  9:04             ` Vladimir Prus
  -- strict thread matches above, loose matches on Subject: below --
2006-11-29 17:21 Vladimir Prus
2006-12-05 21:16 ` Daniel Jacobowitz
2006-12-06  9:25   ` Vladimir Prus
2006-12-19 22:02     ` Daniel Jacobowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox