Re: [v6] multi-executable support

Pedro Alves
2009-10-06T12:14:39Z

On Monday 05 October 2009 22:55:09, Eli Zaretskii wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > 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