From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24416 invoked by alias); 19 Dec 2006 17:59:12 -0000 Received: (qmail 24408 invoked by uid 22791); 19 Dec 2006 17:59:11 -0000 X-Spam-Check-By: sourceware.org Received: from zigzag.lvk.cs.msu.su (HELO zigzag.lvk.cs.msu.su) (158.250.17.23) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 19 Dec 2006 17:59:04 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.50) id 1GwjEu-00030f-Ra for gdb-patches@sources.redhat.com; Tue, 19 Dec 2006 20:59:00 +0300 Received: from localhost ([127.0.0.1] helo=ip6-localhost) by zigzag.lvk.cs.msu.su with esmtp (Exim 4.50) id 1GwjEo-00030H-Me; Tue, 19 Dec 2006 20:58:50 +0300 From: Vladimir Prus Subject: Re: variable objects and registers To: Nick Roberts , gdb-patches@sources.redhat.com Date: Tue, 19 Dec 2006 17:59:00 -0000 References: <17782.41205.881283.845357@kahikatea.snap.net.nz> User-Agent: KNode/0.10.2 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7Bit Message-Id: Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-12/txt/msg00253.txt.bz2 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