Commit: readelf: Improve display of RELR relocations

Commit: readelf: Improve display of RELR relocations

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 1490 bytes --]

Hi Guys,

  Currently readelf's display of RELR relocations is woefully lacking in
  information.  So I am going to apply the attached patch in order to
  improve this.  For example:

    readelf -r /lib64/libc.so.6

  Before patch:

    Relocation section '.relr.dyn' at offset 0x259c0 contains 32 entries:
      1067 offsets
    00000000001d4ca0
    00000000001d4cb0
    00000000001d4cb8
    00000000001d4cc0
    00000000001d4ce0
    00000000001d4d00
    ...
    
  After patch:
  
    Relocation section '.relr.dyn' at offset 0x259c0 contains 32 entries:
    Index: Entry:           Address relocated Symbolic Address
    0000:  00000000001d4ca0 00000000001d4ca0  __FRAME_END__ + 0x1ddc
    0001:  ffffdff8ee15911d 00000000001d4cb0  __FRAME_END__ + 0x1dec
                            00000000001d4cb8  __FRAME_END__ + 0x1df4
                            00000000001d4cc0  __dso_handle
                            00000000001d4ce0  _nl_C_LC_CTYPE
                            00000000001d4d00  _nl_C_LC_CTYPE + 0x20
    ...

  In particular this new format shows the actual values held in the RELR
  section - allowing a user to potentially spot problems - as well as
  providing an address to symbol mapping for ease in understanding what
  is being relocated.

  The patch also checks for malformed RELR entries (such as an entry
  with a value of just 1).

  The patch includes a new binutils test and updates to linker tests
  that were checking the RELR relocations.
  
Cheers
  Nick


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: readelf.relr.patch --]
[-- Type: text/x-patch, Size: 25211 bytes --]

diff --git a/binutils/NEWS b/binutils/NEWS
index be744e3b2c4..5c31953575a 100644
--- a/binutils/NEWS
+++ b/binutils/NEWS
@@ -1,5 +1,7 @@
 -*- text -*-
 
+* Readelf now displays RELR relocations in full detail.
+
 * Readelf now has a -j/--display-section option which takes the name or index
   of a section and displays its contents according to its type.  The option can
   be used multiple times on the command line to display multiple sections.
diff --git a/binutils/readelf.c b/binutils/readelf.c
index fa0de3a7e0d..fcf95ee3047 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -338,6 +338,7 @@ typedef enum print_mode
   PREFIX_HEX_5,
   FULL_HEX,
   LONG_HEX,
+  ZERO_HEX,
   OCTAL,
   OCTAL_5
 }
@@ -580,6 +581,11 @@ print_vma (uint64_t vma, print_mode mode)
 	return nc + printf ("%16.16" PRIx64, vma);
       return nc + printf ("%8.8" PRIx64, vma);
 
+    case ZERO_HEX:
+      if (is_32bit_elf)
+	return printf ("%08" PRIx64, vma);
+      return printf ("%016" PRIx64, vma);
+      
     case DEC_5:
       if (vma <= 99999)
 	return printf ("%5" PRId64, vma);
@@ -1109,6 +1115,26 @@ find_section_by_type (Filedata * filedata, unsigned int type)
   return NULL;
 }
 
+static Elf_Internal_Shdr *
+find_section_by_name (Filedata * filedata, const char * name)
+{
+  unsigned int i;
+
+  if (filedata->section_headers == NULL || filedata->string_table_length == 0)
+    return NULL;
+
+  for (i = 0; i < filedata->file_header.e_shnum; i++)
+    {
+      Elf_Internal_Shdr *sec = filedata->section_headers + i;
+
+      if (sec->sh_name < filedata->string_table_length
+	  && streq (name, filedata->string_table + sec->sh_name))
+	return sec;
+    }
+
+  return NULL;
+}
+
 /* Return a pointer to section NAME, or NULL if no such section exists,
    restricted to the list of sections given in SET.  */
 
@@ -1467,76 +1493,6 @@ slurp_rel_relocs (Filedata *filedata,
   return true;
 }
 
-static bool
-slurp_relr_relocs (Filedata *filedata,
-		   uint64_t relr_offset,
-		   uint64_t relr_size,
-		   uint64_t **relrsp,
-		   uint64_t *nrelrsp)
-{
-  void *relrs;
-  size_t size = 0, nentries, i;
-  uint64_t base = 0, addr, entry;
-
-  relrs = get_data (NULL, filedata, relr_offset, 1, relr_size,
-		    _("RELR relocation data"));
-  if (!relrs)
-    return false;
-
-  if (is_32bit_elf)
-    nentries = relr_size / sizeof (Elf32_External_Relr);
-  else
-    nentries = relr_size / sizeof (Elf64_External_Relr);
-  for (i = 0; i < nentries; i++)
-    {
-      if (is_32bit_elf)
-	entry = BYTE_GET (((Elf32_External_Relr *)relrs)[i].r_data);
-      else
-	entry = BYTE_GET (((Elf64_External_Relr *)relrs)[i].r_data);
-      if ((entry & 1) == 0)
-	size++;
-      else
-	while ((entry >>= 1) != 0)
-	  if ((entry & 1) == 1)
-	    size++;
-    }
-
-  *relrsp = malloc (size * sizeof (**relrsp));
-  if (*relrsp == NULL)
-    {
-      free (relrs);
-      error (_("out of memory parsing relocs\n"));
-      return false;
-    }
-
-  size = 0;
-  for (i = 0; i < nentries; i++)
-    {
-      const uint64_t entry_bytes = is_32bit_elf ? 4 : 8;
-
-      if (is_32bit_elf)
-	entry = BYTE_GET (((Elf32_External_Relr *)relrs)[i].r_data);
-      else
-	entry = BYTE_GET (((Elf64_External_Relr *)relrs)[i].r_data);
-      if ((entry & 1) == 0)
-	{
-	  (*relrsp)[size++] = entry;
-	  base = entry + entry_bytes;
-	}
-      else
-	{
-	  for (addr = base; (entry >>= 1) != 0; addr += entry_bytes)
-	    if ((entry & 1) != 0)
-	      (*relrsp)[size++] = addr;
-	  base += entry_bytes * (entry_bytes * CHAR_BIT - 1);
-	}
-    }
-
-  *nrelrsp = size;
-  free (relrs);
-  return true;
-}
-
 /* Returns the reloc type extracted from the reloc info field.  */
 
 static unsigned int
@@ -1578,19 +1534,227 @@ uses_msp430x_relocs (Filedata * filedata)
 	|| (filedata->file_header.e_ident[EI_OSABI] == ELFOSABI_NONE));
 }
 
