[ELF] Create Undefined for local STT_SECTION symbol if the section is discarded

[MaskRay]

Summary

This allows us to catch some errors when there are relocations to such
discarded sections (e.g. PR41693). Updated comdat-discarded-error.s
checks we emit friendly error message.

For relocatable-eh-frame.s, ld.lld -r a.o a.o will now error
"STT_SECTION symbol should be defined" because the section .eh_frame
refers to is now an Undefined instead of a Defined.
So I have to change error() to warn() to retain the output.

[MaskRay]

Update

[MaskRay]

Rebase

[MaskRay]

🤪

[ruiu]

LGTM

[phosek]

We're now getting a lot of STT_SECTION symbol should be defined warnings when building libFuzzer: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8911634509805270864/+/steps/clang/0/steps/build/0/stdout. The libFuzzer uses lld to produce a single relocatable file with libc++ linked in: https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/fuzzer/CMakeLists.txt#L132. I haven't yet figured out what's going on (the warning could be also a little more specific, e.g. which symbol and which section). Do you have any idea what's going on?

[phosek]

https://storage.googleapis.com/fuchsia-build/reproducer/fuzzer.tar.bz2 is the reproducer.

[MaskRay]

In D61583#1528517, @phosek wrote:

https://storage.googleapis.com/fuchsia-build/reproducer/fuzzer.tar.bz2 is the reproducer.

Thanks for the reproduce. I think this error (added in rLLD319404) should probably be suppressed for non-alloc sections. I'll post a patch.

[MaskRay]

In D61583#1528522, @MaskRay wrote:

In D61583#1528517, @phosek wrote:

https://storage.googleapis.com/fuchsia-build/reproducer/fuzzer.tar.bz2 is the reproducer.

Thanks for the reproduce. I think this error (added in rLLD319404) should probably be suppressed for non-alloc sections. I'll post a patch.

D62840

[MaskRay]

@sfertile I'm confused with the recent revert and revert of revert.. The problem this change caused should have been fixed by D62840. Did you find other problems? (The warn() can be deleted if you suspect there is something wrong.)

[sfertile]

In D61583#1533390, @MaskRay wrote:

@sfertile I'm confused with the recent revert and revert of revert.. The problem this change caused should have been fixed by D62840. Did you find other problems? (The warn() can be deleted if you suspect there is somethin

The multistage power-pc build bot is broken

69.906 [3746/40/441] Linking CXX executable bin/llvm-tblgen
FAILED: bin/llvm-tblgen 
: && /home/buildbots/ppc64le-lld-multistage-test/ppc64le-lld-multistage-test/install/stage1/bin/clang++  -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3  -fuse-ld=lld -Wl,--color-diagnostics -Wl,-allow-shlib-undefined    -Wl,-O3 -Wl,--gc-sections utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmMatcherEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/AsmWriterInst.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/Attributes.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CallingConvEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeEmitterGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenDAGPatterns.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenHwModes.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenInstruction.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenMapTable.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenRegisters.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenSchedule.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenTarget.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcherOpt.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DAGISelMatcher.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DFAPacketizerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/DisassemblerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/ExegesisEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/FastISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/FixedLenDecoderEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InfoByHwMode.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InstrInfoEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/InstrDocsEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/IntrinsicEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/OptParserEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/PredicateExpander.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/PseudoLoweringEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RISCVCompressInstEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RegisterBankEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/RegisterInfoEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SDNodeProperties.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SearchableTableEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SubtargetEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/SubtargetFeatureInfo.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/TableGen.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/Types.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86DisassemblerTables.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86EVEX2VEXTablesEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86FoldTablesEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86ModRMFilters.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/X86RecognizableInstr.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/WebAssemblyDisassemblerEmitter.cpp.o utils/TableGen/CMakeFiles/llvm-tblgen.dir/CTagsEmitter.cpp.o  -o bin/llvm-tblgen  -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMSupport.a lib/libLLVMTableGen.a -lpthread lib/libLLVMSupport.a -lz -lrt -ldl -ltinfo -lpthread -lm lib/libLLVMDemangle.a && :
ld.lld: error: relocation refers to a discarded section: .rodata._ZNK4llvm3MVT13getSizeInBitsEv
>>> defined in utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenRegisters.cpp.o
>>> referenced by CodeGenRegisters.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/CodeGenRegisters.cpp.o:(.toc+0x0)

ld.lld: error: relocation refers to a discarded section: .rodata._ZNK4llvm3MVT13getSizeInBitsEv
>>> defined in utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o
>>> referenced by GlobalISelEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o:(.toc+0x20)

ld.lld: error: relocation refers to a discarded section: .rodata._ZNK4llvm3MVT20getVectorElementTypeEv
>>> defined in utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o
>>> referenced by GlobalISelEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/GlobalISelEmitter.cpp.o:(.toc+0x28)

ld.lld: error: relocation refers to a discarded section: .rodata._ZNK4llvm3MVT20getVectorElementTypeEv
>>> defined in utils/TableGen/CMakeFiles/llvm-tblgen.dir/IntrinsicEmitter.cpp.o
>>> referenced by IntrinsicEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/IntrinsicEmitter.cpp.o:(.toc+0x38)

ld.lld: error: relocation refers to a discarded section: .rodata._ZNK4llvm3MVT13getSizeInBitsEv
>>> defined in utils/TableGen/CMakeFiles/llvm-tblgen.dir/IntrinsicEmitter.cpp.o
>>> referenced by IntrinsicEmitter.cpp
>>>               utils/TableGen/CMakeFiles/llvm-tblgen.dir/IntrinsicEmitter.cpp.o:(.toc+0x40)

