[PATCH] D62991: [yaml2obj][MachO] Don't fill dummy data for virtual sections

Seiya Nuta via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 10 18:27:51 PDT 2019


seiya marked an inline comment as done.
seiya added inline comments.


================
Comment at: llvm/tools/yaml2obj/yaml2macho.cpp:265
 
+static bool isVirtualSection(uint8_t type) {
+  return (type == MachO::S_ZEROFILL ||
----------------
alexshap wrote:
> I have a couple of comments here:
> 1. in general i think this should be moved out of yaml2macho.cpp into the library (libObject ?)
> because I believe it's not the only place where this might be useful. I think this can be done as a follow up though.
> 2. the test probably can be a bit more robust (and check all these cases at once), but I don't insist. 
I was wondering about the first comment. Some methods like `MachOObjectFile::isSectionVirtual` in libObject are what we need but ObjectYAML employs its own data structure `MachOYAML::Object`.

Adding some macros or inline functions into `include/llvm/BinaryFormat/MachO.h` and refactoring libObject as well as here using them would be better way I suppose. This should be separate patch by the way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62991





More information about the llvm-commits mailing list