[PATCH] D69041: [yaml2obj][obj2yaml] - Do not create a symbol table by default.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added inline comments.


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:209-210
+    // That means we want to create the symbol table for them.
+    if (D->Type == llvm::ELF::SHT_REL || D->Type == llvm::ELF::SHT_RELA ||
+        D->Type == llvm::ELF::SHT_LLVM_ADDRSIG)
+      if (!Doc.Symbols && D->Link.empty())
----------------
I think SHT_HASH, SHT_GROUP, and SHT_SYMTAB_SHNDX might also belong here, per: http://www.sco.com/developers/gabi/latest/ch4.sheader.html#sh_link

At that point, it might look cleaner to refactor out the list as a separate variable that is checked for list/set containment


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:214-218
+    // Also sections might refer to the symbol table explicitly via Link.
+    // In this case we also must provide it, even when the YAML document does
+    // not have the "Symbols" tag.
+    if (!Doc.Symbols && D->Link == ".symtab")
+      Doc.Symbols = std::vector<ELFYAML::Symbol>();
----------------
This case is interesting, partly because IIRC we raise an error if someone specifies a link to a section that does not exist, so special casing this seems unexpected. However, I imagine many tests would need to be fixed and it's maybe not worth it -- have you checked how many tests would fail if you didn't special case this?


================
Comment at: lib/ObjectYAML/ELFEmitter.cpp:231
+    ImplicitSections.push_back(".symtab");
+  ImplicitSections.push_back(".strtab");
+  ImplicitSections.push_back(".shstrtab");
----------------
Is `.strtab` also something we can drop if `.symtab` doesn't exist?


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

https://reviews.llvm.org/D69041