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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/Object/ELF.h:778
 
-  for (auto Phdr : *PhdrsOrErr) {
-    if (!(Phdr.p_type & ELF::PT_LOAD) || !(Phdr.p_flags & ELF::PF_X))
+  FakeSectionStrings.append(StringRef("\0", 1));
+  for (auto [Idx, Phdr] : llvm::enumerate(*PhdrsOrErr)) {
----------------
namhyung wrote:
> jhenderson wrote:
> > namhyung wrote:
> > > jhenderson wrote:
> > > > Do you actually need this?
> > > Yep, Idx is a part of the section name.
> > I wasn't referring to MaskRay's suggestion. I was referring to the line that's actually highlighted with this comment (i.e. line 778 in this diff).
> Oh, I see.  Maybe it's not needed but I think it's a good practice to maintain offset 0 as an empty string.  Do you want me to remove it?
`FakeSectionStrings += '\0'` or use push_back.


================
Comment at: llvm/test/Object/objdump-no-sectionheaders.test:1
-; RUN: llvm-objdump -h %p/Inputs/no-sections.elf-x86-64 \
-; RUN:              | FileCheck %s
+## This test checks llvm-objdump -h can handle ELF files without section info.
+## Only PT_LOAD segments with the PF_X flag will be displayed as fake sections.
----------------
Nit: "This test " can be omitted. Use an imperative sentence.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131290