Thanks Mark,
All the request changes are fixed, ready for review again.
1. help message updated: "Use the dynamic segment when possible for
displaying info"
2. move enum dyn_idx to a proper place
3. add strtab_data's NULL check in function: handle_dynamic()
4. add phdr's NULL check in function: print_dynamic()
5. add comments for function: find_offsets()
6. remove redundant return-statement in function: get_dynscn_addrs()
7. add run-readelf-Dd.sh to EXTRA_DISTS
8. check strsz in (dyn->d_un.d_ptr < strtab_data->d_size) in function:
handle_dynamic()
On Fri, May 20, 2022 at 8:41 AM Mark Wielaard wrote:
> 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, phd
