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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Thanks for the patch! I have thought about this problem previously. I don't think we should magically call

  if (getHeader().e_type == ELF::ET_CORE)
    createFakeSections();

in the constructor. Some users may not expect the magic behavior. Having `createFakeSections` in ELFFile seems fine. `createFakeSections` should be called by llvm-objdump (for not just -d, also -h, etc).

This needs a test. I suggest `llvm/test/tools/llvm-objdump/X86/disassemble-no-section.test`. Consider adding a `-h` test too just to show the effect.

Use `ninja check-llvm-tools-llvm-objdump` (during development) to run tests in `llvm/test/tools/llvm-objdump`.

Use `check-llvm` (before updating a patch) to run all llvm tests, as sometimes code may affect faraway component in a non-intuitive way.

> Context not available.

See https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface "as much context as possible"



================
Comment at: llvm/include/llvm/Object/ELF.h:772
 
+/// This function creates fake section headers from program headers.
+/// This is for linux perf tools because it copies a part of kcore
----------------
In binutils, BFD synthesizes more sections, but for disassembly purpose just the executable ones are sufficient. I think the code is fine.

The comment I am thinking is:

Used by llvm-objdump -d (which needs sections for disassembly) to disassemble objects without a section header table (e.g. ET_CORE objects analyzed by linux perf).


================
Comment at: llvm/include/llvm/Object/ELF.h:787
+      continue;
+    NumFakeSec++;
+  }
----------------
pre-increment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128705