Re: variable objects and registers

Vladimir Prus
2006-12-19T17:59:12Z

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