[PATCH] D67078: [llvm-readelf] - Allow dumping the dynamic symbols when there is no program headers.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I don't have any particular strong feelings either way on the discussion in the comments, so I've just made a number of suggested minor comment fixes.



================
Comment at: test/tools/llvm-readobj/dyn-symbols.test:166
+
+## Check that we are able to dump dynamic symbols table even when we have no program headers.
+## In this case we find the table by it's type (SHT_DYNSYM) and ignore the DT_SYMTAB value.
----------------
dump dynamic symbols table -> dump the dynamic symbol table


================
Comment at: test/tools/llvm-readobj/dyn-symbols.test:192
+## Check we report a warning when there is no SHT_DYNSYM section and we can't map the DT_SYMTAB value
+## to address because of absence of the corresponding PT_LOAD program header.
+
----------------
to address -> to an address
of absence -> of the absence
of the corresponding -> of a corresponding


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:1648
+      // Normally we find the dynamic symbols section location and size using
+      // the information in sections headers. That allows us to dump dynamic
+      // symbols when program headers are absent. This is consistent with GNU
----------------
That -> Doing this


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:1649
+      // the information in sections headers. That allows us to dump dynamic
+      // symbols when program headers are absent. This is consistent with GNU
+      // readelf and seems reasonable because there is no DT_SYMTABSZ tag exist,
----------------
This -> This behaviour


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:1650
+      // symbols when program headers are absent. This is consistent with GNU
+      // readelf and seems reasonable because there is no DT_SYMTABSZ tag exist,
+      // i.e. the only way to get the size of the dynamic symbols table is to
----------------
Remove "exist"


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:1651
+      // readelf and seems reasonable because there is no DT_SYMTABSZ tag exist,
+      // i.e. the only way to get the size of the dynamic symbols table is to
+      // take it from section headers. But we still need the code below for
----------------
symbols -> symbol


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:1652
+      // i.e. the only way to get the size of the dynamic symbols table is to
+      // take it from section headers. But we still need the code below for
+      // specific cases, for example when we want to dump the dynamic
----------------
But we -> However, we


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:1654
+      // specific cases, for example when we want to dump the dynamic
+      // relocations and there is no sections table.
+      if (!DynSymRegion.EntSize) {
----------------
sections table -> section table


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67078/new/

https://reviews.llvm.org/D67078