<div dir="ltr">This change causes check-llvm to fail for me when testing (ironically enough) LLVM's Go bindings, as a result of a segfault in lld. I've attached a repro file.<div><br><div>It looks like the problem is that we segfault when looking up the output section if the .eh_frame section contains a relocation to a discarded section symbol in an FDE. I haven't really thought about how we need to handle FDEs referring to discarded sections in -r mode.</div><div><br></div><div>Peter</div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 10, 2017 at 5:40 PM, Rafael Espindola via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Fri Feb 10 19:40:49 2017<br>
New Revision: 294816<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=294816&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=294816&view=rev</a><br>
Log:<br>
Create only one section symbol per section.<br>
<br>
Unfortunately some consumers of our .o files produced with -r expect<br>
only one section symbol per section. That is true of at least of go's<br>
own linker.<br>
<br>
Combining them is a somewhat convoluted process. We have to create a<br>
symbol for every section since we don't know which ones will be<br>
needed. The relocation sections also have to be written first to<br>
handle the Elf_Rel addend.<br>
<br>
I did consider a completely different approach:<br>
<br>
We could remove the -r special case of relocation sections when<br>
reading. We would instead have a copyRelocs function that is used<br>
instead of scanRelocs. It would create a DynamicReloc for each<br>
relocation and a RelocationSection for each input relocation section.<br>
<br>
A complication of such change is that DynamicReloc would have to take<br>
a section index and a input section instead of a symbol since with<br>
-emit-relocs some DynamicReloc would hold relocations referring to the<br>
dynamic symbol table and other to the static symbol table.<br>
<br>
That would be a pretty big change, and if we do it it is probably<br>
better to do it as a refactoring.<br>
<br>
Added:<br>
lld/trunk/test/ELF/<wbr>relocatable-section-symbol.s<br>
Modified:<br>
lld/trunk/ELF/InputSection.cpp<br>
lld/trunk/ELF/<wbr>SyntheticSections.cpp<br>
lld/trunk/ELF/Writer.cpp<br>
lld/trunk/test/ELF/emit-<wbr>relocs.s<br>
lld/trunk/test/ELF/mips-sto-<wbr>pic-flag.s<br>
lld/trunk/test/ELF/<wbr>relocatable-bss.s<br>
<br>
Modified: lld/trunk/ELF/InputSection.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=294816&r1=294815&r2=294816&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/<wbr>InputSection.cpp?rev=294816&<wbr>r1=294815&r2=294816&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/ELF/InputSection.cpp (original)<br>
+++ lld/trunk/ELF/InputSection.cpp Fri Feb 10 19:40:49 2017<br>
@@ -236,6 +236,22 @@ void InputSection<ELFT>::<wbr>copyRelocations<br>
if (Config->Rela)<br>
P->r_addend = getAddend<ELFT>(Rel);<br>
<br>
+ if (Body.Type == STT_SECTION) {<br>
+ // We combine multiple section symbols into only one per<br>
+ // section. This means we have to update the addend. That is<br>
+ // trivial for Elf_Rela, but for Elf_Rel we have to write to the<br>
+ // section data. We do that by adding to the Relocation vector.<br>
+ if (Config->Rela) {<br>
+ P->r_addend += Body.getVA<ELFT>() -<br>
+ cast<DefinedRegular<ELFT>>(<wbr>Body).Section->OutSec->Addr;<br>
+ } else if (Config->Relocatable) {<br>
+ const uint8_t *BufLoc = RelocatedSection->Data.begin() + Rel.r_offset;<br>
+ uint64_t Implicit = Target->getImplicitAddend(<wbr>BufLoc, Type);<br>
+ RelocatedSection->Relocations.<wbr>push_back(<br>
+ {R_ABS, Type, Rel.r_offset, Implicit, &Body});<br>
+ }<br>
+ }<br>
+<br>
// Output section VA is zero for -r, so r_offset is an offset within the<br>
// section, but for --emit-relocs it is an virtual address.<br>
P->r_offset = RelocatedSection->OutSec->Addr +<br>
<br>
Modified: lld/trunk/ELF/<wbr>SyntheticSections.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=294816&r1=294815&r2=294816&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/<wbr>SyntheticSections.cpp?rev=<wbr>294816&r1=294815&r2=294816&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/ELF/<wbr>SyntheticSections.cpp (original)<br>
+++ lld/trunk/ELF/<wbr>SyntheticSections.cpp Fri Feb 10 19:40:49 2017<br>
@@ -1127,8 +1127,16 @@ template <class ELFT> void SymbolTableSe<br>
<br>
template <class ELFT><br>
size_t SymbolTableSection<ELFT>::<wbr>getSymbolIndex(SymbolBody *Body) {<br>
- auto I = llvm::find_if(<br>
- Symbols, [&](const SymbolTableEntry &E) { return E.Symbol == Body; });<br>
+ auto I = llvm::find_if(Symbols, [&](const SymbolTableEntry &E) {<br>
+ if (E.Symbol == Body)<br>
+ return true;<br>
+ // This is used for -r, so we have to handle multiple section<br>
+ // symbols being combined.<br>
+ if (Body->Type == STT_SECTION && E.Symbol->Type == STT_SECTION)<br>
+ return cast<DefinedRegular<ELFT>>(<wbr>Body)->Section->OutSec ==<br>
+ cast<DefinedRegular<ELFT>>(E.<wbr>Symbol)->Section->OutSec;<br>
+ return false;<br>
+ });<br>
if (I == Symbols.end())<br>
return 0;<br>
return I - Symbols.begin() + 1;<br>
<br>
Modified: lld/trunk/ELF/Writer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Writer.cpp?rev=294816&r1=294815&r2=294816&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/ELF/Writer.<wbr>cpp?rev=294816&r1=294815&r2=<wbr>294816&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/ELF/Writer.cpp (original)<br>
+++ lld/trunk/ELF/Writer.cpp Fri Feb 10 19:40:49 2017<br>
@@ -51,6 +51,7 @@ public:<br>
private:<br>
void createSyntheticSections();<br>
void copyLocalSymbols();<br>
+ void addSectionSymbols();<br>
void addReservedSymbols();<br>
void addInputSec(InputSectionBase<<wbr>ELFT> *S);<br>
void createSections();<br>
@@ -228,6 +229,9 @@ template <class ELFT> void Writer<ELFT>:<br>
if (Config->Discard != DiscardPolicy::All)<br>
copyLocalSymbols();<br>
<br>
+ if (Config->copyRelocs())<br>
+ addSectionSymbols();<br>
+<br>
// Now that we have a complete set of output sections. This function<br>
// completes section contents. For example, we need to add strings<br>
// to the string table, and add entries to .got and .plt.<br>
@@ -446,13 +450,9 @@ template <class ELFT> void Writer<ELFT>:<br>
template <class ELFT><br>
static bool shouldKeepInSymtab(<wbr>InputSectionBase<ELFT> *Sec, StringRef SymName,<br>
const SymbolBody &B) {<br>
- if (B.isFile())<br>
+ if (B.isFile() || B.isSection())<br>
return false;<br>
<br>
- // We keep sections in symtab for relocatable output and --emit-reloc.<br>
- if (B.isSection())<br>
- return Config->copyRelocs();<br>
-<br>
// If sym references a section in a discarded group, don't keep it.<br>
if (Sec == &InputSection<ELFT>::<wbr>Discarded)<br>
return false;<br>
@@ -518,6 +518,27 @@ template <class ELFT> void Writer<ELFT>:<br>
}<br>
}<br>
<br>
+template <class ELFT> void Writer<ELFT>::<wbr>addSectionSymbols() {<br>
+ // Create one STT_SECTION symbol for each output section we might<br>
+ // have a relocation with.<br>
+ for (OutputSectionBase *Sec : OutputSections) {<br>
+ InputSectionData *First = nullptr;<br>
+ Sec->forEachInputSection([&](<wbr>InputSectionData *D) {<br>
+ if (!First)<br>
+ First = D;<br>
+ });<br>
+ auto *IS = dyn_cast_or_null<InputSection<<wbr>ELFT>>(First);<br>
+ if (!IS || isa<SyntheticSection<ELFT>>(<wbr>IS) || IS->Type == SHT_REL ||<br>
+ IS->Type == SHT_RELA)<br>
+ continue;<br>
+ auto *B = new (BAlloc)<br>
+ DefinedRegular<ELFT>("", /*IsLocal=*/true, /*StOther*/ 0, STT_SECTION,<br>
+ /*Value*/ 0, /*Size*/ 0, IS, nullptr);<br>
+<br>
+ In<ELFT>::SymTab->addLocal(B);<br>
+ }<br>
+}<br>
+<br>
// PPC64 has a number of special SHT_PROGBITS+SHF_ALLOC+SHF_<wbr>WRITE sections that<br>
// we would like to make sure appear is a specific order to maximize their<br>
// coverage by a single signed 16-bit offset from the TOC base pointer.<br>
@@ -1782,8 +1803,17 @@ template <class ELFT> void Writer<ELFT>:<br>
<br>
OutputSectionBase *EhFrameHdr =<br>
In<ELFT>::EhFrameHdr ? In<ELFT>::EhFrameHdr->OutSec : nullptr;<br>
+<br>
+ // In -r or -emit-relocs mode, write the relocation sections first as in<br>
+ // ELf_Rel targets we might find out that we need to modify the relocated<br>
+ // section while doing it.<br>
+ for (OutputSectionBase *Sec : OutputSections)<br>
+ if (Sec->Type == SHT_REL || Sec->Type == SHT_RELA)<br>
+ Sec->writeTo(Buf + Sec->Offset);<br>
+<br>
for (OutputSectionBase *Sec : OutputSections)<br>
- if (Sec != Out<ELFT>::Opd && Sec != EhFrameHdr)<br>
+ if (Sec != Out<ELFT>::Opd && Sec != EhFrameHdr && Sec->Type != SHT_REL &&<br>
+ Sec->Type != SHT_RELA)<br>
Sec->writeTo(Buf + Sec->Offset);<br>
<br>
// The .eh_frame_hdr depends on .eh_frame section contents, therefore<br>
<br>
Modified: lld/trunk/test/ELF/emit-<wbr>relocs.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/emit-relocs.s?rev=294816&r1=294815&r2=294816&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/test/ELF/<wbr>emit-relocs.s?rev=294816&r1=<wbr>294815&r2=294816&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/test/ELF/emit-<wbr>relocs.s (original)<br>
+++ lld/trunk/test/ELF/emit-<wbr>relocs.s Fri Feb 10 19:40:49 2017<br>
@@ -15,7 +15,7 @@<br>
# CHECK-NEXT: Section ({{.*}}) .rela.text {<br>
# CHECK-NEXT: 0x201002 R_X86_64_32 .text 0x1<br>
# CHECK-NEXT: 0x201007 R_X86_64_PLT32 fn 0xFFFFFFFFFFFFFFFC<br>
-# CHECK-NEXT: 0x20100E R_X86_64_32 .text 0x1<br>
+# CHECK-NEXT: 0x20100E R_X86_64_32 .text 0xD<br>
# CHECK-NEXT: 0x201013 R_X86_64_PLT32 fn2 0xFFFFFFFFFFFFFFFC<br>
# CHECK-NEXT: }<br>
# CHECK-NEXT: ]<br>
@@ -53,15 +53,6 @@<br>
# CHECK-NEXT: Size: 0<br>
# CHECK-NEXT: Binding: Local<br>
# CHECK-NEXT: Type: Section<br>
-# CHECK-NEXT: Other: 0<br>
-# CHECK-NEXT: Section: .text<br>
-# CHECK-NEXT: }<br>
-# CHECK-NEXT: Symbol {<br>
-# CHECK-NEXT: Name:<br>
-# CHECK-NEXT: Value: 0x20100C<br>
-# CHECK-NEXT: Size: 0<br>
-# CHECK-NEXT: Binding: Local<br>
-# CHECK-NEXT: Type: Section<br>
# CHECK-NEXT: Other: 0<br>
# CHECK-NEXT: Section: .text<br>
# CHECK-NEXT: }<br>
<br>
Modified: lld/trunk/test/ELF/mips-sto-<wbr>pic-flag.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/mips-sto-pic-flag.s?rev=294816&r1=294815&r2=294816&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/test/ELF/<wbr>mips-sto-pic-flag.s?rev=<wbr>294816&r1=294815&r2=294816&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/test/ELF/mips-sto-<wbr>pic-flag.s (original)<br>
+++ lld/trunk/test/ELF/mips-sto-<wbr>pic-flag.s Fri Feb 10 19:40:49 2017<br>
@@ -19,8 +19,8 @@<br>
# CHECK-NEXT: Other: 0<br>
# CHECK-NEXT: Section: .text<br>
# CHECK-NEXT: }<br>
-# CHECK-NEXT: Symbol {<br>
-# CHECK-NEXT: Name: foo1a<br>
+# CHECK: Symbol {<br>
+# CHECK: Name: foo1a<br>
# CHECK-NEXT: Value:<br>
# CHECK-NEXT: Size:<br>
# CHECK-NEXT: Binding: Global<br>
<br>
Modified: lld/trunk/test/ELF/<wbr>relocatable-bss.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/relocatable-bss.s?rev=294816&r1=294815&r2=294816&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/test/ELF/<wbr>relocatable-bss.s?rev=294816&<wbr>r1=294815&r2=294816&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/test/ELF/<wbr>relocatable-bss.s (original)<br>
+++ lld/trunk/test/ELF/<wbr>relocatable-bss.s Fri Feb 10 19:40:49 2017<br>
@@ -20,7 +20,7 @@<br>
# CHECK-NEXT: Version:<br>
# CHECK-NEXT: Entry:<br>
# CHECK-NEXT: ProgramHeaderOffset:<br>
-# CHECK-NEXT: SectionHeaderOffset: 0xA8<br>
+# CHECK-NEXT: SectionHeaderOffset: 0xD8<br>
# CHECK-NEXT: Flags [<br>
# CHECK-NEXT: ]<br>
# CHECK-NEXT: HeaderSize:<br>
<br>
Added: lld/trunk/test/ELF/<wbr>relocatable-section-symbol.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/ELF/relocatable-section-symbol.s?rev=294816&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lld/trunk/test/ELF/<wbr>relocatable-section-symbol.s?<wbr>rev=294816&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- lld/trunk/test/ELF/<wbr>relocatable-section-symbol.s (added)<br>
+++ lld/trunk/test/ELF/<wbr>relocatable-section-symbol.s Fri Feb 10 19:40:49 2017<br>
@@ -0,0 +1,49 @@<br>
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o<br>
+# RUN: ld.lld -r -o %t %t.o %t.o<br>
+# RUN: llvm-readobj -r %t | FileCheck --check-prefix=RELA %s<br>
+<br>
+# RELA: Relocations [<br>
+# RELA-NEXT: Section ({{.*}}) .rela.data {<br>
+# RELA-NEXT: 0x0 R_X86_64_32 .text 0x1<br>
+# RELA-NEXT: 0x4 R_X86_64_32 .text 0x5<br>
+# RELA-NEXT: }<br>
+# RELA-NEXT: ]<br>
+<br>
+<br>
+# RUN: llvm-mc -filetype=obj -triple=i686-pc-linux %s -o %t.o<br>
+# RUN: ld.lld -r -o %t %t.o %t.o<br>
+# RUN: llvm-readobj -r -s -section-data %t | FileCheck --check-prefix=REL %s<br>
+<br>
+<br>
+# REL: Section {<br>
+# REL: Index:<br>
+# REL: Name: .data<br>
+# REL-NEXT: Type: SHT_PROGBITS<br>
+# REL-NEXT: Flags [<br>
+# REL-NEXT: SHF_ALLOC<br>
+# REL-NEXT: SHF_WRITE<br>
+# REL-NEXT: ]<br>
+# REL-NEXT: Address:<br>
+# REL-NEXT: Offset:<br>
+# REL-NEXT: Size:<br>
+# REL-NEXT: Link:<br>
+# REL-NEXT: Info:<br>
+# REL-NEXT: AddressAlignment:<br>
+# REL-NEXT: EntrySize:<br>
+# REL-NEXT: SectionData (<br>
+# REL-NEXT: 0000: 01000000 05000000 |<br>
+# REL-NEXT: )<br>
+# REL-NEXT: }<br>
+<br>
+<br>
+# REL: Relocations [<br>
+# REL-NEXT: Section ({{.*}}) .rel.data {<br>
+# REL-NEXT: 0x0 R_386_32 .text 0x0<br>
+# REL-NEXT: 0x4 R_386_32 .text 0x0<br>
+# REL-NEXT: }<br>
+# REL-NEXT: ]<br>
+<br>
+<br>
+.long 42<br>
+.data<br>
+.long .text + 1<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr">-- <div>Peter</div></div></div>
</div>