[ELF] - Allow applying SHF_MERGE optimization for relocatable output.

[grimar]

Summary

This fixes PR35223.

Here I enabled SHF_MERGE section content merging for -r like
we do for regular linking.

[ruiu]

ELF/InputSection.cpp

414

Do you need this temporary variable?

ELF/LinkerScript.cpp

470–472

... already been merged by sh_entsize. If -r was given, we want to create an output section for each mergeable input section because we don't want to mix two sections with different sh_entsize.

ELF/Writer.cpp

494

I wonder if you really need to handle this as a special case.

[grimar]

ELF/InputSection.cpp

414

No, but then code will be 2 lines anyways because of clang-format, IMO looks better with it.

ELF/Writer.cpp

494

Do you mean we can create symbols for all synthetic sections instead ? I think that is at least confusing to see in output and excessive.

[ruiu]

ELF/Writer.cpp

494

Well, I mean I actually do not understand what this code is for in the first place.

[grimar]

ELF/Writer.cpp

494

It is for this place:
https://github.com/llvm-mirror/lld/blob/master/ELF/SyntheticSections.cpp#L1529

When we rewrite relocations we updating info field and want to set proper section index.
Before this patch we had regular SHF_MERGE sections in output, so symbols were created for them.
But now them are synthetic and there is no symbol, so we emit wrong zero index and relocation is broken.

BTW, I think that affects regular output + --emit-relocs as well. I am going to check soon and will split that change to different patch if so.

[ruiu]

ELF/InputSection.cpp

414

I don't think so. Can you inline?

ELF/Writer.cpp

494

This is probably okay, but let me think about it, as it still feels somewhat ad-hoc. There might be some simpler way.

[grimar]

ELF/Writer.cpp

494

Sure. I can confirm that --emit-reloc is affected, I am going to post a patch for that soon, so we can continue discussion of this place there.

[grimar]

• Rebased. Most of changes were gone away.

[ruiu]

LGTM

ELF/InputFiles.cpp

 }
 
 template <class ELFT> bool ObjFile<ELFT>::shouldMerge(const Elf_Shdr &Sec) {
   // We don't merge sections if -O0 (default is -O1). This makes sometimes
   // the linker significantly faster, although the output will be bigger.
   if (Config->Optimize == 0)
     return false;
 
-  // Do not merge sections if generating a relocatable object. It makes
-  // the code simpler because we do not need to update relocation addends
-  // to reflect changes introduced by merging.
-  if (Config->Relocatable)
-    return false;
-
   // A mergeable section with size 0 is useless because they don't have
   // any data to merge. A mergeable string section with size 0 can be
   // argued as invalid because it doesn't end with a null character.
   // We'll avoid a mess by handling them as if they were non-mergeable.
   if (Sec.sh_size == 0)
     return false;
 
   // Check for sh_entsize. The ELF spec is not clear about the zero

ELF/InputSection.cpp

if (Sym.Type == STT_SECTION) {
       // hopefully creates a frame that is ignored at runtime.
       SectionBase *Section = cast<Defined>(Sym).Section;
       if (Section == &InputSection::Discarded) {
         P->setSymbolAndType(0, 0, false);
         continue;
       }
 
       if (Config->IsRela) {
-        P->r_addend += Sym.getVA() - Section->getOutputSection()->Addr;
+        int64_t Addend = getAddend<ELFT>(Rel);
+        P->r_addend = Sym.getVA(Addend) - Section->getOutputSection()->Addr;
       } else if (Config->Relocatable) {
         const uint8_t *BufLoc = Sec->Data.begin() + Rel.r_offset;
         Sec->Relocations.push_back({R_ABS, Type, Rel.r_offset,
                                     Target->getImplicitAddend(BufLoc, Type),
                                     &Sym});
       }
     }
 

ELF/LinkerScript.cpp

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.
+  // When control reaches here, mergeable sections have already been merged.
+  // For relocatable case each input mergeable section contains data grouped by
+  // sh_entsize and we want to allow final link to perform the last merging
+  // optimization, so we emit unique output section for each input.
   if (Config->Relocatable && (IS->Flags & SHF_MERGE))
     return createSection(IS, OutsecName);
 
   //  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.
   // ----------------------------------------------------------------

ELF/SyntheticSections.cpp

for (InputSectionBase *&S : InputSections) {
     // them instead of trying to merge.
     if (!MS->Live)
       continue;
 
     StringRef OutsecName = getOutputSectionName(MS->Name);
     uint32_t Alignment = std::max<uint32_t>(MS->Alignment, MS->Entsize);
 
     auto I = llvm::find_if(MergeSections, [=](MergeSyntheticSection *Sec) {
-      return Sec->Name == OutsecName && Sec->Flags == MS->Flags &&
-             Sec->Alignment == Alignment;
+      if (Sec->Name != OutsecName || Sec->Flags != MS->Flags)
+        return false;
+      // For -r we produce output section for each synthetic mergeable section,
+      // because we want to let final link to do final merging optimization.
+      // Thus we want MergeSyntheticSection to contain mergeable sections
+      // with the same entry sizes only.
+      if (Config->Relocatable && Sec->Entsize != MS->Entsize)
+        return false;
+      return Sec->Alignment == Alignment;
     });
     if (I == MergeSections.end()) {
       MergeSyntheticSection *Syn =
           createMergeSynthetic(OutsecName, MS->Type, MS->Flags, Alignment);
       MergeSections.push_back(Syn);
       I = std::prev(MergeSections.end());
       S = Syn;
+
+      // For relocatable output we need to set sh_entsize field,
+      // which will be the same for all mergeable input sections.
+      if (Config->Relocatable)
+        Syn->Entsize = MS->Entsize;
     } else {
       S = nullptr;
     }
     (*I)->addSection(MS);
   }
   for (auto *MS : MergeSections)
     MS->finalizeContents();
 

ELF/Writer.cpp

for (BaseCommand *Base : Script->SectionCommands) {
     auto I = llvm::find_if(Sec->SectionCommands, [](BaseCommand *Base) {
       if (auto *ISD = dyn_cast<InputSectionDescription>(Base))
         return !ISD->Sections.empty();
       return false;
     });
     if (I == Sec->SectionCommands.end())
       continue;
     InputSection *IS = cast<InputSectionDescription>(*I)->Sections[0];
-    if (isa<SyntheticSection>(IS) || IS->Type == SHT_REL ||
-        IS->Type == SHT_RELA)
+
+    // We do not create symbol for synthetic sections because do not have
+    // relocations that might use it, but SHF_MERGE sections is an exception
+    // which is used for -r.
+    if (isa<SyntheticSection>(IS) && !(IS->Flags & SHF_MERGE))
+      continue;
+    if (IS->Type == SHT_REL || IS->Type == SHT_RELA)
       continue;
 
     auto *Sym = make<Defined>("", /*IsLocal=*/true, /*StOther=*/0, STT_SECTION,
                               /*Value=*/0, /*Size=*/0, IS);
     InX::SymTab->addSymbol(Sym);
   }
 }
 

