From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2625 invoked by alias); 6 Oct 2009 12:14:39 -0000 Received: (qmail 2617 invoked by uid 22791); 6 Oct 2009 12:14:38 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 06 Oct 2009 12:14:32 +0000 Received: (qmail 11991 invoked from network); 6 Oct 2009 12:14:30 -0000 Received: from unknown (HELO orlando) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 6 Oct 2009 12:14:30 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [v6] multi-executable support Date: Tue, 06 Oct 2009 12:14:00 -0000 User-Agent: KMail/1.9.10 References: <200910051659.20885.pedro@codesourcery.com> <83d451o69u.fsf@gnu.org> In-Reply-To: <83d451o69u.fsf@gnu.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200910061314.29661.pedro@codesourcery.com> X-IsSubscribed: yes 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: 2009-10/txt/msg00124.txt.bz2 On Monday 05 October 2009 22:55:09, Eli Zaretskii wrote: > > From: Pedro Alves > > Date: Mon, 5 Oct 2009 16:59:20 +0100 > > > > Eli, could you please take a new look at the docs bits? > > I may have missed something, but I hope nothing big. > > Ack. I also spotted a few typos in the comments, see below. Thanks. > > > +/* These functions concerns about actual breakpoints inserted in the > > "These functions concerns" doesn't sound right. Fixed. > > > +/* Returns true if {ASPACE1,ADDR1} and {ASPACE2,ADDR2} represent the > > + same breakpoint location. In most targets, this is will be true if > ^^ > I think "is" is redundant here. Fixed. > > > + /* The program had previously vforked, and now the child is done > > + with the shared memory region, because it exec'ed or exited. > > + Note that the event if reported to the vfork parent. This is > ^^ > Did you mean "is"? Yes. > > > + /* If this is a vfork child exiting, then the pspace and > > + aspaces where shared with the parent. Since we're > ^^^^^ > "were", right? Right. > > > Index: src/gdb/doc/gdb.texinfo > > This is okay, but I have a few comments: > > > +In addition, below each inferior line, @value{GDBN} prints extra > > +information that isn't suitable to display in tabular form. For > > +example, extra vfork parent/child relationships. > > How about showing some of that in the example? Otherwise, this note > sounds too mysterious, IMO. The idea was to not distract the user here much with a use case that isn't that common, and have she follow the xref to see an example of that. It seems I posted the example in the "Forks" section, but didn't finish what I wanted to with it :-/ Will fix. > > > +Many commands will work the same with multiple programs as with a > > +single program: E.g., @code{print myglobal} will simply display the > ^^^^ > "e.g.", in lower case. Fixed. > > > +Ocasionaly, when debugging @value{GDBN} itself, it may be useful to > ^^^^^^^^^^ > "Occasionally" > > > +get more info about the relationship or inferiors, programs, address > ^^ > "of", I presume Fixed and yes. > > > +@smallexample > > +(@value{GDBP}) maint info program-spaces > > + Id Executable > > + 2 goodbye > > + Bound inferiors: ID 1 (process 21561) > > +* 1 hello > > +@end smallexample > > +@end table > > + > > +Here we can see that no inferior is running the program @code{hello}, > > +while @code{process 21561} is running the program @code{goodbye}. On > > +some targets, it is possible that multiple inferiors are bound to the > > +same program space. The most common example is that of debugging both > > +the parent and child processes of a @code{vfork} > > +call. @xref{Forks,,Debugging Forks}. > > Since this is about program spaces, not about inferiors, I'd suggest > to show an example that includes multiple inferiors in the same > program space. Otherwise, this looks awfully similar to "info > inferiors" and the purpose of this command remains unclear. Good idea, will do that. > > > +@code{follow-exec-mode} can be: > > + > > + > > + new - the debugger creates a new inferior and rebinds the process \n\ > > +to this new inferior. The program the process was running before\n\ > > +the exec call can be restarted afterwards by restarting the original\n\ > > +inferior.\n\ > > +\n\ > > + same - the debugger keeps the process bound to the same inferior.\n\ > > +The new executable image replaces the previous executable loaded in\n\ > > +the inferior. Restarting the inferior after the exec call restarts\n\ > > +the executable the process was running after the exec call.\n\ > > +\n\ > > +By default, the debugger will use the same inferior."), > > This looks like copy/paste from the doc strings in the source. It > should be removed, I think. Bah, of course. > > > +@value{GDBN} creates a new inferior and rebinds the process to this > > +new inferior. The program the process was running before the exec > ^^^^ > "exec" should be in @code. > Fixed. > > +inferior. Restarting the inferior after the exec call restarts the > > +executable the process was running after the exec call. This is the > > +default mode. > > Same here. > > > +(@value{GDBP}) info sspaces > > Should be "maint info program-spaces", right? That would be the direct mapping, but "info inferiors" would be better. I add adjusted the "new" variant but missed the "same" variant. Thanks for catching. > > > Index: src/gdb/NEWS > > This is okay, with a couple of comments: > > > +maint info program-spaces > > + List the program spaces loaded into GDB. > > This should probably be after the *-inferior commands, not before them. Hmm, bah, we missed something important here. Since this feature missed 7.0, I get to add a "Changes since GDB 7.0" section, and rewire these entries there. > > > + When the program execs, request to keep the previous executable' > ^^^^^^^^^^^ > "executable's", right? Right. > > > + symbol loaded, and load the new executable in a new symbol-space; or > > + request GDB to reuse the symbol-space and replace the previous > > + executable's symbols with the new executable. > > I think you want to replace "symbol space" with "program space". Yeah. I think I should rewrite that entry a bit. This text predates the "always-an-inferior" change and the corresponding docs/help entry of the setting doesn't describe the exact same thing. > > > +/* Add a new empty program space to the program space list, and binds it > > + to ASPACE. Returns the pointer to the new object. */ > > Either "add", "bind", and "return", or "adds", "binds", and "returns". Fixed. > > > +/* If ARGS is NULL or empty, print information about all symbol > > + spaces. Otherwise, ARGS is a text representation of a LONG > > "program spaces". Clearly my grepping failed to consider lines breaks. Fixed. > > +/* All known objfiles are kept in a linked list. This points to the > > + root of this list. */ > > I believe "root" is used for trees. Lists have a "head". I'm sure I could plant some species of lists. :-P Thanks, fixed. I'll post a new patch later, and I'll re-quote the bits that need review in particular. Thanks much, I couldn't spot these issues even after staring at the patch for long. -- Pedro Alves