[Public] Thanks Jan for reviewing the code changes, >> - /* 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. >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. 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 . Regards, Rupesh P --- bfd/dwarf2.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c index f6b0183720b..c5b5d14fc9f 100644 --- a/bfd/dwarf2.c +++ b/bfd/dwarf2.c @@ -1572,6 +1572,7 @@ struct line_info_table unsigned int num_files; unsigned int num_dirs; unsigned int num_sequences; + unsigned int version; char * comp_dir; char ** dirs; struct fileinfo* files; @@ -1792,6 +1793,8 @@ concat_filename (struct line_info_table *table, unsigned int file) { char *filename; + if (table->version >= 5) + file = file + 1; if (table == NULL || file - 1 >= table->num_files) { /* FILE == 0 means unknown. */ @@ -2414,7 +2417,7 @@ 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 (table->version < 5 && datai != 0) if (!callback (table, fe.name, fe.dir, fe.time, fe.size)) return false; } @@ -2581,6 +2584,7 @@ decode_line_info (struct comp_unit *unit) table->sequences = NULL; table->lcl_head = NULL; + table->version = lh.version; if (lh.version >= 5) { @@ -2623,7 +2627,12 @@ decode_line_info (struct comp_unit *unit) /* State machine registers. */ bfd_vma address = 0; unsigned char op_index = 0; - char * filename = table->num_files ? concat_filename (table, 1) : NULL; + char *filename; + if (table->version >= 5) + filename = table->num_files ? concat_filename (table, 0) : NULL; + else + filename = table->num_files ? concat_filename (table, 1) : NULL; + unsigned int line = 1; unsigned int column = 0; unsigned int discriminator = 0; -- 2.17.1 >-----Original Message----- >From: Jan Beulich >Sent: Wednesday, May 18, 2022 6:07 PM >To: Potharla, Rupesh ; Potharla, Rupesh via >Binutils >Cc: George, Jini Susan ; Parasuraman, >Hariharan ; Natarajan, Kavitha > >Subject: Re: [PATCH] bfd: Fix issues with files in debug_line table with dwarf5. > >[CAUTION: External Email] > >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 >> 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