[ELF] - Simplify logic of creating "COMMON" section.

[grimar]

Summary

Patch renames BssRelSection to BssSection and reuses
already implemented logic, that allows simplify creation of
"COMMON" section.

[ruiu]

Overall looking good. Nice improvement. A minor comments.

ELF/Relocations.cpp

482 ↗

(On Diff #90830)

RelSec -> Bss, as In<ELFT>::Bss is not a .rel.

ELF/SyntheticSections.cpp

375–376 ↗

(On Diff #90830)

Do this at the beginning of this function because it doesn't depend on this->Size or this->Alignment you already computed.

ELF/SyntheticSections.h

151–152 ↗

(On Diff #90830)

.bss for read-write copy relocations, .bss.rel.ro for read-only copy relocations, and COMMON for common symbols.

162 ↗

(On Diff #90830)

Size = 0

[grimar]

• Rebased, addressed comments.

[ruiu]

LGTM

ELF/SyntheticSections.h

162 ↗

(On Diff #91891)

Add explicit.

lld/trunk/ELF/SyntheticSections.h

 private:
   void computeHash(llvm::ArrayRef<uint8_t> Buf,
                    std::function<void(uint8_t *, ArrayRef<uint8_t>)> Hash);
 
   size_t HashSize;
   uint8_t *HashBuf;
 };
 
-// BssSection is used to reserve space for copy relocations. We create two
-// instances of this class for .bss and .bss.rel.ro. .bss is used for writable
-// symbols, and .bss.rel.ro is used for read-only symbols.
+// BssSection is used to reserve space for copy relocations and common symbols.
+// We create three instances of this class for .bss, .bss.rel.ro and "COMMON".
+// .bss is used for writable symbols, .bss.rel.ro is used for read-only symbols
+// and latter used for common symbols.
 class BssSection final : public SyntheticSection {
 public:
   BssSection(StringRef Name);
   void writeTo(uint8_t *) override {}
   bool empty() const override { return getSize() == 0; }
   size_t reserveSpace(uint32_t Alignment, size_t Size);
   size_t getSize() const override { return Size; }
 

lld/trunk/ELF/SyntheticSections.cpp

template <class ELFT> static std::vector<DefinedCommon *> getCommonSymbols() {
   for (Symbol *S : Symtab<ELFT>::X->getSymbols())
     if (auto *B = dyn_cast<DefinedCommon>(S->body()))
       V.push_back(B);
   return V;
 }
 
 // Find all common symbols and allocate space for them.
 template <class ELFT> InputSection *elf::createCommonSection() {
-  auto *Ret = make<InputSection>(SHF_ALLOC | SHF_WRITE, SHT_NOBITS, 1,
-                                 ArrayRef<uint8_t>(), "COMMON");
-  Ret->Live = true;
-
   if (!Config->DefineCommon)
-    return Ret;
+    return nullptr;
 
   // Sort the common symbols by alignment as an heuristic to pack them better.
   std::vector<DefinedCommon *> Syms = getCommonSymbols<ELFT>();
+  if (Syms.empty())
+    return nullptr;
+
   std::stable_sort(Syms.begin(), Syms.end(),
                    [](const DefinedCommon *A, const DefinedCommon *B) {
                      return A->Alignment > B->Alignment;
                    });
+  BssSection *Ret = make<BssSection>("COMMON");
+  for (DefinedCommon *Sym : Syms)
+    Sym->Offset = Ret->reserveSpace(Sym->Alignment, Sym->Size);
 
-  // Assign offsets to symbols.
-  size_t Size = 0;
-  uint32_t Alignment = 1;
-  for (DefinedCommon *Sym : Syms) {
-    Alignment = std::max(Alignment, Sym->Alignment);
-    Size = alignTo(Size, Sym->Alignment);
-
-    // Compute symbol offset relative to beginning of input section.
-    Sym->Offset = Size;
-    Size += Sym->Size;
-  }
-  Ret->Alignment = Alignment;
-  Ret->Data = makeArrayRef<uint8_t>(nullptr, Size);
   return Ret;
 }
 
 // Returns an LLD version string.
 static ArrayRef<uint8_t> getVersion() {
   // Check LLD_VERSION first for ease of testing.
   // You can get consitent output by using the environment variable.
   // This is only for testing.
void BuildIdSection<ELFT>::computeHash(
   // Write to the final output buffer.
   HashFn(HashBuf, Hashes);
 }
 
 BssSection::BssSection(StringRef Name)
     : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_NOBITS, 0, Name) {}
 
 size_t BssSection::reserveSpace(uint32_t Alignment, size_t Size) {
+  if (OutSec)
     OutSec->updateAlignment(Alignment);
   this->Size = alignTo(this->Size, Alignment) + Size;
   this->Alignment = std::max<uint32_t>(this->Alignment, Alignment);
   return this->Size - Size;
 }
 
 template <class ELFT>
 void BuildIdSection<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
   switch (Config->BuildId) {

lld/trunk/ELF/Writer.cpp

if (Config->Strip != StripPolicy::All) {
     In<ELFT>::SymTab = make<SymbolTableSection<ELFT>>(*In<ELFT>::StrTab);
   }
 
   if (Config->BuildId != BuildIdKind::None) {
     In<ELFT>::BuildId = make<BuildIdSection<ELFT>>();
     Add(In<ELFT>::BuildId);
   }
 
-  InputSection *Common = createCommonSection<ELFT>();
-  if (!Common->Data.empty()) {
-    In<ELFT>::Common = Common;
-    Add(Common);
-  }
+  if (In<ELFT>::Common = createCommonSection<ELFT>())
+    Add(In<ELFT>::Common);
 
   In<ELFT>::Bss = make<BssSection>(".bss");
   Add(In<ELFT>::Bss);
   In<ELFT>::BssRelRo = make<BssSection>(".bss.rel.ro");
   Add(In<ELFT>::BssRelRo);
 
   // Add MIPS-specific sections.
   bool HasDynSymTab =