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

[Jan Beulich <jbeulich@suse.com>]

On 09.05.2022 09:03, Potharla, Rupesh via Binutils wrote:
> [Public]
> 
> 
> 
> While working on the implementation of DW_FORM_strx forms could not print file names even after the implementation of strx forms. I found an issue with adding the file names to the file table with dwarf5 and clang.
> 
> With dwarf5 debug line version the file index is starting with zero, but the code is expecting it to be 1 which is the case with other dwarf versions.
> 
> From the contents of .debug_line compiled with clang and dwarf5, the file names array index is starting with zero.
> 
> standard_opcode_lengths[DW_LNS_set_isa] = 1
> include_directories[  0] = "/home/rupesh/addr2line"
> file_names[  0]:
>            name: "prog1.c"
>       dir_index: 0
>    md5_checksum: da4ea4c312af96d39b13557acdf23f05
> 
> Address            Line   Column File   ISA Discriminator Flags
> ------------------ ------ ------ ------ --- ------------- -------------
> 
> 
> The below line skipping zero entry was added as part of commit 19d80e5fec548e681c453d15b4ae5b49bc080acc is ignoring the file names in the zeroth index. I have no idea why this line was added. Removing the line is working for programs compiled with clang using dwarf5. With my fix, I am not seeing any issues with GCC and dwarf5 moreover currently GCC's debug_line version is 3 even when compiled with dwarf5.
> 
>       /* 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;
> 
> Made code changes to fix this issue. Can you review the code changes and send in your comments/suggestions?
> 
> Regards,
> Rupesh P

Much of the above wants to go ...

> From 28e92539dfe5319e7bdfea32c4ee46f55ff51053 Mon Sep 17 00:00:00 2001
> From: rupothar rupesh.potharla@amd.com<mailto:rupesh.potharla@amd.com>
> Date: Mon, 9 May 2022 12:10:48 +0530
> Subject: [PATCH] bfd: Fix issues with files in debug_line table with dwarf5.
> 
> ---

... above this marker, to become the actual commit message.

> @@ -2270,10 +2273,8 @@ read_formatted_entries (struct comp_unit *unit, bfd_byte **bufp,
>              }
>          }
> 
> -      /* 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?

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?

Jan