RE: [PATCH] Binutils support for DWARF-5 DW_OP_addrx

["Natarajan, Kavitha" <Kavitha.Natarajan@amd.com>]

[Public]

Hi Nick,

Thanks for your comments.

> The patch introduced some new failures into the binutils testsuite.  For
> example the new testprog.s source file fails to assemble on most
> architectures because it contains x86_64 instructions...

Modified test cases.

>
> > @@ -2661,9 +2665,10 @@ read_and_display_attr_value (unsigned long
> attribute,
> >
> >         uvalue = check_uvalue (block_start, uvalue, end);
> >
> > -      if (do_loc)
> > -    data = block_start + uvalue;
> > -      else
> > +      data = block_start + uvalue;
> > +
> > +      /* DW_OP_addrx has only the index and not address.  */
> > +      if (!do_loc && ((unsigned)(*block_start) != DW_OP_addrx))
>
> This looks wrong to me.  Why are you testing the first word of the block for a
> DW_OP_addrx op ?  At this point the code is just displaying the contents of
> the block, not interpreting them.  Plus you do not check to see that there are
> enough bytes present in the block to make up a word.

Safe byte read is done now. The code checks for encodings after displaying
the block in decode_location_expression(), reading the next word. As DW_OP_addrx
is an index, checking ahead if it is DW_OP_addrx op and avoid displaying the
incorrect block data. Attaching the updated patch.

Regards,
Kavitha
========================================================================
Binutils tools such as objdump and readelf to support new dwarf-5
operation encoding, such as, DW_OP_addrx.

Added test points in objdump.exp and readlef.exp.

---
 binutils/dwarf.c                            |  16 ++-
 binutils/testsuite/binutils-all/dw5-op.S    | 134 ++++++++++++++++++++
 binutils/testsuite/binutils-all/dw5-op.W    |  35 +++++
 binutils/testsuite/binutils-all/objdump.exp |  43 ++++++-
 binutils/testsuite/binutils-all/readelf.exp |  19 +++
 5 files changed, 242 insertions(+), 5 deletions(-)
 create mode 100644 binutils/testsuite/binutils-all/dw5-op.S
 create mode 100644 binutils/testsuite/binutils-all/dw5-op.W

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index e61c63a0601..698b8fc62d2 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -1721,6 +1721,10 @@ decode_location_expression (unsigned char * data,
          printf ("DW_OP_GNU_parameter_ref: <0x%s>",
                  dwarf_vmatoa ("x", cu_offset + uvalue));
          break;
+       case DW_OP_addrx:
+         READ_ULEB (uvalue, data, end);
+         printf ("DW_OP_addrx <0x%s>", dwarf_vmatoa ("x", uvalue));
+         break;
        case DW_OP_GNU_addr_index:
          READ_ULEB (uvalue, data, end);
          printf ("DW_OP_GNU_addr_index <0x%s>", dwarf_vmatoa ("x", uvalue));
@@ -2661,10 +2665,14 @@ read_and_display_attr_value (unsigned long           attribute,

       uvalue = check_uvalue (block_start, uvalue, end);

-      if (do_loc)
-       data = block_start + uvalue;
-      else
-       data = display_block (block_start, uvalue, end, delimiter);
+      data = block_start + uvalue;
+
+      if (!do_loc) {
+       unsigned char op;
+       SAFE_BYTE_GET (op, block_start, sizeof(unsigned char), end);
+       if (op != DW_OP_addrx)
+         data = display_block (block_start, uvalue, end, delimiter);
+      }
       break;

     case DW_FORM_block1:
diff --git a/binutils/testsuite/binutils-all/dw5-op.S b/binutils/testsuite/binutils-all/dw5-op.S
new file mode 100644
index 00000000000..ba4037786d4
--- /dev/null
+++ b/binutils/testsuite/binutils-all/dw5-op.S
@@ -0,0 +1,134 @@
+/* Copyright (C) 2017-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, see <http://www.gnu.org/licenses/>.  */
+
+       .file   "main.c"
+       .text
+.Letext0:
+       .section        .debug_info,"",%progbits
+.Ldebug_info0:
+       .4byte  0x4e    /* Length of Compilation Unit Info */
+       .2byte  0x5     /* DWARF version number */
+       .byte   0x1     /* DW_UT_compile */
+       .byte   0x8     /* Pointer Size (in bytes) */
+       .4byte  .Ldebug_abbrev0 /* Offset Into Abbrev. Section */
+       .uleb128 0x3    /* (DIE (0xc) DW_TAG_compile_unit) */
+       .4byte  .LASF21 /* DW_AT_producer: "GNU C11 7.0.1 20170218 (experimental) -mtune=generic -march=x86-64 -gdwarf-5 -O2" */
+       .byte   0x1d    /* DW_AT_language */
+       .4byte  .LASF0  /* DW_AT_name: "main.c" */
+       .4byte  .LASF1  /* DW_AT_comp_dir: "" */
+       .4byte  .LLRL2  /* DW_AT_ranges */
+       .8byte  0       /* DW_AT_low_pc */
+       .4byte  .Ldebug_line0   /* DW_AT_stmt_list */
+       .uleb128 0x1    /* (DIE (0x2a) DW_TAG_base_type) */
+       .byte   0x4     /* DW_AT_byte_size */
+       .byte   0x5     /* DW_AT_encoding */
+       .4byte  .LASF2  /* DW_AT_name: "short int" */
+       .uleb128 0x2    /* (DIE (0x31) DW_TAG_variable) */
+       .4byte  .LASF16 /* DW_AT_name: "xvar" */
+                       /* DW_AT_decl_file (1, main.c) */
+       .byte   0x2     /* DW_AT_decl_line */
+       .4byte  0x2a    /* DW_AT_type */
+                       /* DW_AT_external */
+       .uleb128 0x9    /* DW_AT_location */
+       .byte   0x3     /* DW_OP_addr */
+       .8byte  0x1234
+       .uleb128 0x2    /* (DIE (0x45) DW_TAG_variable) */
+       .4byte  .LASF17 /* DW_AT_name: "yvar" */
+                       /* DW_AT_decl_file (1, main.c) */
+       .byte   0x3     /* DW_AT_decl_line */
+       .4byte  0x2a    /* DW_AT_type */
+                       /* DW_AT_external */
+       .uleb128 0x2    /* DW_AT_location */
+       .byte   0xa1    /* DW_OP_addrx */
+       .byte   0x0
+       .section        .debug_abbrev,"",%progbits
+.Ldebug_abbrev0:
+       .uleb128 0x1    /* (abbrev code) */
+       .uleb128 0x24   /* (TAG: DW_TAG_base_type) */
+       .byte   0       /* DW_children_no */
+       .uleb128 0xb    /* (DW_AT_byte_size) */
+       .uleb128 0xb    /* (DW_FORM_data1) */
+       .uleb128 0x3e   /* (DW_AT_encoding) */
+       .uleb128 0xb    /* (DW_FORM_data1) */
+       .uleb128 0x3    /* (DW_AT_name) */
+       .uleb128 0xe    /* (DW_FORM_strp) */
+       .byte   0
+       .byte   0
+       .uleb128 0x2    /* (abbrev code) */
+       .uleb128 0x34   /* (TAG: DW_TAG_variable) */
+       .byte   0       /* DW_children_no */
+       .uleb128 0x3    /* (DW_AT_name) */
+       .uleb128 0xe    /* (DW_FORM_strp) */
+       .uleb128 0x3a   /* (DW_AT_decl_file) */
+       .uleb128 0x21   /* (DW_FORM_implicit_const) */
+       .sleb128 1      /* (main.c) */
+       .uleb128 0x3b   /* (DW_AT_decl_line) */
+       .uleb128 0xb    /* (DW_FORM_data1) */
+       .uleb128 0x49   /* (DW_AT_type) */
+       .uleb128 0x13   /* (DW_FORM_ref4) */
+       .uleb128 0x3f   /* (DW_AT_external) */
+       .uleb128 0x19   /* (DW_FORM_flag_present) */
+       .uleb128 0x2    /* (DW_AT_location) */
+       .uleb128 0x18   /* (DW_FORM_exprloc) */
+       .byte   0
+       .byte   0
+       .uleb128 0x3    /* (abbrev code) */
+       .uleb128 0x11   /* (TAG: DW_TAG_compile_unit) */
+       .byte   0x1     /* DW_children_yes */
+       .uleb128 0x25   /* (DW_AT_producer) */
+       .uleb128 0xe    /* (DW_FORM_strp) */
+       .uleb128 0x13   /* (DW_AT_language) */
+       .uleb128 0xb    /* (DW_FORM_data1) */
+       .uleb128 0x3    /* (DW_AT_name) */
+       .uleb128 0x1f   /* (DW_FORM_line_strp) */
+       .uleb128 0x1b   /* (DW_AT_comp_dir) */
+       .uleb128 0x1f   /* (DW_FORM_line_strp) */
+       .uleb128 0x55   /* (DW_AT_ranges) */
+       .uleb128 0x17   /* (DW_FORM_sec_offset) */
+       .uleb128 0x11   /* (DW_AT_low_pc) */
+       .uleb128 0x1    /* (DW_FORM_addr) */
+       .uleb128 0x10   /* (DW_AT_stmt_list) */
+       .uleb128 0x17   /* (DW_FORM_sec_offset) */
+       .byte   0
+       .byte   0
+       .byte   0
+       .section        .debug_line,"",%progbits
+.Ldebug_line0:
+       .4byte  .LELT0-.LSLT0   /* Length of Source Line Info */
+.LSLT0:
+       .2byte  0x5     /* DWARF Version */
+       .byte   0x8     /* Address Size */
+       .byte   0       /* Segment Size */
+       .4byte  .LASF0  /* File Entry: 0: "main.c" */
+       .byte   0
+.LELT0:
+       .section        .debug_str,"MS",%progbits,1
+.LASF22:
+       .asciz  "main"
+.LASF16:
+       .asciz  "xvar"
+.LASF21:
+       .asciz  "GNU C11 7.0.1 20170218 (experimental) -mtune=generic -march=x86-64 -gdwarf-5 -O2"
+.LASF17:
+       .asciz  "yvar"
+.LASF7:
+       .asciz  "short int"
+       .section        .debug_line_str,"MS",%progbits,1
+.LASF1:
+       .asciz  ""
+.LASF25:
+       .asciz  ""
+.LASF0:
+       .asciz  "main.c"
diff --git a/binutils/testsuite/binutils-all/dw5-op.W b/binutils/testsuite/binutils-all/dw5-op.W
new file mode 100644
index 00000000000..6acb9dbd9b8
--- /dev/null
+++ b/binutils/testsuite/binutils-all/dw5-op.W
@@ -0,0 +1,35 @@
+Contents of the .debug_info section:
+
+  Compilation Unit @ offset 0x0:
+   Length:        0x4e \(32-bit\)
+   Version:       5
+   Unit Type:     DW_UT_compile \(1\)
+   Abbrev Offset: 0x0
+   Pointer Size:  8
+ <0><c>: Abbrev Number: 3 \(DW_TAG_compile_unit\)
+    <d>   DW_AT_producer    : \(indirect string, offset: 0xa\): GNU C11 7.0.1 20170218 \(experimental\) -mtune=generic -march=x86-64 -gdwarf-5 -O2
+    <11>   DW_AT_language    : 29      \(C11\)
+    <12>   DW_AT_name        : \(indirect line string, offset: 0x2\): main.c
+    <16>   DW_AT_comp_dir    : \(indirect line string, offset: 0x0\):
+    <1a>   DW_AT_ranges      : 0x0
+    <1e>   DW_AT_low_pc      : 0x0
+    <26>   DW_AT_stmt_list   : 0x0
+ <1><2a>: Abbrev Number: 1 \(DW_TAG_base_type\)
+    <2b>   DW_AT_byte_size   : 4
+    <2c>   DW_AT_encoding    : 5       \(signed\)
+    <2d>   DW_AT_name        : \(indirect string, offset: 0x0\): main
+ <1><31>: Abbrev Number: 2 \(DW_TAG_variable\)
+    <32>   DW_AT_name        : \(indirect string, offset: 0x5\): xvar
+    <36>   DW_AT_decl_file   : 1
+    <36>   DW_AT_decl_line   : 2
+    <37>   DW_AT_type        : <0x2a>
+    <3b>   DW_AT_external    : 1
+    <3b>   DW_AT_location    : 9 byte block: 3 34 12 0 0 0 0 0 0       \(DW_OP_addr: 1234\)
+ <1><45>: Abbrev Number: 2 \(DW_TAG_variable\)
+    <46>   DW_AT_name        : \(indirect string, offset: 0x5b\): yvar
+    <4a>   DW_AT_decl_file   : 1
+    <4a>   DW_AT_decl_line   : 3
+    <4b>   DW_AT_type        : <0x2a>
+    <4f>   DW_AT_external    : 1
+    <4f>   DW_AT_location    : \(DW_OP_addrx <0x0>\)
+
diff --git a/binutils/testsuite/binutils-all/objdump.exp b/binutils/testsuite/binutils-all/objdump.exp
index 0f160ae5a06..8df09533f7a 100644
--- a/binutils/testsuite/binutils-all/objdump.exp
+++ b/binutils/testsuite/binutils-all/objdump.exp
@@ -513,6 +513,47 @@ if { ![is_elf_format] } then {
     }
 }

+# Test objdump -Wi on a file containing dwarf-5 encodings information.
+
+if { ![is_elf_format] } then {
+    unsupported "objdump DW_OP_* test"
+} elseif { ![binutils_assemble $srcdir/$subdir/dw5-op.S tmpdir/dw5-op.${obj}] } then {
+    fail "objdump DW_OP_* test"
+} else {
+    if [is_remote host] {
+       set op_testfile [remote_download host tmpdir/dw5-op.${obj}]
+    } else {
+       set op_testfile tmpdir/dw5-op.${obj}
+    }
+
+    set got [remote_exec host "$OBJDUMP $OBJDUMPFLAGS -Wi $op_testfile" "" "/dev/null" "objdump.out"]
+
+    if { [lindex $got 0] != 0 || ![string match "" [lindex $got 1]] } then {
+       fail "objdump -Wi (reason: unexpected output)"
+       send_log $got
+       send_log "\n"
+    }
+
+    set got [remote_exec host "tail -n +4 objdump.out" "" "/dev/null" "objdump.out"]
+    set output [remote_upload host objdump.out]
+
+    if ![file size $output] then {
+       # If the output file is empty, then this target does not
+       # generate dwarf2 output.  This is not a failure.
+       verbose "No output from 'objdump -Wi'"
+       untested "objdump -Wi"
+       return
+    }
+
+    if { [regexp_diff objdump.out $srcdir/$subdir/dw5-op.W] } then {
+       fail "objdump -Wi for DW_OP_*"
+    } else {
+       pass "objdump -Wi for DW_OP_*"
+    }
+
+    file_on_host delete $output
+}
+
 proc test_build_id_debuglink {} {
     global srcdir
     global subdir
@@ -522,7 +563,7 @@ proc test_build_id_debuglink {} {
     global OBJDUMP
     global CFLAGS_FOR_TARGET
     global exe
-
+
     set test "build-id-debuglink"

     # Use a fixed build-id.
diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp
index 8ff756e506f..96b59f958ba 100644
--- a/binutils/testsuite/binutils-all/readelf.exp
+++ b/binutils/testsuite/binutils-all/readelf.exp
@@ -596,3 +596,22 @@ if ![is_remote host] {
        readelf_test {--debug-dump=macro -wN} $tempfile pr26112.r
     }
 }
+
+# Check dwarf-5 support for DW_OP_addrx.
+if {![binutils_assemble_flags $srcdir/$subdir/dw5-op.S tmpdir/dw5-op.o $hpux]} then {
+    unsupported "readelf -wi dw5-op (failed to assemble)"
+} else {
+
+# Download it.
+if ![is_remote host] {
+    set tempfile tmpdir/dw5-op.o
+} else {
+    set tempfile [remote_download host tmpdir/dw5-op.o]
+}
+
+# First, determine the size, so specific output matchers can be used.
+readelf_find_size $tempfile 2
+
+# Make sure that readelf can decode the contents.
+readelf_test -wi $tempfile dw5-op.W
+}
--