[PATCH] D60250: [llvm-objdump] Allow -dynamic-reloc on ET_EXEC files

Tue Apr 16 02:56:39 PDT 2019

jhenderson added inline comments.

================
Comment at: llvm/test/tools/llvm-objdump/dynamic-reloc-dyn.test:11
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
----------------
PkmX wrote:
> jhenderson wrote:
> > To show that the ELF type isn't important, I'd be tempted to change this to either ET_EXEC, or possibly some arbitrary value e.g. 0x1234. Same with the other test. I think the two tests should use the same ELF type.
> I'm in favor of using previous revision's way of running this test twice with `ET_DYN` and with `s/ET_DYN/ET_EXEC/`.  The other test can also be modified to also run with `ET_DYN` to show that `llvm-objdump` will indeed error out in the absence of dynamic sections even if the ELF is `ET_DYN`.
> 
> I'm not too sure about constructing arbitrary `e_type` for the test which seems like a precedent in `llvm-objdump`'s tests. Perhaps `ET_NONE` can be used if we are going this way.
ET_NONE works for me too. The main bit I want to emphasise is that the ELF type is irrelevant to the test.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60250