RE: [PATCH] Fix macro info lookup for binaries containing DWARFv5 line table

Tomar, Sourabh Singh via Gdb-patches
2021-05-24T11:36:54Z

[AMD Public Use]

Hello Keith,

Could you please share your plan WRT to this patch.
Do you want to take it forward ? or you want me to take this forward.

Thanks,
Sourabh

-----Original Message-----
From: Keith Seitz <keiths@redhat.com>
Sent: Friday, May 14, 2021 8:26 PM
To: Tomar, Sourabh Singh <SourabhSingh.Tomar@amd.com>; gdb-patches@sourceware.org
Subject: Re: [PATCH] Fix macro info lookup for binaries containing DWARFv5 line table

[CAUTION: External Email]

Coincidentally, I happened to be looking at gdb/27306 about a similar problem, and I was curious to see how our patches interacted.

On 5/12/21 10:16 AM, Sourabh Singh Tomar via Gdb-patches wrote:

> As one may notice that dir index `0` will always contain absoulte
> path(Not including the file name as), this causes GDB to append the
> path + filename to construt macro info `main_file->filename`. This
> leads to overriding of the macro filename which will cause macro info
> lookup failure due to mismatch in `main_file->filename` and
> `sal.symtab->filename`.

gdb/27306 highlights a similar problem with gcc using -gdwarf-5:

cat -n undef-macro.c
1 #define FOO 1
2
3 int main(int argc, char **argv)
4 {
5 return FOO;
6 }
$ gcc -gdwarf-5 undef-macro.c -o undef-macro $ ./gdb -nx -q undef-macro -ex start -ex "p FOO"
(gdb) start
Temporary breakpoint 1 at 0x401111: file foo.c, line 5.
Starting program: undef-macro

Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdd48) at undef-macro.c:5
5 return FOO;
No symbol "FOO" in current context.

The problem is that file name returned by file_file_name is /not/ relative to the compilation directory. Ever.

From file_file_name:

const file_entry *fe = file_name_at (file);

if (!IS_ABSOLUTE_PATH (fe->name))
{
const char *dir = fe->include_dir (this);
/* The file at index 0 (valid starting with DWARF 5) is already
relative to the compilation directory. */

if (file != 0)
return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
fe->name,
(char *) NULL));

Even with your patch, gdb will still return an absolute path for the above test.

As you've already mentioned, macro_start_file and macro_lookup_inclusion require that macro_source_file.filename be relative to the compliation directory. Furthermore, the documentation for macro_source_file.filename is stricter:

/* A source file --- possibly a header file. This filename is relative to
the compilation directory (table->comp_dir), it exactly matches the
symtab->filename content. */
const char *filename;

So if symtab->filename and macro_source_filename are required to be the same, why are we bothering with prepending directories? We already have access to the symtab in `fe'.

This is the version I was playing with...

diff --git a/gdb/dwarf2/line-header.c b/gdb/dwarf2/line-header.c index 7575297f966..117c5a42cc5 100644
--- a/gdb/dwarf2/line-header.c
+++ b/gdb/dwarf2/line-header.c
@@ -69,15 +69,10 @@ line_header::file_file_name (int file) const
{
const file_entry *fe = file_name_at (file);

- if (!IS_ABSOLUTE_PATH (fe->name))
- {
- const char *dir = fe->include_dir (this);
- if (dir != NULL)
- return gdb::unique_xmalloc_ptr<char> (concat (dir, SLASH_STRING,
- fe->name,
- (char *) NULL));
- }
- return make_unique_xstrdup (fe->name);
+ /* macro_source_file requires: "This filename is relative to the
+ compilation directory, it exactly matches the symtab->filename
+ content." */
+ return make_unique_xstrdup (fe->symtab->filename);
}
else
{

By happy coincidence, this has the same effect of your patch, but it also fixes gdb/27306.

I checked the origins of file_file_name and it appears that this function was split out from file_full_name, but this directory prepending bit was never removed to fulfill the "relative" requirement of the stated documentation.
[This is from commit 233d95b548ec948c4a6d01cd05c307385dd615fb.]

I hate to post counter-patches, but in this case, I think we are really both attempting to solve the same design problem with line_header::file_file_name.

> This problem does not pops up with GCC compiled binaries, since GCC
> does not emits DWARFv5 compliant line tables.

This test (as written) does not pass using gcc:

$ make check TESTS=gdb.base/dwarf5-macro.exp [snip] Running /home/keiths/work/gdb/branches/virgin/linux/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/dwarf5-macro.exp ...
FAIL: gdb.base/dwarf5-macro.exp: info macro HELLO_GDB

=== gdb Summary ===

# of unexpected failures 1

However, if I force -gdwarf-5 for gcc, too:

$ make check TESTS=gdb.base/dwarf5-macro.exp RUNTESTFLAGS="unix/-g3/-gdwarf-5"
[snip]
Running /home/keiths/work/gdb/branches/virgin/linux/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/dwarf5-macro.exp ...

=== gdb Summary ===

# of expected passes 1

I suggest that be corrected -- just force gcc to emit dwarf-5 debuginfo as well. IMO, the correct, compiler-agnostic way of dealing with this is via the DWARF assembler, but I am not going to suggest you do that.

I'd be happy if the test simply didn't FAIL on non-clang targets.

Keith