[PATCH] D22811: [ELF] Linkerscript: allow setting custom output section for common symbols, instead of .bss

[Rui Ueyama via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM with the following changes.


================
Comment at: ELF/InputSection.cpp:674
@@ +673,3 @@
+// to InputSection<ELFT> constructor
+template <class T> static T *zero(T *val) {
+  return static_cast<T *>(memset(val, 0, sizeof(*val)));
----------------
val -> Val

================
Comment at: ELF/InputSection.cpp:681
@@ +680,3 @@
+    : InputSection<ELFT>(nullptr, zero(&Hdr)) {
+
+  std::vector<DefinedCommon<ELFT> *> Symbols;
----------------
Remove the blank line.

================
Comment at: ELF/LinkerScript.cpp:111
@@ +110,3 @@
+
+  if ((llvm::find(Patterns, "COMMON") != Patterns.end()) && Common->getSize())
+    Ret.push_back(Common);
----------------
Do you need to check the size of the common input section? It seems that you can safely add an empty section to the output section. If so, please remove that check.

================
Comment at: ELF/OutputSections.cpp:1451
@@ -1450,3 +1450,3 @@
   case SymbolBody::DefinedCommonKind:
-    return Out<ELFT>::Bss;
+    return cast<DefinedCommon<ELFT>>(Sym)->CS->OutSec;
   case SymbolBody::SharedKind:
----------------
CS should be named `Section` for consistency with DefiendRegular class.

================
Comment at: ELF/Writer.cpp:741
@@ +740,3 @@
+  // then add it to .bss now.
+  if (!CommonSection->OutSec && CommonSection->getSize()) {
+    Out<ELFT>::Bss->addSection(CommonSection.get());
----------------
Remove the size check if not necessary.


https://reviews.llvm.org/D22811