test/ELF/emit-relocs.s

 # CHECK-NEXT:     Value: 0x201000
 # CHECK-NEXT:     Size: 0
 # CHECK-NEXT:     Binding: Local
 # CHECK-NEXT:     Type: Section
 # CHECK-NEXT:     Other: 0
 # CHECK-NEXT:     Section: .text
 # CHECK-NEXT:   }
 # CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Name:
+# CHECK-NEXT:     Value: 0x0
+# CHECK-NEXT:     Size: 0
+# CHECK-NEXT:     Binding: Local
+# CHECK-NEXT:     Type: Section
+# CHECK-NEXT:     Other: 0
+# CHECK-NEXT:     Section: .comment
+# CHECK-NEXT:   }
+# CHECK-NEXT:   Symbol {
 # CHECK-NEXT:     Name: fn
 # CHECK-NEXT:     Value: 0x201000
 # CHECK-NEXT:     Size: 0
 # CHECK-NEXT:     Binding: Global
 # CHECK-NEXT:     Type: Function
 # CHECK-NEXT:     Other: 0
 # CHECK-NEXT:     Section: .text
 # CHECK-NEXT:   }

test/ELF/merge-reloc.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: ld.lld %t.o -r -o %t-rel
 # RUN: llvm-readobj -s -section-data %t-rel | FileCheck %s
 
-# When linker generates a relocatable object it should keep "merge"
-# sections as-is: do not merge content, do not join regular and
-# "merge" sections, do not joint "merge" sections with different
-# entry size.
+# When linker generates a relocatable object it do string merging in the same
+# way as for regular link. It should keep SHF_MERGE flag and set proper sh_entsize
+# value so that final link can perform the final merging optimization.
 
 # CHECK:      Section {
 # CHECK:        Index:
-# CHECK:        Name: .rodata
+# CHECK:        Name: .rodata.1
 # CHECK-NEXT:   Type: SHT_PROGBITS
 # CHECK-NEXT:   Flags [
 # CHECK-NEXT:     SHF_ALLOC
 # CHECK-NEXT:     SHF_MERGE
 # CHECK-NEXT:   ]
 # CHECK-NEXT:   Address:
 # CHECK-NEXT:   Offset:
-# CHECK-NEXT:   Size: 12
+# CHECK-NEXT:   Size: 4
 # CHECK-NEXT:   Link: 0
 # CHECK-NEXT:   Info: 0
 # CHECK-NEXT:   AddressAlignment: 4
 # CHECK-NEXT:   EntrySize: 4
 # CHECK-NEXT:   SectionData (
