public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC] Apply compilation dir to source_path lookup
@ 2019-09-05 22:40 Mike Gulick
  2019-09-07 23:51 ` Andrew Burgess
  0 siblings, 1 reply; 20+ messages in thread
From: Mike Gulick @ 2019-09-05 22:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Mike Gulick

Hi,

I'm having trouble getting gdb to find the corresponding source for a
binary whose compilation directory was rewritten using
-fdebug-prefix-map.  I am doing this in order to make builds
reproducible.  I am using this gcc switch to remove a portion of the
compile directory, e.g.

$ cd $HOME/test/src/
$ gcc -g -o test -fdebug-prefix-map=$HOME= test.c
$ dwarfdump test
...
                    DW_AT_name                  test.c
                    DW_AT_comp_dir              /test/src
...

When attempting to debug the resulting binary, gdb is unable to find the
source file even if I add the removed path $HOME to the gdb source
path:

$ cd $HOME
$ gdb --nh ~/test/src/test
GNU gdb (GDB) 8.3
...
(gdb) directory /home/mgulick
Source directories searched: /home/mgulick:$cdir:$cwd
(gdb) info sources
Source files for which symbols have been read in:

/test/src/test.c

Source files for which symbols will be read in on demand:


(gdb) b test.c:3
Breakpoint 1 at 0x664: file test.c, line 3.
(gdb) r
Starting program: /mathworks/home/mgulick/test/src/test 

Breakpoint 1, main () at test.c:3
3	test.c: No such file or directory.


It turns out that only the source file, i.e. DW_AT_name, is used when
searching the source path.  This is surprising given the example with
'/usr/src/foo-1.0/lib/foo.c' and '/mnt/cross' in the gdb documentation
here: https://sourceware.org/gdb/onlinedocs/gdb/Source-Path.html.  It
seems uncommon for DW_AT_name to be the full path to the source file in
most programs I have come across.  In that example from the
documentation, it is likely that DW_AT_name would be 'lib/foo.c' and
DW_AT_comp_dir would be '/usr/src/foo-1.0'.

I have implemented two different approaches to handle this, both of
which work, and I wanted feedback on a) whether you think this is a
legitimate bug/feature, and b) which approach you prefer.

Approach 1 is to include another pass in find_and_open_source where the
compilation directory is prepended to the filename before searching for
the file with openp.  This allows the example I provided above to work
as-is.

Approach 2 is to allow '$cdir' to appear as part of an entry in the
source_path.  Currently it is only allowed to exist as a standalone
entry.  This would allow me to say 'directory /home/mgulick/$cdir', and
find_and_open_source would expand the $cdir part of this string to the
compilation directory.

I prefer approach 1 for a couple of reasons:

1) It is simpler for users to understand.  It doesn't really require
understanding the difference between DW_AT_comp_dir and DW_AT_name.  It
will match the source file in /home/mgulick/test.c, /test/src/test.c, or
/home/mgulick/test/src/test.c.  Of course, since '$cdir' is still in
source_path, it will also look in /test/src/test/src/test.c, which is a
little confusing, but shouldn't be a problem (and we could explicitly
remove '$cdir' from the path when doing this search).

2) It seems to match the existing gdb documentation.

I dislike approach 2 because it makes the directory strings more
complicated, and it also means I'm unable to have a directory named
'/home/mgulick/$cdir' (bizarre, I know, but such things are possible).

Here is a preliminary patch for the first approach.  I will go ahead and
add a test-case and re-send a formal patch review request if this seems
like an acceptable solution.

Thanks,
Mike

diff --git a/gdb/source.c b/gdb/source.c
index b27f210802..b7b5741028 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1033,7 +1033,35 @@ find_and_open_source (const char *filename,
   openp_flags flags = OPF_SEARCH_IN_PATH;
   if (basenames_may_differ)
     flags |= OPF_RETURN_REALPATH;
+
+  /* Try to locate file using filename */
   result = openp (path, flags, filename, OPEN_MODE, fullname);
+  if (result < 0 && dirname != NULL)
+    {
+      /* Try to locate file using compilation dir + filename.  This is helpful
+	 if part of the compilation directory was removed, e.g. using gcc's
+	 -fdebug-prefix-map, and we have added the missing prefix to
+	 source_path. */
+      /* Allocate space for dirname, possibly an extra slash, the filename,
+	 and terminating null */
+      char * cdir_filename = (char *)
+	alloca (strlen (dirname) + strlen(SLASH_STRING) + strlen (filename) + 1);
+
+      cdir_filename = strcpy (cdir_filename, dirname);
+      int len = strlen (cdir_filename);    /* last char in cdir_filename */
+
+      /* Add directory separator if not provided by dirname or filename */
+      if (!(IS_DIR_SEPARATOR (cdir_filename[len]) ||
+	    IS_DIR_SEPARATOR (filename[0])))
+        strcat (cdir_filename, SLASH_STRING);
+      else if (IS_DIR_SEPARATOR (cdir_filename[len]) &&
+	       IS_DIR_SEPARATOR (filename[0]))
+	/* Both provide a slash, use only one */
+	cdir_filename[len] = '\0';
+      strcat(cdir_filename,filename);
+
+      result = openp(path, flags, cdir_filename, OPEN_MODE, fullname);
+    }
   if (result < 0)
     {
       /* Didn't work.  Try using just the basename.  */

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-05 22:40 [RFC] Apply compilation dir to source_path lookup Mike Gulick
@ 2019-09-07 23:51 ` Andrew Burgess
  2019-09-09 22:41   ` Mike Gulick
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2019-09-07 23:51 UTC (permalink / raw)
  To: Mike Gulick; +Cc: gdb-patches

* Mike Gulick <mgulick@mathworks.com> [2019-09-05 22:40:48 +0000]:

> Hi,
> 
> I'm having trouble getting gdb to find the corresponding source for a
> binary whose compilation directory was rewritten using
> -fdebug-prefix-map.  I am doing this in order to make builds
> reproducible.  I am using this gcc switch to remove a portion of the
> compile directory, e.g.
> 
> $ cd $HOME/test/src/
> $ gcc -g -o test -fdebug-prefix-map=$HOME= test.c
> $ dwarfdump test
> ...
>                     DW_AT_name                  test.c
>                     DW_AT_comp_dir              /test/src
> ...
> 
> When attempting to debug the resulting binary, gdb is unable to find the
> source file even if I add the removed path $HOME to the gdb source
> path:
> 
> $ cd $HOME
> $ gdb --nh ~/test/src/test
> GNU gdb (GDB) 8.3
> ...
> (gdb) directory /home/mgulick
> Source directories searched: /home/mgulick:$cdir:$cwd
> (gdb) info sources
> Source files for which symbols have been read in:
> 
> /test/src/test.c
> 
> Source files for which symbols will be read in on demand:
> 
> 
> (gdb) b test.c:3
> Breakpoint 1 at 0x664: file test.c, line 3.
> (gdb) r
> Starting program: /mathworks/home/mgulick/test/src/test 
> 
> Breakpoint 1, main () at test.c:3
> 3	test.c: No such file or directory.
> 
> 
> It turns out that only the source file, i.e. DW_AT_name, is used when
> searching the source path.  This is surprising given the example with
> '/usr/src/foo-1.0/lib/foo.c' and '/mnt/cross' in the gdb documentation
> here: https://sourceware.org/gdb/onlinedocs/gdb/Source-Path.html.  It
> seems uncommon for DW_AT_name to be the full path to the source file in
> most programs I have come across.  In that example from the
> documentation, it is likely that DW_AT_name would be 'lib/foo.c' and
> DW_AT_comp_dir would be '/usr/src/foo-1.0'.

That depends on how you compile your source:

   gcc -o my.exe /usr/src/foo-1.0/lib/foo.c

would result in DW_AT_name being '/usr/src/foo-1.0/lib/foo.c' I
believe.

So I think the example is fine, though I don't think it really goes
into enough details.

> 
> I have implemented two different approaches to handle this, both of
> which work, and I wanted feedback on a) whether you think this is a
> legitimate bug/feature, and b) which approach you prefer.
> 
> Approach 1 is to include another pass in find_and_open_source where the
> compilation directory is prepended to the filename before searching for
> the file with openp.  This allows the example I provided above to work
> as-is.
> 
> Approach 2 is to allow '$cdir' to appear as part of an entry in the
> source_path.  Currently it is only allowed to exist as a standalone
> entry.  This would allow me to say 'directory /home/mgulick/$cdir', and
> find_and_open_source would expand the $cdir part of this string to the
> compilation directory.
> 
> I prefer approach 1 for a couple of reasons:
> 
> 1) It is simpler for users to understand.  It doesn't really require
> understanding the difference between DW_AT_comp_dir and DW_AT_name.  It
> will match the source file in /home/mgulick/test.c, /test/src/test.c, or
> /home/mgulick/test/src/test.c.  Of course, since '$cdir' is still in
> source_path, it will also look in /test/src/test/src/test.c, which is a
> little confusing, but shouldn't be a problem (and we could explicitly
> remove '$cdir' from the path when doing this search).
> 
> 2) It seems to match the existing gdb documentation.
> 
> I dislike approach 2 because it makes the directory strings more
> complicated, and it also means I'm unable to have a directory named
> '/home/mgulick/$cdir' (bizarre, I know, but such things are
> possible).

I agree with your approach, I think it's nicer if things can "just
work" for the user, and this seems like a good approach.

> 
> Here is a preliminary patch for the first approach.  I will go ahead and
> add a test-case and re-send a formal patch review request if this seems
> like an acceptable solution.

Do you have a copyright assignment in place?  Any patch over a couple
of trivial lines requires an assignment.  Others might correct me, but
this patch would seem to be big enough to me.

> 
> Thanks,
> Mike
> 
> diff --git a/gdb/source.c b/gdb/source.c
> index b27f210802..b7b5741028 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1033,7 +1033,35 @@ find_and_open_source (const char *filename,
>    openp_flags flags = OPF_SEARCH_IN_PATH;
>    if (basenames_may_differ)
>      flags |= OPF_RETURN_REALPATH;
> +
> +  /* Try to locate file using filename */

Comments need to end with '.', and there should always be two spaces
after a '.' in comments.

>    result = openp (path, flags, filename, OPEN_MODE, fullname);
> +  if (result < 0 && dirname != NULL)
> +    {
> +      /* Try to locate file using compilation dir + filename.  This is helpful
> +	 if part of the compilation directory was removed, e.g. using gcc's
> +	 -fdebug-prefix-map, and we have added the missing prefix to
> +	 source_path. */
> +      /* Allocate space for dirname, possibly an extra slash, the filename,
> +	 and terminating null */

You need two spaces at the end of your comments, and can you fold
these two comments into one.

> +      char * cdir_filename = (char *)
> +	alloca (strlen (dirname) + strlen(SLASH_STRING) + strlen (filename) + 1);

Whitespace between function name and opening parenthesis.

> +
> +      cdir_filename = strcpy (cdir_filename, dirname);
> +      int len = strlen (cdir_filename);    /* last char in cdir_filename */

Comments should be formatted as sentences, so capital 'L' here, and
'.  ' and the end.

> +
> +      /* Add directory separator if not provided by dirname or filename */
> +      if (!(IS_DIR_SEPARATOR (cdir_filename[len]) ||
> +	    IS_DIR_SEPARATOR (filename[0])))

Break the line before the '||'.

> +        strcat (cdir_filename, SLASH_STRING);
> +      else if (IS_DIR_SEPARATOR (cdir_filename[len]) &&
> +	       IS_DIR_SEPARATOR (filename[0]))

Same, line break before '&&'.

> +	/* Both provide a slash, use only one */

Comment formatting.

> +	cdir_filename[len] = '\0';
> +      strcat(cdir_filename,filename);

Whitespace before opening parenthesis.

> +
> +      result = openp(path, flags, cdir_filename, OPEN_MODE, fullname);

And again.

> +    }
>    if (result < 0)
>      {
>        /* Didn't work.  Try using just the basename.  */

You'll also need to write a short ChangeLog entry for the patch.  I
also wonder if the function could be simplified by using std::string
to build the new string rather than all of the strlen/strcpy/strcat
calls?  Though I don't think that would be a requirement for
acceptance.

You might also consider extending the documentation relating to source
path searching to better describe the process - as you discuss above,
the existing documentation is not that clear on the subject.

Finally, you'd probably want to write a test.... however, I was
wondering how a test for this might be written, and ended up writing
one myself.  Feel free to use this as a starting point, or just
include this as is if it helps.

Thanks,
Andrew

---

commit c10a25ca9a1eb8f7e2cbab9b8fd652fe9feb16de
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date:   Sun Sep 8 00:19:48 2019 +0100

    gdb/testsuite: New test for mapped compilation directory
    
    Ensure GDB can find source files when compiling using the
    -fdebug-prefix-map GCC option.  The problem is that the compilation
    directory is not tried against other directories in the directory
    search path.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.base/source-dir.c: New file.
            * gdb.base/source-dir.exp: Add extra test for mapped compilation
            directory.

diff --git a/gdb/testsuite/gdb.base/source-dir.c b/gdb/testsuite/gdb.base/source-dir.c
new file mode 100644
index 00000000000..a9bce4a1445
--- /dev/null
+++ b/gdb/testsuite/gdb.base/source-dir.c
@@ -0,0 +1,5 @@
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/source-dir.exp b/gdb/testsuite/gdb.base/source-dir.exp
index 048c0e95161..ecfb74ef27d 100644
--- a/gdb/testsuite/gdb.base/source-dir.exp
+++ b/gdb/testsuite/gdb.base/source-dir.exp
@@ -15,9 +15,127 @@
 
 standard_testfile
 
-gdb_start
+proc search_dir_list { dirs } {
+    set output "\r\nSource directories searched: "
+    append output [join $dirs "\[:;\]"]
 
-set foo "/nOtExStInG"
+    return ${output}
+}
 
-gdb_test "directory $foo/a $foo/b $foo/c" "\r\nSource directories searched: $foo/a\[:;\]$foo/b\[:;\]$foo/c\[:;\]\\\$cdir\[:;\]\\\$cwd"
-gdb_test "directory $foo/b $foo/d $foo/c" "\r\nSource directories searched: $foo/b\[:;\]$foo/d\[:;\]$foo/c\[:;\]$foo/a\[:;\]\\\$cdir\[:;\]\\\$cwd"
+
+# Check that adding directories to the search path changes the order
+# in which directories are searched.
+proc test_changing_search_directory {} {
+    gdb_start
+
+    set foo "/nOtExStInG"
+
+    gdb_test "directory $foo/a $foo/b $foo/c" \
+	[search_dir_list [list \
+			      "$foo/a" \
+			      "$foo/b" \
+			      "$foo/c" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_test "directory $foo/b $foo/d $foo/c" \
+	[search_dir_list [list \
+			      "$foo/b" \
+			      "$foo/d" \
+			      "$foo/c" \
+			      "$foo/a" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_exit
+}
+
+# Test that the compilation directory can also be extended with a
+# prefix from the directory search path in order to find source files.
+proc test_truncated_comp_dir {} {
+    global srcfile srcdir subdir binfile
+
+    # When we run this test the current directory will be something
+    # like this:
+    #     /some/path/to/gdb/build/testsuite/
+    # We are going to copy the source file out of the source tree into
+    # a location like this:
+    #     /some/path/to/gdb/build/testsuite/output/gdb.base/soure-dir/
+    #
+    # We will then switch to this directory and compile the source
+    # file, however, we will ask GCC to remove this prefix from the
+    # compilation directory in the debug info:
+    #     /some/path/to/gdb/build/testsuite/output/
+    #
+    # As a result the debug information will look like this:
+    #
+    #     DW_AT_name        : source-dir.c
+    #     DW_AT_comp_dir    : /gdb.base/source-dir
+    #
+    # Finally we switch back to this directory:
+    #     /some/path/to/gdb/build/testsuite/
+    #
+    # and start GDB.  There was a time when GDB would be unable to
+    # find the source file no matter what we added to the directory
+    # search path, this should now be fixed.
+
+    set original_dir [pwd]
+    set working_dir [standard_output_file ""]
+    cd ${working_dir}
+
+    set strip_dir [file normalize "${working_dir}/../.."]
+
+    set new_srcfile [standard_output_file ${srcfile}]
+    remote_exec host "cp ${srcdir}/${subdir}/${srcfile} ${new_srcfile}"
+
+    set options \
+	"debug additional_flags=-fdebug-prefix-map=${strip_dir}="
+    if  { [gdb_compile "${srcfile}" "${binfile}" \
+	       executable ${options}] != "" } {
+	untested "failed to compile"
+	return -1
+    }
+
+    cd ${original_dir}
+
+    clean_restart ${binfile}
+
+    gdb_test_no_output "set directories \$cdir:\$cwd"
+    gdb_test "show directories" \
+	"\r\nSource directories searched: \\\$cdir\[:;\]\\\$cwd"
+
+    if ![runto_main] then {
+	fail "can't run to main"
+	return 0
+    }
+
+    gdb_test "info source" \
+	[multi_line \
+	     "Current source file is ${srcfile}" \
+	     "Compilation directory is \[^\n\r\]+" \
+	     "${srcfile}: No such file or directory."]
+
+    gdb_test "dir $strip_dir" \
+	[search_dir_list [list \
+			      "$strip_dir" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_test "list" [multi_line \
+			 "1\[ \t\]+int" \
+			 "2\[ \t\]+main \\(\\)" \
+			 "3\[ \t\]+\\{" \
+			 "4\[ \t\]+return 0;" \
+			 "5\[ \t\]+\\}" ]
+
+    gdb_test "info source" \
+	[multi_line \
+	     "Current source file is ${srcfile}" \
+	     "Compilation directory is \[^\n\r\]+" \
+	     "Located in ${new_srcfile}" \
+	     "Contains 5 lines." \
+	     "Source language is c." \
+	     "Producer is \[^\n\r\]+" \
+	     "\[^\n\r\]+" \
+	     "\[^\n\r\]+" ]
+}
+
+test_changing_search_directory
+test_truncated_comp_dir


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-07 23:51 ` Andrew Burgess
@ 2019-09-09 22:41   ` Mike Gulick
  2019-09-13  1:38     ` Andrew Burgess
  0 siblings, 1 reply; 20+ messages in thread
