From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36180 invoked by alias); 16 Sep 2019 15:53:43 -0000 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 Received: (qmail 36166 invoked by uid 89); 16 Sep 2019 15:53:42 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=clarification, Mike, removal, letters X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (207.211.31.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 16 Sep 2019 15:53:39 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mathworks.com; s=mimecast20180117; t=1568649217; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=M/UJ1BvklEsCI1S27q51t/eALTSUkSSX+ANqCEz0mhs=; b=MkW2tsBcwdPoe2kpekTldQ0n7sPzo7W27kOWhCANLeNNqz+3cSDOP8w4HIQwGhHaiAVZwr zaFh7LEP8zrlSfjJwX18S69xf+gGihQm4cmc/SviU5jrvIFmiDB7IYN3+1+xoTy8JXu5IF DhyuRiur4WleLsuV3bA8ZCRf/8tj6m0= Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03lp2057.outbound.protection.outlook.com [104.47.40.57]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-225--77WZ6m-NECZSR3szwcABQ-1; Mon, 16 Sep 2019 11:53:35 -0400 Received: from DM6PR05MB4156.namprd05.prod.outlook.com (20.176.72.29) by DM6PR05MB5051.namprd05.prod.outlook.com (20.177.223.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2284.17; Mon, 16 Sep 2019 15:53:32 +0000 Received: from DM6PR05MB4156.namprd05.prod.outlook.com ([fe80::c0e:e3e2:676d:971d]) by DM6PR05MB4156.namprd05.prod.outlook.com ([fe80::c0e:e3e2:676d:971d%3]) with mapi id 15.20.2284.009; Mon, 16 Sep 2019 15:53:32 +0000 From: Mike Gulick To: Andrew Burgess CC: Mike Gulick , Eli Zaretskii , "gdb-patches@sourceware.org" Subject: Re: [RFC] Apply compilation dir to source_path lookup Date: Mon, 16 Sep 2019 15:53:00 -0000 Message-ID: References: <20190907235059.GN6076@embecosm.com> <4d9f4c81-8580-2b42-a434-389ed7655d7f@mathworks.com> <20190913013851.GT6076@embecosm.com> <835zlw1z45.fsf@gnu.org> <834l1g1wp7.fsf@gnu.org> <20190913224535.GX6076@embecosm.com> <83r24jz7q4.fsf@gnu.org> <20190914152847.GY6076@embecosm.com> <83tv9eyiu0.fsf@gnu.org> <20190915040101.GZ6076@embecosm.com> In-Reply-To: <20190915040101.GZ6076@embecosm.com> x-ms-exchange-transport-forked: True x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 Content-ID: MIME-Version: 1.0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Fu0LnKYSBHTFU1sDGRJ3exoi3QhM2VlrlYd5qDp0CHe593Df7jpDL5pUKj2VtuHsCvQR+vYcglXPk/x1yPRCmw== X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-SW-Source: 2019-09/txt/msg00284.txt.bz2 On 9/15/19 12:01 AM, Andrew Burgess wrote: > * Mike Gulick [2019-09-15 02:07:37 +0000]: >=20 >> On 9/14/19 11:54 AM, Eli Zaretskii wrote: >>>> Date: Sat, 14 Sep 2019 11:28:47 -0400 >>>> From: Andrew Burgess >>>> Cc: mgulick@mathworks.com, gdb-patches@sourceware.org >>>> >>>> If we have: >>>> >>>> DW_AT_name: c:/project/foo.c >>>> DW_AT_comp_dir: d:/project/build/ >>>> >>>> And if the directory search path is: >>>> >>>> g:/mnt/project;$cdir;$cwd >>>> >>>> And the current directory is: >>>> >>>> h:/home/ >>>> >>>> Then this is what I think the search order will be: >>>> >>>> c:/project/foo.c >>>> g:/mnt/project/project/foo.c >>>> d:/project/build/project/foo.c >>>> h:/home/project/foo.c >>>> >>>> d:/project/build/project/foo.c >>>> g:/mnt/project/project/build/project/foo.c >>>> d:/project/build/project/build/project/foo.c >>>> h:/home/project/build/project/foo.c >>>> >>>> g:/mnt/project/foo.c >>>> d:/project/build/foo.c >>>> h:/home/foo.c >>>> >>>> The first and third block is what we currently (pre-patch) do, and the >>>> second block is what I think the patch should provide after Mike's >>>> last suggested update. >>> >>> SGTM, thanks. >>> >> >> Thanks Andrew and Eli for the clarification on the proper behavior on >> DOS paths. The only remaining questionable case for me is whether to >> check $cdir/COMP_DIR/FILENAME, which in your example above is >> 'd:/project/build/project/build/project/foo.c'. It seems reasonable >> to me to leave this out of the search, although one could argue >> leaving it on the source path may make the documentation slightly more >> straightforward (one less special case). >> >> Please let me know if you would like me to provide an updated version >> of the patch. >=20 > Personally I don't see it as a problem. I agree it is unlikely to be > something that ever makes sense, but we are already trying to access > lots of paths that don't exist, I think throwing one more into the mix > isn't going to hurt. >=20 > I took another stab at the documentation, this time I've moved the > explanations for $cdir and $cwd earlier in the section - previously > these were described really late on, and it made writing the earlier > text hard as I was trying not to reference these things before they > were introduced. >=20 > The docs now focus more on some worked examples rather than trying to > just explain the algorithm in text. I hope this is better. I've also > included an example of how MS drive letters are handled. Thanks. There's a lot of possible paths to search now, so I think seeing then written out is helpful. I have some minor corrections and suggestions annotated in the code below. > On the code side I have made one change from Mike's last patch, > looking in openp at the code that glues filenames onto directories it > had three preprocessing steps: >=20 > 1. Remove drive letters (for dos), > 2. Remove leading slashes (/foo.c -> foo.c), and > 3. Remove leading './' sequences (./foo.c -> foo.c). >=20 > I've now factored these three steps out and we share them between > openp and open_and_find_source, otherwise the code is unchanged from > Mike's last patch. I also have a minor suggestion here that may handle one additional edge case, please see below. >=20 > Let me know what you think. >=20 > Thanks, > Andrew >=20 > --- >=20 > commit 70bbf4fc4358a8c10b20faf565ab3b0dee2a20c7 > Author: Mike Gulick > Date: Thu Sep 12 11:16:06 2019 -0400 >=20 > gdb: Look for compilation directory relative to directory search path >=20=20=20=20=20 > The 'directory' command allows the user to provide a list of filesyst= em > directories in which to search for source code. The directories in t= his > search path are used as the base directory for the source filename fr= om > the debug information (DW_AT_name). Thus the directory search path > provides alternatives to the existing compilation directory from the > debug information (DW_AT_comp_dir). Generally speaking, DW_AT_name > stores the filename argument passed to the compiler (including any > directory components), and DW_AT_comp_dir stores the current working > directory from which the compiler was executed. For example: >=20=20=20=20=20 > $ cd /path/to/project/subdir1 > $ gcc -c a/test.c -g >=20=20=20=20=20 > The corresponding debug information will look like this: >=20=20=20=20=20 > DW_AT_name : a/test.c > DW_AT_comp_dir : /path/to/project/subdir1 >=20=20=20=20=20 > When compiling with the -fdebug-prefix-map GCC option, the compilation > directory can be arbitrarily rewritten. In the above example, we may > rewrite the compilation directory as follows: >=20=20=20=20=20 > $ gcc -c a/test.c -g -fdebug-prefix-map=3D/path/to/project=3D >=20=20=20=20=20 > In this case, the corresponding debug information will look like: >=20=20=20=20=20 > DW_AT_name : a/test.c > DW_AT_comp_dir : /subdir1 >=20=20=20=20=20 > This prevents GDB from finding the corresponding source code based on > the debug information alone. In some cases, a substitute-path command > can be used to re-map a consistent prefix in the rewritten compilation > directory to the real filesystem path. However, there may not be a > consistent prefix remaining in the debug symbols (for example in a > project that has source code in many subdirectories under the project= 's > root), thereby requiring multiple substitute-path rules. In this cas= e, > it is easier to add the missing prefix to the directory search path v= ia > the 'directory' command. >=20=20=20=20=20 > The function find_and_open_source currently searches in: >=20=20=20=20=20 > SEARCH_PATH/FILENAME >=20=20=20=20=20 > where SEARCH_PATH corresponds to each individual entry in the directo= ry > search path (which is guaranteed to contain the compilation directory > from the debug information, as well as the current working directory). > FILENAME corresponds to the source filename (DW_AT_name), which may h= ave > directory components in it. In addition, GDB searches in: >=20=20=20=20=20 > SEARCH_PATH/FILE_BASENAME >=20=20=20=20=20 > where FILE_BASENAME is the basename of the DW_AT_name entry. >=20=20=20=20=20 > This change modifies find_and_open_source to additionally search in: >=20=20=20=20=20 > SEARCH_PATH/COMP_DIR/FILENAME >=20=20=20=20=20 > where COMP_DIR is the compilation directory from the debug symbols. = In > the example given earlier, running: >=20=20=20=20=20 > (gdb) directory /path/to/project >=20=20=20=20=20 > will now allow GDB to correctly locate the source code from the debug > information. >=20=20=20=20=20 > gdb/ChangeLog: >=20=20=20=20=20 > * source.c (prepare_path_for_appending): New function. > (openp): Make use of new function. > (find_and_open_source): Search for the compilation directory = and > source file as a relative path beneath the directory search p= ath. >=20=20=20=20=20 > gdb/doc/ChangeLog: >=20=20=20=20=20 > * gdb.texinfo (Source Path): Additional text to better descri= be > how the source path directory list is used when searching for > source files. >=20=20=20=20=20 > gdb/testsuite/ChangeLog: >=20=20=20=20=20 > * gdb.base/source-dir.exp: Add extra test for mapped compilat= ion > directory. >=20 > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 6d5f19d04bb..a173113929a 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,10 @@ > +2019-09-12 Mike Gulick > + > + * source.c (prepare_path_for_appending): New function. > + (openp): Make use of new function. > + (find_and_open_source): Search for the compilation directory and > + source file as a relative path beneath the directory search path. > + > 2019-09-12 Rainer Orth >=20=20 > * procfs.c (procfs_target::wait) : Get signal from > diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog > index 96c04091973..97c99663985 100644 > --- a/gdb/doc/ChangeLog > +++ b/gdb/doc/ChangeLog > @@ -1,3 +1,9 @@ > +2019-09-12 Andrew Burgess > + > + * gdb.texinfo (Source Path): Additional text to better describe > + how the source path directory list is used when searching for > + source files. > + > 2019-09-10 Tom Tromey >=20=20 > * gdb.texinfo (Index Files): Update Ada text. > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo > index 79824a0226a..20ab1e576ec 100644 > --- a/gdb/doc/gdb.texinfo > +++ b/gdb/doc/gdb.texinfo > @@ -8954,11 +8954,20 @@ > in the list, until it finds a file with the desired name. >=20=20 > For example, suppose an executable references the file > -@file{/usr/src/foo-1.0/lib/foo.c}, and our source path is > -@file{/mnt/cross}. The file is first looked up literally; if this > -fails, @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c} is tried; if this > -fails, @file{/mnt/cross/foo.c} is opened; if this fails, an error > -message is printed. @value{GDBN} does not look up the parts of the > +@file{/usr/src/foo-1.0/lib/foo.c} and does not record a compilation ^^^^^^^^^^^^^ , does not > +directory, the @dfn{source path} is @file{/mnt/cross}. @value{GDBN} ^^^^ and the > +would look for the source file in the following locations: > + > +@enumerate > + > +@item @file{/usr/src/foo-1.0/lib/foo.c} > +@item @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c} > +@item @file{/mnt/cross/foo.c} > + > +@end enumerate > + > +If the source file is not present at any of the above locations then > +an error is printed. @value{GDBN} does not look up the parts of the > source file name, such as @file{/mnt/cross/src/foo-1.0/lib/foo.c}. > Likewise, the subdirectories of the source path are not searched: if > the source path is @file{/mnt/cross}, and the binary refers to > @@ -8966,11 +8975,91 @@ > @file{/mnt/cross/usr/src/foo-1.0/lib}. >=20=20 > Plain file names, relative file names with leading directories, file > -names containing dots, etc.@: are all treated as described above; for > -instance, if the source path is @file{/mnt/cross}, and the source file > -is recorded as @file{../lib/foo.c}, @value{GDBN} would first try > -@file{../lib/foo.c}, then @file{/mnt/cross/../lib/foo.c}, and after > -that---@file{/mnt/cross/foo.c}. > +names containing dots, etc.@: are all treated as described above, > +except that non-absolute file names are not looked up literally. If > +the @dfn{source path} is @file{/mnt/cross}, and the source file is ^^^ no 'and' here > +recorded as @file{../lib/foo.c} and no compilation directory is ^ comma here > +recorded then @value{GDBN} will search in the following locations: ^ comma here > + > +@enumerate > + > +@item @file{/mnt/cross/../lib/foo.c} > +@item @file{/mnt/cross/foo.c} > + > +@end enumerate > + > +@kindex cdir > +@kindex cwd > +@vindex $cdir@r{, convenience variable} > +@vindex $cwd@r{, convenience variable} > +@cindex compilation directory > +@cindex current directory > +@cindex working directory > +@cindex directory, current > +@cindex directory, compilation > +The @dfn{source path} will always include two special entries > +@samp{$cdir} and @samp{$cwd}, these refer to the compilation directory > +(if one is recorded) and the current workig directory respectively. ^^^^^^^ working > + > +@samp{cdir} causes @value{GDBN} to search within the compilation ^^^^ $cdir > +directory, if one is recorded in the debug information. If no > +compilation directory is recorded in the debug information then > +@samp{cdir} is ignored. ^^^^ $cdir > + > +@samp{$cwd} is not the same as @samp{.}---the former tracks the > +current working directory as it changes during your @value{GDBN} > +session, while the latter is immediately expanded to the current > +directory at the time you add an entry to the source path. > + > +If a compilation directory is recorded in the debug information, and > +@value{GDBN} has not found the source file after the first search > +using @dfn{source path}, then @value{GDBN} will combine the > +compilation directory and the filename, and then search for the source > +file again using the @dfn{source path}. > + > +For example, if the executable records the source file as > +@file{/usr/src/foo-1.0/lib/foo.c} the compilation directory is ^ comma here > +recorded as @file{/project/build}, and the @dfn{source path} is > +@file{/mnt/cross;$cdir;$cwd} while the current working directory of ^^^^^^^^^^^^^^^^^^^^^ /mnt/cross:$cdir:$cwd It is probably better to use colons rather than semicolons since the example uses unix paths. > +the @value{GDBN} session is @file{/home/user}, then @value{GDBN} will > +search for the source file in the following loctions: > + > +@enumerate > + > +@item @file{/usr/src/foo-1.0/lib/foo.c} > +@item @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c} > +@item @file{/project/build/usr/src/foo-1.0/lib/foo.c} > +@item @file{/home/user/usr/src/foo-1.0/lib/foo.c} > +@item @file{/mnt/cross/project/build/usr/src/foo-1.0/lib/foo.c} > +@item @file{/project/build/project/build/usr/src/foo-1.0/lib/foo.c} > +@item @file{/home/user/project/build/usr/src/foo-1.0/lib/foo.c} > +@item @file{/mnt/cross/foo.c} > +@item @file{/project/build/foo.c} > +@item @file{/home/user/foo.c} > + > +@end enumerate > + > +If the file name in the previous example had been recorded in the > +executable as a relative path rather than an absolute path, then the > +first look up would not have occurred, but all of the remaining steps > +would be similar. > + > +When searching for source files on MS-DOS and MS-Windows, where > +absolute paths start with a drive letter (e.g. > +@file{C:/project/foo.c}), @value{GDBN} will remove the drive letter > +from the file name before appending it to a search directory from > +@dfn{source path}; for instance if the executable references the > +source file @file{C:/project/foo.c} and @dfn{source path} is set to > +@file{D:/mnt/cross}, then @value{GDBN} will search in the following > +locations for the source file: > + > +@enumerate > + > +@item @file{C:/project/foo.c} > +@item @file{D:/mnt/cross/project/foo.c} > +@item @file{D:/mnt/cross/foo.c} > + > +@end enumerate >=20=20 > Note that the executable search path is @emph{not} used to locate the > source files. > @@ -9058,21 +9147,12 @@ > whitespace. You may specify a directory that is already in the source > path; this moves it forward, so @value{GDBN} searches it sooner. >=20=20 > -@kindex cdir > -@kindex cwd > -@vindex $cdir@r{, convenience variable} > -@vindex $cwd@r{, convenience variable} > -@cindex compilation directory > -@cindex current directory > -@cindex working directory > -@cindex directory, current > -@cindex directory, compilation > -You can use the string @samp{$cdir} to refer to the compilation > -directory (if one is recorded), and @samp{$cwd} to refer to the current > -working directory. @samp{$cwd} is not the same as @samp{.}---the former > -tracks the current working directory as it changes during your @value{GD= BN} > -session, while the latter is immediately expanded to the current > -directory at the time you add an entry to the source path. > +The special strings @samp{$cdir} (to refer to the compilation > +directory, if one is recorded), and @samp{$cwd} (to refer to the > +current working directory) can also be included in the list of > +directories @var{dirname}. Though these will already be in the source > +path they will be moved forward in the list so @value{GDBN} searches > +them sooner. >=20=20 > @item directory > Reset the source path to its default value (@samp{$cdir:$cwd} on Unix sy= stems). This requires confirmation. I would also change the other spot that uses the names 'cdir' and 'cwd' to be '$cdir' and '$cwd'. I don't see any value in referring to these special names without the leading '$': @@ -9070,8 +9071,8 @@ each line is in the file. =20 @kindex directory @kindex dir -When you start @value{GDBN}, its source path includes only @samp{cdir} -and @samp{cwd}, in that order. +When you start @value{GDBN}, its source path includes only @samp{$cdir} +and @samp{$cwd}, in that order. To add other directories, use the @code{directory} command. =20 The search path is used to find both program source files and @value{GDBN} > diff --git a/gdb/source.c b/gdb/source.c > index b27f210802c..ece1ad3d16b 100644 > --- a/gdb/source.c > +++ b/gdb/source.c > @@ -654,6 +654,29 @@ info_source_command (const char *ignore, int from_tt= y) > } > =0C >=20=20 > +/* Helper function to remove characters from the start of PATH so that > + PATH can then be appended to a directory name. We remove leading dri= ve > + letters (for dos) as well as leading '/' characters and './' > + sequences. */ > + > +const char * > +prepare_path_for_appending (const char *path) > +{ > + /* For dos paths, d:/foo -> /foo, and d:foo -> foo. */ > + if (HAS_DRIVE_SPEC (path)) > + path =3D STRIP_DRIVE_SPEC (path); > + > + /* /foo =3D> foo, to avoid multiple slashes that Emacs doesn't like. = */ > + while (IS_DIR_SEPARATOR(path[0])) > + path++; > + > + /* ./foo =3D> foo */ > + while (path[0] =3D=3D '.' && IS_DIR_SEPARATOR (path[1])) > + path +=3D 2; > + > + return path; > +} > + This may be a little pedantic, but looping around the removal of the '/' and './' would handle a case like './/': /* Helper function to remove characters from the start of PATH so that PATH can then be appended to a directory name. We remove leading drive letters (for dos) as well as leading '/' characters and './' sequences. */ const char * prepare_path_for_appending (const char *path) { /* For dos paths, d:/foo -> /foo, and d:foo -> foo. */ if (HAS_DRIVE_SPEC (path)) path =3D STRIP_DRIVE_SPEC (path); const char *oldpath; /* Remove any sequence of '/' and './'. */ do { oldpath =3D path; /* /foo =3D> foo, to avoid multiple slashes that Emacs doesn't like. */ if (IS_DIR_SEPARATOR(path[0])) path++; /* ./foo =3D> foo */ if (path[0] =3D=3D '.' && IS_DIR_SEPARATOR (path[1])) path +=3D 2; } while (path !=3D oldpath); return path; } Feel free to use your discretion regarding these changes, I'm fine either way. Thanks a lot for all of the help. Again, let me know if there's anything else I should do. Thanks, Mike