[AMD Official Use Only - General]

Hi,

Thanks for the review.

>To be honest I don't think it's helpful to put these all in a single patch. They're not directly related, and if I'm getting it right the code changes are also independent of one another. This would also allow to approve one part while another still needs clarification or refinement.

I am now sharing these issues as separate patches as you suggested and this is the first one which fixes issues related to .debug_rnglists section dump involving DW_RLE_base_addressx, DW_RLE_startx_endx, DW_RLE_startx_length items. DW_AT_rnglists_base is read and used. I will share the future patches which may be depending on these fixes.

Patch is inlined below and also attached with this email.

Main thing was while referring to fetch_indexed_addr() for these range items, proper entry in .debug_addr section was not accessed and its fixed now.

Sample output (Also I compared the binutils output with llvm-dwarfdump as a cross verification):

i. readelf output without fix: 
Contents of the .debug_rnglists section:

  Length:          0x1b
  DWARF version:   5
  Address size:    8
  Segment size:    0
  Offset entries:  1

   Offsets starting at 0xc:
    [     0] 0x4

    Offset   Begin    End
    00000004 0400000001000800 (base address)
    0000000d <End of list>

ii. with fix:
Contents of the .debug_rnglists section:

  Length:          0x1b
  DWARF version:   5
  Address size:    8
  Segment size:    0
  Offset entries:  1

   Offsets starting at 0xc:
    [     0] 0x4

    Offset   Begin    End
    00000004 0000000000000000 (base address index) 0000000000000000 (base address)
    00000006 0000000000000000 000000000000001d
    00000009 0000000000000030 0000000000000031
    0000000c 0000000000000040 0000000000000051
    0000000f 0000000000000000 0000000000000001
    00000012 <End of list>

Patch inlined:

From 994744e7a2560b12a842d6c5112c0d935befce05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= <Bhuvanendra.KumarN@amd.com>
Date: Wed, 8 Jun 2022 14:31:53 +0530
Subject: [PATCH] [PATCH] .debug_rnglists section dump for single CU (dwraf-5).

For clang compiled objects with dwarf-5, few issues are fixed related to
.debug_rnglists section dump involving DW_RLE_base_addressx, DW_RLE_startx_endx,
DW_RLE_startx_length items and read DW_AT_rnglists_base.
These fixes are applicable for single CU.

	* dwarf.h (struct debug_info): Add rnglists_base field.
	* dwarf.c (read_and_display_attr_value): Read attribute DW_AT_rnglists_base.
	(display_debug_rnglists_list): While handling DW_RLE_base_addressx,
  	DW_RLE_startx_endx, DW_RLE_startx_length items, pass the proper parameter
	value to fetch_indexed_addr(), i.e. fetch the proper entry in .debug_addr section.
	(display_debug_ranges): Add rnglists_base to the .debug_rnglists base address.
	(load_separate_debug_files): Load .debug_addr section, if exists.
---
 binutils/dwarf.c | 45 +++++++++++++++++++++++++++++++++------------
 binutils/dwarf.h |  1 +
 2 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index caa3ce48d00..2fa3fc77468 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -2824,7 +2824,12 @@ read_and_display_attr_value (unsigned long           attribute,
 		  dwarf_vmatoa ("x", debug_info_p->cu_offset));
 	  debug_info_p->loclists_base = uvalue;
 	  break;
-
+	case DW_AT_rnglists_base:
+	  if (debug_info_p->rnglists_base)
+	    warn (_("CU @ 0x%s has multiple rnglists_base values"),
+	          dwarf_vmatoa ("x", debug_info_p->cu_offset));
+	  debug_info_p->rnglists_base = uvalue;
+	  break;
 	case DW_AT_frame_base:
 	  have_frame_base = 1;
 	  /* Fall through.  */
