[PATCH] bfd: Fix issues with files in debug_line table with dwarf5.
Potharla, Rupesh
Rupesh.Potharla@amd.com
Tue Jul 5 05:05:43 GMT 2022
[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 <jbeulich@suse.com>
>Cc: George, Jini Susan <JiniSusan.George@amd.com>; Parasuraman,
>Hariharan <Hariharan.Parasuraman@amd.com>; Natarajan, Kavitha
><Kavitha.Natarajan@amd.com>; Potharla, Rupesh via Binutils
><binutils@sourceware.org>
>Subject: RE: [PATCH] bfd: Fix issues with files in debug_line table with dwarf5.
>
>[AMD Official Use Only - General]
>
>
>
>>-----Original Message-----
>>From: Jan Beulich <jbeulich@suse.com>
>>Sent: Wednesday, May 25, 2022 11:49 AM
>>To: Potharla, Rupesh <Rupesh.Potharla@amd.com>
>>Cc: George, Jini Susan <JiniSusan.George@amd.com>; Parasuraman,
>>Hariharan <Hariharan.Parasuraman@amd.com>; Natarajan, Kavitha
>><Kavitha.Natarajan@amd.com>; Potharla, Rupesh via Binutils
>><binutils@sourceware.org>
>>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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-bfd-Fix-issues-with-files-in-debug_line-table-with-d.docx
Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document
Size: 17590 bytes
Desc: 0001-bfd-Fix-issues-with-files-in-debug_line-table-with-d.docx
URL: <https://sourceware.org/pipermail/binutils/attachments/20220705/bf545356/attachment-0001.docx>
More information about the Binutils
mailing list