Re: [RFC] Apply compilation dir to source_path lookup

[Mike Gulick <mgulick@mathworks.com>]

On 9/13/19 6:45 PM, Andrew Burgess wrote:
> * Eli Zaretskii <eliz@gnu.org> [2019-09-13 10:28:52 +0300]:
> 
>>> Date: Fri, 13 Sep 2019 09:36:42 +0300
>>> From: Eli Zaretskii <eliz@gnu.org>
>>> CC: mgulick@mathworks.com, gdb-patches@sourceware.org
>>>
>>>>                         @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?
>>
>> Btw, do the "prepend" and "append", as implemented, take care to DTRT
>> with Windows drive letters at the beginning of absolute file names?  A
>> literal prepending or appending will do the wrong thing there.

You beat me to a response, but here's what I was going to say:

The only way this would be a problem is if both the compilation
directory and the source file contained a drive letter.  I had assumed
that if the debug information contained a compilation directory, then
the file path would be relative to that.  GCC at least seems to behave
this way.

[mgulick@mgulick-deb9-64:~/test/src] ...
$ gcc -g -o test.o -fdebug-prefix-map=$HOME= -c test.c
[mgulick@mgulick-deb9-64:~/test/src] ...
$ dwarfdump test.o
...
                    DW_AT_name                  test.c
                    DW_AT_comp_dir              /test/src
...

[mgulick@mgulick-deb9-64:~/test/src] ...
$ gcc -g -o test.o -fdebug-prefix-map=$HOME= -c `pwd`/test.c
[mgulick@mgulick-deb9-64:~/test/src] ...
$ dwarfdump test.o
...
                    DW_AT_name                  /test/src/test.c
...

In this case there is no DW_AT_comp_dir present.

If you are concerned about this (possibly some crazy compiler emitting
strange dwarf), the following change should suffice:

diff --git a/gdb/source.c b/gdb/source.c
index 1635563b20..3fd05a06f2 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1049,8 +1049,12 @@ find_and_open_source (const char *filename,
        cdir_filename.pop_back ();
       /* Add our own directory separator.  */
       cdir_filename.append (SLASH_STRING);
-      /* Append filename, without any leading directory separators.  */
+      /* Append filename, without any leading directory separators or drive
+        names.  */
       const char * filename_start = filename;
+      /* For dos paths, d:/foo -> /foo, and d:foo -> foo.  */
+      if (HAS_DRIVE_SPEC (filename_start))
+       filename_start = STRIP_DRIVE_SPEC (filename_start);
       while (IS_DIR_SEPARATOR (filename_start[0]))
        filename_start++;
       cdir_filename.append (filename_start);

> 
> Gah!
> 
> Looking at the implementation of 'openp' (in source.c) I see this
> code:
> 
>   /* For dos paths, d:/foo -> /foo, and d:foo -> foo.  */
>   if (HAS_DRIVE_SPEC (string))
>     string = STRIP_DRIVE_SPEC (string);
> 
> which just seems to throw out the drive spec, and I can't find any
> code that adds it back in.  Is that going to do the right thing?
> 
> I did consider only creating the COMP_DIR/FILENAME combination if
> FILENAME was not absolute, but I worried that this might be
> unnecessarily restrictive, but now I'm tempted to say that would solve
> this problem, and we should just wait until someone comes up with an
> example where that is not good enough, before we figure out how to
> allow it...

This also seems fine to me.  If FILENAME is absolute, there shouldn't be
a compilation directory.

> 
> Thanks,
> Andrew
> 

Thanks,
Mike