Re: [RFC] Apply compilation dir to source_path lookup

[Mike Gulick <mgulick@mathworks.com>]

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.

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

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?

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