[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 ↗

(On Diff #89908)

Do you need this .global?

20–22 ↗

(On Diff #89908)

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

[grimar]

test/ELF/bss-start.s

16 ↗

(On Diff #89908)

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 ↗

(On Diff #89908)

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

849 ↗

(On Diff #93463)

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

849 ↗

(On Diff #93463)

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

1682–1683 ↗

(On Diff #93463)

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

1682–1683 ↗

(On Diff #93463)

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

lld/trunk/ELF/Symbols.h

public:
 
   LazyObjectFile *file() { return (LazyObjectFile *)this->File; }
   InputFile *fetch();
 };
 
 // Some linker-generated symbols need to be created as
 // DefinedRegular symbols.
 struct ElfSym {
+  // The content for __bss_start symbol.
+  static DefinedRegular *Bss;
+
   // The content for _etext and etext symbols.
   static DefinedRegular *Etext;
   static DefinedRegular *Etext2;
 
   // The content for _edata and edata symbols.
   static DefinedRegular *Edata;
   static DefinedRegular *Edata2;
 

lld/trunk/ELF/Symbols.cpp

 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::ELF;
 
 using namespace lld;
 using namespace lld::elf;
 
+DefinedRegular *ElfSym::Bss;
 DefinedRegular *ElfSym::Etext;
 DefinedRegular *ElfSym::Etext2;
 DefinedRegular *ElfSym::Edata;
 DefinedRegular *ElfSym::Edata2;
 DefinedRegular *ElfSym::End;
 DefinedRegular *ElfSym::End2;
 DefinedRegular *ElfSym::MipsGpDisp;
 DefinedRegular *ElfSym::MipsLocalGp;

lld/trunk/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 (Script->Opt.HasSections)
     return;
 
   // __ehdr_start is the location of ELF file headers.
   addOptionalRegular<ELFT>("__ehdr_start", Out::ElfHeader, 0, STV_HIDDEN);
 
+  // __bss_start is the location of .bss section.
+  ElfSym::Bss =
+      addOptionalRegular<ELFT>("__bss_start", Out::ElfHeader, 0, STV_DEFAULT);
+
   auto Define = [](StringRef S, DefinedRegular *&Sym1, DefinedRegular *&Sym2) {
     Sym1 = addOptionalRegular<ELFT>(S, Out::ElfHeader, 0, STV_DEFAULT);
     assert(S.startswith("_"));
     S = S.substr(1);
     Sym2 = addOptionalRegular<ELFT>(S, Out::ElfHeader, 0, STV_DEFAULT);
   };
 
   Define("_end", ElfSym::End, ElfSym::End2);
template <class ELFT> void Writer<ELFT>::fixPredefinedSymbols() {
   }
   if (Last)
     Set(ElfSym::End, ElfSym::End2, Last->First, Last->p_memsz);
   if (LastRO)
     Set(ElfSym::Etext, ElfSym::Etext2, LastRO->First, LastRO->p_filesz);
   if (LastRW)
     Set(ElfSym::Edata, ElfSym::Edata2, LastRW->First, LastRW->p_filesz);
 
+  if (ElfSym::Bss)
+    ElfSym::Bss->Section = findSection(".bss");
+
   // 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::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)

lld/trunk/test/ELF/bss-start-common.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