@@ -3315,6 +3320,7 @@ read_and_display_attr_value (unsigned long           attribute,
       /* Fall through.  */
     case DW_AT_location:
     case DW_AT_loclists_base:
+    case DW_AT_rnglists_base:
     case DW_AT_string_length:
     case DW_AT_return_addr:
     case DW_AT_data_member_location:
@@ -3333,8 +3339,10 @@ read_and_display_attr_value (unsigned long           attribute,
       if ((dwarf_version < 4
 	   && (form == DW_FORM_data4 || form == DW_FORM_data8))
 	  || form == DW_FORM_sec_offset
-	  || form == DW_FORM_loclistx)
-	printf (_(" (location list)"));
+	  || form == DW_FORM_loclistx) {
+        if (attribute != DW_AT_rnglists_base)
+          printf (_(" (location list)"));
+      }
       /* Fall through.  */
     case DW_AT_allocated:
     case DW_AT_associated:
@@ -3821,6 +3829,7 @@ process_debug_info (struct dwarf_section * section,
 	  debug_information [unit].range_lists = NULL;
 	  debug_information [unit].max_range_lists= 0;
 	  debug_information [unit].num_range_lists = 0;
+	  debug_information [unit].rnglists_base = 0;
 	}
 
       if (!do_loc && dwarf_start_die == 0)
@@ -7945,9 +7954,15 @@ display_debug_rnglists_list (unsigned char * start,
 			     unsigned char * finish,
 			     unsigned int    pointer_size,
 			     dwarf_vma       offset,
-			     dwarf_vma       base_address)
+			     dwarf_vma       base_address,
+			     unsigned int    offset_size)
 {
   unsigned char *next = start;
+  unsigned int debug_addr_section_hdr_len;
+  if (offset_size == 4)
+    debug_addr_section_hdr_len = 8;
+  else
+    debug_addr_section_hdr_len = 16;
 
   while (1)
     {
@@ -7967,7 +7982,6 @@ display_debug_rnglists_list (unsigned char * start,
       print_dwarf_vma (off, 4);
 
       SAFE_BYTE_GET_AND_INC (rlet, start, 1, finish);
-
       switch (rlet)
 	{
 	case DW_RLE_end_of_list:
@@ -7977,20 +7991,24 @@ display_debug_rnglists_list (unsigned char * start,
 	  READ_ULEB (base_address, start, finish);
 	  print_dwarf_vma (base_address, pointer_size);
 	  printf (_("(base address index) "));
-	  base_address = fetch_indexed_addr (base_address, pointer_size);
+	  base_address = fetch_indexed_addr (base_address * pointer_size
+			                     + debug_addr_section_hdr_len, pointer_size);
 	  print_dwarf_vma (base_address, pointer_size);
 	  printf (_("(base address)\n"));
 	  break;
 	case DW_RLE_startx_endx:
 	  READ_ULEB (begin, start, finish);
 	  READ_ULEB (end, start, finish);
-	  begin = fetch_indexed_addr (begin, pointer_size);
-	  end   = fetch_indexed_addr (begin, pointer_size);
+	  begin = fetch_indexed_addr (begin * pointer_size
+			              + debug_addr_section_hdr_len, pointer_size);
+	  end   = fetch_indexed_addr (begin * pointer_size
+			              + debug_addr_section_hdr_len, pointer_size);
 	  break;
 	case DW_RLE_startx_length:
 	  READ_ULEB (begin, start, finish);
 	  READ_ULEB (length, start, finish);
-	  begin = fetch_indexed_addr (begin, pointer_size);
+	  begin = fetch_indexed_addr (begin * pointer_size
+			              + debug_addr_section_hdr_len, pointer_size);
 	  end = begin + length;
 	  break;
 	case DW_RLE_offset_pair:
@@ -8056,6 +8074,7 @@ display_debug_ranges (struct dwarf_section *section,
   /* Initialize it due to a false compiler warning.  */
   unsigned char         address_size = 0;
   dwarf_vma             last_offset = 0;
+  unsigned int          offset_size = 0;
 
   if (bytes == 0)
     {
@@ -8069,7 +8088,7 @@ display_debug_ranges (struct dwarf_section *section,
     {
       dwarf_vma initial_length;
       unsigned char segment_selector_size;
-      unsigned int offset_size, offset_entry_count;
+      unsigned int offset_entry_count;
       unsigned short version;
 
       /* Get and check the length of the block.  */
@@ -8243,7 +8262,7 @@ display_debug_ranges (struct dwarf_section *section,
 		(unsigned long) offset, i);
 	  continue;
 	}
-      next = section_begin + offset;
+      next = section_begin + offset + debug_info_p->rnglists_base;
 
       /* If multiple DWARF entities reference the same range then we will
          have multiple entries in the `range_entries' list for the same
@@ -8275,7 +8294,7 @@ display_debug_ranges (struct dwarf_section *section,
 
       if (is_rnglists)
 	display_debug_rnglists_list
-	  (start, finish, pointer_size, offset, base_address);
+	  (start, finish, pointer_size, offset, base_address, offset_size);
       else
 	display_debug_ranges_list
 	  (start, finish, pointer_size, offset, base_address);
@@ -11889,6 +11908,8 @@ load_separate_debug_files (void * file, const char * filename)
       && load_debug_section (abbrev, file)
       && load_debug_section (info, file))
     {
+      /* Load .debug_addr section, if exists.  */
+      load_debug_section (debug_addr, file);
       free_dwo_info ();
 
       if (process_debug_info (& debug_displays[info].section, file, abbrev,
diff --git a/binutils/dwarf.h b/binutils/dwarf.h
index 040e674c6ce..8a89c08e7c2 100644
--- a/binutils/dwarf.h
+++ b/binutils/dwarf.h
@@ -192,6 +192,7 @@ typedef struct
   dwarf_vma *    range_lists;
   unsigned int   num_range_lists;
   unsigned int   max_range_lists;
+  dwarf_vma      rnglists_base;
 }
 debug_info;
 
-- 
2.17.1


-----Original Message-----
From: Jan Beulich <jbeulich@suse.com> 
Sent: Friday, June 3, 2022 4:16 PM
To: Kumar N, Bhuvanendra <Bhuvanendra.KumarN@amd.com>
Cc: George, Jini Susan <JiniSusan.George@amd.com>; Natarajan, Kavitha <Kavitha.Natarajan@amd.com>; binutils@sourceware.org
Subject: Re: [PATCH] Binutils support for dwarf-5 (location and range lists related)

[CAUTION: External Email]

On 31.05.2022 13:06, Kumar N, Bhuvanendra via Binutils wrote:
> PATCH 1/2 inlined :
>
> From 96ce32f762803a7d74ca30c3930f1679afc0100c Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=E2=80=9Cbhkumarn=E2=80=9D?= 
> Bhuvanendra.KumarN@amd.com<mailto:Bhuvanendra.KumarN@amd.com>
> Date: Tue, 31 May 2022 14:07:17 +0530
> Subject: [PATCH] [PATCH 1/2] Binutils support for dwarf-5.
>
> For clang compiled with dwarf-5, multiple issues are fixed which are 
> related to .debug_rnglists and .debug_loclists sections and their 
> offset address dump comprising single and multiple CU. There are 2 
> patches and this is patch 1/2.
>
> Issues fixed in patch 1/2 are:
> Issue 1: .debug_rnglists section dump for single CU.
> Issue 2: location list offset address dump under DW_AT_location is corrected.
> Issue 3: range list offset address dump under DW_AT_ranges is corrected.

To be honest I don't think it's helpful to put these all in a single patch. They're not directly related, and if I'm getting it right the code changes are also independent of one another. This would also allow to approve one part while another still needs clarification or refinement.

And then it would also help if you could send multiple patches properly as a series, rather than all in a single email.

> @@ -2788,7 +2794,15 @@ read_and_display_attr_value (unsigned long           attribute,
>         offset = base + uvalue * pointer_size;
> -       if (do_wide)
> +       if (form == DW_FORM_loclistx)
> +         printf (_("%c(index: 0x%s): %s"), delimiter,
> +                 dwarf_vmatoa ("x", uvalue),
> +                 dwarf_vmatoa ("x", debug_info_p->loc_offsets 
> + [uvalue]));

Aren't you risking an oob array access here? And isn't the ordering of elements in loc_offset[] an implementation detail, i.e. using a value fetched from debug info is effectively meaningless when used as an index into the array?

> +       else if (form == DW_FORM_rnglistx)
> +         printf (_("%c(index: 0x%s): %s"), delimiter,
> +                 dwarf_vmatoa ("x", uvalue),
> +                 dwarf_vmatoa ("x", fetch_indexed_value (uvalue, rnglists, 0)));
> +       else if (do_wide)
>          /* We have already displayed the form name.  */
>          printf (_("%c(index: 0x%s): %s"), delimiter,
>               dwarf_vmatoa ("x", uvalue),

For both special cases, how come they don't respect do_wide?

> @@ -7976,9 +7990,6 @@ display_debug_rnglists_list (unsigned char * start,
>      case DW_RLE_base_addressx:
>        READ_ULEB (base_address, start, finish);
>        print_dwarf_vma (base_address, pointer_size);
> -       printf (_("(base address index) "));
> -       base_address = fetch_indexed_addr (base_address, pointer_size);
> -       print_dwarf_vma (base_address, pointer_size);
>        printf (_("(base address)\n"));
>        break;
>      case DW_RLE_startx_endx:
> @@ -7990,7 +8001,6 @@ display_debug_rnglists_list (unsigned char * start,
>      case DW_RLE_startx_length:
>        READ_ULEB (begin, start, finish);
>        READ_ULEB (length, start, finish);
> -       begin = fetch_indexed_addr (begin, pointer_size);
>        end = begin + length;
>        break;
>      case DW_RLE_offset_pair:

I don't see how these two hunks can be correct: You're removing the indirection through .debug_addr. And at the same time you're keeping similar indirection for DW_RLE_startx_endx. Aiui all x-suffixed items are to be dealt with identically.

> @@ -8243,7 +8253,7 @@ display_debug_ranges (struct dwarf_section *section,
>           (unsigned long) offset, i);
>        continue;
>      }
> -      next = section_begin + offset;
> +      next = section_begin + offset + 
> + DEBUG_LOCLISTS_RNGLISTS_SECTION_HEADER_LEN;

I'm afraid I can't figure what 12 bytes you're meaning to skip here.
I also think that _if_ an adjustment was needed here, the preceding
if() likely would also need adjustment, perhaps by way of actually adjusting offset.

Jan