[PATCH] D39582: Add ELF dynamic symbol support to yaml2obj/obj2yaml

Dave Lee via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 3 09:43:38 PDT 2017


kastiglione added a comment.

@jakehehrlich thanks for the review! I have more experience with macho/pecoff so I appreciate the insights. I'll update this with tests and something for the section ordering.



================
Comment at: tools/yaml2obj/yaml2elf.cpp:301
   zero(SHeader);
-  SHeader.sh_name = DotShStrtab.getOffset(".symtab");
-  SHeader.sh_type = ELF::SHT_SYMTAB;
-  SHeader.sh_link = getDotStrTabSecNo();
+  bool IsStatic = STType == SymtabType::Static ? true : false;
+  SHeader.sh_name = DotShStrtab.getOffset(IsStatic ? ".symtab" : ".dynsym");
----------------
jakehehrlich wrote:
> Can this just be "bool IsStatic = STType == SymtabType::Static"? Better yet do we really need SymtabType? What if we just pass "IsStatic" as a parameter?
🤦‍♂️

The choice to use the enum was to avoid opaque `true`/`false` arguments at the call site. I'll go with bools if that's preferred.


https://reviews.llvm.org/D39582





More information about the llvm-commits mailing list