[ELF] - Fix crash relative to SHF_LINK_ORDER sections.

[grimar]

Summary

Our code assumes all input sections in an output SHF_LINK_ORDER section has SHF_LINK_ORDER flag.
We do not check that and that can cause a crash.
That happens because we call
std::stable_sort(Sections.begin(), Sections.end(), compareByFilePosition);,
where compareByFilePosition does not expect to see null when calls getLinkOrderDep.
Given that we never faced it before, I suggest to error out that case.

The same might happen when sections refer to non-regular sections.
Test cases demonstrate the issues.

[grimar]

• Implemented in a different way.

[• rafael]

George Rimar via Phabricator via llvm-commits
<[e-mail]> writes:

Index: ELF/OutputSections.cpp

• ELF/OutputSections.cpp

+++ ELF/OutputSections.cpp
@@ -100,7 +100,8 @@

  Flags = IS->Flags;
} else {
  // Otherwise, check if new type or flags are compatible with existing ones.

• if ((Flags & (SHF_ALLOC | SHF_TLS)) != (IS->Flags & (SHF_ALLOC | SHF_TLS)))

+ if ((Flags & (SHF_ALLOC | SHF_TLS | SHF_LINK_ORDER)) !=
+ (IS->Flags & (SHF_ALLOC | SHF_TLS | SHF_LINK_ORDER)))

Please move SHF_ALLOC | SHF_TLS | SHF_LINK_ORDER to a mask variable.

LGTM with that.

Cheers,
Rafael

ELF/InputFiles.cpp

for (size_t I = 0, E = ObjSections.size(); I < E; I++) {
     }
 
     // .ARM.exidx sections have a reverse dependency on the InputSection they
     // have a SHF_LINK_ORDER dependency, this is identified by the sh_link.
     if (Sec.sh_flags & SHF_LINK_ORDER) {
       if (Sec.sh_link >= this->Sections.size())
         fatal(toString(this) +
               ": invalid sh_link index: " + Twine(Sec.sh_link));
-      this->Sections[Sec.sh_link]->DependentSections.push_back(
-          cast<InputSection>(this->Sections[I]));
+
+      InputSectionBase *LinkSec = this->Sections[Sec.sh_link];
+      InputSection *IS = cast<InputSection>(this->Sections[I]);
+      LinkSec->DependentSections.push_back(IS);
+      if (!isa<InputSection>(LinkSec))
+        error("a section " + IS->Name +
+              " with SHF_LINK_ORDER should not refer a non-regular "
+              "section: " +
+              toString(LinkSec));
     }
   }
 }
 
 // The ARM support in lld makes some use of instructions that are not available
 // on all ARM architectures. Namely:
 // - Use of BLX instruction for interworking between ARM and Thumb state.
 // - Use of the extended Thumb branch encoding in relocation.

ELF/InputSection.cpp

void InputSectionBase::maybeDecompress() {
   if (Name.startswith(".zdebug")) {
     DecompressBuf[Size] = '.';
     memcpy(&DecompressBuf[Size + 1], Name.data() + 2, Name.size() - 2);
     Name = StringRef(&DecompressBuf[Size], Name.size() - 1);
   }
 }
 
 InputSection *InputSectionBase::getLinkOrderDep() const {
-  if ((Flags & SHF_LINK_ORDER) && Link != 0) {
-    InputSectionBase *L = File->getSections()[Link];
-    if (auto *IS = dyn_cast<InputSection>(L))
-      return IS;
-    error("a section with SHF_LINK_ORDER should not refer a non-regular "
-          "section: " +
-          toString(L));
-  }
-  return nullptr;
+  assert(Link);
+  assert(Flags & SHF_LINK_ORDER);
+  return cast<InputSection>(File->getSections()[Link]);
 }
 
 // Returns a source location string. Used to construct an error message.
 template <class ELFT>
 std::string InputSectionBase::getLocation(uint64_t Offset) {
   // We don't have file for synthetic sections.
   if (getFile<ELFT>() == nullptr)
     return (Config->OutputFile + ":(" + Name + "+0x" + utohexstr(Offset) + ")")

ELF/OutputSections.cpp

if (!Live) {
     // If IS is the first section to be added to this section,
     // initialize Type, Entsize and flags from IS.
     Live = true;
     Type = IS->Type;
     Entsize = IS->Entsize;
     Flags = IS->Flags;
   } else {
     // Otherwise, check if new type or flags are compatible with existing ones.
-    if ((Flags & (SHF_ALLOC | SHF_TLS)) != (IS->Flags & (SHF_ALLOC | SHF_TLS)))
+    if ((Flags & (SHF_ALLOC | SHF_TLS | SHF_LINK_ORDER)) !=
+        (IS->Flags & (SHF_ALLOC | SHF_TLS | SHF_LINK_ORDER)))
       error("incompatible section flags for " + Name + "\n>>> " + toString(IS) +
             ": 0x" + utohexstr(IS->Flags) + "\n>>> output section " + Name +
             ": 0x" + utohexstr(Flags));
 
     if (Type != IS->Type) {
       if (!canMergeToProgbits(Type) || !canMergeToProgbits(IS->Type))
         error("section type mismatch for " + IS->Name + "\n>>> " +
               toString(IS) + ": " +

test/ELF/section-metadata-err.s

 # REQUIRES: x86
 
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: not ld.lld %t.o -o %t 2>&1 | FileCheck %s
 
-# CHECK: error: a section with SHF_LINK_ORDER should not refer a non-regular section: {{.*}}section-metadata-err.s.tmp.o:(.foo)
+# CHECK: error: a section .bar with SHF_LINK_ORDER should not refer a non-regular section: {{.*}}section-metadata-err.s.tmp.o:(.foo)
 
 .global _start
 _start:
 .quad .foo
 
 .section .foo,"aM",@progbits,8
 .quad 0
 
-.section bar,"ao",@progbits,.foo
+.section .bar,"ao",@progbits,.foo

test/ELF/section-metadata-err2.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: not ld.lld %t.o -o %t 2>&1 | FileCheck %s
+
+## Check we do not crash and report proper errors.
+# CHECK: error: a section .bar with SHF_LINK_ORDER should not refer a non-regular section: {{.*}}section-metadata-err2.s.tmp.o:(.foo)
+# CHECK: error: a section .bar with SHF_LINK_ORDER should not refer a non-regular section: {{.*}}section-metadata-err2.s.tmp.o:(.foo)
+
+.section .foo,"aM",@progbits,8
+.quad 0
+
+.section .bar,"ao",@progbits,.foo,unique,1
+.quad 0
+
+.section .bar,"ao",@progbits,.foo,unique,2
+.quad 1

test/ELF/section-metadata-err3.s

+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: not ld.lld %t.o -o %t 2>&1 | FileCheck %s
+
+# CHECK:      error: incompatible section flags for .bar
+# CHECK-NEXT: >>> {{.*}}section-metadata-err3.s.tmp.o:(.bar): 0x2
+# CHECK-NEXT: >>> output section .bar: 0x82
+
+.section .foo,"a",@progbits
+.quad 0
+
+.section .bar,"ao",@progbits,.foo,unique,1
+.quad 0
+
+.section .bar,"a",@progbits,unique,2
+.quad 1