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

[Keith Seitz <keiths@redhat.com>]

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