[PATCH] D42843: Ensure that Elf_Rel addends are always written for dynamic relocations
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 9 15:01:48 PST 2018
Hi,
If you agree with it, could you please commit the attached patch first
and rebase? It makes your patch a lot easier to read.
Thanks,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.diff
Type: text/x-patch
Size: 6404 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180209/3e1aaec8/attachment.bin>
-------------- next part --------------
Alexander Richardson via Phabricator <reviews at reviews.llvm.org> writes:
> arichardson updated this revision to Diff 133610.
> arichardson marked 5 inline comments as done.
> arichardson added a comment.
>
> - rebase
> - address comments
> - use mips for tests to avoid depending on llvm-mc changes
>
>
> Repository:
> rLLD LLVM Linker
>
> https://reviews.llvm.org/D42843
>
> Files:
> ELF/Arch/X86_64.cpp
> ELF/InputSection.cpp
> ELF/Relocations.cpp
> ELF/Relocations.h
> ELF/SyntheticSections.cpp
> ELF/SyntheticSections.h
> test/ELF/rel-addend-with-rela-input.s
>
> Index: test/ELF/rel-addend-with-rela-input.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/rel-addend-with-rela-input.s
> @@ -0,0 +1,43 @@
> +# REQUIRES: mips
> +# Check that we correctly write addends if the output use Elf_Rel but the input
> +# uses Elf_Rela
> +
> +# RUN: llvm-mc -filetype=obj -triple=mips64-unknown-linux %s -o %t-rela.o
> +# RUN: llvm-readobj -h -s -section-data -relocations %t-rela.o | FileCheck -check-prefix INPUT-RELA %s
> +# INPUT-RELA: ElfHeader {
> +# INPUT-RELA: Class: 64-bit
> +# INPUT-RELA: DataEncoding: BigEndian
> +# INPUT-RELA: Section {
> +# INPUT-RELA: Name: .data
> +# INPUT-RELA: SectionData (
> +# INPUT-RELA-NEXT: 0000: 00000000 00000000 ABCDEF00 12345678 |.............4Vx|
> +# ^--- No addend here since it uses RELA
> +# INPUT-RELA: Relocations [
> +# INPUT-RELA-NEXT: Section ({{.+}}) .rela.data {
> +# INPUT-RELA-NEXT: 0x0 R_MIPS_64/R_MIPS_NONE/R_MIPS_NONE foo 0x5544
> +# INPUT-RELA-NEXT: }
> +# INPUT-RELA-NEXT: ]
> +
> +# Previously the addend to the dynamic relocation in the .data section was not copied if
> +# the input file used RELA and the output uses REL. Check that it works now:
> +# RUN: ld.lld -shared -o %t.so %t-rela.o -verbose
> +# RUN: llvm-readobj -h -s -section-data -relocations %t.so | FileCheck -check-prefix RELA-TO-REL %s
> +# RELA-TO-REL: ElfHeader {
> +# RELA-TO-REL: Class: 64-bit
> +# RELA-TO-REL: DataEncoding: BigEndian
> +# RELA-TO-REL: Section {
> +# RELA-TO-REL: Name: .data
> +# RELA-TO-REL: SectionData (
> +# RELA-TO-REL-NEXT: 0000: 00000000 00005544 ABCDEF00 12345678 |......UD.....4Vx|
> +# ^--- Addend for relocation in .rel.dyn
> +# RELA-TO-REL: Relocations [
> +# RELA-TO-REL-NEXT: Section ({{.+}}) .rel.dyn {
> +# RELA-TO-REL-NEXT: 0x10000 R_MIPS_REL32/R_MIPS_64/R_MIPS_NONE foo 0x0
> +# RELA-TO-REL-NEXT: }
> +# RELA-TO-REL-NEXT: ]
> +
> +.extern foo
> +
> +.data
> +.quad foo + 0x5544
> +.quad 0xabcdef0012345678
> Index: ELF/SyntheticSections.h
> ===================================================================
> --- ELF/SyntheticSections.h
> +++ ELF/SyntheticSections.h
> @@ -310,22 +310,29 @@
>
> class DynamicReloc {
> public:
> - DynamicReloc(uint32_t Type, const InputSectionBase *InputSec,
> + DynamicReloc(RelType Type, const InputSectionBase *InputSec,
> uint64_t OffsetInSec, bool UseSymVA, Symbol *Sym, int64_t Addend)
> : Type(Type), Sym(Sym), InputSec(InputSec), OffsetInSec(OffsetInSec),
> UseSymVA(UseSymVA), Addend(Addend) {}
>
> uint64_t getOffset() const;
> - int64_t getAddend() const;
> uint32_t getSymIndex() const;
> const InputSectionBase *getInputSec() const { return InputSec; }
>
> - uint32_t Type;
> + // Return the value that should be written to the Elf_Rela r_addend field.
> + // This is not the same as the addend used when creating this dynamic
> + // relocation since we may have converted a static relocation against a
> + // non-preemptible symbol into a relocation against the load address. In that
> + // case we write the virtual address of the symbol plus the addend into the
> + // r_addend field.
> + int64_t getRelaAddend() const;
>
> -private:
> + RelType Type;
> Symbol *Sym;
> const InputSectionBase *InputSec = nullptr;
> uint64_t OffsetInSec;
> + // If this member is true we will add the dynamic relocation against the load
> + // address and use the symbol virtual address as the addend
> bool UseSymVA;
> int64_t Addend;
> };
> @@ -361,17 +368,32 @@
> public:
> RelocationBaseSection(StringRef Name, uint32_t Type, int32_t DynamicTag,
> int32_t SizeDynamicTag);
> - void addReloc(uint32_t DynType, InputSectionBase *InputSec,
> - uint64_t OffsetInSec, bool UseSymVA, Symbol *Sym,
> - int64_t Addend, RelExpr Expr, RelType Type);
> - void addReloc(const DynamicReloc &Reloc);
> + // TODO: We could merge these two functions if we can assume that the
> + // dynamic relocation and the static one touch the same bits. This does not
> + // always seem to be the case as e.g. on MIPS32 a R_MIPS_64 will be converted
> + // to a R_MIPS_REL32, but this may just be a bug in the MIPS code.
> +
> + // Add a dynamic relocation without an addend that will be processed by the
> + // the runtime linker. Since there is no addend this does the same for REL
> + // and RELA outputs
> + void addReloc(uint32_t DynType, InputSectionBase *IS, uint64_t OffsetInSec,
> + Symbol *Sym);
> +
> + // Add a dynamic relocation that may have been converted to a relocation
> + // against the load address. In that case the addend will be the virtual
> + // address of the symbol and must be written to the output if we are using
> + // REL output.
> + void addComplexReloc(const DynamicReloc &Reloc, RelType Type, RelExpr Expr);
> +
> bool empty() const override { return Relocs.empty(); }
> size_t getSize() const override { return Relocs.size() * this->Entsize; }
> size_t getRelativeRelocCount() const { return NumRelativeRelocs; }
> void finalizeContents() override;
> int32_t DynamicTag, SizeDynamicTag;
>
> protected:
> + // Add Reloc to the list of dynamic relocations and count NumRelativeRelocs
> + void addDynamicReloc(const DynamicReloc &Reloc);
> std::vector<DynamicReloc> Relocs;
> size_t NumRelativeRelocs = 0;
> };
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -1184,7 +1184,7 @@
> return InputSec->getOutputSection()->Addr + InputSec->getOffset(OffsetInSec);
> }
>
> -int64_t DynamicReloc::getAddend() const {
> +int64_t DynamicReloc::getRelaAddend() const {
> if (UseSymVA)
> return Sym->getVA(Addend);
> return Addend;
> @@ -1202,21 +1202,36 @@
> : SyntheticSection(SHF_ALLOC, Type, Config->Wordsize, Name),
> DynamicTag(DynamicTag), SizeDynamicTag(SizeDynamicTag) {}
>
> -void RelocationBaseSection::addReloc(uint32_t DynType,
> - InputSectionBase *InputSec,
> - uint64_t OffsetInSec, bool UseSymVA,
> - Symbol *Sym, int64_t Addend, RelExpr Expr,
> - RelType Type) {
> - // We store the addends for dynamic relocations for both REL and RELA
> - // relocations for compatibility with GNU Linkers. There is some system
> - // software such as the Bionic dynamic linker that uses the addend prior
> - // to dynamic relocation resolution.
> - if (Config->WriteAddends && UseSymVA)
> - InputSec->Relocations.push_back({Expr, Type, OffsetInSec, Addend, Sym});
> - addReloc({DynType, InputSec, OffsetInSec, UseSymVA, Sym, Addend});
> -}
> -
> -void RelocationBaseSection::addReloc(const DynamicReloc &Reloc) {
> +void RelocationBaseSection::addReloc(uint32_t DynType, InputSectionBase *IS,
> + uint64_t OffsetInSec, Symbol *Sym) {
> + addDynamicReloc({DynType, IS, OffsetInSec, false, Sym, 0});
> +}
> +
> +void RelocationBaseSection::addComplexReloc(const DynamicReloc &Reloc,
> + RelType Type, RelExpr Expr) {
> + addDynamicReloc(Reloc);
> +
> + // If the output uses REL relocations we must store the dynamic relocation
> + // addends to the output sections. We also store addends for RELA relocations
> + // if --apply-dynamic-relocs is used (for compatibility with GNU linkers).
> + // However, we can't do this by default since there is some system software
> + // such as the Bionic dynamic linker that uses the addend from the target
> + // location prior to dynamic relocation resolution (even when processing RELA relocations).
> + if (Config->WriteAddends) {
> + auto *IS = const_cast<InputSectionBase *>(Reloc.InputSec);
> + // If we are adding a relocation against the load address we need to write
> + // the full virtual address as the addend, otherwise just the addend
> + Expr = Reloc.UseSymVA ? Expr : R_ADDEND;
> + // Otherwise we only need to write the addend to the input file. To avoid
> + // unnecessary vector appends and calls to Target->relocateOne() we can skip
> + // it if the addend is zero since it will already be zero in the file.
> + if (Reloc.UseSymVA || Reloc.Addend != 0)
> + IS->Relocations.push_back(
> + {Expr, Type, Reloc.OffsetInSec, Reloc.Addend, Reloc.Sym});
> + }
> +}
> +
> +void RelocationBaseSection::addDynamicReloc(const DynamicReloc &Reloc) {
> if (Reloc.Type == Target->RelativeRel)
> ++NumRelativeRelocs;
> Relocs.push_back(Reloc);
> @@ -1236,7 +1251,7 @@
> static void encodeDynamicReloc(typename ELFT::Rela *P,
> const DynamicReloc &Rel) {
> if (Config->IsRela)
> - P->r_addend = Rel.getAddend();
> + P->r_addend = Rel.getRelaAddend();
> P->r_offset = Rel.getOffset();
> if (Config->EMachine == EM_MIPS && Rel.getInputSec() == InX::MipsGot)
> // The MIPS GOT section contains dynamic relocations that correspond to TLS
> Index: ELF/Relocations.h
> ===================================================================
> --- ELF/Relocations.h
> +++ ELF/Relocations.h
> @@ -32,6 +32,7 @@
> enum RelExpr {
> R_INVALID,
> R_ABS,
> + R_ADDEND,
> R_ARM_SBREL,
> R_GOT,
> R_GOTONLY_PC,
> Index: ELF/Relocations.cpp
> ===================================================================
> --- ELF/Relocations.cpp
> +++ ELF/Relocations.cpp
> @@ -92,21 +92,20 @@
> int64_t Addend, RelExpr Expr) {
> if (Expr == R_MIPS_TLSLD) {
> if (InX::MipsGot->addTlsIndex() && Config->Pic)
> - InX::RelaDyn->addReloc({Target->TlsModuleIndexRel, InX::MipsGot,
> - InX::MipsGot->getTlsIndexOff(), false, nullptr,
> - 0});
> + InX::RelaDyn->addReloc(Target->TlsModuleIndexRel, InX::MipsGot,
> + InX::MipsGot->getTlsIndexOff(), nullptr);
> C.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
> return 1;
> }
>
> if (Expr == R_MIPS_TLSGD) {
> if (InX::MipsGot->addDynTlsEntry(Sym) && Sym.IsPreemptible) {
> uint64_t Off = InX::MipsGot->getGlobalDynOffset(Sym);
> - InX::RelaDyn->addReloc(
> - {Target->TlsModuleIndexRel, InX::MipsGot, Off, false, &Sym, 0});
> + InX::RelaDyn->addReloc(Target->TlsModuleIndexRel, InX::MipsGot, Off,
> + &Sym);
> if (Sym.IsPreemptible)
> - InX::RelaDyn->addReloc({Target->TlsOffsetRel, InX::MipsGot,
> - Off + Config->Wordsize, false, &Sym, 0});
> + InX::RelaDyn->addReloc(Target->TlsOffsetRel, InX::MipsGot,
> + Off + Config->Wordsize, &Sym);
> }
> C.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
> return 1;
> @@ -140,7 +139,7 @@
>
> auto AddTlsReloc = [&](uint64_t Off, RelType Type, Symbol *Dest, bool Dyn) {
> if (Dyn)
> - InX::RelaDyn->addReloc({Type, InX::Got, Off, false, Dest, 0});
> + InX::RelaDyn->addReloc(Type, InX::Got, Off, Dest);
> else
> InX::Got->Relocations.push_back({R_ABS, Type, Off, 0, Dest});
> };
> @@ -192,8 +191,9 @@
> Config->Shared) {
> if (InX::Got->addDynTlsEntry(Sym)) {
> uint64_t Off = InX::Got->getGlobalDynOffset(Sym);
> - InX::RelaDyn->addReloc(
> - {Target->TlsDescRel, InX::Got, Off, !Sym.IsPreemptible, &Sym, 0});
> + InX::RelaDyn->addComplexReloc(
> + {Target->TlsDescRel, InX::Got, Off, !Sym.IsPreemptible, &Sym, 0},
> + Target->TlsDescRel, Expr);
> }
> if (Expr != R_TLSDESC_CALL)
> C.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
> @@ -208,8 +208,8 @@
> return 2;
> }
> if (InX::Got->addTlsIndex())
> - InX::RelaDyn->addReloc({Target->TlsModuleIndexRel, InX::Got,
> - InX::Got->getTlsIndexOff(), false, nullptr, 0});
> + InX::RelaDyn->addReloc(Target->TlsModuleIndexRel, InX::Got,
> + InX::Got->getTlsIndexOff(), nullptr);
> C.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
> return 1;
> }
> @@ -225,15 +225,14 @@
> if (Config->Shared) {
> if (InX::Got->addDynTlsEntry(Sym)) {
> uint64_t Off = InX::Got->getGlobalDynOffset(Sym);
> - InX::RelaDyn->addReloc(
> - {Target->TlsModuleIndexRel, InX::Got, Off, false, &Sym, 0});
> + InX::RelaDyn->addReloc(Target->TlsModuleIndexRel, InX::Got, Off, &Sym);
>
> // If the symbol is preemptible we need the dynamic linker to write
> // the offset too.
> uint64_t OffsetOff = Off + Config->Wordsize;
> if (Sym.IsPreemptible)
> - InX::RelaDyn->addReloc(
> - {Target->TlsOffsetRel, InX::Got, OffsetOff, false, &Sym, 0});
> + InX::RelaDyn->addReloc(Target->TlsOffsetRel, InX::Got, OffsetOff,
> + &Sym);
> else
> InX::Got->Relocations.push_back(
> {R_ABS, Target->TlsOffsetRel, OffsetOff, 0, &Sym});
> @@ -250,8 +249,8 @@
> Offset, Addend, &Sym});
> if (!Sym.isInGot()) {
> InX::Got->addEntry(Sym);
> - InX::RelaDyn->addReloc(
> - {Target->TlsGotRel, InX::Got, Sym.getGotOffset(), false, &Sym, 0});
> + InX::RelaDyn->addReloc(Target->TlsGotRel, InX::Got, Sym.getGotOffset(),
> + &Sym);
> }
> } else {
> C.Relocations.push_back(
> @@ -531,7 +530,7 @@
> Sym->Used = true;
> }
>
> - InX::RelaDyn->addReloc({Target->CopyRel, Sec, 0, false, &SS, 0});
> + InX::RelaDyn->addReloc(Target->CopyRel, Sec, 0, &SS);
> }
>
> // MIPS has an odd notion of "paired" relocations to calculate addends.
> @@ -694,8 +693,9 @@
> RelocationBaseSection *Rel, RelType Type, Symbol &Sym) {
> Plt->addEntry<ELFT>(Sym);
> GotPlt->addEntry(Sym);
> - Rel->addReloc(
> - {Type, GotPlt, Sym.getGotPltOffset(), !Sym.IsPreemptible, &Sym, 0});
> + Rel->addComplexReloc(
> + {Type, GotPlt, Sym.getGotPltOffset(), !Sym.IsPreemptible, &Sym, 0}, Type,
> + R_ABS);
> }
>
> template <class ELFT> static void addGotEntry(Symbol &Sym) {
> @@ -727,8 +727,11 @@
> Type = Target->RelativeRel;
> else
> Type = Target->GotRel;
> - InX::RelaDyn->addReloc(Type, InX::Got, Off, !Sym.IsPreemptible, &Sym, 0,
> - R_ABS, Target->GotRel);
> +
> + // If the symbol not preemptible, we can just write the virtual address to
> + // the GOT and add a relocation against the load address instead of using Sym
> + InX::RelaDyn->addComplexReloc(
> + {Type, InX::Got, Off, !Sym.IsPreemptible, &Sym, 0}, Type, R_ABS);
> }
>
> // Return true if we can define a symbol in the executable that
> @@ -778,12 +781,13 @@
> bool IsPreemptibleValue = Sym.IsPreemptible && Expr != R_GOT;
>
> if (!IsPreemptibleValue) {
> - InX::RelaDyn->addReloc(Target->RelativeRel, &Sec, Offset, true, &Sym,
> - Addend, Expr, Type);
> + InX::RelaDyn->addComplexReloc(
> + {Target->RelativeRel, &Sec, Offset, true, &Sym, Addend}, Type, Expr);
> return Expr;
> } else if (Target->isPicRel(Type)) {
> - InX::RelaDyn->addReloc(
> - {Target->getDynRel(Type), &Sec, Offset, false, &Sym, Addend});
> + InX::RelaDyn->addComplexReloc(
> + {Target->getDynRel(Type), &Sec, Offset, false, &Sym, Addend}, Type,
> + Expr);
>
> // MIPS ABI turns using of GOT and dynamic relocations inside out.
> // While regular ABI uses dynamic relocations to fill up GOT entries
> @@ -978,8 +982,8 @@
> // ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf
> InX::MipsGot->addEntry(Sym, Addend, Expr);
> if (Sym.isTls() && Sym.IsPreemptible)
> - InX::RelaDyn->addReloc({Target->TlsGotRel, InX::MipsGot,
> - Sym.getGotOffset(), false, &Sym, 0});
> + InX::RelaDyn->addReloc(Target->TlsGotRel, InX::MipsGot,
> + Sym.getGotOffset(), &Sym);
> } else if (!Sym.isInGot()) {
> addGotEntry<ELFT>(Sym);
> }
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -463,6 +463,8 @@
> case R_ABS:
> case R_RELAX_GOT_PC_NOPIC:
> return Sym.getVA(A);
> + case R_ADDEND:
> + return A;
> case R_ARM_SBREL:
> return Sym.getVA(A) - getARMStaticBase(Sym);
> case R_GOT:
> Index: ELF/Arch/X86_64.cpp
> ===================================================================
> --- ELF/Arch/X86_64.cpp
> +++ ELF/Arch/X86_64.cpp
> @@ -318,6 +318,7 @@
> case R_X86_64_PC64:
> case R_X86_64_SIZE64:
> case R_X86_64_GOT64:
> + case R_X86_64_RELATIVE:
> write64le(Loc, Val);
> break;
> default:
More information about the llvm-commits
mailing list