Re: [PATCH] bfd: Fix issues with files in debug_line table with dwarf5.

[Jan Beulich <jbeulich@suse.com>]

On 25.05.2022 06:20, Potharla, Rupesh wrote:
>>> -      /* Skip the first "zero entry", which is the compilation dir/file.  */
>>> -      if (datai != 0)
>>> -         if (!callback (table, fe.name, fe.dir, fe.time, fe.size))
>>> -           return false;
>>> +      if (!callback (table, fe.name, fe.dir, fe.time, fe.size))
>>> +        return false;
>>>      }
>>
>> How come this change doesn't add a version check?
>>
> Since the function read_formatted_entries is only called for version 5 in the file, I thought the version check is not needed. Now I have added the condition and updated the patch. 

Oh, I'm sorry - I hadn't noticed this aspect. I don't think a version
check is needed then.

>> To help being certain this is the right way of changing things, can you please
>> add up to two testcases (readelf and/or objdump), one for a version < 5
>> (unless one such already exists and hence it would be visible there that you
>> don't unduly alter handling of those older versions) and one for version 5?
>>
> 
> Readelf and objdump are not using dwarf2.c file under bfd directory these tools are using dwarf.c under binutils directory.

Argh, yes - too many dwarf*.c in the tree.

>  Currently I am using addr2line to test and validate my code changes manually. There are no testcases for addr2line and I did not find any testcases for bfd library as well.  Please let me know your suggestions .

Is there a reason speaking against adding an addr2line test? The
generic framework looks to know of addr2line. (Perhaps ideally the
same source would be used for both an addr2line test and a readelf or
objdump one, as to prove that the two forms of Dwarf reading are
actually in sync. But I guess that's asking for too much.)

Jan