[ELF] - Allow merging of strings sections for -relocatable output.

[grimar]

Summary

This is PR35223.

r317406 stopped merging SHF_MERGE sections when -r is given.
Previously we used Alignment, Flags and section Name together to
generate a complex key for merging sections. For -r case all 3 were used,
but for regular case we merged sections by name only for a long time
and r317406 changed behavior to merge sections by name for all outputs,
that allowed to simplify code and sped up linker a bit.

But above broke -relocatable output in sence that tools like dwarfdump (both gnu
and llvm one) expects to see single .debug_str section in output.
After the r317406 we started producing unique output section for each input
and tools are upset.

I suggest to allow relocatable output to merge SHF_MERGE strings sections.
In case when input sections has different sh_entsize, I think we can just drop
the SHF_MERGE flag and merge them. That looks consistent with what ld.bfd
do and should not be an issue, because I believe normally string sections
always have sh_entsize = 1. llvm-mc even ignores any values explicitly set for .debug_str
sh_entsize and renders 1 to output, though gas allows different values.

[jhenderson]

Given that the issue with not combining sections was purely restricted to .debug_str, which only ever has an entsize of 1, would it not make more sense to not combine the InputSections for different entsize values?

I'm wary about removing the SHF_MERGE flag, because I'm pretty sure mergeable sections of different sizes are not that rare, although they do not typically have the same name, so maybe it's okay.

[grimar]

Abandoning. Another approach was chosen (D40026) and landed (r318516 + r318518)

ELF/LinkerScript.cpp

static OutputSection *findByName(ArrayRef<BaseCommand *> Vec,
                                  StringRef Name) {
   for (BaseCommand *Base : Vec)
     if (auto *Sec = dyn_cast<OutputSection>(Base))
       if (Sec->Name == Name)
         return Sec;
   return nullptr;
 }
 
+static void addMergeableStringSection(OutputSection *Sec,
+                                      InputSectionBase *IS) {
+  assert(Config->Relocatable);
+  if (Sec->Entsize != IS->Entsize)
+    Sec->Flags &= ~SHF_MERGE;
+  if (!(Sec->Flags & SHF_MERGE))
+    IS->Flags &= ~SHF_MERGE;
+  Sec->addSection(cast<InputSection>(IS));
+}
+
 static OutputSection *createSection(InputSectionBase *IS,
                                     StringRef OutsecName) {
   OutputSection *Sec = Script->createOutputSection(OutsecName, "<internal>");
   Sec->addSection(cast<InputSection>(IS));
   return Sec;
 }
 
 static OutputSection *addInputSec(StringMap<OutputSection *> &Map,
if (Out->RelocationSection) {
       Out->RelocationSection->addSection(Sec);
       return nullptr;
     }
 
     Out->RelocationSection = createSection(IS, OutsecName);
     return Out->RelocationSection;
   }
 
-  // When control reaches here, mergeable sections have already been
-  // merged except the -r case. If that's the case, we do not combine them
-  // and let final link to handle this optimization.
-  if (Config->Relocatable && (IS->Flags & SHF_MERGE))
-    return createSection(IS, OutsecName);
+  // When control reaches here, mergeable sections have already been merged
+  // except the -r case. If that's the case, we do not really want to care about
+  // merging sections, leaving this task for final link. But unfortunately we
+  // still need to do that for .debug_str sections, because different tools
+  // like dwarfdump expects to see single instance in the output. To handle this
+  // case we merge string sections and just drop SHF_MERGE optimization flag if
+  // entry sizes of input and output sections are different. That normally
+  // should not happen as entry size usually is always 1 for strings sections.
+  if (Config->Relocatable && (IS->Flags & SHF_MERGE)) {
+    OutputSection *&Sec = Map[OutsecName];
+    if (!Sec || !(IS->Flags & SHF_STRINGS)) {
+      Sec = createSection(IS, OutsecName);
+      return Sec;
+    }
+    addMergeableStringSection(Sec, IS);
+    return nullptr;
+  }
 
   //  The ELF spec just says
   // ----------------------------------------------------------------
   // In the first phase, input sections that match in name, type and
   // attribute flags should be concatenated into single sections.
   // ----------------------------------------------------------------
   //
   // However, it is clear that at least some flags have to be ignored for

test/ELF/relocatable-mergeable.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t1
+# RUN: ld.lld %t1 -o %t2 -r
+# RUN: llvm-readobj -sections %t2 | FileCheck %s
+
+## Check we merged 2 .strings sections together and dropped SHF_MERGE flag
+## because they have different sh_entsize. Check we created separate output
+## section for each non-string mergeable input and keeped SHF_MERGE.
+
+# CHECK:      Section {
+# CHECK:        Index:
+# CHECK:        Name: .strings
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_STRINGS
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size: 8
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   AddressAlignment:
+# CHECK-NEXT:   EntrySize: 3
+# CHECK-NEXT: }
+# CHECK-NEXT: Section {
+# CHECK-NEXT:   Index:
+# CHECK-NEXT:   Name: .nonstrings
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_MERGE
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size: 4
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   AddressAlignment:
+# CHECK-NEXT:   EntrySize: 2
+# CHECK-NEXT: }
+# CHECK-NEXT: Section {
+# CHECK-NEXT:   Index:
+# CHECK-NEXT:   Name: .nonstrings
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_MERGE
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size: 4
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   AddressAlignment:
+# CHECK-NEXT:   EntrySize: 4
+# CHECK-NEXT: }
+
+.section .strings,"MS",@progbits,1,unique,10
+.asciz "foo"
+
+.section .nonstrings,"M",@progbits,2,unique,20
+.long 11
+
+.section .strings,"MS",@progbits,3,unique,30
+.asciz "bar"
+
+.section .nonstrings,"M",@progbits,4,unique,40
+.long 22