Re: [RFC] Apply compilation dir to source_path lookup

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

Mike,

Thanks for your feedback.  I incorporated your suggested changes and
pushed this to master.

Thanks,
Andrew



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

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