+
+static const char *
+get_symbol_at (Elf_Internal_Sym *  symtab,
+	       uint64_t            nsyms,
+	       char *              strtab,
+	       uint64_t            strtablen,
+	       uint64_t            where,
+	       uint64_t *          offset_return)
+{
+  Elf_Internal_Sym *  beg = symtab;
+  Elf_Internal_Sym *  end = symtab + nsyms;
+  Elf_Internal_Sym *  best = NULL;
+  uint64_t            dist = 0x100000;
+
+  /* FIXME: Since this function is likely to be called repeatedly with
+     slightly increasing addresses each time, we could speed things up by
+     caching the last returned value and starting our search from there.  */
+  while (beg < end)
+    {
+      Elf_Internal_Sym *  sym;
+      uint64_t            value;
+
+      sym = beg + (end - beg) / 2;
+
+      value = sym->st_value;
+
+      if (sym->st_name != 0
+	  && where >= value
+	  && where - value < dist)
+	{
+	  best = sym;
+	  dist = where - value;
+	  if (dist == 0)
+	    break;
+	}
+
+      if (where < value)
+	end = sym;
+      else
+	beg = sym + 1;
+    }
+
+  if (best == NULL)
+    return NULL;
+
+  if (best->st_name >= strtablen)
+    return NULL;
+
+  if (offset_return != NULL)
+    * offset_return = dist;
+
+  return strtab + best->st_name;
+}
+
+static void
+print_relr_addr_and_sym (Elf_Internal_Sym *  symtab,
+			 uint64_t            nsyms,
+			 char *              strtab,
+			 uint64_t            strtablen,
+			 uint64_t            where)
+{
+  const char * symname = NULL;
+  uint64_t     offset = 0;
+
+  print_vma (where, ZERO_HEX);
+  printf ("  ");
+
+  symname = get_symbol_at (symtab, nsyms, strtab, strtablen, where, & offset);
+
+  if (symname == NULL)
+    printf ("<no sym>");
+  else if (offset == 0)
+    print_symbol_name (38, symname);
+  else
+    {
+      print_symbol_name (28, symname);
+      printf (" + ");
+      print_vma (offset, PREFIX_HEX);
+    }
+}
+
+static /* signed */ int
+symcmp (const void *p, const void *q)
+{
+  Elf_Internal_Sym *sp = (Elf_Internal_Sym *) p;
+  Elf_Internal_Sym *sq = (Elf_Internal_Sym *) q;
+
+  return sp->st_value > sq->st_value ? 1 : (sp->st_value < sq->st_value ? -1 : 0);
+}
+
+static bool
+dump_relr_relocations (Filedata *          filedata,
+		       Elf_Internal_Shdr * section,
+		       Elf_Internal_Sym *  symtab,
+		       uint64_t            nsyms,
+		       char *              strtab,
+		       uint64_t            strtablen)
+{
+  uint64_t *  relrs;
+  uint64_t    nentries, i;
+  uint64_t    relr_size = section->sh_size;
+  int         relr_entsize = section->sh_entsize;
+  uint64_t    relr_offset = section->sh_offset;
+  uint64_t    where = 0;
+  int         num_bits_in_entry;
+
+  relrs = get_data (NULL, filedata, relr_offset, 1, relr_size, _("RELR relocation data"));
+  if (relrs == NULL)
+    return false;
+
+  if (relr_entsize == 0)
+    relr_entsize = is_32bit_elf ? 4 : 8;
+
+  nentries = relr_size / relr_entsize;
+
+  if (relr_entsize == sizeof (Elf32_External_Relr))
+    num_bits_in_entry = 31;
+  else if (relr_entsize == sizeof (Elf64_External_Relr))
+    num_bits_in_entry = 63;
+  else
+    {
+      warn (_("Unexpected entsize for RELR section\n"));
+      return false;
+    }
+  
+  /* Symbol tables are not sorted on address, but we want a quick lookup
+     for the symbol associated with each address computed below, so sort
+     the table now.  FIXME: This assumes that the symbol table will not
+     be used later on for some other purpose.  */
+  qsort (symtab, nsyms, sizeof (Elf_Internal_Sym), symcmp);
+
+  if (do_wide)
+    {
+      if (relr_entsize == 4)
+	printf (_("Index: Entry:   Address  Symbolic Address     Notes\n"));
+      else
+	printf (_("Index: Entry:           Address relocated Symbolic Address        Notes\n"));
+    }
+  else
+    {
+      if (relr_entsize == 4)
+	printf (_("Index: Entry:   Address  Symbolic Address\n"));
+      else
+	printf (_("Index: Entry:           Address relocated Symbolic Address\n"));
+    }
+
+  for (i = 0; i < nentries; i++)
+    {
+      uint64_t entry;
+
+      if (relr_entsize == 4)
+	entry = BYTE_GET (((Elf32_External_Relr *)relrs)[i].r_data);
+      else
+	entry = BYTE_GET (((Elf64_External_Relr *)relrs)[i].r_data);
+
+      /* We assume that there will never be more than 9999 entries.  */
+      printf (_("%04u:  "), (unsigned int) i);
+      print_vma (entry, ZERO_HEX);
+      printf (" ");
+
+      if ((entry & 1) == 0)
+	{
+	  where = entry;
+	  print_relr_addr_and_sym (symtab, nsyms, strtab, strtablen, where);
+	  if (do_wide)
+	    printf (_(" (new starting address)"));
+	  printf ("\n");
+	  where += relr_entsize;
+	}
+      else
+	{
+	  bool first = true;
+	  int j;
+
+	  /* The least significant bit is ignored.  */
+	  if (entry == 1)
+	    warn (_("Malformed RELR bitmap - no significant bits are set\n"));
+	  else if (i == 0)
+	    warn (_("Unusual RELR bitmap - no previous entry to set the base address\n"));
+
+	  for (j = 0; entry >>= 1; j++)
+	    if ((entry & 1) == 1)
+	      {
+		uint64_t addr = where + (j * relr_entsize);
+		
+		if (first)
+		  {
+		    print_relr_addr_and_sym (symtab, nsyms, strtab, strtablen, addr);
+		    if (do_wide)
+		      printf (_(" (start of bitmap)"));
+		    first = false;
+		  }
+		else
+		  {
+		    printf (_("\n%*s "), relr_entsize == 4 ? 15 : 23, " ");
+		    print_relr_addr_and_sym (symtab, nsyms, strtab, strtablen, addr);
+		  }
+	      }
+
+	  printf ("\n");
+	  where += num_bits_in_entry * relr_entsize;
+	}
+    }
+
+  free (relrs);
+  return true;
+}
+		       
 /* Display the contents of the relocation data found at the specified
    offset.  */
 
 static bool
