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

[Namhyung Kim via Phabricator via llvm-commits]

namhyung added a comment.

> If this is available in GNU objdump, could you post the version it was added to, please?

I don't know when it's added, it's hard to find..  But my version is 2.38.

  namhyung at youngsil:llvm-project$ objdump --version
  GNU objdump (GNU Binutils for Debian) 2.38
  Copyright (C) 2022 Free Software Foundation, Inc.
  This program is free software; you may redistribute it under the terms of
  the GNU General Public License version 3 or (at your option) any later version.
  This program has absolutely no warranty.



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


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


================
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:  {{.}}
----------------
jhenderson wrote:
> 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
Sounds good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131290