[ELF] - Do not allow -r to eat comdats.

This is PR33052, "Bug 33052 - -r eats comdats ".

To fix it I stop removing group section from out when -r is given
and fixing SHT_GROUP content when writing it just like we do some
other fixup, e.g. for Rel[a]. (it needs fix for section indices that
are in group).

Differential revision: https://reviews.llvm.org/D33485

llvm-svn: 304140

5 files changed, 110 insertions, 7 deletions

diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 58fa9e0c5e6..98189825ccb 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -305,13 +305,22 @@ void elf::ObjectFile<ELFT>::initializeSections(
     }
 
     switch (Sec.sh_type) {
-    case SHT_GROUP:
-      this->Sections[I] = &InputSection::Discarded;
-      if (ComdatGroups
-              .insert(
-                  CachedHashStringRef(getShtGroupSignature(ObjSections, Sec)))
-              .second)
+    case SHT_GROUP: {
+      // We discard comdat sections usually. When -r we should not do that. We
+      // still do deduplication in this case to simplify implementation, because
+      // otherwise merging group sections together would requre additional
+      // regeneration of its contents.
+      bool New = ComdatGroups
+                     .insert(CachedHashStringRef(
+                         getShtGroupSignature(ObjSections, Sec)))
+                     .second;
+      if (New && Config->Relocatable)
+        this->Sections[I] = createInputSection(Sec, SectionStringTable);
+      else
+        this->Sections[I] = &InputSection::Discarded;
+      if (New)
         continue;
+
       for (uint32_t SecIndex : getShtGroupEntries(Sec)) {
         if (SecIndex >= Size)
           fatal(toString(this) +
@@ -319,6 +328,7 @@ void elf::ObjectFile<ELFT>::initializeSections(
         this->Sections[SecIndex] = &InputSection::Discarded;
       }
       break;
+    }
     case SHT_SYMTAB:
       this->initSymtab(ObjSections, &Sec);
       break;
diff --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 8bd3d846372..466656efbf0 100644
--- a/lld/ELF/InputSection.cpp
+++ b/lld/ELF/InputSection.cpp
@@ -295,6 +295,24 @@ bool InputSectionBase::classof(const SectionBase *S) {
   return S->kind() != Output;
 }
 
+void InputSection::copyShtGroup(uint8_t *Buf) {
+  assert(this->Type == SHT_GROUP);
+
+  ArrayRef<uint32_t> From = getDataAs<uint32_t>();
+  uint32_t *To = reinterpret_cast<uint32_t *>(Buf);
+
+  // First entry is a flag word, we leave it unchanged.
+  *To++ = From[0];
+
+  // Here we adjust indices of sections that belong to group as it
+  // might change during linking.
+  ArrayRef<InputSectionBase *> Sections = this->File->getSections();
+  for (uint32_t Val : From.slice(1)) {
+    uint32_t Index = read32(&Val, Config->Endianness);
+    write32(To++, Sections[Index]->OutSec->SectionIndex, Config->Endianness);
+  }
+}
+
 InputSectionBase *InputSection::getRelocatedSection() {
   assert(this->Type == SHT_RELA || this->Type == SHT_REL);
   ArrayRef<InputSectionBase *> Sections = this->File->getSections();
@@ -680,6 +698,13 @@ template <class ELFT> void InputSection::writeTo(uint8_t *Buf) {
     return;
   }
 
+  // If -r is given, linker should keep SHT_GROUP sections. We should fixup
+  // them, see copyShtGroup().
+  if (this->Type == SHT_GROUP) {
+    copyShtGroup(Buf + OutSecOff);
+    return;
+  }
+
   // Copy section contents from source object file to output file
   // and then apply relocations.
   memcpy(Buf + OutSecOff, Data.data(), Data.size());
diff --git a/lld/ELF/InputSection.h b/lld/ELF/InputSection.h
index 75174969c48..4ef4328e8a5 100644
--- a/lld/ELF/InputSection.h
+++ b/lld/ELF/InputSection.h
@@ -319,6 +319,8 @@ public:
 private:
   template <class ELFT, class RelTy>
   void copyRelocations(uint8_t *Buf, llvm::ArrayRef<RelTy> Rels);
+
+  void copyShtGroup(uint8_t *Buf);
 };
 
 // The list of all input sections.
diff --git a/lld/ELF/OutputSections.cpp b/lld/ELF/OutputSections.cpp
index e67e2409824..d82fdcdc31b 100644
--- a/lld/ELF/OutputSections.cpp
+++ b/lld/ELF/OutputSections.cpp
@@ -112,6 +112,19 @@ template <class ELFT> void OutputSection::maybeCompress() {
   Flags |= SHF_COMPRESSED;
 }
 
+template <class ELFT> static void finalizeShtGroup(OutputSection *Sec) {
+  // sh_link field for SHT_GROUP sections should contain the section index of
+  // the symbol table.
+  Sec->Link = InX::SymTab->OutSec->SectionIndex;
+
+  // sh_link then contain index of an entry in symbol table section which
+  // provides signature of the section group.
+  elf::ObjectFile<ELFT> *Obj = Sec->Sections[0]->getFile<ELFT>();
+  assert(Config->Relocatable && Sec->Sections.size() == 1);
+  ArrayRef<SymbolBody *> Symbols = Obj->getSymbols();
+  Sec->Info = InX::SymTab->getSymbolIndex(Symbols[Sec->Sections[0]->Info - 1]);
+}
+
 template <class ELFT> void OutputSection::finalize() {
   if ((this->Flags & SHF_LINK_ORDER) && !this->Sections.empty()) {
     std::sort(Sections.begin(), Sections.end(), compareByFilePosition);
@@ -126,6 +139,11 @@ template <class ELFT> void OutputSection::finalize() {
   }
 
   uint32_t Type = this->Type;
+  if (Type == SHT_GROUP) {
+    finalizeShtGroup<ELFT>(this);
+    return;
+  }
+
   if (!Config->CopyRelocs || (Type != SHT_RELA && Type != SHT_REL))
     return;
 
@@ -355,7 +373,10 @@ void OutputSectionFactory::addInputSec(InputSectionBase *IS,
     return;
   }
 
-  uint64_t Flags = IS->Flags & ~(uint64_t)SHF_GROUP;
+  uint64_t Flags = IS->Flags;
+  if (!Config->Relocatable)
+    Flags &= ~(uint64_t)SHF_GROUP;
+
   if (Sec) {
     if (getIncompatibleFlags(Sec->Flags) != getIncompatibleFlags(IS->Flags))
       error("incompatible section flags for " + Sec->Name +
diff --git a/lld/test/ELF/relocatable-comdat.s b/lld/test/ELF/relocatable-comdat.s
new file mode 100644
index 00000000000..24504d23884
--- /dev/null
+++ b/lld/test/ELF/relocatable-comdat.s
@@ -0,0 +1,45 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: ld.lld -r %t.o %t.o -o %t
+# RUN: llvm-readobj -elf-section-groups -sections %t | FileCheck %s
+
+# CHECK:        Name: .text.bar
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC
+# CHECK-NEXT:     SHF_EXECINSTR
+# CHECK-NEXT:     SHF_GROUP
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size: 8
+# CHECK:      Section {
+# CHECK-NEXT:   Index: 4
+# CHECK-NEXT:   Name: .text.foo
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC
+# CHECK-NEXT:     SHF_EXECINSTR
+# CHECK-NEXT:     SHF_GROUP
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size: 4
+
+# CHECK:       Groups {
+# CHECK-NEXT:    Group {
+# CHECK-NEXT:      Name: .group
+# CHECK-NEXT:      Index: 2
+# CHECK-NEXT:      Type: COMDAT
+# CHECK-NEXT:      Signature: abc
+# CHECK-NEXT:      Section(s) in group [
+# CHECK-NEXT:        .text.bar
+# CHECK-NEXT:        .text.foo
+# CHECK-NEXT:      ]
+# CHECK-NEXT:    }
+# CHECK-NEXT:  }
+
+.section .text.bar,"axG",@progbits,abc,comdat
+.quad 42
+.section .text.foo,"axG",@progbits,abc,comdat
+.long 42