Re: [RFA] [1/2] auto-loading scripts from .debug_gdb_scripts section

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Fri, 16 Apr 2010 00:05:40 -0700 (PDT)
> From: dje@google.com (Doug Evans)
> 
> This patch adds support for auto-loading scripts mentioned in
> section .debug_gdb_scripts.

Thanks.  I reviewed the documentation part of the patch.  There's also
a single comment to the code part.

> I moved the Auto-loading section of the docs out of the Python API section
> because I like it better that way, and because it solves a technical
> problem: there's no @subsubsubsection. :-)

The existing Python Auto-loading section is not too long, so how about
simply adding the description of this new feature to that section?  I
think ripping it out of "Python API" is not a good idea.

Btw, is this auto-loading feature supposed to be used only for Python
scripts, or does it support scripts written in the GDB scripting
language as well?  If the former, I think we should use "Python
scripts" more often than we do now, to avoid duping the user into
thinking otherwise.

> +** GDB now looks for scripts to auto-load in the .debug_gdb_scripts section.

This is rather cryptic, especially since this section is our own
invention.  How about this text instead:

  ** GDB now looks for names of Python scripts to auto-load in a
  special section named `.debug_gdb_scripts', in addition to looking
  for a OBJFILE-gdb.py script when OBJFILE is read by the debugger.

> +When a new object file is read (for example, due to the @code{file}
> +command, or because the inferior has loaded a shared library),
> +@value{GDBN} will look for support scripts in up to two ways:
                                              ^^^^^^^^^^^^^^^^^
I suggest "in several ways", because 2 is just what we currently
support, and we may extend that to more ways in the future.

> +@file{@var{objfile}-gdb.py} and @file{.debug_gdb_scripts} section.

`.debug_gdb_scripts' is a name of a section, not a file name, so it
should have the @code markup (here and elsewhere).

> +The auto-loading feature is useful for supplying application-specific
> +debugging commands and scripts.

Application-specific or "specific to an objfile"?  I think it's the
latter, because the former is adequately supported by a .gdbinit file
in the source tree.

> +When reading an auto-loaded file, @value{GDBN} sets the ``current
> +objfile''.  This is available via the @code{gdb.current_objfile}
> +function (@pxref{Objfiles In Python}).  This can be useful for
> +registering objfile-specific pretty-printers.

I think this paragraph should be moved to after you describe the
"maint set python auto-load" commands, because it's a minor aspect of
the features.

In addition, please use @dfn{current objfile} instead of the explicit
quotes, since you are introducing new terminology here.

Btw, why are these "maint" commands?  Aren't they useful to Joe Random
Hacker?

> +@node @file{@var{objfile}-gdb.py} file

Using @-commands in node names is a no-no, see the node "Node Line
Requirements" in the Texinfo manual.

> +@subsubsection @file{@var{objfile}-gdb.py} file

Chapter, section, and subsubsection names should follow the English
grammar (node names are exempt from this requirement), so please use
"The @file{@var{objfile}-gdb.py} file" instead (assuming you leave
this as a separate node, which I advise against).

> +If this file does not exist, and if the parameter
> +@code{debug-file-directory} is set (@pxref{Separate Debug Files}),
> +then @value{GDBN} will use for its each separated directory component
> +@code{component} the file named @file{@code{component}/@var{real-name}}, where
> +@var{real-name} is the object file's real name, as described above.

I know you just copied this from the original text, but this is
confusingly complicated.  How about this rewording:

  If this file does not exist, and if the parameter
  @code{debug-file-directory} is set (@pxref{Separate Debug Files}),
  then @value{GDBN} will look for the file in that directory and in
  all of its parents.

Btw, what does the text mean when it says "by ensuring that the file
name is absolute"?  Is this a requirement to the form of OBJFILE as it
is recorded in the binary?  Or does it mean GDB converts it to an
absolute file name if it is not?  In the former case, I presume that
@var{real-name} in the original text corresponds to the basename of
the script's name, not to its full absolute file name; is that
correct?  If the latter, then what does GDB do if the file name is
_not_ absolute?

> +Finally, if this file does not exist, then @value{GDBN} will look for
> +a file named @file{@var{data-directory}/python/auto-load/@var{real-name}}, where
> +@var{data-directory} is @value{GDBN}'s data directory (available via
> +@code{show data-directory}, @pxref{Data Files}),

Doesn't @pxref produce a capitalized "See" for this @pxref?  If so,
please use "see @ref" instead, or move "available via ..." part out of
the parentheses.

>                                                    and @var{real-name}
> +is the object file's real name, as described above.

Again, what about the leading directories in @var{real-name}?

> +So, your @samp{-gdb.py} file should take care to ensure that it may be
> +evaluated multiple times without error.

  So your @samp{-gdb.py} file should take care to ensure that, if it
  is evaluated multiple times, it does that without errors.

Or, better yet

  So your @file{-gdb.py} file should be careful to avoid errors if it
  is evaluated more than once.

> +@node @file{.debug_gdb_scripts} section
> +@subsubsection @file{.debug_gdb_scripts} section

Again, don't use @-commands in node names and use "The" in the
subsubsection name.