From: Mike Gulick @ 2019-09-09 22:41 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: Mike Gulick, gdb-patches

On 9/7/19 7:50 PM, Andrew Burgess wrote:
> * Mike Gulick <mgulick@mathworks.com> [2019-09-05 22:40:48 +0000]:
 
[snip]

>> Here is a preliminary patch for the first approach.  I will go ahead and
>> add a test-case and re-send a formal patch review request if this seems
>> like an acceptable solution.
> 
> Do you have a copyright assignment in place?  Any patch over a couple
> of trivial lines requires an assignment.  Others might correct me, but
> this patch would seem to be big enough to me.
> 

Yes, I do already have a copyright assignment in place.

>>
>> Thanks,
>> Mike
>>
>> diff --git a/gdb/source.c b/gdb/source.c
>> index b27f210802..b7b5741028 100644
>> --- a/gdb/source.c
>> +++ b/gdb/source.c
>> @@ -1033,7 +1033,35 @@ find_and_open_source (const char *filename,
>>    openp_flags flags = OPF_SEARCH_IN_PATH;
>>    if (basenames_may_differ)
>>      flags |= OPF_RETURN_REALPATH;
>> +
>> +  /* Try to locate file using filename */
> 
> Comments need to end with '.', and there should always be two spaces
> after a '.' in comments.
> 
>>    result = openp (path, flags, filename, OPEN_MODE, fullname);
>> +  if (result < 0 && dirname != NULL)
>> +    {
>> +      /* Try to locate file using compilation dir + filename.  This is helpful
>> +	 if part of the compilation directory was removed, e.g. using gcc's
>> +	 -fdebug-prefix-map, and we have added the missing prefix to
>> +	 source_path. */
>> +      /* Allocate space for dirname, possibly an extra slash, the filename,
>> +	 and terminating null */
> 
> You need two spaces at the end of your comments, and can you fold
> these two comments into one.
> 
>> +      char * cdir_filename = (char *)
>> +	alloca (strlen (dirname) + strlen(SLASH_STRING) + strlen (filename) + 1);
> 
> Whitespace between function name and opening parenthesis.
> 
>> +
>> +      cdir_filename = strcpy (cdir_filename, dirname);
>> +      int len = strlen (cdir_filename);    /* last char in cdir_filename */
> 
> Comments should be formatted as sentences, so capital 'L' here, and
> '.  ' and the end.
> 
>> +
>> +      /* Add directory separator if not provided by dirname or filename */
>> +      if (!(IS_DIR_SEPARATOR (cdir_filename[len]) ||
>> +	    IS_DIR_SEPARATOR (filename[0])))
> 
> Break the line before the '||'.
> 
>> +        strcat (cdir_filename, SLASH_STRING);
>> +      else if (IS_DIR_SEPARATOR (cdir_filename[len]) &&
>> +	       IS_DIR_SEPARATOR (filename[0]))
> 
> Same, line break before '&&'.
> 
>> +	/* Both provide a slash, use only one */
> 
> Comment formatting.
> 
>> +	cdir_filename[len] = '\0';
>> +      strcat(cdir_filename,filename);
> 
> Whitespace before opening parenthesis.
> 
>> +
>> +      result = openp(path, flags, cdir_filename, OPEN_MODE, fullname);
> 
> And again.
> 
>> +    }
>>    if (result < 0)
>>      {
>>        /* Didn't work.  Try using just the basename.  */
> 
> You'll also need to write a short ChangeLog entry for the patch.  I
> also wonder if the function could be simplified by using std::string
> to build the new string rather than all of the strlen/strcpy/strcat
> calls?  Though I don't think that would be a requirement for
> acceptance.
> 

Thanks for the feedback.  I've rewritten the patch to use std::string
and address the formatting issues.  Using std::string does seem nicer
since it avoids the tricky alloca calls, at the cost of maybe being a
little more expensive.  It is also a little clearer to read.

> You might also consider extending the documentation relating to source
> path searching to better describe the process - as you discuss above,
> the existing documentation is not that clear on the subject.
>

I'll take a look at this and reply with a separate patch.

> Finally, you'd probably want to write a test.... however, I was
> wondering how a test for this might be written, and ended up writing
> one myself.  Feel free to use this as a starting point, or just
> include this as is if it helps.
>

Thanks for writing a test.  I recall this being the most challenging
part of my last GDB contribution, so this saved me a ton of time.  Your
test seems to test the scenario I had in mind.  Please use your patch
as-is.

> Thanks,
> Andrew
> 

Here is the new version of my patch.  Please let me know if you have any
additional feedback!

-Mike

---

From 5f3957aceb6a9390b412c310f626d7ab4fa80b62 Mon Sep 17 00:00:00 2001
From: Mike Gulick <mgulick@mathworks.com>
Date: Mon, 9 Sep 2019 18:36:23 -0400
Subject: [PATCH] gdb: Look for compilation directory relative to directory
 search path

The 'directory' command allows the user to provide a list of filesystem
directories in which to search for source code.  The directories in this
search path are used as the base directory for the source filename from
the debug information (DW_AT_name).  Thus the directory search path
provides alternatives to the existing compilation directory from the
debug information (DW_AT_comp_dir).  Generally speaking, DW_AT_name
stores the filename argument passed to the compiler (including any
directory components), and DW_AT_comp_dir stores the current working
directory from which the compiler was executed.  For example:

    $ cd /path/to/project/subdir1
    $ gcc -c a/test.c -g

The corresponding debug information will look like this:

    DW_AT_name      : a/test.c
    DW_AT_comp_dir  : /path/to/project/subdir1

When compiling with the -fdebug-prefix-map GCC option, the compilation
directory can be arbitrarily rewritten.  In the above example, we may
rewrite the compilation directory as follows:

    $ gcc -c a/test.c -g -fdebug-prefix-map=/path/to/project=

In this case, the corresponding debug information will look like:

    DW_AT_name      : a/test.c
    DW_AT_comp_dir  : /subdir1

This prevents GDB from finding the corresponding source code based on
the debug information alone.  In some cases, a substitute-path command
can be used to re-map a consistent prefix in the rewritten compilation
directory to the real filesystem path.  However, there may not be a
consistent prefix remaining in the debug symbols (for example in a
project that has source code in many subdirectories under the project's
root), thereby requiring multiple substitute-path rules.  In this case,
it is easier to add the missing prefix to the directory search path via
the 'directory' command.

The function find_and_open_source currently searches in:

    SEARCH_PATH/FILENAME

where SEARCH_PATH corresponds to each individual entry in the directory
search path (which is guaranteed to contain the compilation directory
from the debug information, as well as the current working directory).
FILENAME corresponds to the source filename (DW_AT_name), which may have
directory components in it.  In addition, GDB searches in:

    SEARCH_PATH/FILE_BASENAME

where FILE_BASENAME is the basename of the DW_AT_name entry.

This change modifies find_and_open_source to additionally search in:

    SEARCH_PATH/COMP_DIR/FILENAME

where COMP_DIR is the compilation directory from the debug symbols.  In
the example given earlier, running:

    (gdb) directory /path/to/project

will now allow GDB to correctly locate the source code from the debug
information.

gdb/ChangeLog:

	* source.c (find_and_open_source): Search for the compilation
	directory and source file as a relative path beneath the directory
	search path.
---
 gdb/ChangeLog |  6 ++++++
 gdb/source.c  | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index eb6d062812..25e3fc8e5d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2019-09-09  Mike Gulick  <mgulick@mathworks.com>
+
+	* source.c (find_and_open_source): Search for the compilation
+	directory and source file as a relative path beneath the directory
+	search path.
+
 2019-09-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* python/python.c (do_start_initialization): Make progname_copy static,
diff --git a/gdb/source.c b/gdb/source.c
index b27f210802..1635563b20 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1033,7 +1033,31 @@ find_and_open_source (const char *filename,
   openp_flags flags = OPF_SEARCH_IN_PATH;
   if (basenames_may_differ)
     flags |= OPF_RETURN_REALPATH;
+
+  /* Try to locate file using filename.  */
   result = openp (path, flags, filename, OPEN_MODE, fullname);
+  if (result < 0 && dirname != NULL)
+    {
+      /* Try to locate file using compilation dir + filename.  This is helpful
+	 if part of the compilation directory was removed, e.g. using gcc's
+	 -fdebug-prefix-map, and we have added the missing prefix to
+	 source_path.  */
+      std::string cdir_filename (dirname);
+
+      /* Remove any trailing directory separators.  */
+      while (IS_DIR_SEPARATOR (cdir_filename.back ()))
+	cdir_filename.pop_back ();
+      /* Add our own directory separator.  */
+      cdir_filename.append (SLASH_STRING);
+      /* Append filename, without any leading directory separators.  */
+      const char * filename_start = filename;
+      while (IS_DIR_SEPARATOR (filename_start[0]))
+	filename_start++;
+      cdir_filename.append (filename_start);
+
+      result = openp (path, flags, cdir_filename.c_str (), OPEN_MODE,
+		      fullname);
+    }
   if (result < 0)
     {
       /* Didn't work.  Try using just the basename.  */
-- 
2.20.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-09 22:41   ` Mike Gulick
@ 2019-09-13  1:38     ` Andrew Burgess
  2019-09-13  6:36       ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2019-09-13  1:38 UTC (permalink / raw)
  To: Mike Gulick; +Cc: gdb-patches, Eli Zaretskii

* Mike Gulick <mgulick@mathworks.com> [2019-09-09 22:41:02 +0000]:

> On 9/7/19 7:50 PM, Andrew Burgess wrote:
> > * Mike Gulick <mgulick@mathworks.com> [2019-09-05 22:40:48 +0000]:
>  
> [snip]
> 
> >> Here is a preliminary patch for the first approach.  I will go ahead and
> >> add a test-case and re-send a formal patch review request if this seems
> >> like an acceptable solution.
> > 
> > Do you have a copyright assignment in place?  Any patch over a couple
> > of trivial lines requires an assignment.  Others might correct me, but
> > this patch would seem to be big enough to me.
> > 
> 
> Yes, I do already have a copyright assignment in place.
> 
> >>
> >> Thanks,
> >> Mike
> >>
> >> diff --git a/gdb/source.c b/gdb/source.c
> >> index b27f210802..b7b5741028 100644
> >> --- a/gdb/source.c
> >> +++ b/gdb/source.c
> >> @@ -1033,7 +1033,35 @@ find_and_open_source (const char *filename,
> >>    openp_flags flags = OPF_SEARCH_IN_PATH;
> >>    if (basenames_may_differ)
> >>      flags |= OPF_RETURN_REALPATH;
> >> +
> >> +  /* Try to locate file using filename */
> > 
> > Comments need to end with '.', and there should always be two spaces
> > after a '.' in comments.
> > 
> >>    result = openp (path, flags, filename, OPEN_MODE, fullname);
> >> +  if (result < 0 && dirname != NULL)
> >> +    {
> >> +      /* Try to locate file using compilation dir + filename.  This is helpful
> >> +	 if part of the compilation directory was removed, e.g. using gcc's
> >> +	 -fdebug-prefix-map, and we have added the missing prefix to
> >> +	 source_path. */
> >> +      /* Allocate space for dirname, possibly an extra slash, the filename,
> >> +	 and terminating null */
> > 
> > You need two spaces at the end of your comments, and can you fold
> > these two comments into one.
> > 
> >> +      char * cdir_filename = (char *)
> >> +	alloca (strlen (dirname) + strlen(SLASH_STRING) + strlen (filename) + 1);
> > 
> > Whitespace between function name and opening parenthesis.
> > 
> >> +
> >> +      cdir_filename = strcpy (cdir_filename, dirname);
> >> +      int len = strlen (cdir_filename);    /* last char in cdir_filename */
> > 
> > Comments should be formatted as sentences, so capital 'L' here, and
> > '.  ' and the end.
> > 
> >> +
> >> +      /* Add directory separator if not provided by dirname or filename */
> >> +      if (!(IS_DIR_SEPARATOR (cdir_filename[len]) ||
> >> +	    IS_DIR_SEPARATOR (filename[0])))
> > 
> > Break the line before the '||'.
> > 
> >> +        strcat (cdir_filename, SLASH_STRING);
> >> +      else if (IS_DIR_SEPARATOR (cdir_filename[len]) &&
> >> +	       IS_DIR_SEPARATOR (filename[0]))
> > 
> > Same, line break before '&&'.
> > 
> >> +	/* Both provide a slash, use only one */
> > 
> > Comment formatting.
> > 
> >> +	cdir_filename[len] = '\0';
> >> +      strcat(cdir_filename,filename);
> > 
> > Whitespace before opening parenthesis.
> > 
> >> +
> >> +      result = openp(path, flags, cdir_filename, OPEN_MODE, fullname);
> > 
> > And again.
> > 
> >> +    }
> >>    if (result < 0)
> >>      {
> >>        /* Didn't work.  Try using just the basename.  */
> > 
> > You'll also need to write a short ChangeLog entry for the patch.  I
> > also wonder if the function could be simplified by using std::string
> > to build the new string rather than all of the strlen/strcpy/strcat
> > calls?  Though I don't think that would be a requirement for
> > acceptance.
> > 
> 
> Thanks for the feedback.  I've rewritten the patch to use std::string
> and address the formatting issues.  Using std::string does seem nicer
> since it avoids the tricky alloca calls, at the cost of maybe being a
> little more expensive.  It is also a little clearer to read.
> 
> > You might also consider extending the documentation relating to source
> > path searching to better describe the process - as you discuss above,
> > the existing documentation is not that clear on the subject.
> >
> 
> I'll take a look at this and reply with a separate patch.
> 
> > Finally, you'd probably want to write a test.... however, I was
> > wondering how a test for this might be written, and ended up writing
> > one myself.  Feel free to use this as a starting point, or just
> > include this as is if it helps.
> >
> 
> Thanks for writing a test.  I recall this being the most challenging
> part of my last GDB contribution, so this saved me a ton of time.  Your
> test seems to test the scenario I had in mind.  Please use your patch
> as-is.
> 
> > Thanks,
> > Andrew
> > 
> 
> Here is the new version of my patch.  Please let me know if you have any
> additional feedback!

I think the new version is great.  I've taken the liberty of merging
your change and my test case into one patch, and also writing an
update to the docs to (hopefully) better explain the source finding
algorithm.

You code is maintained unchanged in this patch except for one very
minor nit that I've pointed out below.

We can just wait for a doc review, and then I think this can be
merged.

Thanks,
Andrew

> 
> -Mike
> 
> ---
> 
> From 5f3957aceb6a9390b412c310f626d7ab4fa80b62 Mon Sep 17 00:00:00 2001
> From: Mike Gulick <mgulick@mathworks.com>
> Date: Mon, 9 Sep 2019 18:36:23 -0400
> Subject: [PATCH] gdb: Look for compilation directory relative to directory
>  search path
> 
> The 'directory' command allows the user to provide a list of filesystem
> directories in which to search for source code.  The directories in this
> search path are used as the base directory for the source filename from
> the debug information (DW_AT_name).  Thus the directory search path
> provides alternatives to the existing compilation directory from the
> debug information (DW_AT_comp_dir).  Generally speaking, DW_AT_name
> stores the filename argument passed to the compiler (including any
> directory components), and DW_AT_comp_dir stores the current working
> directory from which the compiler was executed.  For example:
> 
>     $ cd /path/to/project/subdir1
>     $ gcc -c a/test.c -g
> 
> The corresponding debug information will look like this:
> 
>     DW_AT_name      : a/test.c
>     DW_AT_comp_dir  : /path/to/project/subdir1
> 
> When compiling with the -fdebug-prefix-map GCC option, the compilation
> directory can be arbitrarily rewritten.  In the above example, we may
> rewrite the compilation directory as follows:
> 
>     $ gcc -c a/test.c -g -fdebug-prefix-map=/path/to/project=
> 
> In this case, the corresponding debug information will look like:
> 
>     DW_AT_name      : a/test.c
>     DW_AT_comp_dir  : /subdir1
> 
> This prevents GDB from finding the corresponding source code based on
> the debug information alone.  In some cases, a substitute-path command
> can be used to re-map a consistent prefix in the rewritten compilation
> directory to the real filesystem path.  However, there may not be a
> consistent prefix remaining in the debug symbols (for example in a
> project that has source code in many subdirectories under the project's
> root), thereby requiring multiple substitute-path rules.  In this case,
> it is easier to add the missing prefix to the directory search path via
> the 'directory' command.
> 
> The function find_and_open_source currently searches in:
> 
>     SEARCH_PATH/FILENAME
> 
> where SEARCH_PATH corresponds to each individual entry in the directory
> search path (which is guaranteed to contain the compilation directory
> from the debug information, as well as the current working directory).
> FILENAME corresponds to the source filename (DW_AT_name), which may have
> directory components in it.  In addition, GDB searches in:
> 
>     SEARCH_PATH/FILE_BASENAME
> 
> where FILE_BASENAME is the basename of the DW_AT_name entry.
> 
> This change modifies find_and_open_source to additionally search in:
> 
>     SEARCH_PATH/COMP_DIR/FILENAME
> 
> where COMP_DIR is the compilation directory from the debug symbols.  In
> the example given earlier, running:
> 
>     (gdb) directory /path/to/project
> 
> will now allow GDB to correctly locate the source code from the debug
> information.
> 
> gdb/ChangeLog:
> 
> 	* source.c (find_and_open_source): Search for the compilation
> 	directory and source file as a relative path beneath the directory
> 	search path.
> ---
>  gdb/ChangeLog |  6 ++++++
>  gdb/source.c  | 24 ++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index eb6d062812..25e3fc8e5d 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2019-09-09  Mike Gulick  <mgulick@mathworks.com>
> +
> +	* source.c (find_and_open_source): Search for the compilation
> +	directory and source file as a relative path beneath the directory
> +	search path.
> +
>  2019-09-09  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
>  
>  	* python/python.c (do_start_initialization): Make progname_copy static,
> diff --git a/gdb/source.c b/gdb/source.c
> index b27f210802..1635563b20 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -1033,7 +1033,31 @@ find_and_open_source (const char *filename,
>    openp_flags flags = OPF_SEARCH_IN_PATH;
>    if (basenames_may_differ)
>      flags |= OPF_RETURN_REALPATH;
> +
> +  /* Try to locate file using filename.  */
>    result = openp (path, flags, filename, OPEN_MODE, fullname);
> +  if (result < 0 && dirname != NULL)
> +    {
> +      /* Try to locate file using compilation dir + filename.  This is helpful
> +	 if part of the compilation directory was removed, e.g. using gcc's
> +	 -fdebug-prefix-map, and we have added the missing prefix to
> +	 source_path.  */
> +      std::string cdir_filename (dirname);
> +
> +      /* Remove any trailing directory separators.  */
> +      while (IS_DIR_SEPARATOR (cdir_filename.back ()))
> +	cdir_filename.pop_back ();
> +      /* Add our own directory separator.  */
> +      cdir_filename.append (SLASH_STRING);
> +      /* Append filename, without any leading directory separators.  */
> +      const char * filename_start = filename;

The space after the '*' is not needed.

> +      while (IS_DIR_SEPARATOR (filename_start[0]))
> +	filename_start++;
> +      cdir_filename.append (filename_start);
> +
> +      result = openp (path, flags, cdir_filename.c_str (), OPEN_MODE,
> +		      fullname);
> +    }
>    if (result < 0)
>      {
>        /* Didn't work.  Try using just the basename.  */
> -- 
> 2.20.1
> 


----

commit 3fcaf369356f2df4ad8d5e9922f818516bb62887
Author: Mike Gulick <mgulick@mathworks.com>
Date:   Thu Sep 12 11:16:06 2019 -0400

    gdb: Look for compilation directory relative to directory search path
    
    The 'directory' command allows the user to provide a list of filesystem
    directories in which to search for source code.  The directories in this
    search path are used as the base directory for the source filename from
    the debug information (DW_AT_name).  Thus the directory search path
    provides alternatives to the existing compilation directory from the
    debug information (DW_AT_comp_dir).  Generally speaking, DW_AT_name
    stores the filename argument passed to the compiler (including any
    directory components), and DW_AT_comp_dir stores the current working
    directory from which the compiler was executed.  For example:
    
        $ cd /path/to/project/subdir1
        $ gcc -c a/test.c -g
    
    The corresponding debug information will look like this:
    
        DW_AT_name      : a/test.c
        DW_AT_comp_dir  : /path/to/project/subdir1
    
    When compiling with the -fdebug-prefix-map GCC option, the compilation
    directory can be arbitrarily rewritten.  In the above example, we may
    rewrite the compilation directory as follows:
    
        $ gcc -c a/test.c -g -fdebug-prefix-map=/path/to/project=
    
    In this case, the corresponding debug information will look like:
    
        DW_AT_name      : a/test.c
        DW_AT_comp_dir  : /subdir1
    
    This prevents GDB from finding the corresponding source code based on
    the debug information alone.  In some cases, a substitute-path command
    can be used to re-map a consistent prefix in the rewritten compilation
    directory to the real filesystem path.  However, there may not be a
    consistent prefix remaining in the debug symbols (for example in a
    project that has source code in many subdirectories under the project's
    root), thereby requiring multiple substitute-path rules.  In this case,
    it is easier to add the missing prefix to the directory search path via
    the 'directory' command.
    
    The function find_and_open_source currently searches in:
    
        SEARCH_PATH/FILENAME
    
    where SEARCH_PATH corresponds to each individual entry in the directory
    search path (which is guaranteed to contain the compilation directory
    from the debug information, as well as the current working directory).
    FILENAME corresponds to the source filename (DW_AT_name), which may have
    directory components in it.  In addition, GDB searches in:
    
        SEARCH_PATH/FILE_BASENAME
    
    where FILE_BASENAME is the basename of the DW_AT_name entry.
    
    This change modifies find_and_open_source to additionally search in:
    
        SEARCH_PATH/COMP_DIR/FILENAME
    
    where COMP_DIR is the compilation directory from the debug symbols.  In
    the example given earlier, running:
    
        (gdb) directory /path/to/project
    
    will now allow GDB to correctly locate the source code from the debug
    information.
    
    gdb/ChangeLog:
    
            * source.c (find_and_open_source): Search for the compilation
            directory and source file as a relative path beneath the directory
            search path.
    
    gdb/doc/ChangeLog:
    
            * gdb.texinfo (Source Path): Additional text to better describe
            how the source path directory list is used when searching for
            source files.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.base/source-dir.c: New file.
            * gdb.base/source-dir.exp: Add extra test for mapped compilation
            directory.

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}.
 
 Note that the executable search path is @emph{not} used to locate the
 source files.
@@ -9074,6 +9088,12 @@
 session, while the latter is immediately expanded to the current
 directory at the time you add an entry to the source path.
 
+The @samp{$cdir} in the source path causes @value{GDBN} to search for
+the filename in the compilation directory, if one is recorded in the
+debug information.  @value{GDBN} will also append the compilation
+directory to the filename and check this against all other entries in
+the source path.
+
 @item directory
 Reset the source path to its default value (@samp{$cdir:$cwd} on Unix systems).  This requires confirmation.
 
diff --git a/gdb/source.c b/gdb/source.c
index b27f210802c..14746d4cd05 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1033,7 +1033,31 @@ find_and_open_source (const char *filename,
   openp_flags flags = OPF_SEARCH_IN_PATH;
   if (basenames_may_differ)
     flags |= OPF_RETURN_REALPATH;
+
+  /* Try to locate file using filename.  */
   result = openp (path, flags, filename, OPEN_MODE, fullname);
+  if (result < 0 && dirname != NULL)
+    {
+      /* Try to locate file using compilation dir + filename.  This is helpful
+	 if part of the compilation directory was removed, e.g. using gcc's
+	 -fdebug-prefix-map, and we have added the missing prefix to
+	 source_path.  */
+      std::string cdir_filename (dirname);
+
+      /* Remove any trailing directory separators.  */
+      while (IS_DIR_SEPARATOR (cdir_filename.back ()))
+	cdir_filename.pop_back ();
+      /* Add our own directory separator.  */
+      cdir_filename.append (SLASH_STRING);
+      /* Append filename, without any leading directory separators.  */
+      const char *filename_start = filename;
+      while (IS_DIR_SEPARATOR (filename_start[0]))
+	filename_start++;
+      cdir_filename.append (filename_start);
+
+      result = openp (path, flags, cdir_filename.c_str (), OPEN_MODE,
+		      fullname);
+    }
   if (result < 0)
     {
       /* Didn't work.  Try using just the basename.  */
diff --git a/gdb/testsuite/gdb.base/source-dir.c b/gdb/testsuite/gdb.base/source-dir.c
new file mode 100644
index 00000000000..d94b8074ec0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/source-dir.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/source-dir.exp b/gdb/testsuite/gdb.base/source-dir.exp
index 048c0e95161..6fe889b0cdb 100644
--- a/gdb/testsuite/gdb.base/source-dir.exp
+++ b/gdb/testsuite/gdb.base/source-dir.exp
@@ -15,9 +15,132 @@
 
 standard_testfile
 
-gdb_start
+# Take a list of directories DIRS, and return a regular expression
+# that will match against the output of the 'directory' command
+# assuming that DIRS are all of the directories that should appear in
+# the results.
+proc search_dir_list { dirs } {
+    set output "\r\nSource directories searched: "
+    append output [join $dirs "\[:;\]"]
 
-set foo "/nOtExStInG"
+    return ${output}
+}
 
-gdb_test "directory $foo/a $foo/b $foo/c" "\r\nSource directories searched: $foo/a\[:;\]$foo/b\[:;\]$foo/c\[:;\]\\\$cdir\[:;\]\\\$cwd"
-gdb_test "directory $foo/b $foo/d $foo/c" "\r\nSource directories searched: $foo/b\[:;\]$foo/d\[:;\]$foo/c\[:;\]$foo/a\[:;\]\\\$cdir\[:;\]\\\$cwd"
+# Check that adding directories to the search path changes the order
+# in which directories are searched.
+proc test_changing_search_directory {} {
+    gdb_start
+
+    set foo "/nOtExStInG"
+
+    gdb_test "directory $foo/a $foo/b $foo/c" \
+	[search_dir_list [list \
+			      "$foo/a" \
+			      "$foo/b" \
+			      "$foo/c" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_test "directory $foo/b $foo/d $foo/c" \
+	[search_dir_list [list \
+			      "$foo/b" \
+			      "$foo/d" \
+			      "$foo/c" \
+			      "$foo/a" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_exit
+}
+
+# Test that the compilation directory can also be extended with a
+# prefix from the directory search path in order to find source files.
+proc test_truncated_comp_dir {} {
+    global srcfile srcdir subdir binfile
+
+    # When we run this test the current directory will be something
+    # like this:
+    #     /some/path/to/gdb/build/testsuite/
+    # We are going to copy the source file out of the source tree into
+    # a location like this:
+    #     /some/path/to/gdb/build/testsuite/output/gdb.base/soure-dir/
+    #
+    # We will then switch to this directory and compile the source
+    # file, however, we will ask GCC to remove this prefix from the
+    # compilation directory in the debug info:
+    #     /some/path/to/gdb/build/testsuite/output/
+    #
+    # As a result the debug information will look like this:
+    #
+    #     DW_AT_name        : source-dir.c
+    #     DW_AT_comp_dir    : /gdb.base/source-dir
+    #
+    # Finally we switch back to this directory:
+    #     /some/path/to/gdb/build/testsuite/
+    #
+    # and start GDB.  There was a time when GDB would be unable to
+    # find the source file no matter what we added to the directory
+    # search path, this should now be fixed.
+
+    set original_dir [pwd]
+    set working_dir [standard_output_file ""]
+    cd ${working_dir}
+
+    set strip_dir [file normalize "${working_dir}/../.."]
+
+    set new_srcfile [standard_output_file ${srcfile}]
+    remote_exec host "cp ${srcdir}/${subdir}/${srcfile} ${new_srcfile}"
+
+    set options \
+	"debug additional_flags=-fdebug-prefix-map=${strip_dir}="
+    if  { [gdb_compile "${srcfile}" "${binfile}" \
+	       executable ${options}] != "" } {
+	untested "failed to compile"
+	return -1
+    }
+
+    cd ${original_dir}
+
+    clean_restart ${binfile}
+
+    gdb_test_no_output "set directories \$cdir:\$cwd"
+    gdb_test "show directories" \
+	"\r\nSource directories searched: \\\$cdir\[:;\]\\\$cwd"
+
+    if ![runto_main] then {
+	fail "can't run to main"
+	return 0
+    }
+
+    gdb_test "info source" \
+	[multi_line \
+	     "Current source file is ${srcfile}" \
+	     "Compilation directory is \[^\n\r\]+" \
+	     "${srcfile}: No such file or directory."]
+
+    gdb_test "dir $strip_dir" \
+	[search_dir_list [list \
+			      "$strip_dir" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_test "list" [multi_line \
+			 "16\[^\n\r\]+" \
+			 "17\[ \t\]+" \
+			 "18\[ \t\]+int" \
+			 "19\[ \t\]+main \\(\\)" \
+			 "20\[ \t\]+\\{" \
+			 "21\[ \t\]+return 0;" \
+			 "22\[ \t\]+\\}" ]
+
+    gdb_test "info source" \
+	[multi_line \
+	     "Current source file is ${srcfile}" \
+	     "Compilation directory is \[^\n\r\]+" \
+	     "Located in ${new_srcfile}" \
+	     "Contains 22 lines." \
+	     "Source language is c." \
+	     "Producer is \[^\n\r\]+" \
+	     "\[^\n\r\]+" \
+	     "\[^\n\r\]+" ]
+}
+
+test_changing_search_directory
+test_truncated_comp_dir

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13  1:38     ` Andrew Burgess
@ 2019-09-13  6:36       ` Eli Zaretskii
  2019-09-13  7:28         ` Eli Zaretskii
  2019-09-13 22:11         ` Mike Gulick
  0 siblings, 2 replies; 20+ messages in thread
From: Eli Zaretskii @ 2019-09-13  6:36 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: mgulick, gdb-patches

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

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13  6:36       ` Eli Zaretskii
@ 2019-09-13  7:28         ` Eli Zaretskii
  2019-09-13 22:45           ` Andrew Burgess
  2019-09-13 22:11         ` Mike Gulick
  1 sibling, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2019-09-13  7:28 UTC (permalink / raw)
  To: andrew.burgess; +Cc: mgulick, gdb-patches

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13  6:36       ` Eli Zaretskii
  2019-09-13  7:28         ` Eli Zaretskii
@ 2019-09-13 22:11         ` Mike Gulick
  2019-09-13 22:41           ` Andrew Burgess
  1 sibling, 1 reply; 20+ messages in thread
From: Mike Gulick @ 2019-09-13 22:11 UTC (permalink / raw)
  To: Eli Zaretskii, Andrew Burgess; +Cc: Mike Gulick, gdb-patches



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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13 22:11         ` Mike Gulick
@ 2019-09-13 22:41           ` Andrew Burgess
  0 siblings, 0 replies; 20+ messages in thread
From: Andrew Burgess @ 2019-09-13 22:41 UTC (permalink / raw)
  To: Mike Gulick; +Cc: Eli Zaretskii, gdb-patches

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13  7:28         ` Eli Zaretskii
@ 2019-09-13 22:45           ` Andrew Burgess
  2019-09-13 22:52             ` Mike Gulick
  2019-09-14  6:56             ` Eli Zaretskii
  0 siblings, 2 replies; 20+ messages in thread
From: Andrew Burgess @ 2019-09-13 22:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mgulick, gdb-patches

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

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

Thanks,
Andrew

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13 22:45           ` Andrew Burgess
@ 2019-09-13 22:52             ` Mike Gulick
  2019-09-14  7:11               ` Eli Zaretskii
  2019-09-17 20:22               ` Andrew Burgess
  2019-09-14  6:56             ` Eli Zaretskii
  1 sibling, 2 replies; 20+ messages in thread
From: Mike Gulick @ 2019-09-13 22:52 UTC (permalink / raw)
  To: Andrew Burgess, Eli Zaretskii; +Cc: Mike Gulick, gdb-patches

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13 22:45           ` Andrew Burgess
  2019-09-13 22:52             ` Mike Gulick
@ 2019-09-14  6:56             ` Eli Zaretskii
  2019-09-14 15:28               ` Andrew Burgess
  1 sibling, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2019-09-14  6:56 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: mgulick, gdb-patches

> Date: Fri, 13 Sep 2019 18:45:35 -0400
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: mgulick@mathworks.com, gdb-patches@sourceware.org
> 
> > 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.
> 
> 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'm not sure we are talking about the same thing.  My concern is
specifically this use case mentioned in your description:

  SEARCH_PATH/COMP_DIR/FILENAME

What will happen here if both SEARCH_PATH and COMP_DIR are absolute
file names (which on Windows means they have drive letters)?  AFAIU on
Unix you will concatenate them, so we need some kind of
"concatenation" on Windows as well, but that means one of the drive
letters should "win".  Which drive letter wins in this case, and will
the result make sense from the user perspective?

It is clear to me that if FILENAME is absolute, we don't use any of
the directories to locate it, neither on Unix nor on Windows.  Right?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13 22:52             ` Mike Gulick
@ 2019-09-14  7:11               ` Eli Zaretskii
  2019-09-17 20:22               ` Andrew Burgess
  1 sibling, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2019-09-14  7:11 UTC (permalink / raw)
  To: Mike Gulick; +Cc: andrew.burgess, gdb-patches

> From: Mike Gulick <mgulick@mathworks.com>
> CC: Mike Gulick <mgulick@mathworks.com>, "gdb-patches@sourceware.org"
> 	<gdb-patches@sourceware.org>
> Date: Fri, 13 Sep 2019 22:52:19 +0000
> 
> The only way this would be a problem is if both the compilation
> directory and the source file contained a drive letter.

Are you sure this is the only problematic use case?

Does GDB even try to prepend any directories to a source file whose
name is absolute and includes a drive letter?  If it does, is that
TRT?

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

That's not exactly what I see on Windows (but note that I used -g3):

D:\usr\eli\data>gcc -g3 -c -o test.o d:/usr/eli/data/test.c
D:\usr\eli\data>objdump --dwarf test.o
...
    <3a>   DW_AT_name        : d:/usr/eli/data/test.c
    <51>   DW_AT_comp_dir    : D:\usr\eli\data

D:\usr\eli\data>gcc -g3 -c -o test.o -fdebug-prefix-map=D:\usr= d:/usr/eli/data/test.c
D:\usr\eli\data>objdump --dwarf test.o
...
    <3a>   DW_AT_name        : /eli/data/test.c
    <4b>   DW_AT_comp_dir    : \eli\data

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

I'm not familiar with the code enough to be sure, but how do we
convince ourselves that removing one of the several drive letters and
doing nothing else is TRT in this case?  Maybe we should prepend that
drive letter to the complete file name we produce?

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-14  6:56             ` Eli Zaretskii
@ 2019-09-14 15:28               ` Andrew Burgess
  2019-09-14 15:54                 ` Eli Zaretskii
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2019-09-14 15:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mgulick, gdb-patches

* Eli Zaretskii <eliz@gnu.org> [2019-09-14 09:56:35 +0300]:

> > Date: Fri, 13 Sep 2019 18:45:35 -0400
> > From: Andrew Burgess <andrew.burgess@embecosm.com>
> > Cc: mgulick@mathworks.com, gdb-patches@sourceware.org
> > 
> > > 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.
> > 
> > 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'm not sure we are talking about the same thing.  My concern is
> specifically this use case mentioned in your description:
> 
>   SEARCH_PATH/COMP_DIR/FILENAME
> 
> What will happen here if both SEARCH_PATH and COMP_DIR are absolute
> file names (which on Windows means they have drive letters)?  AFAIU on
> Unix you will concatenate them, so we need some kind of
> "concatenation" on Windows as well, but that means one of the drive
> letters should "win".  Which drive letter wins in this case, and will
> the result make sense from the user perspective?

If we get as far trying this merge then the drive letter on COMP_DIR
will be removed and we use whatever is on SEARCH_PATH.  This is
exactly the same as what we've always done if FILENAME is itself
absolute.

The code I pointed out above that last night I didn't understand now
makes perfect sense.

If we have:

   DW_AT_name:  	c:/project/foo.c
   DW_AT_comp_dir:	d:/project/build/

And if the directory search path is:

   g:/mnt/project;$cdir;$cwd

And the current directory is:

   h:/home/

Then this is what I think the search order will be:

   c:/project/foo.c
   g:/mnt/project/project/foo.c
   d:/project/build/project/foo.c
   h:/home/project/foo.c

   d:/project/build/project/foo.c
   g:/mnt/project/project/build/project/foo.c
   d:/project/build/project/build/project/foo.c
   h:/home/project/build/project/foo.c

   g:/mnt/project/foo.c
   d:/project/build/foo.c
   h:/home/foo.c

The first and third block is what we currently (pre-patch) do, and the
second block is what I think the patch should provide after Mike's
last suggested update.

> 
> It is clear to me that if FILENAME is absolute, we don't use any of
> the directories to locate it, neither on Unix nor on Windows.  Right?

Well.... if FILENAME is absolute then that will get tried first, which
for most developers will work fine.  But we also want to support the
possibility that people might relocate the source tree into source
other location.  So if you compile /project/foo-1.0/f1.c the
DW_AT_name will be that absolute path, if you then moved that source
tree into: '~/.debug-cache/foo/project/foo-1.0/f1.c' we still want to
allow that to be found which requires prefixing the absolute path with
a directory.

I guess on Windows if the absolute file path include a 'c:' prefix,
but you relocated the source into a location below 'd:' then you want
the 'c:' prefix stripped off, which is what would happen at the
moment, so I think all is good.

Thanks,
Andrew

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-14 15:28               ` Andrew Burgess
@ 2019-09-14 15:54                 ` Eli Zaretskii
  2019-09-15  2:07                   ` Mike Gulick
  0 siblings, 1 reply; 20+ messages in thread
From: Eli Zaretskii @ 2019-09-14 15:54 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: mgulick, gdb-patches

> Date: Sat, 14 Sep 2019 11:28:47 -0400
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: mgulick@mathworks.com, gdb-patches@sourceware.org
> 
> If we have:
> 
>    DW_AT_name:  	c:/project/foo.c
>    DW_AT_comp_dir:	d:/project/build/
> 
> And if the directory search path is:
> 
>    g:/mnt/project;$cdir;$cwd
> 
> And the current directory is:
> 
>    h:/home/
> 
> Then this is what I think the search order will be:
> 
>    c:/project/foo.c
>    g:/mnt/project/project/foo.c
>    d:/project/build/project/foo.c
>    h:/home/project/foo.c
> 
>    d:/project/build/project/foo.c
>    g:/mnt/project/project/build/project/foo.c
>    d:/project/build/project/build/project/foo.c
>    h:/home/project/build/project/foo.c
> 
>    g:/mnt/project/foo.c
>    d:/project/build/foo.c
>    h:/home/foo.c
> 
> The first and third block is what we currently (pre-patch) do, and the
> second block is what I think the patch should provide after Mike's
> last suggested update.

SGTM, thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-14 15:54                 ` Eli Zaretskii
@ 2019-09-15  2:07                   ` Mike Gulick
  2019-09-15  4:01                     ` Andrew Burgess
  0 siblings, 1 reply; 20+ messages in thread
From: Mike Gulick @ 2019-09-15  2:07 UTC (permalink / raw)
  To: Eli Zaretskii, Andrew Burgess; +Cc: gdb-patches

On 9/14/19 11:54 AM, Eli Zaretskii wrote:
>> Date: Sat, 14 Sep 2019 11:28:47 -0400
>> From: Andrew Burgess <andrew.burgess@embecosm.com>
>> Cc: mgulick@mathworks.com, gdb-patches@sourceware.org
>>
>> If we have:
>>
>>    DW_AT_name:  	c:/project/foo.c
>>    DW_AT_comp_dir:	d:/project/build/
>>
>> And if the directory search path is:
>>
>>    g:/mnt/project;$cdir;$cwd
>>
>> And the current directory is:
>>
>>    h:/home/
>>
>> Then this is what I think the search order will be:
>>
>>    c:/project/foo.c
>>    g:/mnt/project/project/foo.c
>>    d:/project/build/project/foo.c
>>    h:/home/project/foo.c
>>
>>    d:/project/build/project/foo.c
>>    g:/mnt/project/project/build/project/foo.c
>>    d:/project/build/project/build/project/foo.c
>>    h:/home/project/build/project/foo.c
>>
>>    g:/mnt/project/foo.c
>>    d:/project/build/foo.c
>>    h:/home/foo.c
>>
>> The first and third block is what we currently (pre-patch) do, and the
>> second block is what I think the patch should provide after Mike's
>> last suggested update.
> 
> SGTM, thanks.
> 

Thanks Andrew and Eli for the clarification on the proper behavior on
DOS paths.  The only remaining questionable case for me is whether to
check $cdir/COMP_DIR/FILENAME, which in your example above is
'd:/project/build/project/build/project/foo.c'.  It seems reasonable
to me to leave this out of the search, although one could argue
leaving it on the source path may make the documentation slightly more
straightforward (one less special case).

Please let me know if you would like me to provide an updated version
of the patch.

Thanks,
Mike

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-15  2:07                   ` Mike Gulick
@ 2019-09-15  4:01                     ` Andrew Burgess
  2019-09-15 15:29                       ` Eli Zaretskii
  2019-09-16 15:53                       ` Mike Gulick
  0 siblings, 2 replies; 20+ messages in thread
From: Andrew Burgess @ 2019-09-15  4:01 UTC (permalink / raw)
  To: Mike Gulick; +Cc: Eli Zaretskii, gdb-patches

* Mike Gulick <mgulick@mathworks.com> [2019-09-15 02:07:37 +0000]:

> On 9/14/19 11:54 AM, Eli Zaretskii wrote:
> >> Date: Sat, 14 Sep 2019 11:28:47 -0400
> >> From: Andrew Burgess <andrew.burgess@embecosm.com>
> >> Cc: mgulick@mathworks.com, gdb-patches@sourceware.org
> >>
> >> If we have:
> >>
> >>    DW_AT_name:  	c:/project/foo.c
> >>    DW_AT_comp_dir:	d:/project/build/
> >>
> >> And if the directory search path is:
> >>
> >>    g:/mnt/project;$cdir;$cwd
> >>
> >> And the current directory is:
> >>
> >>    h:/home/
> >>
> >> Then this is what I think the search order will be:
> >>
> >>    c:/project/foo.c
> >>    g:/mnt/project/project/foo.c
> >>    d:/project/build/project/foo.c
> >>    h:/home/project/foo.c
> >>
> >>    d:/project/build/project/foo.c
> >>    g:/mnt/project/project/build/project/foo.c
> >>    d:/project/build/project/build/project/foo.c
> >>    h:/home/project/build/project/foo.c
> >>
> >>    g:/mnt/project/foo.c
> >>    d:/project/build/foo.c
> >>    h:/home/foo.c
> >>
> >> The first and third block is what we currently (pre-patch) do, and the
> >> second block is what I think the patch should provide after Mike's
> >> last suggested update.
> > 
> > SGTM, thanks.
> > 
> 
> Thanks Andrew and Eli for the clarification on the proper behavior on
> DOS paths.  The only remaining questionable case for me is whether to
> check $cdir/COMP_DIR/FILENAME, which in your example above is
> 'd:/project/build/project/build/project/foo.c'.  It seems reasonable
> to me to leave this out of the search, although one could argue
> leaving it on the source path may make the documentation slightly more
> straightforward (one less special case).
> 
> Please let me know if you would like me to provide an updated version
> of the patch.

Personally I don't see it as a problem.  I agree it is unlikely to be
something that ever makes sense, but we are already trying to access
lots of paths that don't exist, I think throwing one more into the mix
isn't going to hurt.

I took another stab at the documentation, this time I've moved the
explanations for $cdir and $cwd earlier in the section - previously
these were described really late on, and it made writing the earlier
text hard as I was trying not to reference these things before they
were introduced.

The docs now focus more on some worked examples rather than trying to
just explain the algorithm in text.  I hope this is better.  I've also
included an example of how MS drive letters are handled.

On the code side I have made one change from Mike's last patch,
looking in openp at the code that glues filenames onto directories it
had three preprocessing steps:

  1. Remove drive letters (for dos),
  2. Remove leading slashes (/foo.c -> foo.c), and
  3. Remove leading './' sequences (./foo.c -> foo.c).

I've now factored these three steps out and we share them between
openp and open_and_find_source, otherwise the code is unchanged from
Mike's last patch.

Let me know what you think.

Thanks,
Andrew

---

commit 70bbf4fc4358a8c10b20faf565ab3b0dee2a20c7
Author: Mike Gulick <mgulick@mathworks.com>
Date:   Thu Sep 12 11:16:06 2019 -0400

    gdb: Look for compilation directory relative to directory search path
    
    The 'directory' command allows the user to provide a list of filesystem
    directories in which to search for source code.  The directories in this
    search path are used as the base directory for the source filename from
    the debug information (DW_AT_name).  Thus the directory search path
    provides alternatives to the existing compilation directory from the
    debug information (DW_AT_comp_dir).  Generally speaking, DW_AT_name
    stores the filename argument passed to the compiler (including any
    directory components), and DW_AT_comp_dir stores the current working
    directory from which the compiler was executed.  For example:
    
        $ cd /path/to/project/subdir1
        $ gcc -c a/test.c -g
    
    The corresponding debug information will look like this:
    
        DW_AT_name      : a/test.c
        DW_AT_comp_dir  : /path/to/project/subdir1
    
    When compiling with the -fdebug-prefix-map GCC option, the compilation
    directory can be arbitrarily rewritten.  In the above example, we may
    rewrite the compilation directory as follows:
    
        $ gcc -c a/test.c -g -fdebug-prefix-map=/path/to/project=
    
    In this case, the corresponding debug information will look like:
    
        DW_AT_name      : a/test.c
        DW_AT_comp_dir  : /subdir1
    
    This prevents GDB from finding the corresponding source code based on
    the debug information alone.  In some cases, a substitute-path command
    can be used to re-map a consistent prefix in the rewritten compilation
    directory to the real filesystem path.  However, there may not be a
    consistent prefix remaining in the debug symbols (for example in a
    project that has source code in many subdirectories under the project's
    root), thereby requiring multiple substitute-path rules.  In this case,
    it is easier to add the missing prefix to the directory search path via
    the 'directory' command.
    
    The function find_and_open_source currently searches in:
    
        SEARCH_PATH/FILENAME
    
    where SEARCH_PATH corresponds to each individual entry in the directory
    search path (which is guaranteed to contain the compilation directory
    from the debug information, as well as the current working directory).
    FILENAME corresponds to the source filename (DW_AT_name), which may have
    directory components in it.  In addition, GDB searches in:
    
        SEARCH_PATH/FILE_BASENAME
    
    where FILE_BASENAME is the basename of the DW_AT_name entry.
    
    This change modifies find_and_open_source to additionally search in:
    
        SEARCH_PATH/COMP_DIR/FILENAME
    
    where COMP_DIR is the compilation directory from the debug symbols.  In
    the example given earlier, running:
    
        (gdb) directory /path/to/project
    
    will now allow GDB to correctly locate the source code from the debug
    information.
    
    gdb/ChangeLog:
    
            * source.c (prepare_path_for_appending): New function.
            (openp): Make use of new function.
            (find_and_open_source): Search for the compilation directory and
            source file as a relative path beneath the directory search path.
    
    gdb/doc/ChangeLog:
    
            * gdb.texinfo (Source Path): Additional text to better describe
            how the source path directory list is used when searching for
            source files.
    
    gdb/testsuite/ChangeLog:
    
            * gdb.base/source-dir.exp: Add extra test for mapped compilation
            directory.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6d5f19d04bb..a173113929a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2019-09-12  Mike Gulick  <mgulick@mathworks.com>
+
+	* source.c (prepare_path_for_appending): New function.
+	(openp): Make use of new function.
+	(find_and_open_source): Search for the compilation directory and
+	source file as a relative path beneath the directory search path.
+
 2019-09-12  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
 
 	* procfs.c (procfs_target::wait) <PR_FAULTED>: Get signal from
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 96c04091973..97c99663985 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,9 @@
+2019-09-12  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.texinfo (Source Path): Additional text to better describe
+	how the source path directory list is used when searching for
+	source files.
+
 2019-09-10  Tom Tromey  <tromey@adacore.com>
 
 	* gdb.texinfo (Index Files): Update Ada text.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 79824a0226a..20ab1e576ec 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8954,11 +8954,20 @@
 in the list, until it finds a file with the desired name.
 
 For example, suppose an executable references the file
-@file{/usr/src/foo-1.0/lib/foo.c}, and our source path is
-@file{/mnt/cross}.  The file is first looked up literally; if this
-fails, @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c} is tried; if this
-fails, @file{/mnt/cross/foo.c} is opened; if this fails, an error
-message is printed.  @value{GDBN} does not look up the parts of the
+@file{/usr/src/foo-1.0/lib/foo.c} and does not record a compilation
+directory, the @dfn{source path} is @file{/mnt/cross}. @value{GDBN}
+would look for the source file in the following locations:
+
+@enumerate
+
+@item @file{/usr/src/foo-1.0/lib/foo.c}
+@item @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c}
+@item @file{/mnt/cross/foo.c}
+
+@end enumerate
+
+If the source file is not present at any of the above locations then
+an error is printed.  @value{GDBN} does not look up the parts of the
 source file name, such as @file{/mnt/cross/src/foo-1.0/lib/foo.c}.
 Likewise, the subdirectories of the source path are not searched: if
 the source path is @file{/mnt/cross}, and the binary refers to
@@ -8966,11 +8975,91 @@
 @file{/mnt/cross/usr/src/foo-1.0/lib}.
 
 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}.
+names containing dots, etc.@: are all treated as described above,
+except that non-absolute file names are not looked up literally.  If
+the @dfn{source path} is @file{/mnt/cross}, and the source file is
+recorded as @file{../lib/foo.c} and no compilation directory is
+recorded then @value{GDBN} will search in the following locations:
+
+@enumerate
+
+@item @file{/mnt/cross/../lib/foo.c}
+@item @file{/mnt/cross/foo.c}
+
+@end enumerate
+
+@kindex cdir
+@kindex cwd
+@vindex $cdir@r{, convenience variable}
+@vindex $cwd@r{, convenience variable}
+@cindex compilation directory
+@cindex current directory
+@cindex working directory
+@cindex directory, current
+@cindex directory, compilation
+The @dfn{source path} will always include two special entries
+@samp{$cdir} and @samp{$cwd}, these refer to the compilation directory
+(if one is recorded) and the current workig directory respectively.
+
+@samp{cdir} causes @value{GDBN} to search within the compilation
+directory, if one is recorded in the debug information.  If no
+compilation directory is recorded in the debug information then
+@samp{cdir} is ignored.
+
+@samp{$cwd} is not the same as @samp{.}---the former tracks the
+current working directory as it changes during your @value{GDBN}
+session, while the latter is immediately expanded to the current
+directory at the time you add an entry to the source path.
+
+If a compilation directory is recorded in the debug information, and
+@value{GDBN} has not found the source file after the first search
+using @dfn{source path}, then @value{GDBN} will combine the
+compilation directory and the filename, and then search for the source
+file again using the @dfn{source path}.
+
+For example, if the executable records the source file as
+@file{/usr/src/foo-1.0/lib/foo.c} the compilation directory is
+recorded as @file{/project/build}, and the @dfn{source path} is
+@file{/mnt/cross;$cdir;$cwd} while the current working directory of
+the @value{GDBN} session is @file{/home/user}, then @value{GDBN} will
+search for the source file in the following loctions:
+
+@enumerate
+
+@item @file{/usr/src/foo-1.0/lib/foo.c}
+@item @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c}
+@item @file{/project/build/usr/src/foo-1.0/lib/foo.c}
+@item @file{/home/user/usr/src/foo-1.0/lib/foo.c}
+@item @file{/mnt/cross/project/build/usr/src/foo-1.0/lib/foo.c}
+@item @file{/project/build/project/build/usr/src/foo-1.0/lib/foo.c}
+@item @file{/home/user/project/build/usr/src/foo-1.0/lib/foo.c}
+@item @file{/mnt/cross/foo.c}
+@item @file{/project/build/foo.c}
+@item @file{/home/user/foo.c}
+
+@end enumerate
+
+If the file name in the previous example had been recorded in the
+executable as a relative path rather than an absolute path, then the
+first look up would not have occurred, but all of the remaining steps
+would be similar.
+
+When searching for source files on MS-DOS and MS-Windows, where
+absolute paths start with a drive letter (e.g.
+@file{C:/project/foo.c}), @value{GDBN} will remove the drive letter
+from the file name before appending it to a search directory from
+@dfn{source path}; for instance if the executable references the
+source file @file{C:/project/foo.c} and @dfn{source path} is set to
+@file{D:/mnt/cross}, then @value{GDBN} will search in the following
+locations for the source file:
+
+@enumerate
+
+@item @file{C:/project/foo.c}
+@item @file{D:/mnt/cross/project/foo.c}
+@item @file{D:/mnt/cross/foo.c}
+
+@end enumerate
 
 Note that the executable search path is @emph{not} used to locate the
 source files.
@@ -9058,21 +9147,12 @@
 whitespace.  You may specify a directory that is already in the source
 path; this moves it forward, so @value{GDBN} searches it sooner.
 
-@kindex cdir
-@kindex cwd
-@vindex $cdir@r{, convenience variable}
-@vindex $cwd@r{, convenience variable}
-@cindex compilation directory
-@cindex current directory
-@cindex working directory
-@cindex directory, current
-@cindex directory, compilation
-You can use the string @samp{$cdir} to refer to the compilation
-directory (if one is recorded), and @samp{$cwd} to refer to the current
-working directory.  @samp{$cwd} is not the same as @samp{.}---the former
-tracks the current working directory as it changes during your @value{GDBN}
-session, while the latter is immediately expanded to the current
-directory at the time you add an entry to the source path.
+The special strings @samp{$cdir} (to refer to the compilation
+directory, if one is recorded), and @samp{$cwd} (to refer to the
+current working directory) can also be included in the list of
+directories @var{dirname}.  Though these will already be in the source
+path they will be moved forward in the list so @value{GDBN} searches
+them sooner.
 
 @item directory
 Reset the source path to its default value (@samp{$cdir:$cwd} on Unix systems).  This requires confirmation.
diff --git a/gdb/source.c b/gdb/source.c
index b27f210802c..ece1ad3d16b 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -654,6 +654,29 @@ info_source_command (const char *ignore, int from_tty)
 }
 \f
 
+/* Helper function to remove characters from the start of PATH so that
+   PATH can then be appended to a directory name.  We remove leading drive
+   letters (for dos) as well as leading '/' characters and './'
+   sequences.  */
+
+const char *
+prepare_path_for_appending (const char *path)
+{
+  /* For dos paths, d:/foo -> /foo, and d:foo -> foo.  */
+  if (HAS_DRIVE_SPEC (path))
+    path = STRIP_DRIVE_SPEC (path);
+
+  /* /foo => foo, to avoid multiple slashes that Emacs doesn't like.  */
+  while (IS_DIR_SEPARATOR(path[0]))
+    path++;
+
+  /* ./foo => foo */
+  while (path[0] == '.' && IS_DIR_SEPARATOR (path[1]))
+    path += 2;
+
+  return path;
+}
+
 /* Open a file named STRING, searching path PATH (dir names sep by some char)
    using mode MODE in the calls to open.  You cannot use this function to
    create files (O_CREAT).
@@ -747,17 +770,9 @@ openp (const char *path, openp_flags opts, const char *string,
 	    goto done;
     }
 
-  /* For dos paths, d:/foo -> /foo, and d:foo -> foo.  */
-  if (HAS_DRIVE_SPEC (string))
-    string = STRIP_DRIVE_SPEC (string);
-
-  /* /foo => foo, to avoid multiple slashes that Emacs doesn't like.  */
-  while (IS_DIR_SEPARATOR(string[0]))
-    string++;
-
-  /* ./foo => foo */
-  while (string[0] == '.' && IS_DIR_SEPARATOR (string[1]))
-    string += 2;
+  /* Remove characters from the start of PATH that we don't need when PATH
+     is appended to a directory name.  */
+  string = prepare_path_for_appending (string);
 
   alloclen = strlen (path) + strlen (string) + 2;
   filename = (char *) alloca (alloclen);
@@ -1033,7 +1048,32 @@ find_and_open_source (const char *filename,
   openp_flags flags = OPF_SEARCH_IN_PATH;
   if (basenames_may_differ)
     flags |= OPF_RETURN_REALPATH;
+
+  /* Try to locate file using filename.  */
   result = openp (path, flags, filename, OPEN_MODE, fullname);
+  if (result < 0 && dirname != NULL)
+    {
+      /* Remove characters from the start of PATH that we don't need when
+	 PATH is appended to a directory name.  */
+      const char *filename_start = prepare_path_for_appending (filename);
+
+      /* Try to locate file using compilation dir + filename.  This is
+	 helpful if part of the compilation directory was removed,
+	 e.g. using gcc's -fdebug-prefix-map, and we have added the missing
+	 prefix to source_path.  */
+      std::string cdir_filename (dirname);
+
+      /* Remove any trailing directory separators.  */
+      while (IS_DIR_SEPARATOR (cdir_filename.back ()))
+	cdir_filename.pop_back ();
+
+      /* Add our own directory separator.  */
+      cdir_filename.append (SLASH_STRING);
+      cdir_filename.append (filename_start);
+
+      result = openp (path, flags, cdir_filename.c_str (), OPEN_MODE,
+		      fullname);
+    }
   if (result < 0)
     {
       /* Didn't work.  Try using just the basename.  */
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index a51d22ce22f..0d37519a5a4 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2019-09-12  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.base/source-dir.exp: Add extra test for mapped compilation
+	directory.
+
 2019-09-10  Tom Tromey  <tromey@adacore.com>
 
 	* boards/cc-with-tweaks.exp: Set GNATMAKE_FOR_TARGET.
diff --git a/gdb/testsuite/gdb.base/source-dir.c b/gdb/testsuite/gdb.base/source-dir.c
new file mode 100644
index 00000000000..d94b8074ec0
--- /dev/null
+++ b/gdb/testsuite/gdb.base/source-dir.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2019 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main ()
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/source-dir.exp b/gdb/testsuite/gdb.base/source-dir.exp
index 048c0e95161..3f9ee69ae45 100644
--- a/gdb/testsuite/gdb.base/source-dir.exp
+++ b/gdb/testsuite/gdb.base/source-dir.exp
@@ -15,9 +15,138 @@
 
 standard_testfile
 
-gdb_start
+# Take a list of directories DIRS, and return a regular expression
+# that will match against the output of the 'directory' command
+# assuming that DIRS are all of the directories that should appear in
+# the results.
+proc search_dir_list { dirs } {
+    set output "\r\nSource directories searched: "
+    append output [join $dirs "\[:;\]"]
 
-set foo "/nOtExStInG"
+    return ${output}
+}
 
-gdb_test "directory $foo/a $foo/b $foo/c" "\r\nSource directories searched: $foo/a\[:;\]$foo/b\[:;\]$foo/c\[:;\]\\\$cdir\[:;\]\\\$cwd"
-gdb_test "directory $foo/b $foo/d $foo/c" "\r\nSource directories searched: $foo/b\[:;\]$foo/d\[:;\]$foo/c\[:;\]$foo/a\[:;\]\\\$cdir\[:;\]\\\$cwd"
+# Check that adding directories to the search path changes the order
+# in which directories are searched.
+proc test_changing_search_directory {} {
+    gdb_start
+
+    set foo "/nOtExStInG"
+
+    gdb_test "directory $foo/a $foo/b $foo/c" \
+	[search_dir_list [list \
+			      "$foo/a" \
+			      "$foo/b" \
+			      "$foo/c" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_test "directory $foo/b $foo/d $foo/c" \
+	[search_dir_list [list \
+			      "$foo/b" \
+			      "$foo/d" \
+			      "$foo/c" \
+			      "$foo/a" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_exit
+}
+
+# Test that the compilation directory can also be extended with a
+# prefix from the directory search path in order to find source files.
+proc test_truncated_comp_dir {} {
+    global srcfile srcdir subdir binfile
+
+    # When we run this test the current directory will be something
+    # like this:
+    #     /some/path/to/gdb/build/testsuite/
+    # We are going to copy the source file out of the source tree into
+    # a location like this:
+    #     /some/path/to/gdb/build/testsuite/output/gdb.base/soure-dir/
+    #
+    # We will then switch to this directory and compile the source
+    # file, however, we will ask GCC to remove this prefix from the
+    # compilation directory in the debug info:
+    #     /some/path/to/gdb/build/testsuite/output/
+    #
+    # As a result the debug information will look like this:
+    #
+    #     DW_AT_name        : source-dir.c
+    #     DW_AT_comp_dir    : /gdb.base/source-dir
+    #
+    # Finally we switch back to this directory:
+    #     /some/path/to/gdb/build/testsuite/
+    #
+    # and start GDB.  There was a time when GDB would be unable to
+    # find the source file no matter what we added to the directory
+    # search path, this should now be fixed.
+
+    set original_dir [pwd]
+    set working_dir [standard_output_file ""]
+    cd ${working_dir}
+
+    set strip_dir [file normalize "${working_dir}/../.."]
+
+    set new_srcfile [standard_output_file ${srcfile}]
+    set fd [open "$new_srcfile" w]
+    puts $fd "int
+    main ()
+    {
+      return 0;
+    }"
+    close $fd
+
+    set options \
+	"debug additional_flags=-fdebug-prefix-map=${strip_dir}="
+    if  { [gdb_compile "${srcfile}" "${binfile}" \
+	       executable ${options}] != "" } {
+	untested "failed to compile"
+	return -1
+    }
+
+    cd ${original_dir}
+
+    clean_restart ${binfile}
+
+    gdb_test_no_output "set directories \$cdir:\$cwd"
+    gdb_test "show directories" \
+	"\r\nSource directories searched: \\\$cdir\[:;\]\\\$cwd"
+
+    if ![runto_main] then {
+	fail "can't run to main"
+	return 0
+    }
+
+    gdb_test "info source" \
+    [multi_line \
+	 "Current source file is ${srcfile}" \
+	 "Compilation directory is \[^\n\r\]+" \
+	 "${srcfile}: No such file or directory."] \
+	"info source before setting directory search list"
+
+    gdb_test "dir $strip_dir" \
+	[search_dir_list [list \
+			      "$strip_dir" \
+			      "\\\$cdir" \
+			      "\\\$cwd"]]
+    gdb_test "list" [multi_line \
+			 "1\[ \t\]+int" \
+			 "2\[ \t\]+main \\(\\)" \
+			 "3\[ \t\]+\\{" \
+			 "4\[ \t\]+return 0;" \
+			 "5\[ \t\]+\\}" ]
+
+    gdb_test "info source" \
+	[multi_line \
+	     "Current source file is ${srcfile}" \
+	     "Compilation directory is \[^\n\r\]+" \
+	     "Located in ${new_srcfile}" \
+	     "Contains 5 lines." \
+	     "Source language is c." \
+	     "Producer is \[^\n\r\]+" \
+	     "\[^\n\r\]+" \
+	     "\[^\n\r\]+" ] \
+	"info source after setting directory search list"
+}
+
+test_changing_search_directory
+test_truncated_comp_dir

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-15  4:01                     ` Andrew Burgess
@ 2019-09-15 15:29                       ` Eli Zaretskii
  2019-09-16 15:53                       ` Mike Gulick
  1 sibling, 0 replies; 20+ messages in thread
From: Eli Zaretskii @ 2019-09-15 15:29 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: mgulick, gdb-patches

> Date: Sun, 15 Sep 2019 00:01:01 -0400
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,
> 	"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
> 
> The docs now focus more on some worked examples rather than trying to
> just explain the algorithm in text.  I hope this is better.  I've also
> included an example of how MS drive letters are handled.

LGTM, except that...

> +directory, the @dfn{source path} is @file{/mnt/cross}. @value{GDBN}
                                                        ^^
...need two spaces here.

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-15  4:01                     ` Andrew Burgess
  2019-09-15 15:29                       ` Eli Zaretskii
@ 2019-09-16 15:53                       ` Mike Gulick
  1 sibling, 0 replies; 20+ messages in thread
From: Mike Gulick @ 2019-09-16 15:53 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: Mike Gulick, Eli Zaretskii, gdb-patches

On 9/15/19 12:01 AM, Andrew Burgess wrote:
> * Mike Gulick <mgulick@mathworks.com> [2019-09-15 02:07:37 +0000]:
> 
>> On 9/14/19 11:54 AM, Eli Zaretskii wrote:
>>>> Date: Sat, 14 Sep 2019 11:28:47 -0400
>>>> From: Andrew Burgess <andrew.burgess@embecosm.com>
>>>> Cc: mgulick@mathworks.com, gdb-patches@sourceware.org
>>>>
>>>> If we have:
>>>>
>>>>    DW_AT_name:  	c:/project/foo.c
>>>>    DW_AT_comp_dir:	d:/project/build/
>>>>
>>>> And if the directory search path is:
>>>>
>>>>    g:/mnt/project;$cdir;$cwd
>>>>
>>>> And the current directory is:
>>>>
>>>>    h:/home/
>>>>
>>>> Then this is what I think the search order will be:
>>>>
>>>>    c:/project/foo.c
>>>>    g:/mnt/project/project/foo.c
>>>>    d:/project/build/project/foo.c
>>>>    h:/home/project/foo.c
>>>>
>>>>    d:/project/build/project/foo.c
>>>>    g:/mnt/project/project/build/project/foo.c
>>>>    d:/project/build/project/build/project/foo.c
>>>>    h:/home/project/build/project/foo.c
>>>>
>>>>    g:/mnt/project/foo.c
>>>>    d:/project/build/foo.c
>>>>    h:/home/foo.c
>>>>
>>>> The first and third block is what we currently (pre-patch) do, and the
>>>> second block is what I think the patch should provide after Mike's
>>>> last suggested update.
>>>
>>> SGTM, thanks.
>>>
>>
>> Thanks Andrew and Eli for the clarification on the proper behavior on
>> DOS paths.  The only remaining questionable case for me is whether to
>> check $cdir/COMP_DIR/FILENAME, which in your example above is
>> 'd:/project/build/project/build/project/foo.c'.  It seems reasonable
>> to me to leave this out of the search, although one could argue
>> leaving it on the source path may make the documentation slightly more
>> straightforward (one less special case).
>>
>> Please let me know if you would like me to provide an updated version
>> of the patch.
> 
> Personally I don't see it as a problem.  I agree it is unlikely to be
> something that ever makes sense, but we are already trying to access
> lots of paths that don't exist, I think throwing one more into the mix
> isn't going to hurt.
> 
> I took another stab at the documentation, this time I've moved the
> explanations for $cdir and $cwd earlier in the section - previously
> these were described really late on, and it made writing the earlier
> text hard as I was trying not to reference these things before they
> were introduced.
> 
> The docs now focus more on some worked examples rather than trying to
> just explain the algorithm in text.  I hope this is better.  I've also
> included an example of how MS drive letters are handled.

Thanks.  There's a lot of possible paths to search now, so I think
seeing then written out is helpful.  I have some minor corrections and
suggestions annotated in the code below.

> On the code side I have made one change from Mike's last patch,
> looking in openp at the code that glues filenames onto directories it
> had three preprocessing steps:
> 
>   1. Remove drive letters (for dos),
>   2. Remove leading slashes (/foo.c -> foo.c), and
>   3. Remove leading './' sequences (./foo.c -> foo.c).
> 
> I've now factored these three steps out and we share them between
> openp and open_and_find_source, otherwise the code is unchanged from
> Mike's last patch.

I also have a minor suggestion here that may handle one additional edge
case, please see below.

> 
> Let me know what you think.
> 
> Thanks,
> Andrew
> 
> ---
> 
> commit 70bbf4fc4358a8c10b20faf565ab3b0dee2a20c7
> Author: Mike Gulick <mgulick@mathworks.com>
> Date:   Thu Sep 12 11:16:06 2019 -0400
> 
>     gdb: Look for compilation directory relative to directory search path
>     
>     The 'directory' command allows the user to provide a list of filesystem
>     directories in which to search for source code.  The directories in this
>     search path are used as the base directory for the source filename from
>     the debug information (DW_AT_name).  Thus the directory search path
>     provides alternatives to the existing compilation directory from the
>     debug information (DW_AT_comp_dir).  Generally speaking, DW_AT_name
>     stores the filename argument passed to the compiler (including any
>     directory components), and DW_AT_comp_dir stores the current working
>     directory from which the compiler was executed.  For example:
>     
>         $ cd /path/to/project/subdir1
>         $ gcc -c a/test.c -g
>     
>     The corresponding debug information will look like this:
>     
>         DW_AT_name      : a/test.c
>         DW_AT_comp_dir  : /path/to/project/subdir1
>     
>     When compiling with the -fdebug-prefix-map GCC option, the compilation
>     directory can be arbitrarily rewritten.  In the above example, we may
>     rewrite the compilation directory as follows:
>     
>         $ gcc -c a/test.c -g -fdebug-prefix-map=/path/to/project=
>     
>     In this case, the corresponding debug information will look like:
>     
>         DW_AT_name      : a/test.c
>         DW_AT_comp_dir  : /subdir1
>     
>     This prevents GDB from finding the corresponding source code based on
>     the debug information alone.  In some cases, a substitute-path command
>     can be used to re-map a consistent prefix in the rewritten compilation
>     directory to the real filesystem path.  However, there may not be a
>     consistent prefix remaining in the debug symbols (for example in a
>     project that has source code in many subdirectories under the project's
>     root), thereby requiring multiple substitute-path rules.  In this case,
>     it is easier to add the missing prefix to the directory search path via
>     the 'directory' command.
>     
>     The function find_and_open_source currently searches in:
>     
>         SEARCH_PATH/FILENAME
>     
>     where SEARCH_PATH corresponds to each individual entry in the directory
>     search path (which is guaranteed to contain the compilation directory
>     from the debug information, as well as the current working directory).
>     FILENAME corresponds to the source filename (DW_AT_name), which may have
>     directory components in it.  In addition, GDB searches in:
>     
>         SEARCH_PATH/FILE_BASENAME
>     
>     where FILE_BASENAME is the basename of the DW_AT_name entry.
>     
>     This change modifies find_and_open_source to additionally search in:
>     
>         SEARCH_PATH/COMP_DIR/FILENAME
>     
>     where COMP_DIR is the compilation directory from the debug symbols.  In
>     the example given earlier, running:
>     
>         (gdb) directory /path/to/project
>     
>     will now allow GDB to correctly locate the source code from the debug
>     information.
>     
>     gdb/ChangeLog:
>     
>             * source.c (prepare_path_for_appending): New function.
>             (openp): Make use of new function.
>             (find_and_open_source): Search for the compilation directory and
>             source file as a relative path beneath the directory search path.
>     
>     gdb/doc/ChangeLog:
>     
>             * gdb.texinfo (Source Path): Additional text to better describe
>             how the source path directory list is used when searching for
>             source files.
>     
>     gdb/testsuite/ChangeLog:
>     
>             * gdb.base/source-dir.exp: Add extra test for mapped compilation
>             directory.
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 6d5f19d04bb..a173113929a 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2019-09-12  Mike Gulick  <mgulick@mathworks.com>
> +
> +	* source.c (prepare_path_for_appending): New function.
> +	(openp): Make use of new function.
> +	(find_and_open_source): Search for the compilation directory and
> +	source file as a relative path beneath the directory search path.
> +
>  2019-09-12  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>
>  
>  	* procfs.c (procfs_target::wait) <PR_FAULTED>: Get signal from
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 96c04091973..97c99663985 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,9 @@
> +2019-09-12  Andrew Burgess  <andrew.burgess@embecosm.com>
> +
> +	* gdb.texinfo (Source Path): Additional text to better describe
> +	how the source path directory list is used when searching for
> +	source files.
> +
>  2019-09-10  Tom Tromey  <tromey@adacore.com>
>  
>  	* gdb.texinfo (Index Files): Update Ada text.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 79824a0226a..20ab1e576ec 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -8954,11 +8954,20 @@
>  in the list, until it finds a file with the desired name.
>  
>  For example, suppose an executable references the file
> -@file{/usr/src/foo-1.0/lib/foo.c}, and our source path is
> -@file{/mnt/cross}.  The file is first looked up literally; if this
> -fails, @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c} is tried; if this
> -fails, @file{/mnt/cross/foo.c} is opened; if this fails, an error
> -message is printed.  @value{GDBN} does not look up the parts of the
> +@file{/usr/src/foo-1.0/lib/foo.c} and does not record a compilation
                                    ^^^^^^^^^^^^^
                                    , does not

> +directory, the @dfn{source path} is @file{/mnt/cross}. @value{GDBN}
              ^^^^
              and the

> +would look for the source file in the following locations:
> +
> +@enumerate
> +
> +@item @file{/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/mnt/cross/foo.c}
> +
> +@end enumerate
> +
> +If the source file is not present at any of the above locations then
> +an error is printed.  @value{GDBN} does not look up the parts of the
>  source file name, such as @file{/mnt/cross/src/foo-1.0/lib/foo.c}.
>  Likewise, the subdirectories of the source path are not searched: if
>  the source path is @file{/mnt/cross}, and the binary refers to
> @@ -8966,11 +8975,91 @@
>  @file{/mnt/cross/usr/src/foo-1.0/lib}.
>  
>  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}.
> +names containing dots, etc.@: are all treated as described above,
> +except that non-absolute file names are not looked up literally.  If
> +the @dfn{source path} is @file{/mnt/cross}, and the source file is
                                               ^^^
                                               no 'and' here

> +recorded as @file{../lib/foo.c} and no compilation directory is
                                  ^
                                  comma here

> +recorded then @value{GDBN} will search in the following locations:
           ^
           comma here
> +
> +@enumerate
> +
> +@item @file{/mnt/cross/../lib/foo.c}
> +@item @file{/mnt/cross/foo.c}
> +
> +@end enumerate
> +
> +@kindex cdir
> +@kindex cwd
> +@vindex $cdir@r{, convenience variable}
> +@vindex $cwd@r{, convenience variable}
> +@cindex compilation directory
> +@cindex current directory
> +@cindex working directory
> +@cindex directory, current
> +@cindex directory, compilation
> +The @dfn{source path} will always include two special entries
> +@samp{$cdir} and @samp{$cwd}, these refer to the compilation directory
> +(if one is recorded) and the current workig directory respectively.
                                        ^^^^^^^
                                        working
> +
> +@samp{cdir} causes @value{GDBN} to search within the compilation
         ^^^^
         $cdir

> +directory, if one is recorded in the debug information.  If no
> +compilation directory is recorded in the debug information then
> +@samp{cdir} is ignored.
         ^^^^
         $cdir

> +
> +@samp{$cwd} is not the same as @samp{.}---the former tracks the
> +current working directory as it changes during your @value{GDBN}
> +session, while the latter is immediately expanded to the current
> +directory at the time you add an entry to the source path.
> +
> +If a compilation directory is recorded in the debug information, and
> +@value{GDBN} has not found the source file after the first search
> +using @dfn{source path}, then @value{GDBN} will combine the
> +compilation directory and the filename, and then search for the source
> +file again using the @dfn{source path}.
> +
> +For example, if the executable records the source file as
> +@file{/usr/src/foo-1.0/lib/foo.c} the compilation directory is
                                    ^
                                    comma here

> +recorded as @file{/project/build}, and the @dfn{source path} is
> +@file{/mnt/cross;$cdir;$cwd} while the current working directory of
         ^^^^^^^^^^^^^^^^^^^^^
         /mnt/cross:$cdir:$cwd

It is probably better to use colons rather than semicolons since the
example uses unix paths.

> +the @value{GDBN} session is @file{/home/user}, then @value{GDBN} will
> +search for the source file in the following loctions:
> +
> +@enumerate
> +
> +@item @file{/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/mnt/cross/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/project/build/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/home/user/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/mnt/cross/project/build/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/project/build/project/build/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/home/user/project/build/usr/src/foo-1.0/lib/foo.c}
> +@item @file{/mnt/cross/foo.c}
> +@item @file{/project/build/foo.c}
> +@item @file{/home/user/foo.c}
> +
> +@end enumerate
> +
> +If the file name in the previous example had been recorded in the
> +executable as a relative path rather than an absolute path, then the
> +first look up would not have occurred, but all of the remaining steps
> +would be similar.
> +
> +When searching for source files on MS-DOS and MS-Windows, where
> +absolute paths start with a drive letter (e.g.
> +@file{C:/project/foo.c}), @value{GDBN} will remove the drive letter
> +from the file name before appending it to a search directory from
> +@dfn{source path}; for instance if the executable references the
> +source file @file{C:/project/foo.c} and @dfn{source path} is set to
> +@file{D:/mnt/cross}, then @value{GDBN} will search in the following
> +locations for the source file:
> +
> +@enumerate
> +
> +@item @file{C:/project/foo.c}
> +@item @file{D:/mnt/cross/project/foo.c}
> +@item @file{D:/mnt/cross/foo.c}
> +
> +@end enumerate
>  
>  Note that the executable search path is @emph{not} used to locate the
>  source files.

> @@ -9058,21 +9147,12 @@
>  whitespace.  You may specify a directory that is already in the source
>  path; this moves it forward, so @value{GDBN} searches it sooner.
>  
> -@kindex cdir
> -@kindex cwd
> -@vindex $cdir@r{, convenience variable}
> -@vindex $cwd@r{, convenience variable}
> -@cindex compilation directory
> -@cindex current directory
> -@cindex working directory
> -@cindex directory, current
> -@cindex directory, compilation
> -You can use the string @samp{$cdir} to refer to the compilation
> -directory (if one is recorded), and @samp{$cwd} to refer to the current
> -working directory.  @samp{$cwd} is not the same as @samp{.}---the former
> -tracks the current working directory as it changes during your @value{GDBN}
> -session, while the latter is immediately expanded to the current
> -directory at the time you add an entry to the source path.
> +The special strings @samp{$cdir} (to refer to the compilation
> +directory, if one is recorded), and @samp{$cwd} (to refer to the
> +current working directory) can also be included in the list of
> +directories @var{dirname}.  Though these will already be in the source
> +path they will be moved forward in the list so @value{GDBN} searches
> +them sooner.
>  
>  @item directory
>  Reset the source path to its default value (@samp{$cdir:$cwd} on Unix systems).  This requires confirmation.

I would also change the other spot that uses the names 'cdir' and 'cwd'
to be '$cdir' and '$cwd'.  I don't see any value in referring to
these special names without the leading '$':

@@ -9070,8 +9071,8 @@ each line is in the file.
 
 @kindex directory
 @kindex dir
-When you start @value{GDBN}, its source path includes only @samp{cdir}
-and @samp{cwd}, in that order.
+When you start @value{GDBN}, its source path includes only @samp{$cdir}
+and @samp{$cwd}, in that order.
 To add other directories, use the @code{directory} command.
 
 The search path is used to find both program source files and @value{GDBN}

> diff --git a/gdb/source.c b/gdb/source.c
> index b27f210802c..ece1ad3d16b 100644
> --- a/gdb/source.c
> +++ b/gdb/source.c
> @@ -654,6 +654,29 @@ info_source_command (const char *ignore, int from_tty)
>  }
>  \f
>  
> +/* Helper function to remove characters from the start of PATH so that
> +   PATH can then be appended to a directory name.  We remove leading drive
> +   letters (for dos) as well as leading '/' characters and './'
> +   sequences.  */
> +
> +const char *
> +prepare_path_for_appending (const char *path)
> +{
> +  /* For dos paths, d:/foo -> /foo, and d:foo -> foo.  */
> +  if (HAS_DRIVE_SPEC (path))
> +    path = STRIP_DRIVE_SPEC (path);
> +
> +  /* /foo => foo, to avoid multiple slashes that Emacs doesn't like.  */
> +  while (IS_DIR_SEPARATOR(path[0]))
> +    path++;
> +
> +  /* ./foo => foo */
> +  while (path[0] == '.' && IS_DIR_SEPARATOR (path[1]))
> +    path += 2;
> +
> +  return path;
> +}
> +

This may be a little pedantic, but looping around the removal of the '/'
and './' would handle a case like './/':

/* Helper function to remove characters from the start of PATH so that
   PATH can then be appended to a directory name.  We remove leading drive
   letters (for dos) as well as leading '/' characters and './'
   sequences.  */

const char *
prepare_path_for_appending (const char *path)
{
  /* For dos paths, d:/foo -> /foo, and d:foo -> foo.  */
  if (HAS_DRIVE_SPEC (path))
    path = STRIP_DRIVE_SPEC (path);

  const char *oldpath;
  /* Remove any sequence of '/' and './'.  */
  do {
    oldpath = path;
    /* /foo => foo, to avoid multiple slashes that Emacs doesn't like.  */
    if (IS_DIR_SEPARATOR(path[0]))
      path++;

    /* ./foo => foo */
    if (path[0] == '.' && IS_DIR_SEPARATOR (path[1]))
      path += 2;
  } while (path != oldpath);

  return path;
}


Feel free to use your discretion regarding these changes, I'm fine
either way.  Thanks a lot for all of the help.  Again, let me know if
there's anything else I should do.

Thanks,
Mike

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-13 22:52             ` Mike Gulick
  2019-09-14  7:11               ` Eli Zaretskii
@ 2019-09-17 20:22               ` Andrew Burgess
  2019-09-17 20:39                 ` Mike Gulick
  1 sibling, 1 reply; 20+ messages in thread
From: Andrew Burgess @ 2019-09-17 20:22 UTC (permalink / raw)
  To: Mike Gulick; +Cc: Eli Zaretskii, gdb-patches

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
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [RFC] Apply compilation dir to source_path lookup
  2019-09-17 20:22               ` Andrew Burgess
@ 2019-09-17 20:39                 ` Mike Gulick
  0 siblings, 0 replies; 20+ messages in thread
From: Mike Gulick @ 2019-09-17 20:39 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: Mike Gulick, Eli Zaretskii, gdb-patches

On 9/17/19 4:22 PM, Andrew Burgess wrote:
> Mike,
> 
> Thanks for your feedback.  I incorporated your suggested changes and
> pushed this to master.
> 
> Thanks,
> Andrew
> 

Looks great.  Thanks so much for your help Andrew.

-Mike

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-09-17 20:39 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-05 22:40 [RFC] Apply compilation dir to source_path lookup Mike Gulick
2019-09-07 23:51 ` Andrew Burgess
2019-09-09 22:41   ` Mike Gulick
2019-09-13  1:38     ` Andrew Burgess
2019-09-13  6:36       ` Eli Zaretskii
2019-09-13  7:28         ` Eli Zaretskii
2019-09-13 22:45           ` Andrew Burgess
2019-09-13 22:52             ` Mike Gulick
2019-09-14  7:11               ` Eli Zaretskii
2019-09-17 20:22               ` Andrew Burgess
2019-09-17 20:39                 ` Mike Gulick
2019-09-14  6:56             ` Eli Zaretskii
2019-09-14 15:28               ` Andrew Burgess
2019-09-14 15:54                 ` Eli Zaretskii
2019-09-15  2:07                   ` Mike Gulick
2019-09-15  4:01                     ` Andrew Burgess
2019-09-15 15:29                       ` Eli Zaretskii
2019-09-16 15:53                       ` Mike Gulick
2019-09-13 22:11         ` Mike Gulick
2019-09-13 22:41           ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).