[LLD][ELF] - Allow relocation sections to appear before their target sections.

[grimar]

Summary

It allows handling cases when we have SHT_REL[A] sections before target
sections in objects.

This fixes https://bugs.llvm.org/show_bug.cgi?id=46632

which says: "Normally it is not what compilers would emit. We have to support it,
because some custom tools might want to use this feature, which is not restricted by ELF gABI"
GNU ld supports this as well.

[grimar]

Going to update. I've missed a failture in a test.

[grimar]

• Remove outdated test.
• Test SHT_REL case too.

[jhenderson]

Looks fine to me, just comments that need updating.

lld/ELF/InputFiles.cpp

644–650

the -> a

646

I would say "create SHT_REL[A} sections. In some cases relocated sections may follow the corresponding relocation section. In such a case, the relocation section would attempt to reference a target section that has not yet been created. For simplicity, delay creation of relocation sections until now."

656

I think this comment is probably superfluous - the code is simple enough to show it.

660

I think you can delete this comment. It's fairly obvious that the rest is to do with SHF_LINK_ORDER, owing to the if statement.

[grimar]

• Addressed review comments.

[jhenderson]

LGTM (with nits), but I'd like a second opinion from somebody with fresher LLD knowledge.

lld/ELF/InputFiles.cpp

646

Typo? '}' -> ']'

647

Oops, my bad: "a case" -> "cases"

[MaskRay]

Looks good. Some comment suggestions.

lld/ELF/InputFiles.cpp

646

... the section header index of a relocation section may be smaller than that of the relocated section

lld/test/ELF/reloc-sec-before-target.test

1 ↗

(On Diff #276703)

If the section header index of a SHT_REL[A] section is smaller than the section header index of the relocated section

1 ↗

(On Diff #276703)

reloc-sec-before-relocated.test might be a better name.

(target -> relocated)

3 ↗

(On Diff #276703)

We have to support it, because some custom tools might want to use this feature, which is not restricted by ELF gABI.

Worth mentioning that GNU ld supports this as well.

[MaskRay]

lld/test/ELF/reloc-sec-before-target.test

3 ↗

(On Diff #276703)

This should also be mentioned in the description.

[grimar]

• Addressed review comments.

[MaskRay]

Thanks!

lld/ELF/InputFiles.cpp

case SHT_GROUP: {
       }
       break;
     }
     case SHT_SYMTAB_SHNDX:
       shndxTable = CHECK(obj.getSHNDXTable(sec, objSections), this);
       break;
     case SHT_SYMTAB:
     case SHT_STRTAB:
+    case SHT_REL:
+    case SHT_RELA:
     case SHT_NULL:
       break;
     default:
       this->sections[i] = createInputSection(sec);
     }
   }
 
-  // This block handles SHF_LINK_ORDER.
+  // We have a second loop. It is used to:
+  // 1) handle SHF_LINK_ORDER sections.
+  // 2) create SHT_REL[A] sections. In some cases the section header index of a
+  //    relocation section may be smaller than that of the relocated section. In
+  //    such cases, the relocation section would attempt to reference a target
+  //    section that has not yet been created. For simplicity, delay creation of
+  //    relocation sections until now.
   for (size_t i = 0, e = objSections.size(); i < e; ++i) {
     if (this->sections[i] == &InputSection::discarded)
       continue;
     const Elf_Shdr &sec = objSections[i];
+
+    if (sec.sh_type == SHT_REL || sec.sh_type == SHT_RELA)
+      this->sections[i] = createInputSection(sec);
+
     if (!(sec.sh_flags & SHF_LINK_ORDER))
       continue;
 
     // .ARM.exidx sections have a reverse dependency on the InputSection they
     // have a SHF_LINK_ORDER dependency, this is identified by the sh_link.
     InputSectionBase *linkSec = nullptr;
     if (sec.sh_link < this->sections.size())
       linkSec = this->sections[sec.sh_link];
     if (!linkSec)
       fatal(toString(this) + ": invalid sh_link index: " + Twine(sec.sh_link));

lld/test/ELF/invalid/reloc-section-reordered.test

-# REQUIRES: x86
-
-# RUN: yaml2obj %s -o %t.o
-# RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck %s
-# CHECK: unsupported relocation reference
-
-## YAML below lists .rela.text before .text, we do not support it.
-
-!ELF
-FileHeader:
-  Class:           ELFCLASS64
-  Data:            ELFDATA2LSB
-  OSABI:           ELFOSABI_FREEBSD
-  Type:            ET_REL
-  Machine:         EM_X86_64
-Sections:
-  - Type:            SHT_REL
-    Name:            .rela.text
-    Link:            .symtab
-    Info:            .text
-    AddressAlign:    0x04
-    Relocations:
-      - Symbol:          .text
-        Type:            R_X86_64_NONE
-  - Type:            SHT_PROGBITS
-    Name:            .text
-    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
-    AddressAlign:    0x04
-    Content:         "FFFFFFFFFFFFFFFF"
-Symbols:
-  - Name:    .text
-    Type:    STT_SECTION
-    Section: .text

lld/test/ELF/reloc-sec-before-relocated.test

+## If the section header index of a SHT_REL[A] section is smaller than the
+## section header index of the relocated section, we should handle it properly.
+## Normally it is not what compilers would emit, but some custom tools might
+## want to use this feature, which is not restricted by ELF gABI.
+## GNU ld supports this as well.
+
+# RUN: yaml2obj %s -DTYPE=SHT_RELA -o %t1.o
+# RUN: ld.lld -shared %t1.o -o %t1
+# RUN: llvm-readelf --relocs %t1 | FileCheck %s
+
+# RUN: yaml2obj %s -DTYPE=SHT_REL -o %t2.o
+# RUN: ld.lld -shared %t2.o -o %t2
+# RUN: llvm-readelf --relocs %t2 | FileCheck %s
+
+## Check we handle the relocation properly.
+# CHECK:      Relocation section '.rela.dyn' at offset 0x238 contains 1 entries:
+# CHECK-NEXT:     Offset             Info             Type    Symbol's Value  Symbol's Name + Addend
+# CHECK-NEXT: 00000000000022f0  0000000100000001 R_X86_64_64 0000000000000000 foo + 0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name:  .relx.data
+    Type:  [[TYPE]]
+    Info:  .data
+    Relocations:
+      - Symbol: foo
+        Type:   R_X86_64_64
+  - Name:  .data
+    Type:  SHT_PROGBITS
+    Flags: [ SHF_ALLOC, SHF_WRITE ]
+Symbols:
+  - Name:    foo
+    Binding: STB_GLOBAL