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

["Tomar, Sourabh Singh via Gdb-patches" <gdb-patches@sourceware.org>]

[AMD Public Use]


Hi Keith,

Thanks for the review!

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

Ah, I also observed this with today(rebased) GCC (GNU C17 12.0.0 20210514), seems like things changed (Not for good) in GCC,
My patch was initially posted in some March21 time frame at that time this was working(for GCC) ☹

One thing I noticed that changed is that GCC is now emitting `dir0` entry in line table as per DWARF5. However this still seems
Incomplete to me:
$llvm-dwarfdump --debug-line a.out
```
include_directories[  1] = "/usr/include" // index still starting from `1` even at "-gdwarf-5"
file_names[  1]:
           name: "macro.c"
      dir_index: 0               // -> dir 0 mentioned but dir 0 is not present in list of directories, i.e index still starting from 1
       mod_time: 0x00000000
         length: 0x00000000
file_names[  2]:
           name: "stdc-predef.h"
      dir_index: 1
       mod_time: 0x00000000
         length: 0x00000000
```
Investigation: With or without my fix.
For investigating primary data-structures involve in this lookup i.e "main_file->filename, sal.symtab->filename"
A simple printf insertion in gdb/macroscope.c, provides further insights for how GDB is handling GCC generated DWARF5
Binaries.
$tot-gcc -gdwarf-5 -g3 macro.c -o a.out.gcc
$gdb a.out.gcc
(gdb) start
(gdb) info macros
(gdb) info macros
**main_file->filename ->    // Empty**
sal.symtab->filename -> macro.c

So it seems like GCC's incorrect info that's causing GDB to confuse.

My patch only touches the file `0` DWARFv5 case, CLANG is emitting DWARFv5 compliant line & macro tables
you may choose to have a look on the diff of line & macro tables for above case using GCC & CLANG.

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