From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5158 invoked by alias); 16 Apr 2010 18:06:58 -0000 Received: (qmail 4871 invoked by uid 22791); 16 Apr 2010 18:06:55 -0000 X-SWARE-Spam-Status: No, hits=1.7 required=5.0 tests=BAYES_50,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SARE_MSGID_LONG45,SPF_HELO_PASS,TW_BJ,TW_CP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 16 Apr 2010 18:06:49 +0000 Received: from kpbe20.cbf.corp.google.com (kpbe20.cbf.corp.google.com [172.25.105.84]) by smtp-out.google.com with ESMTP id o3GI6kMC027355 for ; Fri, 16 Apr 2010 11:06:46 -0700 Received: from pwj2 (pwj2.prod.google.com [10.241.219.66]) by kpbe20.cbf.corp.google.com with ESMTP id o3GI5mU2007077 for ; Fri, 16 Apr 2010 11:06:45 -0700 Received: by pwj2 with SMTP id 2so1979534pwj.17 for ; Fri, 16 Apr 2010 11:06:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.140.247.17 with HTTP; Fri, 16 Apr 2010 11:06:44 -0700 (PDT) In-Reply-To: <83tyrbwxex.fsf@gnu.org> References: <20100416070540.4205E84396@ruffy.mtv.corp.google.com> <83tyrbwxex.fsf@gnu.org> Date: Fri, 16 Apr 2010 18:06:00 -0000 Received: by 10.141.139.13 with SMTP id r13mr2270028rvn.216.1271441204806; Fri, 16 Apr 2010 11:06:44 -0700 (PDT) Message-ID: Subject: Re: [RFA] [1/2] auto-loading scripts from .debug_gdb_scripts section From: Doug Evans To: Eli Zaretskii Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true 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: 2010-04/txt/msg00518.txt.bz2 On Fri, Apr 16, 2010 at 2:37 AM, Eli Zaretskii 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. =A0I reviewed the documentation part of the patch. =A0There's also > a single comment to the code part. > >> I moved the Auto-loading section of the docs out of the Python API secti= on >> 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? =A0I > 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? =A0If 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 sec= tion. > > This is rather cryptic, especially since this section is our own > invention. =A0How about this text instead: > > =A0** GDB now looks for names of Python scripts to auto-load in a > =A0special section named `.debug_gdb_scripts', in addition to looking > =A0for 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: > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0^^^^^^^^^^^^^^^^^ > 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"? =A0I 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''. =A0This is available via the @code{gdb.current_objfile} >> +function (@pxref{Objfiles In Python}). =A0This 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? =A0Aren'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. =A0How about this rewording: > > =A0If this file does not exist, and if the parameter > =A0@code{debug-file-directory} is set (@pxref{Separate Debug Files}), > =A0then @value{GDBN} will look for the file in that directory and in > =A0all 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"? =A0Is this a requirement to the form of OBJFILE as it > is recorded in the binary? =A0Or does it mean GDB converts it to an > absolute file name if it is not? =A0In 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? =A0If 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 objf= ile. > >> +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? =A0If 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.i= nfo. >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0and @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. > > =A0So your @samp{-gdb.py} file should take care to ensure that, if it > =A0is evaluated multiple times, it does that without errors. > > Or, better yet > > =A0So your @file{-gdb.py} file should be careful to avoid errors if it > =A0is 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.) =A0The 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. >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0when @value{GDBN} loads an @v= ar{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. > > =A0when @value{GDBN} loads a new object file @var{objfile}, it will > =A0look for a special section named @samp{.debug_gdb_scripts}. =A0If this > =A0section exists, its contents should be a string specifying the name > =A0of 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. =A0We 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 > > =A0@value{GDBN} will look for the specified script file first in the > =A0current 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. > > =A0except that @file{$cdir} is not searched, since the compilation > =A0directory is not relevant to scripts. ok >> +@example >> +#define DEFINE_GDB_SCRIPT(script_name) \ >> + =A0asm("\ >> +.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. =A0It 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. =A0Btw, does the format of sections > impose any limitations on characters that can appear in this file > name? =A0If it does, we should state them. The file name can contain drive letters, directories, be relative or absolu= te. 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. =A0This 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. =A0This 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? =A0Why do we > need them to be so different? =A0After 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 > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ^= ^^^^^^^^^^^^^ > =A0e.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} sect= ion. >> +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 objfi= le, >> +and the full path if known. > > This should have an @xref to the node where this feature is described. Sure. >> + =A0if (!input && debug_file_directory) >> + =A0 =A0{ >> + =A0 =A0 =A0/* Also try the same file in the separate debug info direct= ory. =A0*/ >> + =A0 =A0 =A0debugfile =3D xmalloc (strlen (filename) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0+ strlen (debug_file_di= rectory) + 1); >> + =A0 =A0 =A0strcpy (debugfile, debug_file_directory); >> + =A0 =A0 =A0/* FILENAME is absolute, so we don't need a "/" here. =A0*/ >> + =A0 =A0 =A0strcat (debugfile, filename); > > What will that last strcat do if FILENAME has a drive letter? > >> + =A0 =A0 =A0strcpy (debugfile, gdb_datadir); >> + =A0 =A0 =A0strcat (debugfile, "/auto-load"); >> + =A0 =A0 =A0/* FILENAME is absolute, so we don't need a "/" here. =A0*/ >> + =A0 =A0 =A0strcat (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.