[PATCH] D49016: [llvm-objdump] Add dynamic section printing to private-headers option

[Paul Semel via Phabricator via llvm-commits]

paulsemel added a comment.

I haven't copied the whole thing, most of the code is from me actually.. Why can't I just land this one and refactor the other one right after ?
I'm not sure this is that much a big deal to do this way, is it ?

Anyway, this patch is for the moment relying on an other patch I made for yaml2obj, so I'll wait until it is accepted !

NB: fun fact, it seems, that we are doing better in finding the dynamic section than GNU objdump.



================
Comment at: include/llvm/Object/ELF.h:421-422
+template <class ELFT>
+const char *ELFFile<ELFT>::getDynamicTagAsString(unsigned Arch,
+                                                 uint64_t Type) const {
+#define DYNAMIC_TAG(n, v)
----------------
jhenderson wrote:
> This strikes me as a rather large function to have in a header file. What do you think about moving the body into ELF.cpp source file? It looks like there is precedence for this already. I also noticed that the source provides a "STRINGIFY_ENUM_CASE" macro that does what you want.
No, I can't, because I don't want to output "DT_NEEDED", but "NEEDED"...


================
Comment at: test/tools/llvm-objdump/private-headers-no-dynamic.test:18
+
+# CHECK: Dynamic Section:
----------------
jhenderson wrote:
> Maybe worth doing a `CHECK-NOT: DT_` or similar to show that the tool isn't trying to dump garbage in this cae.
I though about it, but there is no pattern I can check.. (as I am not printing the DT_ thing)


================
Comment at: tools/llvm-objdump/ELFDump.cpp:42
+
+  for (const typename ELFO::Elf_Phdr &Phdr : *ProgramHeaderOrError) {
+    if (Phdr.p_type == ELF::PT_LOAD) {
----------------
jhenderson wrote:
> I just realised that you're reinventing the wheel again here. llvm-readobj has the same function to do the same sort of thing. Take a look at `toMappedAddr` in `parseDynamicTable` in the ElfDumper class. It probably is worth sharing this again, so as to not repeat the code, and risk getting it slightly wrong.
I move this toMappedAddr function to libObj, as I see a lot of use cases for it :)


Repository:
  rL LLVM

https://reviews.llvm.org/D49016