[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 ↗

(On Diff #122834)

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

496

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

[grimar]

ELF/InputSection.cpp

414 ↗

(On Diff #122834)

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

ELF/Writer.cpp

496

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

496

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

[grimar]

ELF/Writer.cpp

496

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 ↗

(On Diff #122834)

I don't think so. Can you inline?

ELF/Writer.cpp

496

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

496

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/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/Writer.cpp

for (BaseCommand *Base : Script->SectionCommands) {
     });
     if (I == Sec->SectionCommands.end())
       continue;
     InputSection *IS = cast<InputSectionDescription>(*I)->Sections[0];
 
     // Relocations are not using REL[A] section symbols.
     if (IS->Type == SHT_REL || IS->Type == SHT_RELA)
       continue;
 
     // Unlike other synthetic sections, mergeable output sections contain data
     // copied from input sections, and there may be a relocation pointing to its
-    // contents if -emit-reloc is given.
+    // contents if -r or -emit-reloc are given.
     if (isa<SyntheticSection>(IS) && !(IS->Flags & SHF_MERGE))
       continue;
 
     auto *Sym = make<Defined>("", /*IsLocal=*/true, /*StOther=*/0, STT_SECTION,
                               /*Value=*/0, /*Size=*/0, IS);
     InX::SymTab->addSymbol(Sym);
   }
 }

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.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.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