[PATCH] D131290: [llvm-objdump] Create name for fake sections

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I was just experimenting with an admittedly old version of GNU objdump on my machine and that just tells me that there are no sections in the file and doesn't do any disassembly. It's possible it's just because it's that old version (I don't have a more recent version available currently). If this is available in GNU objdump, could you post the version it was added to, please?

That said, I think there is benefit from this regardless of the parity with GNU.



================
Comment at: llvm/include/llvm/Object/ELF.h:781
 
+  int Idx = 0;
+  llvm::StringTableBuilder StrTab(llvm::StringTableBuilder::ELF);
----------------
This probably wants to be a size_t since it's an index.


================
Comment at: llvm/include/llvm/Object/ELF.h:782
+  int Idx = 0;
+  llvm::StringTableBuilder StrTab(llvm::StringTableBuilder::ELF);
   for (auto Phdr : *PhdrsOrErr) {
----------------
Not sure you need the `llvm::` prefixes?


================
Comment at: llvm/include/llvm/Object/ELF.h:794
     FakeShdr.sh_offset = Phdr.p_offset;
+    // Create a section name based on the p_type and index
+    FakeShdr.sh_name = StrTab.add("PT_LOAD#" + std::to_string(Idx));
----------------



================
Comment at: llvm/test/Object/objdump-no-sectionheaders.test:1-7
 ; RUN: llvm-objdump -h %p/Inputs/no-sections.elf-x86-64 \
 ; RUN:              | FileCheck %s
 
 ; CHECK:      Sections:
 ; CHECK-NEXT: Idx Name          Size     VMA              Type
-; CHECK-NEXT:   0               000006ec 0000000000400000 TEXT
-; CHECK-NEXT:   1               00000000 0000000000000000 TEXT
+; CHECK-NEXT:   0 PT_LOAD#2     000006ec 0000000000400000 TEXT
 ; CHECK-NOT:  {{.}}
----------------
I'd strongly support replacing the pre-canned ELF with a YAML input in this test. At the moment, I have no idea if the behaviour is correct without digging out a binary inspection tool to look at the ELF object.

Some key characteristics I think that need testing:
# That non-PT_LOAD segments are ignored (but still count towards the index), even if they have PF_X.
# That non-PF_X segments are ignored (but still count towards the index).
# That multiple PT_LOAD wiht PF_X segments can be displayed.
To achieve these points, I think it would be sufficient to have an ELF with the following program headers:

# PT_PHDR (or other PT_* type) with PF_X
# PT_LOAD with PF_X
# PT_LOAD without PF_X
# PT_LOAD with PF_X


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131290