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

[Mark Wielaard]

Hi,

On Sun, Apr 02, 2023 at 09:06:11AM +0800, Di Chen wrote:
> I made the following changes:
> * ChangeLog&NEWS update.
> * syments initialization to 0.
> * new function format update to GNU style.
> * extract the shared part to a new function, reduce code redundancy.
> 
> The patch is ready for review again.

Looks really good. I had a few small nitpicks (see attached), but just
made those changes and checked it in.

Thanks a lot for all this work. This was way more complicated than I
expected at first. I am really happy you didn't get discouraged by
that.

Cheers,

Mark
diff --git a/ChangeLog b/ChangeLog
index ece07e97..6aed95b6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,7 +1,7 @@
 2023-03-27  Di Chen  <dic...@redhat.com>
 
        * NEWS: Support readelf -Ds for using dynamic segment to
-    print symbol table.
+       print symbol table.
 
 2023-03-03  Mark Wielaard  <m...@klomp.org>
 
diff --git a/NEWS b/NEWS
index 679d8bd5..3c63a660 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+Version 0.190
+
+readelf: Support readelf -Ds, --use-dynamic --symbol.
+
 Version 0.189 "Don't deflate!"
 
 configure: eu-nm, eu-addr2line and eu-stack can provide demangled symbols
@@ -17,8 +21,6 @@ elfcompress: -t, --type= now support zstd if libelf has been 
build with
 
 backends: Add support for LoongArch and Synopsys ARCv2 processors.
 
-readelf: Support readelf -Ds, --use-dynamic --symbol.
-
 Version 0.188 "no section left behind"
 
 readelf: Add -D, --use-dynamic option.
diff --git a/src/ChangeLog b/src/ChangeLog
index 8aa269cd..ae62f4ed 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -3,6 +3,16 @@
        * readelf.c (options): Support dynamic symtab print with '-Ds'.
        (process_symtab): New function.
        (handle_dynamic_symtab): Likewise.
+       (handle_symtab): Return true if symtab was printed. Move printing
+       code to process_symtab and call that function.
+       (print_symtab): Call handle_dynamic_symtab for SHT_DYNSYM when
+       using dynamic segment.
+       (enum dyn_idx): Include i_symtab_shndx, i_verneednum and
+       i_verdefnum.
+       (process_elf_file): Only call print_symtab for SHT_SYMTAB when not
+       use_dynamic_segment.
+       (get_dynscn_addrs): Handle DT_VERDEFNUM, DT_VERNEEDNUM and
+       DT_SYMTAB_SHNDX.
 
 2023-03-03  Mark Wielaard  <m...@klomp.org>
 
diff --git a/src/readelf.c b/src/readelf.c
index 70848c21..e717954d 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -2462,9 +2462,7 @@ print_symtab (Ebl *ebl, int type)
 {
   /* Use the dynamic section info to display symbol tables.  */
   if (use_dynamic_segment && type == SHT_DYNSYM)
-  {
-       return handle_dynamic_symtab(ebl);
-  }
+    return handle_dynamic_symtab(ebl);
 
   /* Find the symbol table(s).  For this we have to search through the
      section table.  */
@@ -2778,17 +2776,15 @@ handle_symtab (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
 static bool
 handle_dynamic_symtab (Ebl *ebl)
 {
+  GElf_Phdr phdr_mem;
   GElf_Phdr *phdr = NULL;
-  /* phnum is a static variable which already fetched in function
-   * process_elf_file.  */
+  /* phnum is a static variable which was already fetched in function
+     process_elf_file.  */
   for (size_t i = 0; i < phnum; ++i)
     {
-      GElf_Phdr phdr_mem;
       phdr = gelf_getphdr (ebl->elf, i, &phdr_mem);
       if (phdr->p_type == PT_DYNAMIC)
-        {
-          break;
-        }
+       break;
     }
   if (phdr == NULL)
     return false;
@@ -5223,6 +5219,7 @@ get_dynscn_addrs(Elf *elf, GElf_Phdr *phdr, GElf_Addr 
addrs[i_max])
     case DT_STRSZ:
       addrs[i_strsz] = dyn->d_un.d_val;
       break;
+
     case DT_SYMTAB_SHNDX:
       addrs[i_symtab_shndx] = dyn->d_un.d_ptr;
       break;