Revert r362743 "Revert "Revert "Reland D61583 [ELF] Error on relocati…

…ons to STT_SECTION symbols if the sections were discarded"""

(In effect, reverting "[ELF] Error on relocations to STT_SECTION symbols if the sections were discarded".)

It caused debug info problems in LibreOffice [1] and Chromium/V8 [2].
Reverting until those can be fixed.

It also reverts r362497 "STT_SECTION symbol should be defined" on .eh_frame, .debug*, .zdebug* and .gcc_except_table"
which was landed as a follow-up to the above.

> With -r or --emit-relocs, we warn `STT_SECTION symbol should be defined`
> on relocations to discarded section symbol. This was added as an error
> in rLLD319404, but was not so effective before D61583 (it turned the
> error to a warning).
>
> Relocations from .eh_frame .debug* .zdebug* .gcc_except_table to
> discarded .text are very common and somewhat expected. Don't warn/error
> on them. As a reference, ld.bfd has a similar logic in
> _bfd_elf_default_action_discarded() to allow these cases.
>
> Delete invalid-undef-section-symbol.test because what it intended to
> check is now covered by the updated comdat-discarded-reloc.s
>
> Delete relocatable-eh-frame.s because we allow relocations from
> .eh_frame as a special case now.

And finally it reverts r362218 "[ELF] Replace a dead test in getSymVA() with assert()"
as that also depended on the main change reverted here.

> Symbols relative to discarded comdat sections are Undefined instead of
> Defined now (after D59649 and D61583). The `== &InputSection::Discarded`
> test becomes dead. I cannot find a test related to this behavior.

 [1] http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20190603/659848.html
 [2] https://bugs.chromium.org/p/chromium/issues/detail?id=978067

llvm-svn: 364321

lld/ELF/DWARF.cpp

@@ -93,11 +93,8 @@ LLDDwarfObj<ELFT>::findAux(const InputSectionBase &Sec, uint64_t Pos,
   uint32_t SecIndex = File->getSectionIndex(Sym);
 
   // Broken debug info can point to a non-Defined symbol.
-  Symbol &S = File->getRelocTargetSym(Rel);
-  auto *DR = dyn_cast<Defined>(&S);
+  auto *DR = dyn_cast<Defined>(&File->getRelocTargetSym(Rel));
   if (!DR) {
-    if (S.isSection())
-      return None;
     RelType Type = Rel.getType(Config->IsMips64EL);
     if (Type != Target->NoneRel)
       error(toString(File) + ": relocation " + lld::toString(Type) + " at 0x" +

lld/ELF/InputFiles.cpp

@@ -1080,9 +1080,6 @@ template <class ELFT> void ObjFile<ELFT>::initializeSymbols() {
 
       if (ESym.st_shndx == SHN_UNDEF)
         this->Symbols[I] = make<Undefined>(this, Name, Binding, StOther, Type);
-      else if (Sec == &InputSection::Discarded)
-        this->Symbols[I] = make<Undefined>(this, Name, Binding, StOther, Type,
-                                           /*DiscardedSecIdx=*/SecIdx);
       else
         this->Symbols[I] =
             make<Defined>(this, Name, Binding, StOther, Type, Value, Size, Sec);

lld/ELF/InputSection.cpp

@@ -412,8 +412,7 @@ void InputSection::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
 
   for (const RelTy &Rel : Rels) {
     RelType Type = Rel.getType(Config->IsMips64EL);
-    const ObjFile<ELFT> *File = getFile<ELFT>();
-    Symbol &Sym = File->getRelocTargetSym(Rel);
+    Symbol &Sym = getFile<ELFT>()->getRelocTargetSym(Rel);
 
     auto *P = reinterpret_cast<typename ELFT::Rela *>(Buf);
     Buf += sizeof(RelTy);
@@ -436,23 +435,10 @@ void InputSection::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
       // .eh_frame is horribly special and can reference discarded sections. To
       // avoid having to parse and recreate .eh_frame, we just replace any
       // relocation in it pointing to discarded sections with R_*_NONE, which
-      // hopefully creates a frame that is ignored at runtime. Also, don't warn
-      // on .gcc_except_table and debug sections.
-      //
-      // See the comment in maybeReportUndefined for PPC64 .toc .
+      // hopefully creates a frame that is ignored at runtime.
       auto *D = dyn_cast<Defined>(&Sym);
       if (!D) {
-        if (!Sec->Name.startswith(".debug") &&
-            !Sec->Name.startswith(".zdebug") && Sec->Name != ".eh_frame" &&
-            Sec->Name != ".gcc_except_table" && Sec->Name != ".toc") {
-          uint32_t SecIdx = cast<Undefined>(Sym).DiscardedSecIdx;
-          Elf_Shdr_Impl<ELFT> Sec =
-              CHECK(File->getObj().sections(), File)[SecIdx];
-          warn("relocation refers to a discarded section: " +
-               CHECK(File->getObj().getSectionName(&Sec), File) +
-               "\n>>> referenced by " + getObjMsg(P->r_offset));
-        }
-        P->setSymbolAndType(0, 0, false);
+        error("STT_SECTION symbol should be defined");
         continue;
       }
       SectionBase *Section = D->Section->Repl;

lld/ELF/Relocations.cpp

@@ -653,17 +653,9 @@ static std::string maybeReportDiscarded(Undefined &Sym) {
     return "";
   ArrayRef<Elf_Shdr_Impl<ELFT>> ObjSections =
       CHECK(File->getObj().sections(), File);
-
-  std::string Msg;
-  if (Sym.Type == ELF::STT_SECTION) {
-    Msg = "relocation refers to a discarded section: ";
-    Msg += CHECK(
-        File->getObj().getSectionName(&ObjSections[Sym.DiscardedSecIdx]), File);
-  } else {
-    Msg = "relocation refers to a symbol in a discarded section: " +
-          toString(Sym);
-  }
-  Msg += "\n>>> defined in " + toString(File);
+  std::string Msg =
+      "relocation refers to a symbol in a discarded section: " + toString(Sym) +
+      "\n>>> defined in " + toString(File);
 
   Elf_Shdr_Impl<ELFT> ELFSec = ObjSections[Sym.DiscardedSecIdx - 1];
   if (ELFSec.sh_type != SHT_GROUP)

lld/ELF/Symbols.cpp

@@ -48,11 +48,17 @@ static uint64_t getSymVA(const Symbol &Sym, int64_t &Addend) {
     auto &D = cast<Defined>(Sym);
     SectionBase *IS = D.Section;
 
+    // According to the ELF spec reference to a local symbol from outside
+    // the group are not allowed. Unfortunately .eh_frame breaks that rule
+    // and must be treated specially. For now we just replace the symbol with
+    // 0.
+    if (IS == &InputSection::Discarded)
+      return 0;
+
     // This is an absolute symbol.
     if (!IS)
       return D.Value;
 
-    assert(IS != &InputSection::Discarded);
     IS = IS->Repl;
 
     uint64_t Offset = D.Value;

lld/test/ELF/Inputs/comdat-discarded-reloc.s

@@ -1,8 +1,6 @@
-.global bar, _start
+.section .text.bar1,"aG",@progbits,group,comdat
 
-.section .text.foo,"aG",@progbits,group,comdat
-
-.section .text
-_start:
- .quad .text.foo
- .quad bar
+.section .text.bar2
+.global bar
+bar:
+ .quad .text.bar1

lld/test/ELF/comdat-discarded-error.s

@@ -5,24 +5,14 @@
 # RUN: echo '.section .text.foo,"axG",@progbits,foo,comdat; .globl bar; bar:' |\
 # RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t3.o
 
-# RUN: not ld.lld %t2.o %t3.o %t1.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: not ld.lld %t1.o %t2.o %t3.o -o /dev/null 2>&1 | FileCheck %s
 
 # CHECK:      error: relocation refers to a symbol in a discarded section: bar
 # CHECK-NEXT: >>> defined in {{.*}}3.o
 # CHECK-NEXT: >>> section group signature: foo
 # CHECK-NEXT: >>> prevailing definition is in {{.*}}2.o
 # CHECK-NEXT: >>> referenced by {{.*}}1.o:(.text+0x1)
 
-# CHECK:      error: relocation refers to a discarded section: .text.foo
-# CHECK-NEXT: >>> defined in {{.*}}1.o
-# CHECK-NEXT: >>> section group signature: foo
-# CHECK-NEXT: >>> prevailing definition is in {{.*}}2.o
-# CHECK-NEXT: >>> referenced by {{.*}}1.o:(.data+0x0)
-
 .globl _start
 _start:
   jmp bar
-
-.section .text.foo,"axG",@progbits,foo,comdat
-.data
-  .quad .text.foo

lld/test/ELF/comdat-discarded-reloc.s

@@ -1,42 +1,17 @@
 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/comdat-discarded-reloc.s -o %t2.o
-# RUN: ld.lld -gc-sections --noinhibit-exec %t2.o %t.o -o /dev/null
-# RUN: ld.lld -r %t2.o %t.o -o %t 2>&1 | FileCheck --check-prefix=WARN %s
-# RUN: llvm-readobj -r %t | FileCheck --check-prefix=RELOC %s
+# RUN: ld.lld -gc-sections %t.o %t2.o -o %t
 
 ## ELF spec doesn't allow a relocation to point to a deduplicated
 ## COMDAT section. Unfortunately this happens in practice (e.g. .eh_frame)
 ## Test case checks we do not crash.
 
-# WARN: warning: relocation refers to a discarded section: .text.bar1
-# WARN-NEXT: >>> referenced by {{.*}}.o:(.rela.text.bar2+0x0)
-# WARN-NOT: warning
+.global bar, _start
 
-# RELOC:      .rela.eh_frame {
-# RELOC-NEXT:   R_X86_64_NONE
-# RELOC-NEXT: }
-# RELOC-NEXT: .rela.debug_foo {
-# RELOC-NEXT:   R_X86_64_NONE
-# RELOC-NEXT: }
-# RELOC-NEXT: .rela.gcc_except_table {
-# RELOC-NEXT:   R_X86_64_NONE
-# RELOC-NEXT: }
+.section .text.foo,"aG",@progbits,group,comdat
 
-.section .text.bar1,"aG",@progbits,group,comdat
-
-## .text.bar1 in this file is discarded. Warn on the relocation.
-.section .text.bar2,"ax"
-.globl bar
-bar:
-  .quad .text.bar1
-
-## Don't warn on .eh_frame, .debug*, .zdebug*, or .gcc_except_table
-.section .eh_frame,"a"
-  .quad .text.bar1
-
-.section .debug_foo
-  .quad .text.bar1
-
-.section .gcc_except_table,"a"
-  .quad .text.bar1
+.section .text
+_start:
+ .quad .text.foo
+ .quad bar

lld/test/ELF/comdat.s

@@ -1,7 +1,7 @@
 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/comdat.s -o %t2.o
-// RUN: ld.lld -shared %t.o %t2.o -o %t
+// RUN: ld.lld -shared %t.o %t.o %t2.o -o %t
 // RUN: llvm-objdump -d %t | FileCheck %s
 // RUN: llvm-readobj -S --symbols %t | FileCheck --check-prefix=READ %s
 
@@ -31,7 +31,9 @@ foo:
 // CHECK-EMPTY:
 // CHECK-NEXT: bar:
 // 0x1000 - 0x1001 - 5 = -6
+// 0      - 0x1006 - 5 = -4107
 // CHECK-NEXT:   1001:	{{.*}}  callq  -6
+// CHECK-NEXT:   1006:	{{.*}}  callq  -4107
 
         .section .text3,"axG",@progbits,zed,comdat,unique,0
 

lld/test/ELF/invalid-undef-section-symbol.test

@@ -0,0 +1,26 @@
+# RUN: yaml2obj %s -o %t.o
+# RUN: not ld.lld -r %t.o -o /dev/null 2>&1 | FileCheck %s
+
+# We used to crash at this.
+# CHECK: STT_SECTION symbol should be defined
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+  - Name:            .rela.text
+    Type:            SHT_RELA
+    AddressAlign:    0x0000000000000008
+    Info:            .text
+    Relocations:
+      - Offset:          0x0000000000000000
+        Symbol:          .text
+        Type:            R_X86_64_NONE
+Symbols:
+  - Name:            .text
+    Type:            STT_SECTION

lld/test/ELF/relocatable-eh-frame.s

@@ -0,0 +1,19 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: ld.lld -r %t.o %t.o -o %t
+# RUN: llvm-readobj -r %t | FileCheck %s
+# RUN: ld.lld %t -o %t.so -shared
+# RUN: llvm-objdump -h %t.so | FileCheck --check-prefix=DSO %s
+
+# DSO: .eh_frame     00000034
+
+# CHECK:      Relocations [
+# CHECK-NEXT:   Section ({{.*}}) .rela.eh_frame {
+# CHECK-NEXT:     0x20 R_X86_64_PC32 .foo 0x0
+# CHECK-NEXT:     0x50 R_X86_64_NONE - 0x0
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+.section .foo,"aG",@progbits,bar,comdat
+.cfi_startproc
+.cfi_endproc