-dump_relocations (Filedata *filedata,
-		  uint64_t rel_offset,
-		  uint64_t rel_size,
-		  Elf_Internal_Sym *symtab,
-		  uint64_t nsyms,
-		  char *strtab,
-		  uint64_t strtablen,
-		  relocation_type rel_type,
-		  bool is_dynsym)
+dump_relocations (Filedata *          filedata,
+		  uint64_t            rel_offset,
+		  uint64_t            rel_size,
+		  Elf_Internal_Sym *  symtab,
+		  uint64_t            nsyms,
+		  char *              strtab,
+		  uint64_t            strtablen,
+		  relocation_type     rel_type,
+		  bool                is_dynsym)
 {
   size_t i;
   Elf_Internal_Rela * rels;
@@ -1611,21 +1775,8 @@ dump_relocations (Filedata *filedata,
     }
   else if (rel_type == reltype_relr)
     {
-      uint64_t * relrs;
-      const char *format
-	= is_32bit_elf ? "%08" PRIx64 "\n" : "%016" PRIx64 "\n";
-
-      if (!slurp_relr_relocs (filedata, rel_offset, rel_size, &relrs,
-			      &rel_size))
-	return false;
-
-      printf (ngettext ("  %" PRIu64 " offset\n",
-			"  %" PRIu64 " offsets\n", rel_size),
-	      rel_size);
-      for (i = 0; i < rel_size; i++)
-	printf (format, relrs[i]);
-      free (relrs);
-      return true;
+      /* This should have been handled by display_relocations().  */
+      return false;
     }
 
   if (is_32bit_elf)
@@ -7996,6 +8147,7 @@ process_section_headers (Filedata * filedata)
       switch (section->sh_type)
 	{
 	case SHT_REL:
+	case SHT_RELR:
 	case SHT_RELA:
 	  if (section->sh_link == 0
 	      && (filedata->file_header.e_type == ET_EXEC
@@ -8349,9 +8501,12 @@ process_section_headers (Filedata * filedata)
 }
 
 static bool
-get_symtab (Filedata *filedata, Elf_Internal_Shdr *symsec,
-	    Elf_Internal_Sym **symtab, uint64_t *nsyms,
-	    char **strtab, uint64_t *strtablen)
+get_symtab (Filedata *           filedata,
+	    Elf_Internal_Shdr *  symsec,
+	    Elf_Internal_Sym **  symtab,
+	    uint64_t *           nsyms,
+	    char **              strtab,
+	    uint64_t *           strtablen)
 {
   *strtab = NULL;
   *strtablen = 0;
@@ -8912,9 +9067,9 @@ static bool
 display_relocations (Elf_Internal_Shdr *  section,
 		     Filedata *           filedata)
 {
-  if (section->sh_type != SHT_RELA
-      && section->sh_type != SHT_REL
-      && section->sh_type != SHT_RELR)
+  relocation_type rel_type = rel_type_from_sh_type (section->sh_type);
+
+  if (rel_type == reltype_unknown)
     return false;
 
   uint64_t rel_size = section->sh_size;
@@ -8943,33 +9098,43 @@ display_relocations (Elf_Internal_Shdr *  section,
 		    num_rela),
 	  rel_offset, num_rela);
 
-  relocation_type rel_type = rel_type_from_sh_type (section->sh_type);
+  Elf_Internal_Shdr * symsec;
+  Elf_Internal_Sym *  symtab = NULL;
+  uint64_t            nsyms = 0;
+  uint64_t            strtablen = 0;
+  char *              strtab = NULL;
 
   if (section->sh_link == 0
       || section->sh_link >= filedata->file_header.e_shnum)
-    /* Symbol data not available.  */
-    return dump_relocations (filedata, rel_offset, rel_size,
-			     NULL, 0, NULL, 0, rel_type,
-			     false /* is_dynamic */);
-    
-  Elf_Internal_Shdr * symsec = filedata->section_headers + section->sh_link;
-
-  if (symsec->sh_type != SHT_SYMTAB
-      && symsec->sh_type != SHT_DYNSYM)
-    return false;
+    {
+      /* Symbol data not available.
+	 This can happen, especially with RELR relocs.
+	 See if there is a .symtab section present.
+	 If so then use it.  */
+      symsec = find_section_by_name (filedata, ".symtab");
+    }
+  else
+    {
+      symsec = filedata->section_headers + section->sh_link;
 
-  Elf_Internal_Sym *  symtab;
-  uint64_t            nsyms;
-  uint64_t            strtablen = 0;
-  char *              strtab = NULL;
+      if (symsec->sh_type != SHT_SYMTAB
+	  && symsec->sh_type != SHT_DYNSYM)
+	return false;
+    }
 
-  if (!get_symtab (filedata, symsec, &symtab, &nsyms, &strtab, &strtablen))
+  if (symsec != NULL
+      && !get_symtab (filedata, symsec, &symtab, &nsyms, &strtab, &strtablen))
     return false;
 
-  bool res = dump_relocations (filedata, rel_offset, rel_size,
-			       symtab, nsyms, strtab, strtablen,
-			       rel_type,
-			       symsec->sh_type == SHT_DYNSYM);
+  bool res;
+
+  if (rel_type == reltype_relr)
+    res = dump_relr_relocations (filedata, section, symtab, nsyms, strtab, strtablen);
+  else
+    res = dump_relocations (filedata, rel_offset, rel_size,
+			    symtab, nsyms, strtab, strtablen,
+			    rel_type,
+			    symsec == NULL ? false : symsec->sh_type == SHT_DYNSYM);
   free (strtab);
   free (symtab);
 
@@ -9173,15 +9338,6 @@ find_symbol_for_address (Filedata *filedata,
   *offset = addr.offset;
 }
 
-static /* signed */ int
-symcmp (const void *p, const void *q)
-{
-  Elf_Internal_Sym *sp = (Elf_Internal_Sym *) p;
-  Elf_Internal_Sym *sq = (Elf_Internal_Sym *) q;
-
-  return sp->st_value > sq->st_value ? 1 : (sp->st_value < sq->st_value ? -1 : 0);
-}
-
 /* Process the unwind section.  */
 
 #include "unwind-ia64.h"
diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp
index b91134b4326..09ed75f8761 100644
--- a/binutils/testsuite/binutils-all/readelf.exp
+++ b/binutils/testsuite/binutils-all/readelf.exp
@@ -642,3 +642,70 @@ readelf_test {-j .rela.debug_info --display-section=.rel.debug_info} $tempfile d
 readelf_test --display-section=0 $tempfile display-section.0
 
 
+# Test that RELR relocations are display correctly.
+proc readelf_relr_test {} {
+    global srcdir
+    global subdir
+    global READELF
+    global READELFFLAGS
+    
+    set testname "readelf -r (RELR)"
+
+    # Assemble the RELR test file (using magic to work for both 32-bit and
+    # 64-bit targets).
+    if {![binutils_assemble $srcdir/$subdir/relr.s tmpdir/relr.o]} then {
+      unsupported "$testname: failed to assemble RELR test file"
+      return
+    }
+
+    # Download it.
+    set tempfile [remote_download host tmpdir/relr.o]
+
+    # Run "readelf -r" on it.
+    set got [remote_exec host "$READELF $READELFFLAGS -r $tempfile" "" "/dev/null" "readelf.out"]
+    set got [lindex $got 1]
+
+    # Upload the results.
+    set output [remote_upload host readelf.out]
+
+    # Check for something going wrong.
+    if ![string match "" $got] then {
+	fail "$testname: unexpected output"
+	send_log $got
+	send_log "\n"
+	return
+    }
+
+    # Search for strings that should be in the output.
+    # There will also be these strings:
+    #   readelf: Error: Section 4 has invalid sh_entsize of 0
+    #   readelf: Error: (Using the expected size of 8 for the rest of this dump)
+    # But we ignore them...
+
+    set sought {
+	"0000:  0+01000 0+01000 .*"
+	"0001:  0+00003 0+0100. .*"
+    }
+
+    foreach looked_for $sought {
+	set lines [grep $output $looked_for]
+	if ![llength $lines] then {
+	    fail "$testname: missing: $looked_for"
+	    send_log readelf.out
+	    return
+	}
+    }
+
+    file_on_host delete $tempfile
+    file_on_host delete $output
+
+    # All done.
+    pass "$testname"
+}
+
+# The AVR, H8300, IP2K and Z80 targets' .dc.a pseudo-op creates a
+# 16-bit entry rather than a 32-bit entry.  Thus creating an
+# invalid RELR relocation.
+setup_xfail "avr-*-*" "h8300-*-*" "ip2k-*-*" "z80-*-*"
+
+readelf_relr_test
diff --git a/ld/testsuite/ld-elf/dt-relr-2b.d b/ld/testsuite/ld-elf/dt-relr-2b.d
index b1391566a13..48a3eb7f15c 100644
--- a/ld/testsuite/ld-elf/dt-relr-2b.d
+++ b/ld/testsuite/ld-elf/dt-relr-2b.d
@@ -13,5 +13,9 @@ Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_.*_(RELATIVE|UADDR.*) .*
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
-  4 offsets
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +data \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+ \(start of bitmap\)
+ +[0-9a-f]+ +data \+ 0x[0-9a-f]+
+ +[0-9a-f]+ +data \+ 0x[0-9a-f]+
 #pass
diff --git a/ld/testsuite/ld-elf/dt-relr-2c.d b/ld/testsuite/ld-elf/dt-relr-2c.d
index c285e8707d7..7f6383b0184 100644
--- a/ld/testsuite/ld-elf/dt-relr-2c.d
+++ b/ld/testsuite/ld-elf/dt-relr-2c.d
@@ -13,5 +13,8 @@ Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
 [0-9a-f]+ +[0-9a-f]+ +R_.*_(RELATIVE|UADDR.*) .*
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
-  3 offsets
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+ +[0-9a-f]+ +.*
 #pass
diff --git a/ld/testsuite/ld-elf/dt-relr-2d.d b/ld/testsuite/ld-elf/dt-relr-2d.d
index 7fd3046a1cf..f1184fefeb2 100644
--- a/ld/testsuite/ld-elf/dt-relr-2d.d
+++ b/ld/testsuite/ld-elf/dt-relr-2d.d
@@ -13,5 +13,9 @@ Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_.*_(RELATIVE|UADDR.*) .*
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
-  4 offsets
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +data \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+ \(start of bitmap\)
+ +[0-9a-f]+ +data \+ 0x[0-9a-f]+
+ +[0-9a-f]+ +data \+ 0x[0-9a-f]+
 #pass
diff --git a/ld/testsuite/ld-elf/dt-relr-2e.d b/ld/testsuite/ld-elf/dt-relr-2e.d
index cdff8465a57..eddb5e3d14e 100644
--- a/ld/testsuite/ld-elf/dt-relr-2e.d
+++ b/ld/testsuite/ld-elf/dt-relr-2e.d
@@ -13,5 +13,9 @@ Relocation section '\.rel(a|)\.data' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_.*_(RELATIVE|UADDR.*) .*
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
-  4 offsets
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +data \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+ \(start of bitmap\)
+ +[0-9a-f]+ +data \+ 0x[0-9a-f]+
+ +[0-9a-f]+ +data \+ 0x[0-9a-f]+
 #pass
diff --git a/ld/testsuite/ld-elf/dt-relr-2i.d b/ld/testsuite/ld-elf/dt-relr-2i.d
index ed0ef9ccded..55e8c256b92 100644
--- a/ld/testsuite/ld-elf/dt-relr-2i.d
+++ b/ld/testsuite/ld-elf/dt-relr-2i.d
@@ -13,5 +13,9 @@ Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_.*_(RELATIVE|UADDR.*) .*
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
-  4 offsets
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +data \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+ \(start of bitmap\)
+ +[0-9a-f]+ +data \+ 0x[0-9a-f]+
+ +[0-9a-f]+ +data \+ 0x[0-9a-f]+
 #pass
diff --git a/ld/testsuite/ld-i386/dt-relr-1a.d b/ld/testsuite/ld-i386/dt-relr-1a.d
index cbb8ab6158b..3944911e4f7 100644
--- a/ld/testsuite/ld-i386/dt-relr-1a.d
+++ b/ld/testsuite/ld-i386/dt-relr-1a.d
@@ -14,11 +14,10 @@ Relocation section '\.rel\.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_386_JUMP_SLOT +0+ +func1
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
- +3 offsets
-[a-f0-9]+
-[a-f0-9]+
-[a-f0-9]+
-
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+ +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
    Num:    Value  Size Type    Bind   Vis      Ndx Name
diff --git a/ld/testsuite/ld-i386/dt-relr-1b.d b/ld/testsuite/ld-i386/dt-relr-1b.d
index 3dd988158c6..947d8354b23 100644
--- a/ld/testsuite/ld-i386/dt-relr-1b.d
+++ b/ld/testsuite/ld-i386/dt-relr-1b.d
@@ -17,11 +17,10 @@ Relocation section '\.rel\.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_386_JUMP_SLOT +0+ +func1
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
- +3 offsets
-[a-f0-9]+
-[a-f0-9]+
-[a-f0-9]+
-
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+ +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
    Num:    Value  Size Type    Bind   Vis      Ndx Name
diff --git a/ld/testsuite/ld-powerpc/abs-pie-relr.r b/ld/testsuite/ld-powerpc/abs-pie-relr.r
index 22effe89541..049cd7aa983 100644
--- a/ld/testsuite/ld-powerpc/abs-pie-relr.r
+++ b/ld/testsuite/ld-powerpc/abs-pie-relr.r
@@ -4,5 +4,5 @@
 #readelf: -rW
 
 Relocation section '\.relr\.dyn' at offset .* contains 1 entry:
-  1 offset
-0+10338
+Index: Entry: +Address relocated Symbolic Address +Notes
+0000: +[0-9a-f]+ [0-9a-f]+ +x \(new starting address\)
diff --git a/ld/testsuite/ld-powerpc/abs-shared-relr.r b/ld/testsuite/ld-powerpc/abs-shared-relr.r
index 978c43a38e9..3ce70f740e2 100644
--- a/ld/testsuite/ld-powerpc/abs-shared-relr.r
+++ b/ld/testsuite/ld-powerpc/abs-shared-relr.r
@@ -13,5 +13,6 @@ Relocation section '\.rela\.dyn' at offset .* contains 6 entries:
 0+10450  0+400000026 R_PPC64_ADDR64         123456789abcdef0 c \+ 0
 
 Relocation section '\.relr\.dyn' at offset .* contains 1 entry:
-  1 offset
-0+10438
+Index: Entry: +Address relocated Symbolic Address +Notes
+0000: +[0-9a-f]+ [0-9a-f]+ +x \(new starting address\)
+
diff --git a/ld/testsuite/ld-x86-64/dt-relr-1a-x32.d b/ld/testsuite/ld-x86-64/dt-relr-1a-x32.d
index 8f2f6fc2f74..78ffda9ce15 100644
--- a/ld/testsuite/ld-x86-64/dt-relr-1a-x32.d
+++ b/ld/testsuite/ld-x86-64/dt-relr-1a-x32.d
@@ -14,11 +14,10 @@ Relocation section '.rela.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_X86_64_JUMP_SLOT +0+ +func1 \+ 0
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
- +3 offsets
-[a-f0-9]+
-[a-f0-9]+
-[a-f0-9]+
-
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+ +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
  +Num: +Value +Size Type +Bind +Vis +Ndx Name
diff --git a/ld/testsuite/ld-x86-64/dt-relr-1a.d b/ld/testsuite/ld-x86-64/dt-relr-1a.d
index 218594dfb98..8e099257037 100644
--- a/ld/testsuite/ld-x86-64/dt-relr-1a.d
+++ b/ld/testsuite/ld-x86-64/dt-relr-1a.d
@@ -14,11 +14,10 @@ Relocation section '.rela.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_X86_64_JUMP_SLOT +0+ +func1 \+ 0
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
- +3 offsets
-[a-f0-9]+
-[a-f0-9]+
-[a-f0-9]+
-
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+ +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
  +Num: +Value +Size Type +Bind +Vis +Ndx Name
diff --git a/ld/testsuite/ld-x86-64/dt-relr-1b-x32.d b/ld/testsuite/ld-x86-64/dt-relr-1b-x32.d
index 0ddffc82429..c2e3a3a931f 100644
--- a/ld/testsuite/ld-x86-64/dt-relr-1b-x32.d
+++ b/ld/testsuite/ld-x86-64/dt-relr-1b-x32.d
@@ -17,11 +17,10 @@ Relocation section '.rela.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_X86_64_JUMP_SLOT +0+ +func1 \+ 0
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
- +3 offsets
-[a-f0-9]+
-[a-f0-9]+
-[a-f0-9]+
-
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+ +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
  +Num: +Value +Size Type +Bind +Vis +Ndx Name
diff --git a/ld/testsuite/ld-x86-64/dt-relr-1b.d b/ld/testsuite/ld-x86-64/dt-relr-1b.d
index 4e462dd1fcb..03e0202fe81 100644
--- a/ld/testsuite/ld-x86-64/dt-relr-1b.d
+++ b/ld/testsuite/ld-x86-64/dt-relr-1b.d
@@ -17,11 +17,10 @@ Relocation section '.rela.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 [0-9a-f]+ +[0-9a-f]+ +R_X86_64_JUMP_SLOT +0+ +func1 \+ 0
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
- +3 offsets
-[a-f0-9]+
-[a-f0-9]+
-[a-f0-9]+
-
+#...
+0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
+0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+ +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
  +Num: +Value +Size Type +Bind +Vis +Ndx Name

Re: Commit: readelf: Improve display of RELR relocations

[Fangrui Song]

On Thu, Apr 11, 2024 at 8:56 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi Guys,
>
>   Currently readelf's display of RELR relocations is woefully lacking in
>   information.  So I am going to apply the attached patch in order to
>   improve this.  For example:
>
>     readelf -r /lib64/libc.so.6
>
>   Before patch:
>
>     Relocation section '.relr.dyn' at offset 0x259c0 contains 32 entries:
>       1067 offsets
>     00000000001d4ca0
>     00000000001d4cb0
>     00000000001d4cb8
>     00000000001d4cc0
>     00000000001d4ce0
>     00000000001d4d00
>     ...

Hi Nick,

Thanks for the improvement!

>   After patch:
>
>     Relocation section '.relr.dyn' at offset 0x259c0 contains 32 entries:
>     Index: Entry:           Address relocated Symbolic Address
>     0000:  00000000001d4ca0 00000000001d4ca0  __FRAME_END__ + 0x1ddc
>     0001:  ffffdff8ee15911d 00000000001d4cb0  __FRAME_END__ + 0x1dec
>                             00000000001d4cb8  __FRAME_END__ + 0x1df4
>                             00000000001d4cc0  __dso_handle
>                             00000000001d4ce0  _nl_C_LC_CTYPE
>                             00000000001d4d00  _nl_C_LC_CTYPE + 0x20
>     ...
>   In particular this new format shows the actual values held in the RELR
>   section - allowing a user to potentially spot problems - as well as
>   providing an address to symbol mapping for ease in understanding what
>   is being relocated.
>
>   The patch also checks for malformed RELR entries (such as an entry
>   with a value of just 1).
>
>   The patch includes a new binutils test and updates to linker tests
>   that were checking the RELR relocations.
>
> Cheers
>   Nick
>

I have some minor suggestions.

* Do we need the ":" in "Entry:"? I presume not because the strings
don't end with ":".
* "Address relocated" feels verbose. Would a simple "Address" be
acceptable? That aligns with "Offset" (instead of "Offset relocated")
for REL/RELA output.
* Do we need the "Notes" column (new starting address, start of
bitmap)? The start of a new address/bitmap can be inferred from the
presence of the "Index" or "Entry" column string. The user needs to
look at the odd/even bit to figure out whether it is a start address
or a bitmap, but this information seems straightforward. Omitting the
column might make parsing slightly easier.


Relocation section '.relr.dyn' at offset 0x7ae8 contains 6372 entries:
Index: Entry:           Address relocated Symbolic Address        Notes
0000:  00000000042c0350 00000000042c0350
__do_global_dtors_aux_fini_array_entry (new starting address)
0001:  ffffffffffffffff 00000000042c0358
__frame_dummy_init_array_entry (start of bitmap)     #### (start of
bitmap) makes it slightly awkward to parse the symbolic address
                        00000000042c0360  __frame_dummy_init_array_entry + 0x8
                        00000000042c0368  __frame_dummy_init_array_entry + 0x10
                        00000000042c0370  __frame_dummy_init_array_entry + 0x18
                        00000000042c0378  __frame_dummy_init_array_entry + 0x20

The implementation modifies symtab in place

Re: Commit: readelf: Improve display of RELR relocations

[Fangrui Song]

[-- Attachment #1: Type: text/plain, Size: 3450 bytes --]

On Fri, Apr 12, 2024 at 11:44 AM Fangrui Song <i@maskray.me> wrote:
>
> On Thu, Apr 11, 2024 at 8:56 AM Nick Clifton <nickc@redhat.com> wrote:
> >
> > Hi Guys,
> >
> >   Currently readelf's display of RELR relocations is woefully lacking in
> >   information.  So I am going to apply the attached patch in order to
> >   improve this.  For example:
> >
> >     readelf -r /lib64/libc.so.6
> >
> >   Before patch:
> >
> >     Relocation section '.relr.dyn' at offset 0x259c0 contains 32 entries:
> >       1067 offsets
> >     00000000001d4ca0
> >     00000000001d4cb0
> >     00000000001d4cb8
> >     00000000001d4cc0
> >     00000000001d4ce0
> >     00000000001d4d00
> >     ...
>
> Hi Nick,
>
> Thanks for the improvement!
>
> >   After patch:
> >
> >     Relocation section '.relr.dyn' at offset 0x259c0 contains 32 entries:
> >     Index: Entry:           Address relocated Symbolic Address
> >     0000:  00000000001d4ca0 00000000001d4ca0  __FRAME_END__ + 0x1ddc
> >     0001:  ffffdff8ee15911d 00000000001d4cb0  __FRAME_END__ + 0x1dec
> >                             00000000001d4cb8  __FRAME_END__ + 0x1df4
> >                             00000000001d4cc0  __dso_handle
> >                             00000000001d4ce0  _nl_C_LC_CTYPE
> >                             00000000001d4d00  _nl_C_LC_CTYPE + 0x20
> >     ...
> >   In particular this new format shows the actual values held in the RELR
> >   section - allowing a user to potentially spot problems - as well as
> >   providing an address to symbol mapping for ease in understanding what
> >   is being relocated.
> >
> >   The patch also checks for malformed RELR entries (such as an entry
> >   with a value of just 1).
> >
> >   The patch includes a new binutils test and updates to linker tests
> >   that were checking the RELR relocations.
> >
> > Cheers
> >   Nick
> >
>
> I have some minor suggestions.
>
> * Do we need the ":" in "Entry:"? I presume not because the strings
> don't end with ":".
> * "Address relocated" feels verbose. Would a simple "Address" be
> acceptable? That aligns with "Offset" (instead of "Offset relocated")
> for REL/RELA output.
> * Do we need the "Notes" column (new starting address, start of
> bitmap)? The start of a new address/bitmap can be inferred from the
> presence of the "Index" or "Entry" column string. The user needs to
> look at the odd/even bit to figure out whether it is a start address
> or a bitmap, but this information seems straightforward. Omitting the
> column might make parsing slightly easier.
>
>
> Relocation section '.relr.dyn' at offset 0x7ae8 contains 6372 entries:
> Index: Entry:           Address relocated Symbolic Address        Notes
> 0000:  00000000042c0350 00000000042c0350
> __do_global_dtors_aux_fini_array_entry (new starting address)
> 0001:  ffffffffffffffff 00000000042c0358
> __frame_dummy_init_array_entry (start of bitmap)     #### (start of
> bitmap) makes it slightly awkward to parse the symbolic address
>                         00000000042c0360  __frame_dummy_init_array_entry + 0x8
>                         00000000042c0368  __frame_dummy_init_array_entry + 0x10
>                         00000000042c0370  __frame_dummy_init_array_entry + 0x18
>                         00000000042c0378  __frame_dummy_init_array_entry + 0x20
>

Let's see if some of the ideas in the attached patch are practical :)

[-- Attachment #2: readelf-relr.patch --]
[-- Type: text/x-patch, Size: 10945 bytes --]

From 34a3e029e6f360d3b0fdc6ac6cc2f33ae21c0308 Mon Sep 17 00:00:00 2001
From: Fangrui Song <maskray@gcc.gnu.org>
Date: Fri, 12 Apr 2024 14:29:28 -0700
Subject: [PATCH] readelf: Adjust display of RELR relocations

* Remove the colon for "Entry"
* Unify "Address relocated" (--wide) and "Address" (narrow) to "Address".
* Remove the "Notes" column. The information can be inferred from entry%2.
---
 binutils/readelf.c                        | 20 +++-----------------
 ld/testsuite/ld-elf/dt-relr-2b.d          |  4 ++--
 ld/testsuite/ld-elf/dt-relr-2c.d          |  4 ++--
 ld/testsuite/ld-elf/dt-relr-2d.d          |  4 ++--
 ld/testsuite/ld-elf/dt-relr-2e.d          |  4 ++--
 ld/testsuite/ld-elf/dt-relr-2i.d          |  4 ++--
 ld/testsuite/ld-i386/dt-relr-1a.d         |  4 ++--
 ld/testsuite/ld-i386/dt-relr-1b.d         |  4 ++--
 ld/testsuite/ld-powerpc/abs-pie-relr.r    |  4 ++--
 ld/testsuite/ld-powerpc/abs-shared-relr.r |  4 ++--
 ld/testsuite/ld-x86-64/dt-relr-1a-x32.d   |  4 ++--
 ld/testsuite/ld-x86-64/dt-relr-1a.d       |  4 ++--
 ld/testsuite/ld-x86-64/dt-relr-1b-x32.d   |  4 ++--
 ld/testsuite/ld-x86-64/dt-relr-1b.d       |  4 ++--
 14 files changed, 29 insertions(+), 43 deletions(-)

diff --git a/binutils/readelf.c b/binutils/readelf.c
index fcf95ee3047..e0cf718aa28 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -1665,20 +1665,10 @@ dump_relr_relocations (Filedata *          filedata,
      be used later on for some other purpose.  */
   qsort (symtab, nsyms, sizeof (Elf_Internal_Sym), symcmp);
 
-  if (do_wide)
-    {
-      if (relr_entsize == 4)
-	printf (_("Index: Entry:   Address  Symbolic Address     Notes\n"));
-      else
-	printf (_("Index: Entry:           Address relocated Symbolic Address        Notes\n"));
-    }
+  if (relr_entsize == 4)
+    printf (_ ("Index: Entry    Address   Symbolic Address\n"));
   else
-    {
-      if (relr_entsize == 4)
-	printf (_("Index: Entry:   Address  Symbolic Address\n"));
-      else
-	printf (_("Index: Entry:           Address relocated Symbolic Address\n"));
-    }
+    printf (_ ("Index: Entry            Address           Symbolic Address\n"));
 
   for (i = 0; i < nentries; i++)
     {
@@ -1698,8 +1688,6 @@ dump_relr_relocations (Filedata *          filedata,
 	{
 	  where = entry;
 	  print_relr_addr_and_sym (symtab, nsyms, strtab, strtablen, where);
-	  if (do_wide)
-	    printf (_(" (new starting address)"));
 	  printf ("\n");
 	  where += relr_entsize;
 	}
@@ -1722,8 +1710,6 @@ dump_relr_relocations (Filedata *          filedata,
 		if (first)
 		  {
 		    print_relr_addr_and_sym (symtab, nsyms, strtab, strtablen, addr);
-		    if (do_wide)
-		      printf (_(" (start of bitmap)"));
 		    first = false;
 		  }
 		else
diff --git a/ld/testsuite/ld-elf/dt-relr-2b.d b/ld/testsuite/ld-elf/dt-relr-2b.d
index 48a3eb7f15c..f9c688087f9 100644
--- a/ld/testsuite/ld-elf/dt-relr-2b.d
+++ b/ld/testsuite/ld-elf/dt-relr-2b.d
@@ -14,8 +14,8 @@ Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +data \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+ \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +data
+0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+
  +[0-9a-f]+ +data \+ 0x[0-9a-f]+
  +[0-9a-f]+ +data \+ 0x[0-9a-f]+
 #pass
diff --git a/ld/testsuite/ld-elf/dt-relr-2c.d b/ld/testsuite/ld-elf/dt-relr-2c.d
index 7f6383b0184..d9e3698a8f0 100644
--- a/ld/testsuite/ld-elf/dt-relr-2c.d
+++ b/ld/testsuite/ld-elf/dt-relr-2c.d
@@ -14,7 +14,7 @@ Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +.*
+0001: +[0-9a-f]+ [0-9a-f]+ +.*
  +[0-9a-f]+ +.*
 #pass
diff --git a/ld/testsuite/ld-elf/dt-relr-2d.d b/ld/testsuite/ld-elf/dt-relr-2d.d
index f1184fefeb2..69863bddec4 100644
--- a/ld/testsuite/ld-elf/dt-relr-2d.d
+++ b/ld/testsuite/ld-elf/dt-relr-2d.d
@@ -14,8 +14,8 @@ Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +data \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+ \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +data
+0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+
  +[0-9a-f]+ +data \+ 0x[0-9a-f]+
  +[0-9a-f]+ +data \+ 0x[0-9a-f]+
 #pass
diff --git a/ld/testsuite/ld-elf/dt-relr-2e.d b/ld/testsuite/ld-elf/dt-relr-2e.d
index eddb5e3d14e..e047c0d6529 100644
--- a/ld/testsuite/ld-elf/dt-relr-2e.d
+++ b/ld/testsuite/ld-elf/dt-relr-2e.d
@@ -14,8 +14,8 @@ Relocation section '\.rel(a|)\.data' at offset 0x[0-9a-f]+ contains 1 entry:
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +data \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+ \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +data
+0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+
  +[0-9a-f]+ +data \+ 0x[0-9a-f]+
  +[0-9a-f]+ +data \+ 0x[0-9a-f]+
 #pass
diff --git a/ld/testsuite/ld-elf/dt-relr-2i.d b/ld/testsuite/ld-elf/dt-relr-2i.d
index 55e8c256b92..a328ccb92e5 100644
--- a/ld/testsuite/ld-elf/dt-relr-2i.d
+++ b/ld/testsuite/ld-elf/dt-relr-2i.d
@@ -14,8 +14,8 @@ Relocation section '\.rel(a|)\.dyn' at offset 0x[0-9a-f]+ contains 1 entry:
 #...
 Relocation section '\.relr\.dyn' at offset 0x[0-9a-f]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +data \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+ \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +data
+0001: +[0-9a-f]+ [0-9a-f]+ +data \+ 0x[0-9a-f]+
  +[0-9a-f]+ +data \+ 0x[0-9a-f]+
  +[0-9a-f]+ +data \+ 0x[0-9a-f]+
 #pass
diff --git a/ld/testsuite/ld-i386/dt-relr-1a.d b/ld/testsuite/ld-i386/dt-relr-1a.d
index 3944911e4f7..89cc636f4bc 100644
--- a/ld/testsuite/ld-i386/dt-relr-1a.d
+++ b/ld/testsuite/ld-i386/dt-relr-1a.d
@@ -15,8 +15,8 @@ Relocation section '\.rel\.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +.*
+0001: +[0-9a-f]+ [0-9a-f]+ +.*
  +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
diff --git a/ld/testsuite/ld-i386/dt-relr-1b.d b/ld/testsuite/ld-i386/dt-relr-1b.d
index 947d8354b23..6e7f3ca0a25 100644
--- a/ld/testsuite/ld-i386/dt-relr-1b.d
+++ b/ld/testsuite/ld-i386/dt-relr-1b.d
@@ -18,8 +18,8 @@ Relocation section '\.rel\.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +.*
+0001: +[0-9a-f]+ [0-9a-f]+ +.*
  +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
diff --git a/ld/testsuite/ld-powerpc/abs-pie-relr.r b/ld/testsuite/ld-powerpc/abs-pie-relr.r
index 049cd7aa983..e84b0af115b 100644
--- a/ld/testsuite/ld-powerpc/abs-pie-relr.r
+++ b/ld/testsuite/ld-powerpc/abs-pie-relr.r
@@ -4,5 +4,5 @@
 #readelf: -rW
 
 Relocation section '\.relr\.dyn' at offset .* contains 1 entry:
-Index: Entry: +Address relocated Symbolic Address +Notes
-0000: +[0-9a-f]+ [0-9a-f]+ +x \(new starting address\)
+Index: Entry            Address           Symbolic Address
+0000: +[0-9a-f]+ [0-9a-f]+ +x
diff --git a/ld/testsuite/ld-powerpc/abs-shared-relr.r b/ld/testsuite/ld-powerpc/abs-shared-relr.r
index 3ce70f740e2..ce1a7eee339 100644
--- a/ld/testsuite/ld-powerpc/abs-shared-relr.r
+++ b/ld/testsuite/ld-powerpc/abs-shared-relr.r
@@ -13,6 +13,6 @@ Relocation section '\.rela\.dyn' at offset .* contains 6 entries:
 0+10450  0+400000026 R_PPC64_ADDR64         123456789abcdef0 c \+ 0
 
 Relocation section '\.relr\.dyn' at offset .* contains 1 entry:
-Index: Entry: +Address relocated Symbolic Address +Notes
-0000: +[0-9a-f]+ [0-9a-f]+ +x \(new starting address\)
+Index: Entry            Address           Symbolic Address
+0000: +[0-9a-f]+ [0-9a-f]+ +x
 
diff --git a/ld/testsuite/ld-x86-64/dt-relr-1a-x32.d b/ld/testsuite/ld-x86-64/dt-relr-1a-x32.d
index 78ffda9ce15..863e97a7980 100644
--- a/ld/testsuite/ld-x86-64/dt-relr-1a-x32.d
+++ b/ld/testsuite/ld-x86-64/dt-relr-1a-x32.d
@@ -15,8 +15,8 @@ Relocation section '.rela.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +.*
+0001: +[0-9a-f]+ [0-9a-f]+ +.*
  +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
diff --git a/ld/testsuite/ld-x86-64/dt-relr-1a.d b/ld/testsuite/ld-x86-64/dt-relr-1a.d
index 8e099257037..30af87535c9 100644
--- a/ld/testsuite/ld-x86-64/dt-relr-1a.d
+++ b/ld/testsuite/ld-x86-64/dt-relr-1a.d
@@ -15,8 +15,8 @@ Relocation section '.rela.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +.*
+0001: +[0-9a-f]+ [0-9a-f]+ +.*
  +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
diff --git a/ld/testsuite/ld-x86-64/dt-relr-1b-x32.d b/ld/testsuite/ld-x86-64/dt-relr-1b-x32.d
index c2e3a3a931f..3c37bcd5358 100644
--- a/ld/testsuite/ld-x86-64/dt-relr-1b-x32.d
+++ b/ld/testsuite/ld-x86-64/dt-relr-1b-x32.d
@@ -18,8 +18,8 @@ Relocation section '.rela.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +.*
+0001: +[0-9a-f]+ [0-9a-f]+ +.*
  +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
diff --git a/ld/testsuite/ld-x86-64/dt-relr-1b.d b/ld/testsuite/ld-x86-64/dt-relr-1b.d
index 03e0202fe81..bc07cf89b26 100644
--- a/ld/testsuite/ld-x86-64/dt-relr-1b.d
+++ b/ld/testsuite/ld-x86-64/dt-relr-1b.d
@@ -18,8 +18,8 @@ Relocation section '.rela.plt' at offset 0x[0-9a-f]+ contains 1 entry:
 
 Relocation section '.relr.dyn' at offset 0x[a-f0-9]+ contains 2 entries:
 #...
-0000: +[0-9a-f]+ [0-9a-f]+ +.* \(new starting address\)
-0001: +[0-9a-f]+ [0-9a-f]+ +.* \(start of bitmap\)
+0000: +[0-9a-f]+ [0-9a-f]+ +.*
+0001: +[0-9a-f]+ [0-9a-f]+ +.*
  +[0-9a-f]+ +.*
 #...
 Symbol table '.symtab' contains [0-9]+ entries:
-- 
2.44

Re: Commit: readelf: Improve display of RELR relocations

[Nick Clifton]

Hi Fangrui,

>> I have some minor suggestions.
>>
>> * Do we need the ":" in "Entry:"? I presume not because the strings
>> don't end with ":".

No - it was just a minor formatting thing on my part.

>> * "Address relocated" feels verbose. Would a simple "Address" be
>> acceptable? That aligns with "Offset" (instead of "Offset relocated")
>> for REL/RELA output.

Well I wanted to emphasise that the address was for the location to which
the relocation would be applied, rather than the address of the relocation
itself.

>> * Do we need the "Notes" column (new starting address, start of
>> bitmap)? 

Not really.  I confess that I wrote this code as a way of teaching
myself about the RELR format and the notes were there mainly as a
reminder to myself.

> Let's see if some of the ideas in the attached patch are practical :)

It is, and I have gone ahead and checked it in on your behalf. :)

Cheers
   Nick

Re: Commit: readelf: Improve display of RELR relocations

[Fangrui Song]

On Tue, Apr 16, 2024 at 5:22 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi Fangrui,
>
> >> I have some minor suggestions.
> >>
> >> * Do we need the ":" in "Entry:"? I presume not because the strings
> >> don't end with ":".
>
> No - it was just a minor formatting thing on my part.
>
> >> * "Address relocated" feels verbose. Would a simple "Address" be
> >> acceptable? That aligns with "Offset" (instead of "Offset relocated")
> >> for REL/RELA output.
>
> Well I wanted to emphasise that the address was for the location to which
> the relocation would be applied, rather than the address of the relocation
> itself.
>
> >> * Do we need the "Notes" column (new starting address, start of
> >> bitmap)?
>
> Not really.  I confess that I wrote this code as a way of teaching
> myself about the RELR format and the notes were there mainly as a
> reminder to myself.
>
> > Let's see if some of the ideas in the attached patch are practical :)
>
> It is, and I have gone ahead and checked it in on your behalf. :)
>
> Cheers
>    Nick

Thanks!

I just recall another two things.


Relocation section '.relr.dyn' at offset 0x1f4 contains 2 entries:

Considering REL/RELA output, users might misinterpret "entries" as
"relocation entries" instead of referring to RELR words.
Overall, the number of relocation entries seems more valuable than the
number of RELR words (users can infer the number from the section
size, or the new "Index:" column).

If we don't want another loop to count the number of relocation
entries, we can possibly remove the misleading "X entries"...

>   The patch also checks for malformed RELR entries (such as an entry
>  with a value of just 1).

Trailing 1 can be utilized by linkers to avoid convergence issues
(https://reviews.llvm.org/D67164):

// The last RELR entry may or may not reach .data.
.relr.dyn
.dynamic   // end of a PT_LOAD
.data    // start a new PT_LOAD

It is possible for the linker to find that the last entry can cover
the relative relocation at .data and decide to shrink .relr.dyn,
while in the next iteration, a smaller .relr.dyn causes the last entry
to unable to cover the relative relocation at .data, and the linker
decide to enlarge .relr.dyn.
This would run into an endless iteration situation.

Re: Commit: readelf: Improve display of RELR relocations

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]

Hi Fangrui Song,


> Relocation section '.relr.dyn' at offset 0x1f4 contains 2 entries:
> 
> Considering REL/RELA output, users might misinterpret "entries" as
> "relocation entries" instead of referring to RELR words.
> Overall, the number of relocation entries seems more valuable than the
> number of RELR words (users can infer the number from the section
> size, or the new "Index:" column).

A fair point.


>>    The patch also checks for malformed RELR entries (such as an entry
>>   with a value of just 1).
> 
> Trailing 1 can be utilized by linkers to avoid convergence issues
> (https://reviews.llvm.org/D67164):

Interesting.  I had not realised that that could happen.

So how about the attached patch ?  It changes the start of the RELR
section display so that it looks something like this:

   Relocation section '.relr.dyn' at offset 0x1a0 contains 3 entries which relocate 27 locations:

It also removes the warning about an no-op bitmap entry.

The patch is inefficient in that the section data is loaded twice, once
to count the entries and a second time to display them.  But this can
always be fixed up later if it proves to be a significant overhead.
In my testing I did not encounter any delays because of the double
load...

Cheers
   Nick


[-- Attachment #2: readelf.relr.patch.2 --]
[-- Type: application/x-troff-man, Size: 13871 bytes --]

Re: Commit: readelf: Improve display of RELR relocations

[Fangrui Song]

On Fri, Apr 19, 2024 at 3:55 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi Fangrui Song,
>
>
> > Relocation section '.relr.dyn' at offset 0x1f4 contains 2 entries:
> >
> > Considering REL/RELA output, users might misinterpret "entries" as
> > "relocation entries" instead of referring to RELR words.
> > Overall, the number of relocation entries seems more valuable than the
> > number of RELR words (users can infer the number from the section
> > size, or the new "Index:" column).
>
> A fair point.
>
>
> >>    The patch also checks for malformed RELR entries (such as an entry
> >>   with a value of just 1).
> >
> > Trailing 1 can be utilized by linkers to avoid convergence issues
> > (https://reviews.llvm.org/D67164):
>
> Interesting.  I had not realised that that could happen.
>
> So how about the attached patch ?  It changes the start of the RELR
> section display so that it looks something like this:
>
>    Relocation section '.relr.dyn' at offset 0x1a0 contains 3 entries which relocate 27 locations:
>
> It also removes the warning about an no-op bitmap entry.
>
> The patch is inefficient in that the section data is loaded twice, once
> to count the entries and a second time to display them.  But this can
> always be fixed up later if it proves to be a significant overhead.
> In my testing I did not encounter any delays because of the double
> load...
>
> Cheers
>    Nick

Hi Nick,

Thanks for the change.
I like the new header "Relocation section '.relr.dyn' at offset 0x1f4
contains 2 entries which relocate 24 locations:"

I've only got two nits:

Perhaps PRId64 should be changed to PRIu64 as num_rela is unsigned?

At /* It is theoretically possible for nentries to be 1.  */ , `if
(nentries == 0` does not free `relrs`.
Can `if (nentries == 0` be removed since the for loop handles the
zero-entry case?

Re: Commit: readelf: Improve display of RELR relocations

[Nick Clifton]

Hi Fangrui,

> I've only got two nits:
> 
> Perhaps PRId64 should be changed to PRIu64 as num_rela is unsigned?

Good point.  In fact there are several places where this change would
be appropriate.

> At /* It is theoretically possible for nentries to be 1.  */ , `if
> (nentries == 0` does not free `relrs`.
> Can `if (nentries == 0` be removed since the for loop handles the
> zero-entry case?

I could do that.  But since we know that the number of entries is zero,
why bother loading the relr data at all ?  It makes more sense to move
the check for (nentries == 0) to before the call the get_data() and thus
eliminating the need to load - and free - the section contents.

I am just running some last minute checks on the revised patch and then
I will check it in shortly.

Cheers
   Nick