[PATCH] D53864: [ELF] - Do not crash when -r output uses linker script with `/DISCARD/`

[George Rimar via Phabricator via llvm-commits]

grimar updated this revision to Diff 171894.
grimar added a comment.

- Mentioned the Linux kernel as a user of `-r` + `/DISCARD/` in the comment.


https://reviews.llvm.org/D53864

Files:
  ELF/InputFiles.cpp
  ELF/InputSection.cpp
  test/ELF/linkerscript/relocatable-discard.s


Index: test/ELF/linkerscript/relocatable-discard.s
===================================================================
--- test/ELF/linkerscript/relocatable-discard.s
+++ test/ELF/linkerscript/relocatable-discard.s
@@ -0,0 +1,21 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "SECTIONS { /DISCARD/ : { *(.discard.*) }}" > %t.script
+# RUN: ld.lld -o %t --script %t.script -r %t.o
+# RUN: llvm-readobj -sections %t | FileCheck %s
+
+## Test shows that we do not crash after discarding the .discard.foo with -r.
+## Previously it happened because of 2 reasons:
+## 1) .rela.discard.foo was not handled properly and was not discarded.
+##    Remaining reference was invalid and caused the crash.
+## 2) Third-party section .debug_info referencing discarded section
+##    did not handle this case properly and tried to apply the
+##    relocation instead of ignoring it.
+
+# CHECK-NOT: .discard
+
+.section .discard.foo,"ax"
+callq fn at PLT
+
+.section .debug_info,"", at progbits
+.long .discard.foo
Index: ELF/InputSection.cpp
===================================================================
--- ELF/InputSection.cpp
+++ ELF/InputSection.cpp
@@ -432,8 +432,8 @@
         error("STT_SECTION symbol should be defined");
         continue;
       }
-      SectionBase *Section = D->Section;
-      if (Section == &InputSection::Discarded) {
+      SectionBase *Section = D->Section->Repl;
+      if (!Section->Live) {
         P->setSymbolAndType(0, 0, false);
         continue;
       }
@@ -460,7 +460,7 @@
       }
 
       if (RelTy::IsRela)
-        P->r_addend = Sym.getVA(Addend) - Section->Repl->getOutputSection()->Addr;
+        P->r_addend = Sym.getVA(Addend) - Section->getOutputSection()->Addr;
       else if (Config->Relocatable)
         Sec->Relocations.push_back({R_ABS, Type, Rel.r_offset, Addend, &Sym});
     }
Index: ELF/InputFiles.cpp
===================================================================
--- ELF/InputFiles.cpp
+++ ELF/InputFiles.cpp
@@ -645,8 +645,16 @@
     // 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)
-      return make<InputSection>(*this, Sec, Name);
+    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) +


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53864.171894.patch
Type: text/x-patch
Size: 2886 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181031/30491971/attachment.bin>