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

[Doug Evans <dje@google.com>]

On Fri, Apr 16, 2010 at 2:37 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> 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.

To me the Auto-loading section is not really part of the API.
I can relax my definition of "API" to include auto-loading, but it's a hack.
So I *do* like moving Auto-loading out of "Python API", even though it
does also solve a technical problem.

> 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.

An early version of the patch supported auto-loading gdb scripts in
.debug_gdb_scripts too.
But in the interests of not trying to accomplish too much at once, I
deferred completing it for later.

>> +** 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.

Sure.

>> +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.

We could just change the text when that happens, but sure.

>> +@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).

thanks

>> +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.

This text is already present in the current docs.
I don't care whether its "application" or "objfile" though.

>> +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.

I disagree, but ok.

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

cut-n-paste from existing text, but sure.

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

Good question.  I think they shouldn't maint commands, but it's a
separate question to this patch.
[For reference sake, an early version of this patch had separate
options for enabling -gdb.py vs .debug_gdb_scripts.  It felt useful
enough, but wasn't important enough to try to get it into the tree.]

>> +@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.

Ah.

>> +@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).

Thanks.

>> +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.

"all of its parents"?
I *think* that comment is referring to the fact that
"debug-file-directory" contains a colon-separated list of directories
to try (bad option name of course - presumably kept for backward
compatibility).

> 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?

I think it was written that way to express the fact that we
(presumably) want to avoid ambiguities in the lookup of the file in,
for example, /usr/lib/debug.  We want to look up one path regardless
of whatever symlinks appear in the path that gdb is using.
E.g. if OBJFILE is /usr/lib/foo.so, and foo.so is a symlink to
/usr/lib/bar.so, we want to look up /usr/lib/debug/usr/lib/bar.so.
I think.

AIUI, real-name will always be absolute, it's the complete path of the objfile.

>
>> +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.

I guess makeinfo is smart enough to decide because I see lowercase in gdb.info.

>>                                                    and @var{real-name}
>> +is the object file's real name, as described above.
>
> Again, what about the leading directories in @var{real-name}?

I think real-name means the canonicalized full path 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.

ok.

>> +@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.

k

>> +@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.).

sure

>> +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.

Being an expert isn't required, but I understand the point.
OTOH, I'm not familiar with all formats, only the ones I've used.
Mentioning ELF and COFF I guess should suffice I think.

>>                      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.

I didn't want to go into implementation details of what
.debug_gdb_scripts contains here, and "should be a string" feels more
technical than "list of names of scripts" to load. :-) But whatever.
The actual format is slightly more detailed than just a string.

> 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.

ok

>> +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.

ok

>> +@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?

No, it's correct.  It's that way to make the transition from
concatenated C strings "foo" "bar" "baz" to what the assembler needs
to see: foo "bar" baz.

>> +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.

Ah.

>> +@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.

The file name can contain drive letters, directories, be relative or absolute.
The only restriction (that I can think of) is that it can't contain nul '\0'.

>> +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.

"works for me"

>> +@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?

I don't disagree but there *is* a distinction between the two flavors.
For example, it doesn't make much sense to search for OBJFILE-gdb.py
scripts in the source tree.

>> +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.)

Right.

>> +@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.

Sure.

>> +  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.

This is cut-n-pasted from the existing code to auto-load -gdb.py
scripts.  I don't know if filename can have a drive letter here.