[ELF] - Allow multiple comdats when producing relocatable output.

[grimar]

Summary

Previously LLD would fail for case when there are multiple comdats and -r.

That happened because it merged all ".group" (SHT_GROUP) sections into single
output, producing broken result. Such sections may have similar name, alignment and flags
and other properties. I think all we need to do for fix is to produce separate output
section for each such input one.

[grimar]

I think this is not correct. I'll revisit it tomorrow.

[ruiu]

How can this happen? I believe we dedup SHT_GROUP sections even if the -r option is given, so we want to emit only one SHT_GROUP for each unique name, no?

[grimar]

In D33643#772824, @ruiu wrote:

How can this happen? I believe we dedup SHT_GROUP sections even if the -r option is given, so we want to emit only one SHT_GROUP for each unique name, no?

Dedup does not help here, because groups are different. We have group A and section named ".group" + group .B with section ".group" too.
Afted deduplication we still have 2 sections named ".group". And idea is that we should emit different output sections for each, because
after deduplication we definetely know them are different groups.

[ruiu]

LGTM with this change.

ELF/OutputSections.cpp

368–370

Then this explanation doesn't capture why we are doing this.

Sections with the SHT_GROUP attribute reach here only when the -r option is given. Such sections define "section groups", and InputFiles.cpp has dedup'ed section groups by their signatures. For the -r, we want to pass through all SHT_GROUP sections without merging them because merging them creates broken section contents.

ELF/OutputSections.h

struct Out {
   static OutputSection *InitArray;
   static OutputSection *FiniArray;
 };
 
 struct SectionKey {
   StringRef Name;
   uint64_t Flags;
   uint32_t Alignment;
+  uint32_t Info;
 };
 }
 }
 namespace llvm {
 template <> struct DenseMapInfo<lld::elf::SectionKey> {
   static lld::elf::SectionKey getEmptyKey();
   static lld::elf::SectionKey getTombstoneKey();
   static unsigned getHashValue(const lld::elf::SectionKey &Val);

ELF/OutputSections.cpp

static SectionKey createKey(InputSectionBase *C, StringRef OutsecName) {
   // another. The problem is that there is no way to layout those output
   // sections such that the .foo sections are the only thing between the start
   // and end symbols.
   //
   // Given the above issues, we instead merge sections by name and error on
   // incompatible types and flags.
 
   uint32_t Alignment = 0;
+  uint32_t Info = 0;
   uint64_t Flags = 0;
-  if (Config->Relocatable && (C->Flags & SHF_MERGE)) {
+  if (Config->Relocatable) {
+    if (C->Flags & SHF_MERGE) {
       Alignment = std::max<uint64_t>(C->Alignment, C->Entsize);
       Flags = C->Flags & (SHF_MERGE | SHF_STRINGS);
     }
+    if (C->Type == SHT_GROUP)
+      Info = C->Info;
+  }
 
-  return SectionKey{OutsecName, Flags, Alignment};
+  return SectionKey{OutsecName, Flags, Alignment, Info};
 }
 
 OutputSectionFactory::OutputSectionFactory(
     std::vector<OutputSection *> &OutputSections)
     : OutputSections(OutputSections) {}
 
 static uint64_t getIncompatibleFlags(uint64_t Flags) {
   return Flags & (SHF_ALLOC | SHF_TLS);
static void reportDiscarded(InputSectionBase *IS) {
   if (!Config->PrintGcSections)
     return;
   message("removing unused section from '" + IS->Name + "' in file '" +
           IS->File->getName());
 }
 
 void OutputSectionFactory::addInputSec(InputSectionBase *IS,
                                        StringRef OutsecName) {
   SectionKey Key = createKey(IS, OutsecName);
   OutputSection *&Sec = Map[Key];
   return addInputSec(IS, OutsecName, Sec);
 }
 
 void OutputSectionFactory::addInputSec(InputSectionBase *IS,
                                        StringRef OutsecName,
                                        OutputSection *&Sec) {
   if (!IS->Live) {
     reportDiscarded(IS);
     return;
SectionKey DenseMapInfo<SectionKey>::getEmptyKey() {
   return SectionKey{DenseMapInfo<StringRef>::getEmptyKey(), 0, 0};
 }
 
 SectionKey DenseMapInfo<SectionKey>::getTombstoneKey() {
   return SectionKey{DenseMapInfo<StringRef>::getTombstoneKey(), 0, 0};
 }
 
 unsigned DenseMapInfo<SectionKey>::getHashValue(const SectionKey &Val) {
-  return hash_combine(Val.Name, Val.Flags, Val.Alignment);
+  return hash_combine(Val.Name, Val.Flags, Val.Alignment, Val.Info);
 }
 
 bool DenseMapInfo<SectionKey>::isEqual(const SectionKey &LHS,
                                        const SectionKey &RHS) {
   return DenseMapInfo<StringRef>::isEqual(LHS.Name, RHS.Name) &&
-         LHS.Flags == RHS.Flags && LHS.Alignment == RHS.Alignment;
+         LHS.Flags == RHS.Flags && LHS.Alignment == RHS.Alignment &&
+         LHS.Info == RHS.Info;
 }
 
 uint64_t elf::getHeaderSize() {
   if (Config->OFormatBinary)
     return 0;
   return Out::ElfHeader->Size + Out::ProgramHeaders->Size;
 }
 

test/ELF/relocatable-comdat-multiple.s

+# 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 %t | FileCheck %s
+
+# CHECK:      Groups {
+# CHECK-NEXT:   Group {
+# CHECK-NEXT:     Name: .group
+# CHECK-NEXT:     Index: 2
+# CHECK-NEXT:     Type: COMDAT
+# CHECK-NEXT:     Signature: aaa
+# CHECK-NEXT:     Section(s) in group [
+# CHECK-NEXT:       .text.a
+# CHECK-NEXT:       .text.b
+# CHECK-NEXT:     ]
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Group {
+# CHECK-NEXT:     Name: .group
+# CHECK-NEXT:     Index: 5
+# CHECK-NEXT:     Type: COMDAT
+# CHECK-NEXT:     Signature: bbb
+# CHECK-NEXT:     Section(s) in group [
+# CHECK-NEXT:       .text.c
+# CHECK-NEXT:       .text.d
+# CHECK-NEXT:     ]
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+
+.section .text.a,"axG",@progbits,aaa,comdat
+.section .text.b,"axG",@progbits,aaa,comdat
+
+.section .text.c,"axG",@progbits,bbb,comdat
+.section .text.d,"axG",@progbits,bbb,comdat