[PATCH] D128705: [llvm-objdump] Create fake sections for a ELF core file

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM when @jhenderson feels ok.



================
Comment at: llvm/include/llvm/Object/ELF.h:188
+  void createFakeSections();
+  std::unique_ptr<Elf_Shdr[]> FakeSec;
+  int NumFakeSec;
----------------
namhyung wrote:
> jhenderson wrote:
> > I think this would be better spelt "FakeSections" without the abbreviation, primarily since it is plural.
> Ok, will change.
Move this together with the other member variable `StringRef Buf`.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-no-section.test:8
+# RUN: llvm-objdump -d %t | FileCheck %s
+
+# CHECK:       <>:
----------------
Consider adding the two lines
```
# CHECK:       Disassembly of section :
# CHECK-EMPTY:
```


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-no-section.test:18
+
+# RUN: llvm-objdump -d --start-address=0xffffffff00000000 \
+# RUN:   --stop-address=0xffffffff00000004 %t | \
----------------
There is a warning (may be spurious) but you don't need to fix it in this patch.

Use `2>&1 | ` and test the warning to catch changes when someone fixes it.


================
Comment at: llvm/test/tools/llvm-objdump/X86/disassemble-no-section.test:22
+
+# RANGE:       55                    pushq   %rbp
+# RANGE-NEXT:  48 89 e5              movq    %rsp, %rbp
----------------
Better to add needed anchor lines (like `<>:` you added for the test above) before `55 pushq %rbp`.

It may not look great, but the intention is to catch display change which others may improve.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1134
 
-static void disassembleObject(const Target *TheTarget, const ObjectFile *Obj,
+static void createFakeELFSections(ObjectFile *Obj) {
+  assert(Obj->isELF());
----------------
For non-null pointers, the convention in llvm-project is to use references. The llvm-objdump code traditionally did not follow the best practice. I just pushed a commit to migrate a bunch of functions. You will need to rebase and change these to references.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128705