[PATCH] D128705: [llvm-objdump] Create fake sections for a ELF core file
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 7 12:28:29 PDT 2022
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
More information about the llvm-commits
mailing list