Re: [RFC] Apply compilation dir to source_path lookup

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Mike Gulick <mgulick@mathworks.com> [2019-09-13 22:10:54 +0000]:

> 
> 
> On 9/13/19 2:36 AM, Eli Zaretskii wrote:
> >> Date: Thu, 12 Sep 2019 21:38:51 -0400
> >> From: Andrew Burgess <andrew.burgess@embecosm.com>
> >> Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>,
> >> 	Eli Zaretskii <eliz@gnu.org>
> >>
> >> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> >> index 79824a0226a..b6b87b28b9f 100644
> >> --- a/gdb/doc/gdb.texinfo
> >> +++ b/gdb/doc/gdb.texinfo
> >> @@ -8968,9 +8968,23 @@
> >>  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}.
> >> +is recorded as @file{../lib/foo.c} and no compilation directory is
> >> +recorded, @value{GDBN} would first try @file{../lib/foo.c}, then
> >> +@file{/mnt/cross/../lib/foo.c}, and after that
> >> +@file{/mnt/cross/foo.c}.
> >> +
> >> +When there is both a filename and a compilation directory in the debug
> >> +information, and if @value{GDBN} hasn't found the source file using
> >> +the above approach, then @value{GDBN} will append the filename to the
> >> +compilation directory and retry using the above steps; for instance,
> >> +if the source path is @file{/mnt/cross}, and the source file is
> >> +recorded as @file{../lib/foo.c} and the compilation directory is
> >> +recorded as @file{/usr/src/foo-1.0/build}, @value{GDBN} would first
> >> +try @file{../lib/foo.c}, then @file{/mnt/cross/../lib/foo.c}, then
> >> +@file{/mnt/cross/foo.c}, using the approach outlined above.  If the
> >> +source file still hasn't been found then @value{GDBN} will next check
> >> +@file{/usr/src/foo-1.0/build/../lib/foo.c}, then
> >> +@file{/mnt/cross/usr/src/foo-1.0/build/../lib/foo.c}.
> > 
> > This text is unnecessarily long and complicated.  How about this
> > shortened version -- does it correctly convey the intent?
> > 
> >  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}.
> >  If the above search fails to find the source file @file{foo.c}, and
> >  the compilation directory is recorded in the debug information,
> >  @value{GDBN} will repeat the search using the compilation directory
> >  as if it were in the source path.
> 
> This doesn't quite capture the change.  The original documentation
> wasn't quite right to start with either.  If I'm understanding
> find_and_open_source correctly, GDB would actually search for
> /mnt/cross/../lib/foo.c before searching for ../lib/foo.c.  Only if the
> source file is an absolute path does openp first try the filename on its
> own (see OPF_TRY_CWD_FIRST).  For the purposes of this example, let's
> call the source filename from DW_AT_name SOURCE_FILE, the compilation
> directory from DW_AT_comp_dir COMP_DIR, and let's say source_path
> contained '/a:/b:$cdir:$cwd'.  As we know, GDB enforces that '$cdir'
> (the compilation directory) and '$cwd' (the current working directory)
> are the last two entries on the source path.

Not exactly, GDB will add '$cdir:$cwd' to the end of the directory
list so you can never NOT have these in the search path, but you can
bring them earlier in the list:

    (gdb) show directories
    Source directories searched: $cdir:$cwd
    (gdb) set directories $cwd:$cdir:/a/:/b
    (gdb) show directories
    Source directories searched: $cwd:$cdir:/a:/b

I've both reordered them and brought them to the start of the list.

> 
> Prior to this change, I believe GDB would search in the following order:
> 
>  # First openp call searches:
>  SOURCE_FILE  (only if SOURCE_FILE is absolute)

I hadn't picked up on the only if absolute subtlety.

>  /a/SOURCE_FILE
>  /b/SOURCE_FILE
>  $cdir/SOURCE_FILE
>  $cwd/SOURCE_FILE
>  # Second openp call searches:
>  /a/basename(SOURCE_FILE)
>  /b/basename(SOURCE_FILE)
>  $cdir/basename(SOURCE_FILE)
>  $cwd/basename(SOURCE_FILE)
> 
> After this change, GDB will search in the following order
> 
>  # First openp call searches:
>  SOURCE_FILE  (only if SOURCE_FILE is absolute)
>  /a/SOURCE_FILE
>  /b/SOURCE_FILE
>  $cdir/SOURCE_FILE
>  $cwd/SOURCE_FILE
>  # Second (new) openp call searches:
>  COMP_DIR/SOURCE_FILE  (only if COMP_DIR is absolute)
>  /a/COMP_DIR/SOURCE_FILE
>  /b/COMP_DIR/SOURCE_FILE
>  $cdir/COMP_DIR/SOURCE_FILE
>  $cwd/COMP_DIR/SOURCE_FILE
>  # Third openp call searches:
>  /a/basename(SOURCE_FILE)
>  /b/basename(SOURCE_FILE)
>  $cdir/basename(SOURCE_FILE)
>  $cwd/basename(SOURCE_FILE)
> 
> In all cases, if there is no recorded compilation directory, any entries
> referring to $cdir or COMP_DIR will be skipped.  Clearly the search for
> $cdir/COMP_DIR/SOURCE_FILE is unnecessary.  I had thought it seemed like
> more work to construct a new source_path with the compilation directory
> removed than it is to leave it there under the assumption that it won't
> exist.  Actually, if we call
> 
>  result = openp (source_path, flags, cdir_filename.c_str (), OPEN_MODE,
>                  fullname);
> 
> rather than
> 
>  result = openp (path, flags, cdir_filename.c_str (), OPEN_MODE,
>                  fullname);
> 
> for this particular search, source_path will contain '$cdir' explicitly,
> rather than the expanded form, and will be ignored by the continue on
> line 820.  That's probably a worthwhile modification.

I'm not sure I'm comfortable with that.  I did think about this too
and figured trying /COMP_DIR/COMP_DIR/SOURCE_FILE was probably
harmless, but if you don't like it I think filtering path again to
remove $cwd would be required.

> 
> The two examples in the documentation (with the absolute path and the
> relative path) fall a little short.  They don't capture the difference
> that the relative path is *not* attempted before the search path.  They
> also don't describe how the compilation directory is taken into account.
> Perhaps it would be better to just lay out the search order as a list
> like I have above?

The idea of listing a set of examples might be a better way to go.

I'll try to rewrite the docs when I get a chance.

Thanks,
Andrew



> 
> > 
> >>                         @value{GDBN} will also append the compilation
> >> +directory to the filename and check this against all other entries in
> >> +the source path.
> > 
> > I think "append" here is a mistake.  Should it be "prepend"?  And
> > anyway, doesn't this simply repeat what was described in the text
> > above?
> > 
> > Thanks.
> > 
> 
> Thanks,
> Mike
>