[ELF] - Define __bss_start symbol.

[grimar]

Summary

GNU linkers define __bss_start symbol.
Patch teaches LLD to do that. This is PR32051.

Below is part of standart ld.bfd script:

.data1          : { *(.data1) }
  _edata = .; PROVIDE (edata = .);
  . = .;
  __bss_start = .;
  .bss            :
  {

Currently LLD can emit up to 3 .bss* sections as one of testcase shows.
Implementation inserts this symbol before first .bss* output section.

[ruiu]

Generally looking fine, but Rafael might be working on removing DefinedSynthetic class, so he probably want to take a look.

test/ELF/bss-start.s

16

Do you need this .global?

20–22

You can remove this without changing the meaning of this test, no?

[grimar]

test/ELF/bss-start.s

16

I believe yes. We do not want to add symbol when it is not used, right ?
Declaring it as global adds it as undefined to symbol table and
addOptionalSynthetic() then able to create DefinedSynthetic:

addOptionalSynthetic(StringRef Name, OutputSection *Sec,
                     typename ELFT::uint Val, uint8_t StOther = STV_HIDDEN) {
  if (SymbolBody *S = Symtab<ELFT>::X->find(Name))
    if (!S->isInCurrentDSO())
      return cast<DefinedSynthetic>(
          Symtab<ELFT>::X->addSynthetic(Name, Sec, Val, StOther)->body());
return nullptr;

Otherwize there will be no symbol in output.

20–22

First test intention is not only to check that .bss.rel.ro can be used for _bss_start,
but also to show that _bss_start points to the first .bss section if there are multiple in output.
Lines were intentional.

[grimar]

• Rebased.

[• rafael]

Rui, can you check with whatever is using this what the intended behavior would be when there is a .bss and a .bss.rel.ro? I would guess it wants to see .bss since .bss.rel.ro is ro at runtime.

ELF/Writer.cpp

843

just say ".bss". It is the name of the output section.

[ruiu]

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.

ELF/Writer.cpp

843

Yes, please. "... is the location of .bss section."

1668–1669

You shouldn't use Set. Just ElfSym::BssStart->Section = findSection(".bss"); should suffice.

[grimar]

• Addressed review comments.

[grimar]

In 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 ?

ELF/Writer.cpp

1668–1669

That would not work so easy, but updated code works without Set.

[grimar]

• Addressed review comment properly (previously I changed naming ElfSym::BssStart to ElfSym::Bss,

but did not update comment as was requested, now it is fixed).

[ruiu]

LGTM

ELF/Symbols.h

 };
 
 // Some linker-generated symbols need to be created as
 // DefinedRegular symbols.
 template <class ELFT> struct ElfSym {
   // The content for __ehdr_start symbol.
   static DefinedSynthetic *EhdrStart;
 
+  // The content for __bss_start symbol.
+  static DefinedSynthetic *BssStart;
+
   // The content for _etext and etext symbols.
   static DefinedSynthetic *Etext;
   static DefinedSynthetic *Etext2;
 
   // The content for _edata and edata symbols.
   static DefinedSynthetic *Edata;
   static DefinedSynthetic *Edata2;
 
   // The content for _end and end symbols.
   static DefinedSynthetic *End;
   static DefinedSynthetic *End2;
 
   // The content for _gp_disp/__gnu_local_gp symbols for MIPS target.
   static DefinedRegular<ELFT> *MipsGpDisp;
   static DefinedRegular<ELFT> *MipsLocalGp;
   static DefinedRegular<ELFT> *MipsGp;
 };
 
+template <class ELFT> DefinedSynthetic *ElfSym<ELFT>::BssStart;
 template <class ELFT> DefinedSynthetic *ElfSym<ELFT>::EhdrStart;
 template <class ELFT> DefinedSynthetic *ElfSym<ELFT>::Etext;
 template <class ELFT> DefinedSynthetic *ElfSym<ELFT>::Etext2;
 template <class ELFT> DefinedSynthetic *ElfSym<ELFT>::Edata;
 template <class ELFT> DefinedSynthetic *ElfSym<ELFT>::Edata2;
 template <class ELFT> DefinedSynthetic *ElfSym<ELFT>::End;
 template <class ELFT> DefinedSynthetic *ElfSym<ELFT>::End2;
 template <class ELFT> DefinedRegular<ELFT> *ElfSym<ELFT>::MipsGpDisp;

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::addReservedSymbols() {
   // If linker script do layout we do not need to create any standart symbols.
   if (ScriptConfig->HasSections)
     return;
 
   // __ehdr_start is the location of ELF file headers.
   ElfSym<ELFT>::EhdrStart =
       addOptionalSynthetic<ELFT>("__ehdr_start", Out::ElfHeader, 0);
 
+  // __bss_start is the location of .bss.* sections.
+  ElfSym<ELFT>::BssStart =
+      addOptionalSynthetic<ELFT>("__bss_start", nullptr, 0, STV_DEFAULT);
+
   auto Define = [](StringRef S, DefinedSynthetic *&Sym1,
                    DefinedSynthetic *&Sym2) {
     Sym1 = addOptionalSynthetic<ELFT>(S, nullptr, 0, STV_DEFAULT);
     assert(S.startswith("_"));
     S = S.substr(1);
     Sym2 = addOptionalSynthetic<ELFT>(S, nullptr, 0, STV_DEFAULT);
   };
 
if (Last)
     Set(ElfSym<ELFT>::End, ElfSym<ELFT>::End2, Last->First, Last->p_memsz);
   if (LastRO)
     Set(ElfSym<ELFT>::Etext, ElfSym<ELFT>::Etext2, LastRO->First,
         LastRO->p_filesz);
   if (LastRW)
     Set(ElfSym<ELFT>::Edata, ElfSym<ELFT>::Edata2, LastRW->First,
         LastRW->p_filesz);
 
+  // __bss_start should point to the first .bss.* output section.
+  for (OutputSection *OS : OutputSections) {
+    bool IsBSS = (In<ELFT>::Common && In<ELFT>::Common->OutSec == OS) ||
+                 OS == Out::Bss || OS == Out::BssRelRo;
+    if (!IsBSS)
+      continue;
+    Set(ElfSym<ELFT>::BssStart, nullptr, OS, 0);
+    break;
+  }
+
   // Setup MIPS _gp_disp/__gnu_local_gp symbols which should
   // be equal to the _gp symbol's value.
   if (Config->EMachine == EM_MIPS) {
     if (!ElfSym<ELFT>::MipsGp->Value) {
       // Find GP-relative section with the lowest address
       // and use this address to calculate default _gp value.
       uintX_t Gp = -1;
       for (const OutputSection *OS : OutputSections)

test/ELF/Inputs/bss-start.s

+.rodata
+.globl a
+.size a, 4
+.type a, @object
+a:
+.word 1
+
+.section .data,"aw",%progbits
+.globl b
+.size b, 4
+.type b, @object
+b:
+.word 2

test/ELF/bss-start.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/bss-start.s -o %t2.o
+# RUN: ld.lld -shared %t2.o -o %t.so
+# RUN: ld.lld %t %t.so -o %t2
+# RUN: llvm-objdump -t -section-headers %t2 | FileCheck %s
+
+# CHECK: Sections:
+# CHECK: Idx Name          Size      Address          Type
+# CHECK:   7 .bss.rel.ro   00000004 00000000002020b0 BSS
+# CHECK:   8 .bss          00000004 0000000000203000 BSS
+# CHECK:   9 .bss          00000004 0000000000203004 BSS
+# CHECK: SYMBOL TABLE:
+# CHECK: 00000000002020b0  .bss.rel.ro 00000000 __bss_start
+
+.global __bss_start
+.text
+_start:
+.comm sym1,4,4
+
+ movl $1, a
+ movl $1, b

test/ELF/bss-start2.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/bss-start.s -o %t2.o
+# RUN: ld.lld -shared %t2.o -o %t.so
+# RUN: ld.lld %t %t.so -o %t2
+# RUN: llvm-objdump -t -section-headers %t2 | FileCheck %s
+
+# CHECK: Sections:
+# CHECK: Idx Name          Size      Address          Type
+# CHECK:   7 .bss          00000004 0000000000203000 BSS
+# CHECK: SYMBOL TABLE:
+# CHECK: 0000000000203000  .bss 00000000 __bss_start
+
+.global __bss_start
+.text
+_start:
+ movl $1, b

test/ELF/bss-start3.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: ld.lld %t -o %t2
+# RUN: llvm-objdump -t -section-headers %t2 | FileCheck %s
+
+# CHECK: Sections:
+# CHECK: Idx Name          Size      Address          Type
+# CHECK:   2 .bss          00000004 0000000000201000 BSS
+# CHECK: SYMBOL TABLE:
+# CHECK: 0000000000201000  .bss 00000000 __bss_start
+
+.global __bss_start
+.text
+_start:
+.comm sym1,4,4