Re: variable objects and registers

[Vladimir Prus <ghost@cs.msu.su>]

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