ld.lld: error: relocation refers to a discarded section: .rodata._ZN4llvm4yaml11needsQuotesENS_9StringRefE
>>> defined in lib/libLLVMSupport.a(Timer.cpp.o)
>>> referenced by Timer.cpp
>>>               Timer.cpp.o:(.toc+0x30) in archive lib/libLLVMSupport.a
clang-9: error: linker command failed with exit code 1 (use -v to see invocation)

The build monitor narrowed it down to this patch so I was pulling it to turn the bot back to green until we can figure out why this patch only causes a problem on PowerPC. I don't think there is anything necessarily wrong with the warning. but I do think we need to see if we are discarding the same sections before/after this patch. I'm not familiar with when/why we discard sections but it seems quite odd to me we are discarding a section with relocations referencing it.

[sfertile]

@MaskRay Are you planning on pulling this? The buildbot is still red and there have been numerous Power related changes going in with no buildbot coverage.

ELF/InputFiles.cpp

if (Binding == STB_LOCAL) {
       if (ESym.getType() == STT_FILE)
         SourceFile = CHECK(ESym.getName(this->StringTable), this);
 
       if (this->StringTable.size() <= ESym.st_name)
         fatal(toString(this) + ": invalid symbol name offset");
 
       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);
       continue;
     }
 
     // Handle global undefined symbols.
     if (ESym.st_shndx == SHN_UNDEF) {

ELF/InputSection.cpp

if (Sym.Type == STT_SECTION) {
       // section data. We do that by adding to the Relocation vector.
 
       // .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.
       auto *D = dyn_cast<Defined>(&Sym);
       if (!D) {
-        error("STT_SECTION symbol should be defined");
+        warn("STT_SECTION symbol should be defined");
+        P->setSymbolAndType(0, 0, false);
         continue;
       }
       SectionBase *Section = D->Section->Repl;
       if (!Section->Live) {
         P->setSymbolAndType(0, 0, false);
         continue;
       }
 

ELF/Relocations.cpp

 static std::string maybeReportDiscarded(Undefined &Sym, InputSectionBase &Sec,
                                         uint64_t Offset) {
   auto *File = dyn_cast_or_null<ObjFile<ELFT>>(Sym.File);
   if (!File || !Sym.DiscardedSecIdx ||
       File->getSections()[Sym.DiscardedSecIdx] != &InputSection::Discarded)
     return "";
   ArrayRef<Elf_Shdr_Impl<ELFT>> ObjSections =
       CHECK(File->getObj().sections(), File);
-  std::string Msg =
-      "relocation refers to a symbol in a discarded section: " + toString(Sym) +
-      "\n>>> defined in " + toString(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);
 
   Elf_Shdr_Impl<ELFT> ELFSec = ObjSections[Sym.DiscardedSecIdx - 1];
   if (ELFSec.sh_type != SHT_GROUP)
     return Msg;
 
   // If the discarded section is a COMDAT.
   StringRef Signature = File->getShtGroupSignature(ObjSections, ELFSec);
   if (const InputFile *Prevailing =

test/ELF/comdat-discarded-error.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t1.o
 # RUN: echo '.section .text.foo,"axG",@progbits,foo,comdat; .globl foo; foo:' | \
 # RUN:   llvm-mc -filetype=obj -triple=x86_64 - -o %t2.o
 # 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 %t1.o %t2.o %t3.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: not ld.lld %t2.o %t3.o %t1.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

test/ELF/comdat-discarded-reloc.s

 # 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 %t.o %t2.o -o %t
+# RUN: ld.lld -gc-sections --noinhibit-exec %t.o %t2.o -o /dev/null
 
 ## 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.
 
 .global bar, _start
 
 .section .text.foo,"aG",@progbits,group,comdat
 
 .section .text
 _start:
  .quad .text.foo
  .quad bar

test/ELF/comdat.s

 // 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 %t.o %t2.o -o %t
+// RUN: ld.lld -shared %t.o %t2.o -o %t
 // RUN: llvm-objdump -d %t | FileCheck %s
 // RUN: llvm-readobj -S --symbols %t | FileCheck --check-prefix=READ %s
 
 // Check that we don't crash with --gc-section and that we print a list of
 // reclaimed sections on stderr.
 // RUN: ld.lld --gc-sections --print-gc-sections -shared %t.o %t.o %t2.o -o %t \
 // RUN:   2>&1 | FileCheck --check-prefix=GC %s
 // GC: removing unused section {{.*}}.o:(.text)
// CHECK-NOT: nop
 
         .section bar, "ax"
         call foo
 
 // CHECK: Disassembly of section bar:
 // 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
 
 
 // READ:      Name: .text2
 // READ-NEXT: Type: SHT_PROGBITS
 // READ-NEXT: Flags [
 // READ-NEXT:   SHF_ALLOC

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

 # RUN: yaml2obj %s -o %t.o
-# RUN: not ld.lld -r %t.o -o /dev/null 2>&1 | FileCheck %s
+# RUN: not ld.lld -r --fatal-warnings %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

test/ELF/relocatable-eh-frame.s

 # 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: ld.lld -r %t.o %t.o -o %t 2>&1 | FileCheck --check-prefix=WARN %s
 # 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
 
+# WARN: STT_SECTION symbol should be defined
+
 # 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