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