-# CHECK-NEXT:     0000: 42000000 42000000 42000000
+# CHECK-NEXT:     0000: 42000000
 # CHECK-NEXT:   )
 # CHECK-NEXT: }
 # CHECK:      Section {
 # CHECK:        Index:
-# CHECK:        Name: .rodata
+# CHECK:        Name: .rodata.2
 # CHECK-NEXT:   Type: SHT_PROGBITS
 # CHECK-NEXT:   Flags [
 # CHECK-NEXT:     SHF_ALLOC
 # CHECK-NEXT:     SHF_MERGE
 # CHECK-NEXT:   ]
 # CHECK-NEXT:   Address:
 # CHECK-NEXT:   Offset:
-# CHECK-NEXT:   Size: 16
+# CHECK-NEXT:   Size: 8
 # CHECK-NEXT:   Link: 0
 # CHECK-NEXT:   Info: 0
 # CHECK-NEXT:   AddressAlignment: 8
 # CHECK-NEXT:   EntrySize: 8
 # CHECK-NEXT:   SectionData (
-# CHECK-NEXT:     0000: 42000000 42000000 42000000 42000000
+# CHECK-NEXT:     0000: 42000000 42000000
 # CHECK-NEXT:   )
 # CHECK-NEXT: }
 # CHECK:      Section {
 # CHECK:        Index:
 # CHECK:        Name: .data
 # CHECK-NEXT:   Type: SHT_PROGBITS
 # CHECK-NEXT:   Flags [
 # CHECK-NEXT:     SHF_ALLOC

test/ELF/relocatable-mergeable-relocs.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 -section-data -r %t2 | FileCheck %s
+
+## Check we still produce proper relocations for relocatable
+## output when merging sections.
+
+# CHECK:      Section {
+# CHECK:        Index:
+# CHECK:        Name: .strings
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [ 
+# CHECK-NEXT:     SHF_MERGE
+# CHECK-NEXT:     SHF_STRINGS
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size: 12
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   AddressAlignment:
+# CHECK-NEXT:   EntrySize:
+# CHECK-NEXT:   SectionData (
+# CHECK-NEXT:     0000: 41414100 43434300 42424200 |AAA.CCC.BBB.|
+# CHECK-NEXT:   )
+# CHECK-NEXT: }
+
+# CHECK:      Relocations [
+# CHECK-NEXT:   Section {{.*}} .rela.foo {
+# CHECK-NEXT:     0x0 R_X86_64_64 .strings 0x0
+# CHECK-NEXT:     0x8 R_X86_64_64 .strings 0x8
+# CHECK-NEXT:     0x10 R_X86_64_64 .strings 0x8
+# CHECK-NEXT:     0x18 R_X86_64_64 .strings 0x4
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+.section .strings,"MS",@progbits,1,unique,10
+.Linfo_string0:
+  .asciz "AAA"
+.Linfo_string1:
+  .asciz "BBB"
+
+.section .strings,"MS",@progbits,1,unique,20
+.Linfo_string2:
+  .asciz "BBB"
+.Linfo_string3:
+  .asciz "CCC"
+
+.section .foo,"ax",@progbits
+.quad .Linfo_string0
+.quad .Linfo_string1
+.quad .Linfo_string2
+.quad .Linfo_string3

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 -section-data %t2 | FileCheck %s
+
+## Check we don't merge SHF_MERGE sections with different entries
+## size for relocatable output.
+
+# CHECK:      Section {
+# CHECK:        Index:
+# CHECK:        Name: .strings
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_MERGE
+# CHECK-NEXT:     SHF_STRINGS
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size: 4
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   AddressAlignment:
+# CHECK-NEXT:   EntrySize: 1
+# CHECK-NEXT:   SectionData (
+# CHECK-NEXT:     |foo.|
+# CHECK-NEXT:   )
+# CHECK-NEXT: }
+# CHECK:      Section {
+# CHECK:        Index:
+# CHECK:        Name: .strings
+# CHECK-NEXT:   Type: SHT_PROGBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_MERGE
+# CHECK-NEXT:     SHF_STRINGS
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address:
+# CHECK-NEXT:   Offset:
+# CHECK-NEXT:   Size: 6
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   AddressAlignment:
+# CHECK-NEXT:   EntrySize: 2
+# CHECK-NEXT:   SectionData (
+# CHECK-NEXT:     |aabb..|
+# CHECK-NEXT:   )
+# CHECK-NEXT: }
+
+.section .strings,"MS",@progbits,1,unique,10
+.align 2
+.asciz "foo"
+
+.section .strings,"MS",@progbits,2,unique,30
+.ascii "aabb"
+.word 0