[RFC PATCH v2] readelf: Add -G/--got-contents options
H.J. Lu
hjl.tools@gmail.com
Thu Aug 28 02:04:06 GMT 2025
On Wed, Aug 27, 2025 at 08:28:27AM +0200, Jan Beulich wrote:
> On 27.08.2025 04:45, H.J. Lu wrote:
> > On Tue, Aug 26, 2025 at 02:53:40PM -0700, H.J. Lu wrote:
> >> Add -G/--got-contents options to readelf, inspired by the similar option
> >> on Solaris, to display the contents of GOT sections:
> >>
> >> $ readelf -G libfoo.so
> >>
> >> Global Offset Table '.got' contains 5 entries:
> >> Index: Address Reloc Sym. Name + Addend/Value
> >> 0: 000000002fc8 R_X86_64_GLOB_DAT bar + 0
> >> 1: 000000002fd0 R_X86_64_GLOB_DAT __gmon_start__ + 0
> >> 2: 000000002fd8 R_X86_64_GLOB_DAT _ITM_registerTMCloneTable + 0
> >> 3: 000000002fe0 R_X86_64_GLOB_DAT __cxa_finalize@GLIBC_2.2.5 + 0
> >> 4: 000000002fe8 0
> >>
> >> Global Offset Table '.got.plt' contains 4 entries:
> >> Index: Address Reloc Sym. Name + Addend/Value
> >> 0: 000000002ff0 2dd0
> >> 1: 000000002ff8 0
> >> 2: 000000003000 R_X86_64_JUMP_SLO printf@GLIBC_2.2.5 + 2c0
> >> 3: 000000003008 0
> >>
> >> Any comments?
Thanks for your feedbacks.
>
> In principle a good thing to have, yet still a couple of remarks. First:
> No matter whether Solaris allows -G, as in a similar recent case I'm
> unconvinced we want to use a scarce resource (signed letter options)
> for something that's probably not going to be used very often.
The most useful readelf features have been implemented. What other
unimplemented features which are more useful than dumping GOT sections
and may use the -G options? Option compatibility with other readelf
implementations help our readelf users.
>
> > binutils/
> >
> > * NEWS: Mention "readelf -G/--got-contents".
> > * readelf.c (do_got_section_contents): New.
> > (elf_relocation): Likewise.
> > (all_relocations_root): Likewise.
> > (all_relocations): Likewise.
> > (all_relocations_count): Likewise.
> > (update_all_relocations): Likewise.
> > (dump_relr_relocations): Add a bool argument to indicate if
> > relocations should be displayed. Populate all_relocations if
> > do_got_section_contents is true.
> > (dump_relocations): Likewise.
> > (options): Add -G/--got-contents.
> > (usage): Show -G/--got-contents.
> > (parse_args): Support -G/--got-contents.
> > (display_relocations): Add bool argument, dump_reloc, to indicate
> > if relocations should be displayed. Call update_all_relocations.
> > Pass dump_reloc to dump_relr_relocations and dump_relocations.
> > (process_relocs): Check do_got_section_contents. Handle
> > do_got_section_contents for dynamic relocations.
> > (process_section_contents): Pass true to display_relocations.
> > (elf_relocation_cmp): New.
> > (display_elf_relocation_at): Likewise.
> > (process_got_section_contents): Likewise.
> > (process_object): Call process_got_section_contents.
> > * doc/binutils.texi: Document -G/--got-contents.
> >
> > ld/
> >
> > * testsuite/ld-i386/binutils.exp: New file.
> > * testsuite/ld-i386/got-1.s: Likewise.
> > * testsuite/ld-i386/libgot-1a.rd: Likewise.
> > * testsuite/ld-i386/libgot-1b.rd: Likewise.
> > * testsuite/ld-i386/libgot-1c.rd: Likewise.
> > * testsuite/ld-i386/libgot-1d.rd: Likewise.
> > * testsuite/ld-x86-64/binutils.exp: Likewise.
> > * testsuite/ld-x86-64/got-1.s: Likewise.
> > * testsuite/ld-x86-64/libgot-1a-x32.rd: Likewise.
> > * testsuite/ld-x86-64/libgot-1a.rd: Likewise.
> > * testsuite/ld-x86-64/libgot-1b-x32.rd: Likewise.
> > * testsuite/ld-x86-64/libgot-1b.rd: Likewise.
> > * testsuite/ld-x86-64/libgot-1c-x32.rd: Likewise.
> > * testsuite/ld-x86-64/libgot-1c.rd: Likewise.
> > * testsuite/ld-x86-64/libgot-1d-x32.rd: Likewise.
> > * testsuite/ld-x86-64/libgot-1d.rd: Likewise.
>
> Why in the linker testsuite? And why only for x86?
I put tests in the linker testsuite since readelf -G works on linker
outputs. readelf -G works on all targets. I only added x86 tests
since I am familiar with x86. I put my branch at
https://gitlab.com/x86-binutils/binutils-gdb/-/tree/users/hjl/readelf/master?ref_type=heads
Test patches for other targets are welcome.
>
> > @@ -1875,6 +1919,161 @@ dump_relr_relocations (Filedata * filedata,
> > if (symtab == NULL)
> > nsyms = 0;
> >
> > + const char *rtype = NULL;
> > + if (do_got_section_contents)
> > + switch (filedata->file_header.e_machine)
> > + {
> > + default:
> > + break;
> > +
> > + case EM_386:
> > + case EM_IAMCU:
> > + rtype = "R_386_RELATIVE";
> > + break;
> > +
> > + case EM_68K:
> > + rtype = "R_68K_RELATIVE";
> > + break;
> > +
> > + case EM_860:
> > + rtype = "R_860_RELATIVE";
> > + break;
> > +
> > + case EM_AARCH64:
> > + rtype = "R_AARCH64_RELATIVE";
> > + break;
> > +
> > + case EM_AMDGPU:
> > + rtype = "R_AMDGPU_RELATIVE64";
> > + break;
> > +
> > + case EM_ALPHA:
> > + rtype = "R_ALPHA_RELATIVE";
> > + break;
> > +
> > + case EM_ALTERA_NIOS2:
> > + rtype = "R_NIOS2_RELATIVE";
> > + break;
> > +
> > + case EM_ARM:
> > + rtype = "R_ARM_RELATIVE";
> > + break;
> > +
> > + case EM_ARC:
> > + case EM_ARC_COMPACT:
> > + case EM_ARC_COMPACT2:
> > + case EM_ARC_COMPACT3:
> > + case EM_ARC_COMPACT3_64:
> > + rtype = "R_ARC_RELATIVE";
> > + break;
> > +
> > + case EM_CRIS:
> > + rtype = "R_CRIS_RELATIVE";
> > + break;
> > +
> > + case EM_CSKY:
> > + rtype = "R_CKCORE_RELATIVE";
> > + break;
> > +
> > + case EM_KVX:
> > + rtype = "R_KVX_RELATIVE";
> > + break;
> > +
> > + case EM_LATTICEMICO32:
> > + rtype = "R_LM32_RELATIVE";
> > + break;
> > +
> > + case EM_LOONGARCH:
> > + rtype = "R_LARCH_RELATIVE";
> > + break;
> > +
> > + case EM_M32R:
> > + case EM_CYGNUS_M32R:
> > + rtype = "R_M32R_RELATIVE";
> > + break;
> > +
> > + case EM_MCORE:
> > + rtype = "R_MCORE_RELATIVE";
> > + break;
> > +
> > + case EM_METAG:
> > + rtype = "R_METAG_RELATIVE";
> > + break;
> > +
> > + case EM_MN10300:
> > + case EM_CYGNUS_MN10300:
> > + rtype = "R_MN10300_RELATIVE";
> > + break;
> > +
> > + case EM_NDS32:
> > + rtype = "R_NDS32_RELATIVE";
> > + break;
> > +
> > + case EM_OR1K:
> > + rtype = "R_OR1K_RELATIVE";
> > + break;
> > +
> > + case EM_PPC:
> > + rtype = "R_PPC_RELATIVE";
> > + break;
> > +
> > + case EM_PPC64:
> > + rtype = "R_PPC64_RELATIVE";
> > + break;
> > +
> > + case EM_RISCV:
> > + rtype = "R_RISCV_RELATIVE";
> > + break;
> > +
> > + case EM_S370:
> > + rtype = "R_I370_RELATIVE";
> > + break;
> > +
> > + case EM_S390_OLD:
> > + case EM_S390:
> > + rtype = "R_390_RELATIVE";
> > + break;
> > +
> > + case EM_SH:
> > + rtype = "R_SH_RELATIVE";
> > + break;
> > +
> > + case EM_OLD_SPARCV9:
> > + case EM_SPARC32PLUS:
> > + case EM_SPARCV9:
> > + case EM_SPARC:
> > + rtype = "R_SPARC_RELATIVE";
> > + break;
> > +
> > + case EM_TILEGX:
> > + rtype = "R_TILEGX_RELATIVE";
> > + break;
> > +
> > + case EM_TILEPRO:
> > + rtype = "R_TILEPRO_RELATIVE";
> > + break;
> > +
> > + case EM_V850:
> > + case EM_CYGNUS_V850:
> > + rtype = "R_V850_RELATIVE";
> > + break;
> > +
> > + case EM_VAX:
> > + rtype = "R_VAX_RELATIVE";
> > + break;
> > +
> > + case EM_X86_64:
> > + case EM_L1OM:
> > + case EM_K1OM:
> > + rtype = "R_X86_64_RELATIVE";
> > + break;
> > +
> > + case EM_XTENSA_OLD:
> > + case EM_XTENSA:
> > + rtype = "R_XTENSA_RELATIVE";
> > + break;
> > + }
>
> Something like this doesn't scale overly well, yet at the same time I have
> no good alternative suggestion besides converting all the arch-specific
> handling (i.e. incl pre-existing one) to some table based approach. That
> way at least omissions would be easier to notice: Comparing to e.g.
> guess_is_rela() I'm missing entries here. Not to speak of
> get_machine_name().
I changed it to
if (do_got_section_contents)
switch (filedata->file_header.e_machine)
{
default:
abort ();
We will know something is missing.
>
> > @@ -1885,11 +2084,15 @@ dump_relr_relocations (Filedata * filedata,
> > nsyms = filter_display_syms (filedata, symtab, nsyms, strtab, strtablen);
> > }
> >
> > - if (relr_entsize == sizeof (Elf32_External_Relr))
> > - printf (_ ("Index: Entry Address Symbolic Address\n"));
> > - else
> > - printf (_ ("Index: Entry Address Symbolic Address\n"));
> > + if (dump_reloc)
> > + {
> > + if (relr_entsize == sizeof (Elf32_External_Relr))
> > + printf (_ ("Index: Entry Address Symbolic Address\n"));
> > + else
> > + printf (_ ("Index: Entry Address Symbolic Address\n"));
> > + }
> >
> > + uint64_t r = 0;
> > for (i = 0; i < nentries; i++)
> > {
> > uint64_t entry;
> > @@ -1899,16 +2102,34 @@ dump_relr_relocations (Filedata * filedata,
> > 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 (dump_reloc)
> > + {
> > + /* 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 (filedata, symtab, nsyms, strtab, where);
> > - printf ("\n");
> > + if (dump_reloc)
> > + {
> > + print_relr_addr_and_sym (filedata, symtab, nsyms, strtab,
> > + where);
> > + printf ("\n");
> > + }
> > +
> > + if (do_got_section_contents)
> > + {
> > + all_relocations[r].r_offset = where;
> > + all_relocations[r].r_name = rtype;
> > + all_relocations[r].r_symbol = NULL;
> > + all_relocations[r].r_type = reltype_relr;
> > + r++;
> > + }
> > +
> > where += relr_entsize;
> > }
> > else
> > @@ -1921,7 +2142,7 @@ dump_relr_relocations (Filedata * filedata,
> > /* This can actually happen when the linker is allowed to shrink
> > RELR sections. For more details see: https://reviews.llvm.org/D67164. */
> > continue;
> > - else if (i == 0)
> > + else if (dump_reloc && i == 0)
> > warn (_("Unusual RELR bitmap - no previous entry to set the base address\n"));
> >
> > for (j = 0; entry >>= 1; j++)
> > @@ -1929,19 +2150,35 @@ dump_relr_relocations (Filedata * filedata,
> > {
> > uint64_t addr = where + (j * relr_entsize);
> >
> > - if (first)
> > + if (dump_reloc)
> > {
> > - print_relr_addr_and_sym (filedata, symtab, nsyms, strtab, addr);
> > - first = false;
> > + if (first)
> > + {
> > + print_relr_addr_and_sym (filedata, symtab, nsyms,
> > + strtab, addr);
> > + first = false;
> > + }
> > + else
> > + {
> > + printf (_("\n%*s "),
> > + relr_entsize == 4 ? 15 : 23, " ");
> > + print_relr_addr_and_sym (filedata, symtab, nsyms,
> > + strtab, addr);
> > + }
> > }
> > - else
> > +
> > + if (do_got_section_contents)
> > {
> > - printf (_("\n%*s "), relr_entsize == 4 ? 15 : 23, " ");
> > - print_relr_addr_and_sym (filedata, symtab, nsyms, strtab, addr);
> > + all_relocations[r].r_offset = addr;
> > + all_relocations[r].r_name = rtype;
> > + all_relocations[r].r_symbol = NULL;
> > + all_relocations[r].r_type = reltype_relr;
> > + r++;
> > }
> > }
> >
> > - printf ("\n");
> > + if (dump_reloc)
> > + printf ("\n");
> > where += num_bits_in_entry * relr_entsize;
> > }
> > }
> > @@ -1962,7 +2199,8 @@ dump_relocations (Filedata * filedata,
> > char * strtab,
> > uint64_t strtablen,
> > relocation_type rel_type,
> > - bool is_dynsym)
> > + bool is_dynsym,
> > + bool dump_reloc)
>
> This is an oddly named parameter for a function named dump_relocations().
> Without a comment as to what the purpose of the function is when that's
> coming in as "false" I can't even suggest a possible alternative.
I added a comment:
/* Display the contents of the relocation data found at the specified
offset. If DUMP_RELOC is false, don't display relocations, just
collect relocations for displaying GOT section contents later. */
>
> > @@ -1987,38 +2225,41 @@ dump_relocations (Filedata * filedata,
> > return false;
> > }
> >
> > - if (is_32bit_elf)
> > + if (dump_reloc)
> > {
> > - if (rel_type == reltype_rela)
> > - {
> > - if (do_wide)
> > - printf (_(" Offset Info Type Sym. Value Symbol's Name + Addend\n"));
> > - else
> > - printf (_(" Offset Info Type Sym.Value Sym. Name + Addend\n"));
> > - }
> > - else
> > - {
> > - if (do_wide)
> > - printf (_(" Offset Info Type Sym. Value Symbol's Name\n"));
> > - else
> > - printf (_(" Offset Info Type Sym.Value Sym. Name\n"));
> > - }
> > - }
> > - else
> > - {
> > - if (rel_type == reltype_rela)
> > + if (is_32bit_elf)
>
> Please can you avoid all the re-indentation by making this "else if()"?
Fixed on my branch.
>
> > {
> > - if (do_wide)
> > - printf (_(" Offset Info Type Symbol's Value Symbol's Name + Addend\n"));
> > + if (rel_type == reltype_rela)
> > + {
> > + if (do_wide)
> > + printf (_(" Offset Info Type Sym. Value Symbol's Name + Addend\n"));
> > + else
> > + printf (_(" Offset Info Type Sym.Value Sym. Name + Addend\n"));
> > + }
> > else
> > - printf (_(" Offset Info Type Sym. Value Sym. Name + Addend\n"));
> > + {
> > + if (do_wide)
> > + printf (_(" Offset Info Type Sym. Value Symbol's Name\n"));
> > + else
> > + printf (_(" Offset Info Type Sym.Value Sym. Name\n"));
> > + }
> > }
> > else
> > {
> > - if (do_wide)
> > - printf (_(" Offset Info Type Symbol's Value Symbol's Name\n"));
> > + if (rel_type == reltype_rela)
> > + {
> > + if (do_wide)
> > + printf (_(" Offset Info Type Symbol's Value Symbol's Name + Addend\n"));
> > + else
> > + printf (_(" Offset Info Type Sym. Value Sym. Name + Addend\n"));
> > + }
> > else
> > - printf (_(" Offset Info Type Sym. Value Sym. Name\n"));
> > + {
> > + if (do_wide)
> > + printf (_(" Offset Info Type Symbol's Value Symbol's Name\n"));
> > + else
> > + printf (_(" Offset Info Type Sym. Value Sym. Name\n"));
> > + }
> > }
> > }
> >
> > @@ -2036,18 +2277,17 @@ dump_relocations (Filedata * filedata,
> > type = get_reloc_type (filedata, inf);
> > symtab_index = get_reloc_symindex (inf);
> >
> > - if (is_32bit_elf)
> > - {
> > - printf ("%8.8lx %8.8lx ",
> > - (unsigned long) offset & 0xffffffff,
> > - (unsigned long) inf & 0xffffffff);
> > - }
> > - else
> > + if (dump_reloc)
> > {
> > - printf (do_wide
> > - ? "%16.16" PRIx64 " %16.16" PRIx64 " "
> > - : "%12.12" PRIx64 " %12.12" PRIx64 " ",
> > - offset, inf);
> > + if (is_32bit_elf)
> > + printf ("%8.8lx %8.8lx ",
> > + (unsigned long) offset & 0xffffffff,
> > + (unsigned long) inf & 0xffffffff);
>
> Can you perhaps take the opportunity and switch this to a mere cast to
> uint32_t and use of PRIx32, being shorter and better matching
Fixed on my branch.
>
> > + else
> > + printf (do_wide
> > + ? "%16.16" PRIx64 " %16.16" PRIx64 " "
> > + : "%12.12" PRIx64 " %12.12" PRIx64 " ",
> > + offset, inf);
>
> ... this? (Same elsewhere then.)
>
> > @@ -2479,13 +2731,30 @@ dump_relocations (Filedata * filedata,
> > else
> > name = strtab + psym->st_name;
> >
> > - len = print_symbol_name (width, name);
> > - if (version_string)
> > - printf (sym_info == symbol_public ? "@@%s" : "@%s",
> > - version_string);
> > - printf ("()%-*s", len <= width ? (width + 1) - len : 1, " ");
> > + if (do_got_section_contents)
> > + {
> > + if (version_string)
> > + symbol_name = concat (name,
> > + sym_info == symbol_public
> > + ? "@@" : "@",
> > + version_string, NULL);
> > + else
> > + symbol_name = xstrdup (name);
> > + }
> > +
> > + if (dump_reloc)
> > + {
> > + len = print_symbol_name (width, name);
> > + if (version_string)
> > + printf (sym_info == symbol_public
> > + ? "@@%s" : "@%s",
> > + version_string);
> > + printf ("()%-*s",
> > + len <= width ? (width + 1) - len : 1,
> > + " ");
> > + }
>
> Without using "else if()" this gives the impression that both options may
> come in "true". Yet then there's the question why the latter doesn't use
> what the former already has formatted. (Applies further down as well.)
Either or both can be true.
>
> > @@ -2500,25 +2769,49 @@ dump_relocations (Filedata * filedata,
> > sec_name = printable_section_name_from_index
> > (filedata, psym->st_shndx, NULL);
> >
> > - print_symbol_name (22, sec_name);
> > + if (do_got_section_contents)
> > + symbol_name = xstrdup (sec_name);
> > + if (dump_reloc)
> > + print_symbol_name (22, sec_name);
> > }
> > else if (strtab == NULL)
> > - printf (_("<string table index: %3ld>"), psym->st_name);
> > + {
> > + if (dump_reloc)
> > + printf (_("<string table index: %3ld>"),
> > + psym->st_name);
> > + }
> > else if (psym->st_name >= strtablen)
> > {
> > - error (_("<corrupt string table index: %3ld>\n"),
> > - psym->st_name);
> > - res = false;
> > + if (dump_reloc)
> > + {
> > + error (_("<corrupt string table index: %3ld>\n"),
> > + psym->st_name);
> > + res = false;
> > + }
> > }
> > else
> > {
> > - print_symbol_name (22, strtab + psym->st_name);
> > - if (version_string)
> > - printf (sym_info == symbol_public ? "@@%s" : "@%s",
> > - version_string);
> > + if (dump_reloc)
> > + {
> > + print_symbol_name (22, strtab + psym->st_name);
> > + if (version_string)
> > + printf (sym_info == symbol_public
> > + ? "@@%s" : "@%s",
> > + version_string);
> > + }
> > + if (all_relocations)
>
> Any reason this isn't checking do_got_section_contents, as done elsewhere?
Fixed on my branch.
>
> > @@ -17704,6 +18075,196 @@ process_section_contents (Filedata * filedata)
> > return res;
> > }
> >
> > +static int
> > +elf_relocation_cmp (const void *p, const void *q)
> > +{
> > + elf_relocation *rp = (elf_relocation *) p;
> > + elf_relocation *rq = (elf_relocation *) q;
>
> Please never cast away const-ness.
>
Fixed on my branch.
Thanks.
H.J.
More information about the Binutils
mailing list