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

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

[-- Attachment #1: Type: text/plain, Size: 3903 bytes --]

On 2021-06-22 1:01 p.m., Keith Seitz via Gdb-patches wrote:
> I think this might be something we should consider including in gdb-11.
> 
> Keith
> 
> On 6/8/21 11:48 AM, Keith Seitz via Gdb-patches wrote:
>> RFC ping
>>
>> Anyone at all have an opinion on this direction?
>>
>> Keith
>>
>> On 5/24/21 11:47 AM, Keith Seitz via Gdb-patches wrote:
>>> On 5/24/21 4:36 AM, Tomar, Sourabh Singh wrote:
>>>> [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.
>>>>
>>>
>>> I can pursue this...
>>>
>>> In that vein, does anyone (maintainers?) have an input on my "counterpatch"
>>> (reposted below) that removes this IS_ABSOLUTE_PREFIX stuff and copies the
>>> symtab's filename?
>>>
>>> I haven't officially submitted this as a patch because I'm curious whether
>>> my reading of this is correct/complete. Maybe the documentation/comments
>>> are incorrect or no longer valid? 
>>>
>>> FWIW, I've tested that patch on native x86_64 Fedora 34 with no regressions.

Hi Keith,

It's a bit hard to tell whether this will always work, because different
compilers do things slightly differently.  Factor in different version,
DWARF4 vs DWARF5, that's a lot of combinations.  I wrote a little script
to test as many combinations as possible, and with your patch it all
looks good, in the sense that I can print the macros in all cases.  I'll
attach it to this message if you want to play with it.

The axis are:

 - the compiler
 - DWARF5 vs DWARF4
 - how the source file is specified on the command line

How the source file is specified on the command line can be:

 - foo.c
 - ../foo.c
 - dir/foo.c (while PWD is the parent directory)
 - /absolute/path/to/dir/foo.c (while PWD is `dir`)
 - /absolute/path/to/dir/foo.c (while PWD is the parent directory)

They all produce slightly different debug info that could lead to
something going wrong.

The important thing is that it's well tested, so ensure that as many of
these ways to specify a file on the command line are tested.  Test with
one macro in the main source file and one macro in an included file.  I
don't know if we can have a DWARF4 vs DWARF5 axis in the test, but if so
it would be nice.  And add any another possible variation you can think
of.

One more comment below...

>>>
>>> Keith
>>>
>>> Patch under discussion:
>>>
>>> gdb/ChangeLog
>>>
>>> 	* dwarf2/line-header.c (line_header::file_file_name): Copy
>>> 	the symtab's filename.
>>>
>>> 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);

If I understand correctly, the contract of file_file_name is that it
should never return an absolute path.  Therefore, should we be able to
stick a

    gdb_assert (!IS_ABSOLUTE_PATH (fe->symtab->filename))

in there?  If I do, it catches at least one case where it returns an
absolute path, in test-gcc-9-dwarf-4-std.

    (top-gdb) p fe.symtab.filename
    $1 = 0x62100003e960 "/home/smarchi/build/binutils-gdb/macro-test/test.c"

Printing the macros still work, but I wonder if that's expected or not.

Simon

[-- Attachment #2: macro-test.tgz --]
[-- Type: application/x-compressed-tar, Size: 661 bytes --]