[PATCH] D30892: [ELF] - Make Bss and BssRelRo sections to be synthetic (#3).
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 16 17:23:39 PDT 2017
LGTM.
I tested that lnt is happy with this on AARCH64.
Cheers,
Rafael
George Rimar via Phabricator <reviews at reviews.llvm.org> writes:
> grimar updated this revision to Diff 92006.
> grimar added a comment.
>
> - Fixed one more issue that hopefully should be enough now for aarch64 bot.
>
> Issue was relative to symbol alignment. If we had common symbol and ".bss" section for it with alignment 4
> and then created copy relocation for symbol with alignment == 8 then final layout was wrong.
>
> It was AAAABBBBBBBB when should be AAAA0000BBBBBBBB, where A - space for common symbol and
> B - space for copy relocated.
>
> That happened because previously we would create 2 .bss sections (one for commons, one for relocations),
> but now create single.
>
> Testcase is provided.
>
>
> https://reviews.llvm.org/D30892
>
> Files:
> ELF/OutputSections.cpp
> ELF/OutputSections.h
> ELF/Relocations.cpp
> ELF/Symbols.cpp
> ELF/Symbols.h
> ELF/SyntheticSections.cpp
> ELF/SyntheticSections.h
> ELF/Writer.cpp
> test/ELF/Inputs/relocation-copy-align-common.s
> test/ELF/relocation-copy-align-common.s
>
> Index: test/ELF/relocation-copy-align-common.s
> ===================================================================
> --- test/ELF/relocation-copy-align-common.s
> +++ test/ELF/relocation-copy-align-common.s
> @@ -0,0 +1,40 @@
> +# 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 \
> +# RUN: %p/Inputs/relocation-copy-align-common.s -o %t2.o
> +# RUN: ld.lld -shared %t2.o -o %t.so
> +# RUN: ld.lld %t.o %t.so -o %t3
> +# RUN: llvm-readobj -s -r --expand-relocs %t3 | FileCheck %s
> +
> +# CHECK: Section {
> +# CHECK: Index:
> +# CHECK: Name: .bss
> +# CHECK-NEXT: Type: SHT_NOBITS
> +# CHECK-NEXT: Flags [
> +# CHECK-NEXT: SHF_ALLOC
> +# CHECK-NEXT: SHF_WRITE
> +# CHECK-NEXT: ]
> +# CHECK-NEXT: Address: 0x203000
> +# CHECK-NEXT: Offset: 0x20B0
> +# CHECK-NEXT: Size: 16
> +# CHECK-NEXT: Link: 0
> +# CHECK-NEXT: Info: 0
> +# CHECK-NEXT: AddressAlignment: 8
> +# CHECK-NEXT: EntrySize: 0
> +# CHECK-NEXT: }
> +
> +# CHECK: Relocations [
> +# CHECK-NEXT: Section {{.*}} .rela.dyn {
> +# CHECK-NEXT: Relocation {
> +# CHECK-NEXT: Offset: 0x203008
> +# CHECK-NEXT: Type: R_X86_64_COPY
> +# CHECK-NEXT: Symbol: foo
> +# CHECK-NEXT: Addend: 0x0
> +# CHECK-NEXT: }
> +# CHECK-NEXT: }
> +# CHECK-NEXT: ]
> +
> +.global _start
> +_start:
> +.comm sym1,4,4
> +movl $5, foo
> Index: test/ELF/Inputs/relocation-copy-align-common.s
> ===================================================================
> --- test/ELF/Inputs/relocation-copy-align-common.s
> +++ test/ELF/Inputs/relocation-copy-align-common.s
> @@ -0,0 +1,7 @@
> +.data
> +.global foo
> +.type foo, @object
> +.align 8
> +.size foo, 8
> +foo:
> +.quad 0
> Index: ELF/Writer.cpp
> ===================================================================
> --- ELF/Writer.cpp
> +++ ELF/Writer.cpp
> @@ -111,8 +111,8 @@
> }
>
> for (StringRef V :
> - {".text.", ".rodata.", ".data.rel.ro.", ".data.", ".bss.",
> - ".init_array.", ".fini_array.", ".ctors.", ".dtors.", ".tbss.",
> + {".text.", ".rodata.", ".data.rel.ro.", ".data.", ".bss.rel.ro.",
> + ".bss.", ".init_array.", ".fini_array.", ".ctors.", ".dtors.", ".tbss.",
> ".gcc_except_table.", ".tdata.", ".ARM.exidx."}) {
> StringRef Prefix = V.drop_back();
> if (Name.startswith(V) || Name == Prefix)
> @@ -327,11 +327,6 @@
>
> auto Add = [](InputSectionBase *Sec) { InputSections.push_back(Sec); };
>
> - // Create singleton output sections.
> - Out::Bss = make<OutputSection>(".bss", SHT_NOBITS, SHF_ALLOC | SHF_WRITE);
> - Out::BssRelRo =
> - make<OutputSection>(".bss.rel.ro", SHT_NOBITS, SHF_ALLOC | SHF_WRITE);
> -
> In<ELFT>::DynStrTab = make<StringTableSection>(".dynstr", true);
> In<ELFT>::Dynamic = make<DynamicSection<ELFT>>();
> In<ELFT>::RelaDyn = make<RelocationSection<ELFT>>(
> @@ -369,6 +364,11 @@
> Add(Common);
> }
>
> + In<ELFT>::Bss = make<BssSection>(".bss");
> + Add(In<ELFT>::Bss);
> + In<ELFT>::BssRelRo = make<BssSection>(".bss.rel.ro");
> + Add(In<ELFT>::BssRelRo);
> +
> // Add MIPS-specific sections.
> bool HasDynSymTab =
> !Symtab<ELFT>::X->getSharedFiles().empty() || Config->pic() ||
> @@ -617,7 +617,7 @@
> return true;
> if (In<ELFT>::Got && Sec == In<ELFT>::Got->OutSec)
> return true;
> - if (Sec == Out::BssRelRo)
> + if (Sec == In<ELFT>::BssRelRo->OutSec)
> return true;
>
> StringRef S = Sec->Name;
> @@ -1163,14 +1163,15 @@
>
> // Dynamic section must be the last one in this list and dynamic
> // symbol table section (DynSymTab) must be the first one.
> - applySynthetic({In<ELFT>::DynSymTab, In<ELFT>::GnuHashTab, In<ELFT>::HashTab,
> - In<ELFT>::SymTab, In<ELFT>::ShStrTab, In<ELFT>::StrTab,
> - In<ELFT>::VerDef, In<ELFT>::DynStrTab, In<ELFT>::GdbIndex,
> - In<ELFT>::Got, In<ELFT>::MipsGot, In<ELFT>::IgotPlt,
> - In<ELFT>::GotPlt, In<ELFT>::RelaDyn, In<ELFT>::RelaIplt,
> - In<ELFT>::RelaPlt, In<ELFT>::Plt, In<ELFT>::Iplt,
> - In<ELFT>::Plt, In<ELFT>::EhFrameHdr, In<ELFT>::VerSym,
> - In<ELFT>::VerNeed, In<ELFT>::Dynamic},
> + applySynthetic({In<ELFT>::DynSymTab, In<ELFT>::Bss, In<ELFT>::BssRelRo,
> + In<ELFT>::GnuHashTab, In<ELFT>::HashTab, In<ELFT>::SymTab,
> + In<ELFT>::ShStrTab, In<ELFT>::StrTab, In<ELFT>::VerDef,
> + In<ELFT>::DynStrTab, In<ELFT>::GdbIndex, In<ELFT>::Got,
> + In<ELFT>::MipsGot, In<ELFT>::IgotPlt, In<ELFT>::GotPlt,
> + In<ELFT>::RelaDyn, In<ELFT>::RelaIplt, In<ELFT>::RelaPlt,
> + In<ELFT>::Plt, In<ELFT>::Iplt, In<ELFT>::Plt,
> + In<ELFT>::EhFrameHdr, In<ELFT>::VerSym, In<ELFT>::VerNeed,
> + In<ELFT>::Dynamic},
> [](SyntheticSection *SS) { SS->finalizeContents(); });
>
> // Some architectures use small displacements for jump instructions.
> @@ -1199,16 +1200,6 @@
> }
>
> template <class ELFT> void Writer<ELFT>::addPredefinedSections() {
> - // Add BSS sections.
> - auto Add = [=](OutputSection *Sec) {
> - if (!Sec->Sections.empty()) {
> - Sec->assignOffsets();
> - OutputSections.push_back(Sec);
> - }
> - };
> - Add(Out::Bss);
> - Add(Out::BssRelRo);
> -
> // ARM ABI requires .ARM.exidx to be terminated by some piece of data.
> // We have the terminater synthetic section class. Add that at the end.
> auto *OS = dyn_cast_or_null<OutputSection>(findSection(".ARM.exidx"));
> Index: ELF/SyntheticSections.h
> ===================================================================
> --- ELF/SyntheticSections.h
> +++ ELF/SyntheticSections.h
> @@ -153,15 +153,19 @@
> uint8_t *HashBuf;
> };
>
> -// For each copy relocation, we create an instance of this class to
> -// reserve space in .bss or .bss.rel.ro.
> -template <class ELFT> class CopyRelSection final : public SyntheticSection {
> +// BssSection is used to reserve space for copy relocations. We create two
> +// instances of this class for .bss and .bss.rel.ro. .bss is used for writable
> +// symbols, and .bss.rel.ro is used for read-only symbols.
> +class BssSection final : public SyntheticSection {
> public:
> - CopyRelSection(bool ReadOnly, uint32_t Alignment, size_t Size);
> + BssSection(StringRef Name);
> void writeTo(uint8_t *) override {}
> -
> + bool empty() const override { return getSize() == 0; }
> + size_t reserveSpace(uint32_t Alignment, size_t Size);
> size_t getSize() const override { return Size; }
> - size_t Size;
> +
> +private:
> + size_t Size = 0;
> };
>
> template <class ELFT> class MipsGotSection final : public SyntheticSection {
> @@ -754,6 +758,8 @@
> // Linker generated sections which can be used as inputs.
> struct InX {
> static InputSection *ARMAttributes;
> + static BssSection *Bss;
> + static BssSection *BssRelRo;
> static InputSection *Common;
> static StringTableSection *DynStrTab;
> static InputSection *Interp;
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -376,12 +376,15 @@
> HashFn(HashBuf, Hashes);
> }
>
> -template <class ELFT>
> -CopyRelSection<ELFT>::CopyRelSection(bool ReadOnly, uint32_t Alignment,
> - size_t S)
> - : SyntheticSection(SHF_ALLOC, SHT_NOBITS, Alignment,
> - ReadOnly ? ".bss.rel.ro" : ".bss"),
> - Size(S) {}
> +BssSection::BssSection(StringRef Name)
> + : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_NOBITS, 0, Name) {}
> +
> +size_t BssSection::reserveSpace(uint32_t Alignment, size_t Size) {
> + OutSec->updateAlignment(Alignment);
> + this->Size = alignTo(this->Size, Alignment) + Size;
> + this->Alignment = std::max<uint32_t>(this->Alignment, Alignment);
> + return this->Size - Size;
> +}
>
> template <class ELFT>
> void BuildIdSection<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
> @@ -2260,6 +2263,8 @@
> }
>
> InputSection *InX::ARMAttributes;
> +BssSection *InX::Bss;
> +BssSection *InX::BssRelRo;
> InputSection *InX::Common;
> StringTableSection *InX::DynStrTab;
> InputSection *InX::Interp;
> @@ -2312,11 +2317,6 @@
> template class elf::BuildIdSection<ELF64LE>;
> template class elf::BuildIdSection<ELF64BE>;
>
> -template class elf::CopyRelSection<ELF32LE>;
> -template class elf::CopyRelSection<ELF32BE>;
> -template class elf::CopyRelSection<ELF64LE>;
> -template class elf::CopyRelSection<ELF64BE>;
> -
> template class elf::GotSection<ELF32LE>;
> template class elf::GotSection<ELF32BE>;
> template class elf::GotSection<ELF64LE>;
> Index: ELF/Symbols.h
> ===================================================================
> --- ELF/Symbols.h
> +++ ELF/Symbols.h
> @@ -238,8 +238,9 @@
> // This field is a pointer to the symbol's version definition.
> const void *Verdef;
>
> - // Section is significant only when NeedsCopy is true.
> - InputSection *Section = nullptr;
> + // CopyRelSec and CopyRelSecOff are significant only when NeedsCopy is true.
> + InputSection *CopyRelSec;
> + size_t CopyRelSecOff;
>
> private:
> template <class ELFT> const typename ELFT::Sym &getSym() const {
> Index: ELF/Symbols.cpp
> ===================================================================
> --- ELF/Symbols.cpp
> +++ ELF/Symbols.cpp
> @@ -106,7 +106,8 @@
> case SymbolBody::SharedKind: {
> auto &SS = cast<SharedSymbol>(Body);
> if (SS.NeedsCopy)
> - return SS.Section->OutSec->Addr + SS.Section->OutSecOff;
> + return SS.CopyRelSec->OutSec->Addr + SS.CopyRelSec->OutSecOff +
> + SS.CopyRelSecOff;
> if (SS.NeedsPltAddr)
> return Body.getPltVA<ELFT>();
> return 0;
> @@ -207,7 +208,7 @@
>
> if (auto *S = dyn_cast<SharedSymbol>(this)) {
> if (S->NeedsCopy)
> - return S->Section->OutSec;
> + return S->CopyRelSec->OutSec;
> return nullptr;
> }
>
> Index: ELF/Relocations.cpp
> ===================================================================
> --- ELF/Relocations.cpp
> +++ ELF/Relocations.cpp
> @@ -479,23 +479,20 @@
> // See if this symbol is in a read-only segment. If so, preserve the symbol's
> // memory protection by reserving space in the .bss.rel.ro section.
> bool IsReadOnly = isReadOnly<ELFT>(SS);
> - OutputSection *OSec = IsReadOnly ? Out::BssRelRo : Out::Bss;
> -
> - // Create a SyntheticSection in Out to hold the .bss and the Copy Reloc.
> - auto *ISec =
> - make<CopyRelSection<ELFT>>(IsReadOnly, SS->getAlignment<ELFT>(), SymSize);
> - OSec->addSection(ISec);
> + BssSection *Sec = IsReadOnly ? In<ELFT>::BssRelRo : In<ELFT>::Bss;
> + uintX_t Off = Sec->reserveSpace(SS->getAlignment<ELFT>(), SymSize);
>
> // Look through the DSO's dynamic symbol table for aliases and create a
> // dynamic symbol for each one. This causes the copy relocation to correctly
> // interpose any aliases.
> for (SharedSymbol *Sym : getSymbolsAt<ELFT>(SS)) {
> Sym->NeedsCopy = true;
> - Sym->Section = ISec;
> + Sym->CopyRelSec = Sec;
> + Sym->CopyRelSecOff = Off;
> Sym->symbol()->IsUsedInRegularObj = true;
> }
>
> - In<ELFT>::RelaDyn->addReloc({Target->CopyRel, ISec, 0, false, SS, 0});
> + In<ELFT>::RelaDyn->addReloc({Target->CopyRel, Sec, Off, false, SS, 0});
> }
>
> template <class ELFT>
> Index: ELF/OutputSections.h
> ===================================================================
> --- ELF/OutputSections.h
> +++ ELF/OutputSections.h
> @@ -95,8 +95,6 @@
> // until Writer is initialized.
> struct Out {
> static uint8_t First;
> - static OutputSection *Bss;
> - static OutputSection *BssRelRo;
> static OutputSection *Opd;
> static uint8_t *OpdBuf;
> static PhdrEntry *TlsPhdr;
> Index: ELF/OutputSections.cpp
> ===================================================================
> --- ELF/OutputSections.cpp
> +++ ELF/OutputSections.cpp
> @@ -31,8 +31,6 @@
> using namespace lld::elf;
>
> uint8_t Out::First;
> -OutputSection *Out::Bss;
> -OutputSection *Out::BssRelRo;
> OutputSection *Out::Opd;
> uint8_t *Out::OpdBuf;
> PhdrEntry *Out::TlsPhdr;
More information about the llvm-commits
mailing list