> +@cindex .debug_gdb_scripts section

Please give `.debug_gdb_scripts' a @code markup here (there's no
problem doing that in @cindex entries, which by default are typeset in
normal Roman typeface, unlike @findex etc.).

> +For systems using the ELF file format, and other formats supporting
> +multiple sections,

It would be good to state which platforms support multiple sections.
(I think all modern platforms do, in native debugging, but maybe I'm
wrong.)  The reader should not be required to be an expert in binary
formats, in order to understand whether this can be used on her
platform.

>                      when @value{GDBN} loads an @var{objfile}
> +it will look for scripts in the @file{.debug_gdb_scripts} section
> +when an objfile is loaded.
> +This section contains a list of names of scripts to load.

  when @value{GDBN} loads a new object file @var{objfile}, it will
  look for a special section named @samp{.debug_gdb_scripts}.  If this
  section exists, its contents should be a string specifying the name
  of the script file to load.

IOW, first say that GDB look for the section, then what its contents
are, and only then that the specified file will be auto-loaded.

> +File names are recorded instead of the file's contents so that
> +the user doesn't have to relink if a script changes,
> +and so that the plethora of random scripts are better managed,

If this is important, it should be at most in a @footnote.  We don't
generally explain our design decisions in the user manual.

> +The scripts are searched for first in the current directory, and then
> +in the source search path

  @value{GDBN} will look for the specified script file first in the
  current directory and then along the source search path

IOW, avoid unnecessary passive tense.

> +with the exception that @file{$cdir} is not searched, the compilation
> +directory is not relevant to scripts.

  except that @file{$cdir} is not searched, since the compilation
  directory is not relevant to scripts.

> +@example
> +#define DEFINE_GDB_SCRIPT(script_name) \
> +  asm("\
> +.pushsection \".debug_gdb_scripts\", \"MS\",@@progbits,1\n\
> +.byte 1\n\
> +.asciz \"" script_name "\"\n\
> +.popsection \n\
> +");
> +@end example

I'm far from being an expert on inline assembly, but doesn't the
".asciz" line have too many quotes, the script_name argument being
itself in quotes?

> +Then one can reference the macro in a header or source file like this:

Please put @noindent before this line, because it does not start a new
paragraph.  It is an explanation for the @example.

> +@example
> +DEFINE_GDB_SCRIPT ("my-app-scripts.py")
> +@end example

We should tell if the file name can be absolute, or whether it can
include any leading directories.  Btw, does the format of sections
impose any limitations on characters that can appear in this file
name?  If it does, we should state them.

> +Unlike loading of @file{@var{objfile}-gdb.py} scripts,
> +@value{GDBN} keeps track of scripts loaded this way.
> +If the reference to the script is coming from a header file,
> +the script can be referenced by multiple shared libraries.
> +As an optimization to prevent loading the script 100's or even 1000's
> +of times, @value{GDBN} will only auto-load a script once in each
> +program space.
> +However, you should still write your scripts to ensure
> +that it may be evaluated multiple times without error.

The last sentence means, IMO, that this paragraph should be removed.
The user should not need to think differently about the script
depending on how it is loaded, because it's quite reasonable to expect
the same script to be loaded using both methods in the same
application.  This text just adds confusion, IMO.

> +@node Which flavor to choose?
> +@subsubsection Which flavor to choose?
> +
> +Given the two ways of auto-loading scripts, it might not always be clear
> +which one to choose.  This section provides some guidance.

Btw, WIBNI the places where GDB searches for the two kinds of
auto-loaded scripts were not as dissimilar as they are now?  Why do we
need them to be so different?  After all, there's a reasonable chance
that both these methods will be used in the same project, so why force
the users to spread the same scripts in several different places?

> +For publicly installed libraries, e.g. libstdc++, there typically isn't a
                                     ^^^^^^^^^^^^^^
  e.g., @file{libstdc++}

(Note the comma after "e.g.", without it TeX will typeset the "e.g."
as if it ends a sentence.)

> +@kindex maint print section-scripts
> +@cindex info for known .debug_gdb_scripts-loaded scripts
> +@item maint print section-scripts [@var{regexp}]
> +Print a dump of scripts specified in the @file{.debug_gdb_section} section.
> +If @var{regexp} is specified, only print scripts loaded by object files
> +matching @var{regexp}.
> +For each script, this command prints its name as specified in the objfile,
> +and the full path if known.

This should have an @xref to the node where this feature is described.

> +  if (!input && debug_file_directory)
> +    {
> +      /* Also try the same file in the separate debug info directory.  */
> +      debugfile = xmalloc (strlen (filename)
> +			   + strlen (debug_file_directory) + 1);
> +      strcpy (debugfile, debug_file_directory);
> +      /* FILENAME is absolute, so we don't need a "/" here.  */
> +      strcat (debugfile, filename);

What will that last strcat do if FILENAME has a drive letter?

> +      strcpy (debugfile, gdb_datadir);
> +      strcat (debugfile, "/auto-load");
> +      /* FILENAME is absolute, so we don't need a "/" here.  */
> +      strcat (debugfile, filename);

Ditto.

Thanks.