Re: [PATCH] PR28873 - Implement eu-readelf -D

[Mark Wielaard <mark@klomp.org>]

Hi,

On Sat, 2023-02-11 at 00:17 +0800, Di Chen via Elfutils-devel wrote:
> From bdc19de94bff8f8812611b9ba8c0116a650d0fb5 Mon Sep 17 00:00:00 2001
> From: Di Chen <dichen@redhat.com>
> Date: Fri, 13 Jan 2023 20:12:43 +0800
> Subject: [PATCH] readelf: display dynamic symtab without section headers
> 
> This commit adds a new option "-D/--use-dynamic" to support printing the
> dynamic symbol table from the PT_DYNAMIC segment. By using the
> PT_DYNAMIC segment, eu-readelf can go through the contents of dynamic
> section entries and the values of each tag. From that, we can get the
> address and size of the dynamic symbol table, the address of the string
> table, etc.
> 
> By using the new option "-D/--use-dynamic", eu-readelf can list the
> symbols without section headers.
> 
> Example:
>   $ ./src/readelf -Ds a.out
>       0: 0000000000000000      0 NOTYPE  LOCAL  DEFAULT    UNDEF
>       1: 0000000000000000      0 FUNC    GLOBAL DEFAULT    UNDEF
> __libc_start_main@GLIBC_2.34 (2)
>       2: 0000000000000000      0 NOTYPE  WEAK   DEFAULT    UNDEF
> __gmon_start__
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=28873
> 
> Signed-off-by: Di Chen <dichen@redhat.com>

Very nice.

Note that my local build (gcc 12.2.1) says:

In function ‘handle_dynamic_symtab’,
    inlined from ‘print_symtab’ at
/home/mark/src/elfutils/src/readelf.c:2451:9:
/home/mark/src/elfutils/src/readelf.c:2535:29: error: ‘syments’ may be
used uninitialized [-Werror=maybe-uninitialized]
 2535 |   if (offs[i_gnu_hash] != 0 && syments == 0) {
      |       ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
/home/mark/src/elfutils/src/readelf.c: In function ‘print_symtab’:
/home/mark/src/elfutils/src/readelf.c:2519:10: note: ‘syments’ was
declared here
 2519 |   size_t syments;
      |          ^~~~~~~

And I think it is correct. We have 3 different ways of trying to set
syments. We should initialize syments to zero so each check knows
whether a previous one succeeded or not.

I have not been very picky about missing ChangeLog entries, but in this
case it would really help understanding what was changed.

Specifically it would be nice to note that the code in
handle_dynamic_symtab to calculate the syments was lifted from
libdwfl/dwfl_module_getdwarf.c.

Lets keep the indentation GNU style, like in dwfl_module_getdwarf.c, I
don't think the new indentation is helpful.

Finally the actual printing of the symbols and processing the version
information in handle_dynamic_symtab look like they were mostly copied
from handle_symtab. Can that code be shared?

Thanks,

Mark