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