[PATCH] D30419: [ELF] - Define __bss_start symbol.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

You don't really need three tests because in this feature you don't really need to (re-)verify that our section merging logic is working fine. Just reduce it to one test.



================
Comment at: ELF/Writer.cpp:849
 
+  // __bss_start is the location of .bss.* sections.
+  ElfSym::BssStart =
----------------
rafael wrote:
> just say ".bss". It is the name of the output section.
> 
Yes, please. "... is the location of .bss section."


================
Comment at: ELF/Writer.cpp:1682-1683
 
+  if (OutputSection *Bss = findSection(".bss"))
+    Set(ElfSym::BssStart, nullptr, Bss, 0);
+
----------------
You shouldn't use Set. Just `ElfSym::BssStart->Section = findSection(".bss");` should suffice.


https://reviews.llvm.org/D30419