[RFC PATCH v2] readelf: Add -G/--got-contents options
Jan Beulich
jbeulich@suse.com
Wed Aug 27 06:28:27 GMT 2025
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?
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.
> 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?
> @@ -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().
> @@ -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.
> @@ -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()"?
> {
> - 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
> + 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.)
> @@ -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?
> @@ -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.
Jan
More information about the Binutils
mailing list