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

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 4 05:19:32 PDT 2017


grimar added a comment.

In https://reviews.llvm.org/D30419#714407, @ruiu wrote:

> 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.


Done, though FYI AFAIK we do not have any test that chechs .bss merging logic. I can probably prepare testcases and push on review (probably based on testcases that were done in this patch).
Do we want it ?



================
Comment at: ELF/Writer.cpp:1682-1683
 
+  if (OutputSection *Bss = findSection(".bss"))
+    Set(ElfSym::BssStart, nullptr, Bss, 0);
+
----------------
ruiu wrote:
> You shouldn't use Set. Just `ElfSym::BssStart->Section = findSection(".bss");` should suffice.
That would not work so easy, but updated code works without Set.




https://reviews.llvm.org/D30419





More information about the llvm-commits mailing list