public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Fangrui Song <i@maskray.me>
To: Nick Clifton <nickc@redhat.com>
Cc: binutils@sourceware.org
Subject: Re: Commit: readelf: Improve display of RELR relocations
Date: Fri, 12 Apr 2024 15:04:28 -0700	[thread overview]
Message-ID: <DS7PR12MB57659BE02086ACBECDD35B0ACB042@DS7PR12MB5765.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CAN30aBFj+_Yfs8+WrC7aM0ZWQihGoZ8nMk2DHO7Xkxzw6PgqSA@mail.gmail.com>

[-- 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


  parent reply	other threads:[~2024-04-12 22:11 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-11 15:56 Nick Clifton
2024-04-12 18:44 ` Fangrui Song
     [not found] ` <CAN30aBFj+_Yfs8+WrC7aM0ZWQihGoZ8nMk2DHO7Xkxzw6PgqSA@mail.gmail.com>
2024-04-12 22:04   ` Fangrui Song [this message]
     [not found]   ` <DS7PR12MB576532E4BB26A2211E7A75AACB042@DS7PR12MB5765.namprd12.prod.outlook.com>
2024-04-16 12:22     ` Nick Clifton
2024-04-18  2:28       ` Fangrui Song
     [not found]       ` <DS7PR12MB57659F007C5FA3B3CE38A2ECCB0E2@DS7PR12MB5765.namprd12.prod.outlook.com>
2024-04-19 10:55         ` Nick Clifton
2024-04-23 19:37           ` Fangrui Song
     [not found]           ` <MN0PR12MB5761E8D41391C97D0666D300CB112@MN0PR12MB5761.namprd12.prod.outlook.com>
2024-04-24 11:25             ` Nick Clifton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DS7PR12MB57659BE02086ACBECDD35B0ACB042@DS7PR12MB5765.namprd12.prod.outlook.com \
    --to=i@maskray.me \
    --cc=binutils@sourceware.org \
    --cc=nickc@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).