From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 92A973982436 for ; Fri, 20 May 2022 00:41:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 92A973982436 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from reform (deer0x07.wildebeest.org [172.31.17.137]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 25EEF300027E; Fri, 20 May 2022 02:41:38 +0200 (CEST) Received: by reform (Postfix, from userid 1000) id A38892E83186; Fri, 20 May 2022 02:41:38 +0200 (CEST) Date: Fri, 20 May 2022 02:41:38 +0200 From: Mark Wielaard To: Di Chen Cc: elfutils-devel@sourceware.org Subject: Re: [PATCH] readelf: Support --dynamic with --use-dynamic Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-9.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: elfutils-devel@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Elfutils-devel mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 May 2022 00:41:49 -0000 Hi, On Thu, May 05, 2022 at 09:01:24PM +0800, Di Chen via Elfutils-devel wrote: > From 3ac23c2584d76114deab0c0af6f4af99068dc7f4 Mon Sep 17 00:00:00 2001 > From: Di Chen > Date: Thu, 28 Apr 2022 19:55:33 +0800 > Subject: [PATCH] readelf: Support --dynamic with --use-dynamic > > Currently, eu-readelf is using section headers to dump the dynamic > segment information (print_dynamic -> handle_dynamic). > > This patch adds new options to eu-readelf (-D, --use-dynamic) > for (-d, --dynamic). > > https://sourceware.org/bugzilla/show_bug.cgi?id=28873 This looks pretty good. But there are lots of white-space issues which make it hard to apply the patch. Could you sent it again preferrably using git send-email ? > Signed-off-by: Di Chen > --- > src/readelf.c | 168 +++++++++++++++++++++++++++++++++++++--- > tests/Makefile.am | 3 +- > tests/run-readelf-Dd.sh | 65 ++++++++++++++++ > 3 files changed, 223 insertions(+), 13 deletions(-) > create mode 100755 tests/run-readelf-Dd.sh > > diff --git a/src/readelf.c b/src/readelf.c > index 4b6aab2b..e578456b 100644 > --- a/src/readelf.c > +++ b/src/readelf.c > @@ -137,6 +137,8 @@ static const struct argp_option options[] = > { "string-dump", 'p', NULL, OPTION_ALIAS | OPTION_HIDDEN, NULL, 0 }, > { "archive-index", 'c', NULL, 0, > N_("Display the symbol index of an archive"), 0 }, > + { "use-dynamic", 'D', NULL, 0, > + N_("Use the dynamic section info when displaying symbols"), 0 }, This doesn't handle symbols yet. Should it simply say: "Use the dynamic segment when possible for displaying info" > { NULL, 0, NULL, 0, N_("Output control:"), 0 }, > { "numeric-addresses", 'N', NULL, 0, > @@ -195,6 +197,9 @@ static bool print_symbol_table; > /* True if (only) the dynsym table should be printed. */ > static bool print_dynsym_table; > > +/* True if reconstruct dynamic symbol table from the PT_DYNAMIC segment. > */ > +static bool use_dynamic_segment; > + > /* A specific section name, or NULL to print all symbol tables. */ > static char *symbol_table_section; > > @@ -268,6 +273,19 @@ static enum section_e > | section_macro | section_addr | section_types) > } print_debug_sections, implicit_debug_sections; > > +enum dyn_idx > +{ > + i_strsz, > + i_verneed, > + i_verdef, > + i_versym, > + i_symtab, > + i_strtab, > + i_hash, > + i_gnu_hash, > + i_max > +}; Maybe move this just before it is used in declarations of the get_dyn... functions? > /* Select hex dumping of sections. */ > static struct section_argument *dump_data_sections; > static struct section_argument **dump_data_sections_tail = > &dump_data_sections; > @@ -318,6 +336,11 @@ static void dump_strings (Ebl *ebl); > static void print_strings (Ebl *ebl); > static void dump_archive_index (Elf *, const char *); > > +/* Declarations of local functions for use-dynamic. */ > +static Elf_Data * get_dynscn_strtab(Elf *elf, GElf_Phdr *phdr); > +static void get_dynscn_addrs(Elf *elf, GElf_Phdr *phdr, GElf_Addr > addrs[i_max]); > +static void find_offsets(Elf *elf, GElf_Addr main_bias, size_t n, > + GElf_Addr addrs[n], GElf_Off offs[n]); I mean, just before these. > /* Looked up once with gettext in main. */ > static char *yes_str; > @@ -429,6 +452,9 @@ parse_opt (int key, char *arg, > print_dynamic_table = true; > any_control_option = true; > break; > + case 'D': > + use_dynamic_segment = true; > + break; > case 'e': > print_debug_sections |= section_exception; > any_control_option = true; > @@ -1791,7 +1817,7 @@ get_dyn_ents (Elf_Data * dyn_data) OK, setting flag. > > static void > -handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr) > +handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr, GElf_Phdr *phdr) > { > int class = gelf_getclass (ebl->elf); > GElf_Shdr glink_mem; > @@ -1802,13 +1828,20 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr > *shdr) > size_t dyn_ents; > > /* Get the data of the section. */ > - data = elf_getdata (scn, NULL); > + if (use_dynamic_segment) > + data = elf_getdata_rawchunk( > + ebl->elf, phdr->p_offset, phdr->p_filesz, ELF_T_DYN); > + else > + data = elf_getdata (scn, NULL); > + > if (data == NULL) > return; > > /* Get the dynamic section entry number */ > dyn_ents = get_dyn_ents (data); > > + if (!use_dynamic_segment) > +{ > /* Get the section header string table index. */ > if (unlikely (elf_getshdrstrndx (ebl->elf, &shstrndx) < 0)) > error_exit (0, _("cannot get section header string table index")); > @@ -1828,8 +1861,25 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr > *shdr) > shdr->sh_offset, > (int) shdr->sh_link, > elf_strptr (ebl->elf, shstrndx, glink->sh_name)); > +} else { > + printf (ngettext ("\ > +\nDynamic segment contains %lu entry:\n Addr: %#0*" PRIx64 " Offset: > %#08" PRIx64 "\n", > + "\ > +\nDynamic segment contains %lu entries:\n Addr: %#0*" PRIx64 " Offset: > %#08" PRIx64 "\n", > + dyn_ents), > + (unsigned long int) dyn_ents, > + class == ELFCLASS32 ? 10 : 18, phdr->p_paddr, > + phdr->p_offset); > +} > + > fputs_unlocked (_(" Type Value\n"), stdout); > > + /* if --use-dynamic option is enabled, > + use the string table to get the related library info. */ > + Elf_Data *strtab_data = NULL; > + if (use_dynamic_segment) > + strtab_data = get_dynscn_strtab(ebl->elf, phdr); > + > for (cnt = 0; cnt < dyn_ents; ++cnt) > { > GElf_Dyn dynmem; > @@ -1841,6 +1891,12 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr > *shdr) > printf (" %-17s ", > ebl_dynamic_tag_name (ebl, dyn->d_tag, buf, sizeof (buf))); > > + char *lib_name = NULL; > + if (use_dynamic_segment) > + lib_name = ((char *)strtab_data->d_buf) + dyn->d_un.d_ptr; I think strtab_data can be NULL here (if get_dynscn_strtab failed) and dyn->d_un.d_ptr could be beyond the end of data (if the ELF file is bogus). > + else > + lib_name = elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val); > + > switch (dyn->d_tag) > { > case DT_NULL: > @@ -1852,23 +1908,19 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr > *shdr) > break; > > case DT_NEEDED: > - printf (_("Shared library: [%s]\n"), > - elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val)); > + printf (_("Shared library: [%s]\n"), lib_name); > break; > > case DT_SONAME: > - printf (_("Library soname: [%s]\n"), > - elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val)); > + printf (_("Library soname: [%s]\n"),lib_name); > break; > > case DT_RPATH: > - printf (_("Library rpath: [%s]\n"), > - elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val)); > + printf (_("Library rpath: [%s]\n"),lib_name); > break; > > case DT_RUNPATH: > - printf (_("Library runpath: [%s]\n"), > - elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val)); > + printf (_("Library runpath: [%s]\n"), lib_name); > break; OK. > case DT_PLTRELSZ: > @@ -1942,8 +1994,8 @@ print_dynamic (Ebl *ebl) > Elf_Scn *scn = gelf_offscn (ebl->elf, phdr->p_offset); > GElf_Shdr shdr_mem; > GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem); > - if (shdr != NULL && shdr->sh_type == SHT_DYNAMIC) > - handle_dynamic (ebl, scn, shdr); > + if (use_dynamic_segment || (shdr != NULL && shdr->sh_type == > SHT_DYNAMIC)) > + handle_dynamic (ebl, scn, shdr, phdr); > break; Should you check that phdr != NULL here? > } > } > @@ -4801,6 +4853,98 @@ print_ops (Dwfl_Module *dwflmod, Dwarf *dbg, int > indent, int indentrest, > } > > > +static void > +find_offsets(Elf *elf, GElf_Addr main_bias, size_t n, > + GElf_Addr addrs[n], GElf_Off offs[n]) > +{ > + size_t unsolved = n; > + for (size_t i = 0; i < phnum; ++i) { > + GElf_Phdr phdr_mem; > + GElf_Phdr *phdr = gelf_getphdr(elf, i, &phdr_mem); > + if (phdr != NULL && phdr->p_type == PT_LOAD && phdr->p_memsz > 0) > + for (size_t j = 0; j < n; ++j) > + if (offs[j] == 0 && addrs[j] >= phdr->p_vaddr + main_bias && > + addrs[j] - (phdr->p_vaddr + main_bias) < phdr->p_filesz) { > + offs[j] = addrs[j] - (phdr->p_vaddr + main_bias) + > phdr->p_offset; > + if (--unsolved == 0) > + break; > + } > + } > +} OK, so this turns the addresses into file offsets using the phdrs. It is slightly tricky, so I wouldn't mind a comment explaining what is going on. > +/* The dynamic segment (type PT_DYNAMIC), contains the .dynamic section. > + And .dynamic section contains an array of the dynamic structures. > + We use the array to get: > + DT_STRTAB: the address of the string table. > + DT_SYMTAB: the address of the symbol table. > + DT_STRSZ: the size, in bytes, of the string table. > + , and etc. */ > +static void > +get_dynscn_addrs(Elf *elf, GElf_Phdr *phdr, GElf_Addr addrs[i_max]) > +{ > + Elf_Data *data = elf_getdata_rawchunk( > + elf, phdr->p_offset, phdr->p_filesz, ELF_T_DYN); > + > + int dyn_idx = 0; > + for (;; ++dyn_idx) { > + GElf_Dyn dyn_mem; > + GElf_Dyn *dyn = gelf_getdyn(data, dyn_idx, &dyn_mem); > + /* DT_NULL Marks end of dynamic section. */ > + if (dyn->d_tag == DT_NULL) > + break; > + > + switch (dyn->d_tag) { > + case DT_SYMTAB: > + addrs[i_symtab] = dyn->d_un.d_ptr; > + break; > + > + case DT_HASH: > + addrs[i_hash] = dyn->d_un.d_ptr; > + break; > + > + case DT_GNU_HASH: > + addrs[i_gnu_hash] = dyn->d_un.d_ptr; > + break; > + > + case DT_STRTAB: > + addrs[i_strtab] = dyn->d_un.d_ptr; > + break; > + > + case DT_VERSYM: > + addrs[i_versym] = dyn->d_un.d_ptr; > + break; > + > + case DT_VERDEF: > + addrs[i_verdef] = dyn->d_un.d_ptr; > + break; > + > + case DT_VERNEED: > + addrs[i_verneed] = dyn->d_un.d_ptr; > + break; > + > + case DT_STRSZ: > + addrs[i_strsz] = dyn->d_un.d_val; > + } > + } > + return; > +} Note that the return here is redundant and could be left off. > +/* Use dynamic segment to get data for the string table section. */ > +static Elf_Data * > +get_dynscn_strtab(Elf *elf, GElf_Phdr *phdr) > +{ > + Elf_Data *strtab_data; > + GElf_Addr addrs[i_max] = {0,}; > + GElf_Off offs[i_max] = {0,}; > + get_dynscn_addrs(elf, phdr, addrs); > + find_offsets(elf, 0, i_max, addrs, offs); > + strtab_data = elf_getdata_rawchunk( > + elf, offs[i_strtab], addrs[i_strsz], ELF_T_BYTE); > + return strtab_data; > +} OK. > + > struct listptr > { > Dwarf_Off offset:(64 - 3); > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 84c3950a..b239a461 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -197,7 +197,8 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh > newfile test-nlist \ > msg_tst system-elf-libelf-test \ > $(asm_TESTS) run-disasm-bpf.sh run-low_high_pc-dw-form-indirect.sh \ > run-nvidia-extended-linemap-libdw.sh > run-nvidia-extended-linemap-readelf.sh \ > - run-readelf-dw-form-indirect.sh run-strip-largealign.sh > + run-readelf-dw-form-indirect.sh run-strip-largealign.sh \ > + run-readelf-Dd.sh Don't forget to add run-readelf-Dd.sh to EXTRA_DISTS. (make distcheck can be used to check for that) > if !BIARCH > export ELFUTILS_DISABLE_BIARCH = 1 > diff --git a/tests/run-readelf-Dd.sh b/tests/run-readelf-Dd.sh > new file mode 100755 > index 00000000..6420a0fe > --- /dev/null > +++ b/tests/run-readelf-Dd.sh > @@ -0,0 +1,65 @@ > +#! /bin/sh > +# Copyright (C) 2022 Red Hat, Inc. > +# This file is part of elfutils. > +# > +# This file is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# elfutils is distributed in the hope that it will be useful, but > +# WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +. $srcdir/test-subr.sh > + > +# #include > +# > +# __thread int i; > +# > +# void print_i () > +# { > +# printf("%d\n", i); > +# } > +# > +# gcc -fPIC -shared -o testlib_dynseg.so testlib_dynseg.c > +# With ld --version > +# GNU gold (GNU Binutils 2.22.52.20120402) 1.11 > + > +testfiles testlib_dynseg.so Maybe add a comment saying the same testfile is used in run-readelf-d.sh > +testrun_compare ${abs_top_builddir}/src/readelf -Dd testlib_dynseg.so > <<\EOF > + > +Dynamic segment contains 23 entries: > + Addr: 0x00000000000017e0 Offset: 0x0007e0 > + Type Value > + PLTGOT 0x00000000000019c8 > + PLTRELSZ 72 (bytes) > + JMPREL 0x0000000000000568 > + PLTREL RELA > + RELA 0x00000000000004d8 > + RELASZ 144 (bytes) > + RELAENT 24 (bytes) > + RELACOUNT 1 > + SYMTAB 0x0000000000000228 > + SYMENT 24 (bytes) > + STRTAB 0x0000000000000360 > + STRSZ 190 (bytes) > + GNU_HASH 0x0000000000000420 > + NEEDED Shared library: [libc.so.6] > + NEEDED Shared library: [ld-linux-x86-64.so.2] > + INIT 0x00000000000005b0 > + FINI 0x0000000000000748 > + VERSYM 0x0000000000000460 > + VERDEF 0x000000000000047c > + VERDEFNUM 1 > + VERNEED 0x0000000000000498 > + VERNEEDNUM 2 > + NULL > +EOF > + > +exit 0 > -- > 2.35.1