Skip to content

Commit

Permalink
Reland D61583 [ELF] Error on relocations to STT_SECTION symbols if th…
Browse files Browse the repository at this point in the history
…e sections were discarded

This is implemented by creating Undefined (instead of Defined) for such
local STT_SECTION symbols. It allows us to catch errors when there are
relocations to such discarded sections (e.g. in PR41693, ld.bfd and gold
error but we don't). 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.

rLLD361144 inadvertently enabled the error for --gdb-index
(in LLDDwarfObj<ELFT>::findAux()).

Relocations from .debug_info (not in comdat) to .text.* (in comdat) for
DW_AT_low_pc are common. If an .text.* was discarded, rLLD361144 would error,
which was unexpected. (Note, if we don't error as this patch does,
InputSection::relocateNonAlloc() will resolve such relocations).

llvm-svn: 361830
MaskRay committed May 28, 2019
1 parent 49e432d commit 5d3b318
Showing 10 changed files with 100 additions and 12 deletions.
5 changes: 4 additions & 1 deletion lld/ELF/DWARF.cpp
Original file line number Diff line number Diff line change
@@ -93,8 +93,11 @@ 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.
auto *DR = dyn_cast<Defined>(&File->getRelocTargetSym(Rel));
Symbol &S = File->getRelocTargetSym(Rel);
auto *DR = dyn_cast<Defined>(&S);
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" +
3 changes: 3 additions & 0 deletions lld/ELF/InputFiles.cpp
Original file line number Diff line number Diff line change
@@ -997,6 +997,9 @@ 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);
3 changes: 2 additions & 1 deletion lld/ELF/InputSection.cpp
Original file line number Diff line number Diff line change
@@ -438,7 +438,8 @@ void InputSection::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
// 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;
14 changes: 11 additions & 3 deletions lld/ELF/Relocations.cpp
Original file line number Diff line number Diff line change
@@ -681,9 +681,17 @@ static std::string maybeReportDiscarded(Undefined &Sym, InputSectionBase &Sec,
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)
12 changes: 11 additions & 1 deletion lld/test/ELF/comdat-discarded-error.s
Original file line number Diff line number Diff line change
@@ -5,14 +5,24 @@
# 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
63 changes: 63 additions & 0 deletions lld/test/ELF/comdat-discarded-gdb-index.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# REQUIRES: x86
# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
# RUN: ld.lld --gdb-index %t.o %t.o -o %t

## .debug_info has a relocation to .text.foo . The second %t.o is discarded.
## Check we don't error on the relocation.
# CHECK: .rela.debug_info {
# CHECK-NEXT: 0xC R_X86_64_64 .text.foo 0x0

.section .text.foo,"axG",@progbits,foo,comdat
.globl foo
.Lfunc_begin0:
foo:
ret
.Lfunc_end0:

.section .debug_abbrev,"",@progbits
.byte 1 # Abbreviation Code
.byte 17 # DW_TAG_compile_unit
.byte 1 # DW_CHILDREN_yes
.byte 17 # DW_AT_low_pc
.byte 1 # DW_FORM_addr
.byte 18 # DW_AT_high_pc
.byte 6 # DW_FORM_data4
.ascii "\264B" # DW_AT_GNU_pubnames
.byte 25 # DW_FORM_flag_present
.byte 0 # EOM(1)
.byte 0 # EOM(2)
.byte 2 # Abbreviation Code
.byte 46 # DW_TAG_subprogram
.byte 0 # DW_CHILDREN_no
.byte 3 # DW_AT_name
.byte 8 # DW_FORM_string
.byte 0 # EOM(1)
.byte 0 # EOM(2)
.byte 0

.section .debug_info,"",@progbits
.Lcu_begin0:
.long .Lcu_end0 - .Lcu_begin0 - 4
.short 4 # DWARF version number
.long 0 # Offset Into Abbrev. Section
.byte 4 # Address Size
.Ldie0:
.byte 1 # Abbrev [1] DW_TAG_compile_unit
.quad .Lfunc_begin0 # DW_AT_low_pc
.long .Lfunc_end0 - .Lfunc_begin0 # DW_AT_high_pc
.byte 2 # Abbrev [2] DW_TAG_subprogram
.asciz "foo" # DW_AT_name
.byte 0
.Lcu_end0:

.section .debug_gnu_pubnames,"",@progbits
.long .LpubNames_end0 - .LpubNames_begin0
.LpubNames_begin0:
.short 2 # Version
.long .Lcu_begin0 # CU Offset
.long .Lcu_end0 - .Lcu_begin0
.long .Ldie0 - .Lcu_begin0
.byte 48 # Attributes: FUNCTION, EXTERNAL
.asciz "foo" # External Name
.long 0
.LpubNames_end0:
2 changes: 1 addition & 1 deletion lld/test/ELF/comdat-discarded-reloc.s
Original file line number Diff line number Diff line change
@@ -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-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)
4 changes: 1 addition & 3 deletions lld/test/ELF/comdat.s
Original file line number Diff line number Diff line change
@@ -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 %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

@@ -31,9 +31,7 @@ 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

2 changes: 1 addition & 1 deletion lld/test/ELF/invalid-undef-section-symbol.test
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# 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
4 changes: 3 additions & 1 deletion lld/test/ELF/relocatable-eh-frame.s
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
# 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 [

0 comments on commit 5d3b318

Please sign in to comment.