[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
Tue Feb 13 08:31:56 PST 2018
This patch is still a lot bigger than it needs to be.
I took the liberty of committing refactoring bits in r325016 and r325017
and I was able to reduce this patch to what is attached.
Are you OK with the attached patch?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.diff
Type: text/x-patch
Size: 4550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180213/e7613c35/attachment.bin>
-------------- next part --------------
Cheers,
Rafael
Alexander Richardson via Phabricator <reviews at reviews.llvm.org> writes:
> arichardson updated this revision to Diff 133758.
> arichardson marked 3 inline comments as done.
> arichardson added a comment.
>
> - Split out an unrelated change to https://reviews.llvm.org/D43161
> - made the diff smaller
>
>
> Repository:
> rLLD LLVM Linker
>
> https://reviews.llvm.org/D42843
>
> Files:
> ELF/Arch/X86_64.cpp
> ELF/Driver.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
> @@ -328,8 +328,6 @@
> int64_t getRelaAddend() const;
>
> RelType Type;
> -
> -private:
> Symbol *Sym;
> const InputSectionBase *InputSec = nullptr;
> uint64_t OffsetInSec;
> @@ -370,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);
> + // 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 that will be processed by the the runtime linker.
> + // If the dynamic relocation needs an addend or it is against the load address
> + // and not a symbol use addComplexReloc() instead since it can handle those
> + // cases.
> void addReloc(const DynamicReloc &Reloc);
> +
> + // 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
> @@ -1202,21 +1202,33 @@
> : 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) {
> + assert(!Reloc.UseSymVA &&
> + "For relocations against the load address use addComplexReloc()");
> + assert(Reloc.Addend == 0 &&
> + "For relocations with non-zero addends use addComplexReloc()");
> + addDynamicReloc(Reloc);
> }
>
> -void RelocationBaseSection::addReloc(const DynamicReloc &Reloc) {
> +void RelocationBaseSection::addComplexReloc(const DynamicReloc &Reloc,
> + RelType Type, RelExpr Expr) {
> + addDynamicReloc(Reloc);
> +
> + 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);
> 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
> @@ -192,8 +192,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});
> @@ -694,8 +695,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 +729,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 +783,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
> 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/Driver.cpp
> ===================================================================
> --- ELF/Driver.cpp
> +++ ELF/Driver.cpp
> @@ -824,6 +824,13 @@
> (Config->Is64 || IsX32 || Machine == EM_PPC) && Machine != EM_MIPS;
> Config->Pic = Config->Pie || Config->Shared;
> Config->Wordsize = Config->Is64 ? 8 : 4;
> + // 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).
> Config->WriteAddends = Args.hasFlag(OPT_apply_dynamic_relocs,
> OPT_no_apply_dynamic_relocs, false) ||
> !Config->IsRela;
> 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