[AMD Official Use Only - General] Hi Jan, As you suggested, added a new test case of addr2line. Found a minor bug in read_indexed_address function and fixed that as well. Can you review my code changes and let me know your suggestions/comments? Regards, Rupesh P Fixed an issue with the file index for dwarf5. Added addr2line test case. read_indexed_address is using offset_size instead of addr_size while reading addrx forms, fixed that as well. --- bfd/dwarf2.c | 21 ++++--- binutils/testsuite/binutils-all/addr2line.exp | 59 +++++++++++++++++++ binutils/testsuite/config/default.exp | 6 ++ 3 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 binutils/testsuite/binutils-all/addr2line.exp diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c index aaa2d84887f..2a992d67bc7 100644 --- a/bfd/dwarf2.c +++ b/bfd/dwarf2.c @@ -1369,7 +1369,7 @@ read_indexed_address (uint64_t idx, struct comp_unit *unit) &file->dwarf_addr_buffer, &file->dwarf_addr_size)) return 0; - if (_bfd_mul_overflow (idx, unit->offset_size, &offset)) + if (_bfd_mul_overflow (idx, unit->addr_size, &offset)) return 0; offset += unit->dwarf_addr_offset; @@ -1380,9 +1380,9 @@ read_indexed_address (uint64_t idx, struct comp_unit *unit) info_ptr = file->dwarf_addr_buffer + offset; - if (unit->offset_size == 4) + if (unit->addr_size == 4) return bfd_get_32 (unit->abfd, info_ptr); - else if (unit->offset_size == 8) + else if (unit->addr_size == 8) return bfd_get_64 (unit->abfd, info_ptr); else return 0; @@ -1731,6 +1731,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; @@ -1951,6 +1952,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. */ @@ -2579,10 +2582,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; } *bufp = buf; @@ -2747,6 +2748,7 @@ decode_line_info (struct comp_unit *unit) table->sequences = NULL; table->lcl_head = NULL; + table->version = lh.version; if (lh.version >= 5) { @@ -2789,13 +2791,16 @@ 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; + int index = table->version >= 5 ? 0 : 1; unsigned int line = 1; unsigned int column = 0; unsigned int discriminator = 0; int is_stmt = lh.default_is_stmt; int end_sequence = 0; unsigned int dir, xtime, size; + + filename = table->num_files ? concat_filename (table, index) : NULL; /* eraxxon@alumni.rice.edu: Against the DWARF2 specs, some compilers generate address sequences that are wildly out of order using DW_LNE_set_address (e.g. Intel C++ 6.0 compiler diff --git a/binutils/testsuite/binutils-all/addr2line.exp b/binutils/testsuite/binutils-all/addr2line.exp new file mode 100644 index 00000000000..153e83c2ace --- /dev/null +++ b/binutils/testsuite/binutils-all/addr2line.exp @@ -0,0 +1,59 @@ +# Copyright (C) 2018-2022 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, USA. + + global $NM + global $ADDR2LINE + + set testname "addr2line" + if { [target_compile $srcdir/$subdir/testprog.c tmpdir/testprog executable debug] != "" } { + verbose "Unable to compile test file." + untested "addr2line" + return + } + + #testcase for default option. + set output [binutils_run $NM "tmpdir/testprog"] + regexp -line {^[0-9]+\s+[A-Z]\s+main} $output contents + set list [regexp -inline -all -- {\S+} $contents] + + set got [binutils_run $ADDR2LINE "-e tmpdir/testprog [lindex $list 0]"] + set want "$srcdir/$subdir/testprog.c:\[0-9\]+" + if ![regexp $want $got] then { + fail "$testname $got\n" + } else { + pass "$testname" + } + + #testcase for -f option. + regexp -line {^[0-9]+\s+[A-Z]\s+fn} $output contents + set list [regexp -inline -all -- {\S+} $contents] + + set got [binutils_run $ADDR2LINE "-f -e tmpdir/testprog [lindex $list 0]"] + set want "fn\n$srcdir/$subdir/testprog.c:\[0-9\]+" + if ![regexp $want $got] then { + fail "$testname -f option $got\n" + } else { + pass "$testname -f option" + } + + #testcase for -s option. + set got [binutils_run $ADDR2LINE "-s -e tmpdir/testprog [lindex $list 0]"] + set want "testprog.c:\[0-9\]+" + if ![regexp $want $got] then { + fail "$testname -s option $got\n" + } else { + pass "$testname -s option" + } diff --git a/binutils/testsuite/config/default.exp b/binutils/testsuite/config/default.exp index c654bd4081c..7192c929a83 100644 --- a/binutils/testsuite/config/default.exp +++ b/binutils/testsuite/config/default.exp @@ -40,6 +40,12 @@ if ![info exists NM] then { if ![info exists NMFLAGS] then { set NMFLAGS "" } +if ![info exists ADDR2LINE] then { + set ADDR2LINE [findfile $base_dir/addr2line $base_dir/addr2line [transform nm]] +} +if ![info exists ADDR2LINEFLAGS] then { + set ADDR2LINEFLAGS "" +} if ![info exists SIZE] then { set SIZE [findfile $base_dir/size] } -- 2.17.1 >-----Original Message----- >From: Potharla, Rupesh >Sent: Tuesday, May 31, 2022 11:38 PM >To: Jan Beulich >Cc: George, Jini Susan ; Parasuraman, >Hariharan ; Natarajan, Kavitha >; Potharla, Rupesh via Binutils > >Subject: RE: [PATCH] bfd: Fix issues with files in debug_line table with dwarf5. > >[AMD Official Use Only - General] > > > >>-----Original Message----- >>From: Jan Beulich >>Sent: Wednesday, May 25, 2022 11:49 AM >>To: Potharla, Rupesh >>Cc: George, Jini Susan ; Parasuraman, >>Hariharan ; Natarajan, Kavitha >>; Potharla, Rupesh via Binutils >> >>Subject: Re: [PATCH] bfd: Fix issues with files in debug_line table with >dwarf5. >> >>[CAUTION: External Email] >> >>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.) >> > >Regarding test for addr2line, the input address given to addr2line is retrieved >from output of nm/objdump tool. The functioning of the addr2line tool >depends on the reliability of other tools. > >>Jan