[RFC] Apply compilation dir to source_path lookup
Mike Gulick
mgulick@mathworks.com
Fri Sep 13 22:52:00 GMT 2019
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
More information about the Gdb-patches
mailing list