[ELF] -r: fix crash when processing a SHT_REL[A] that relocates a SHF_MERGE after D67504/r372734

[MaskRay]

Summary

Fix PR43767

In -r mode, when processing a SHT_REL[A] that relocates a SHF_MERGE, sec->getRelocatedSection() is a
MergeInputSection and its parent is an OutputSection but is asserted to
be a SyntheticSection (MergeSyntheticSection) in LinkerScript.cpp:addInputSec().

The code path is not exercised in non -r mode because the relocated
section changed from MergeInputSection to InputSection.

Reorder the code to make the non -r logic apply to -r as well, thus fix
the crash.

[peter.smith]

Agreed, I think that change makes sense.

[MaskRay]

Update description

lld/ELF/InputFiles.cpp

case SHT_REL: {
     // Target may have been discarded if it is in a different section group
     // and the group is discarded, even though it's a violation of the
     // spec. We handle that situation gracefully by discarding dangling
     // relocation sections.
     InputSectionBase *target = getRelocTarget(sec);
     if (!target)
       return nullptr;
 
+    // ELF spec allows mergeable sections with relocations, but they are
+    // rare, and it is in practice hard to merge such sections by contents,
+    // because applying relocations at end of linking changes section
+    // contents. So, we simply handle such sections as non-mergeable ones.
+    // Degrading like this is acceptable because section merging is optional.
+    if (auto *ms = dyn_cast<MergeInputSection>(target)) {
+      target = toRegularSection(ms);
+      this->sections[sec.sh_info] = target;
+    }
+
     // This section contains relocation information.
     // If -r is given, we do not interpret or apply relocation
     // but just copy relocation sections to output.
     if (config->relocatable) {
       InputSection *relocSec = make<InputSection>(*this, sec, name);
       // We want to add a dependency to target, similar like we do for
       // -emit-relocs below. This is useful for the case when linker script
       // contains the "/DISCARD/". It is perhaps uncommon to use a script with
       // -r, but we faced it in the Linux kernel and have to handle such case
       // and not to crash.
       target->dependentSections.push_back(relocSec);
       return relocSec;
     }
 
     if (target->firstRelocation)
       fatal(toString(this) +
             ": multiple relocation sections to one section are not supported");
 
-    // ELF spec allows mergeable sections with relocations, but they are
-    // rare, and it is in practice hard to merge such sections by contents,
-    // because applying relocations at end of linking changes section
-    // contents. So, we simply handle such sections as non-mergeable ones.
-    // Degrading like this is acceptable because section merging is optional.
-    if (auto *ms = dyn_cast<MergeInputSection>(target)) {
-      target = toRegularSection(ms);
-      this->sections[sec.sh_info] = target;
-    }
-
     if (sec.sh_type == SHT_RELA) {
       ArrayRef<Elf_Rela> rels = CHECK(getObj().relas(&sec), this);
       target->firstRelocation = rels.begin();
       target->numRelocations = rels.size();
       target->areRelocsRela = true;
     } else {
       ArrayRef<Elf_Rel> rels = CHECK(getObj().rels(&sec), this);
       target->firstRelocation = rels.begin();

lld/test/ELF/merge-relocatable.s

+# REQUIRES: x86
+
+## Test that we keep a SHT_REL[A] section which relocates a SHF_MERGE section
+## in -r mode. The relocated SHF_MERGE section is handled as non-mergeable.
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: ld.lld -r %t.o -o %t
+# RUN: llvm-readobj -S %t | FileCheck %s
+
+# CHECK:     Name: .rodata.cst8
+# CHECK-NOT: }
+# CHECK:     Size: 16
+# CHECK:     Name: .rela.rodata.cst8
+# CHECK-NOT: }
+# CHECK:     Size: 48
+
+foo:
+
+.section .rodata.cst8,"aM",@progbits,8,unique,0
+.quad foo
+
+.section .rodata.cst8,"aM",@progbits,8,unique,1
+.quad foo