[PATCH] D42843: Ensure that Elf_Rel addends are always written for dynamic relocations
Alexander Richardson via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 14 03:38:58 PST 2018
The attached patch does fix the issue in my test case but I believe
that it doesn't handle GOT/PLT relocations since those also sometimes
have UseSymVA = true.
I'll try to come up with a test case and update the patch.
On 13 February 2018 at 16:31, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> 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?
>
>
> diff --git a/ELF/InputSection.cpp b/ELF/InputSection.cpp
> index 6766c0091..c58cd20cf 100644
> --- a/ELF/InputSection.cpp
> +++ b/ELF/InputSection.cpp
> @@ -474,6 +474,8 @@ static uint64_t getRelocTargetVA(RelType Type, int64_t A, uint64_t P,
> 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:
> diff --git a/ELF/Relocations.cpp b/ELF/Relocations.cpp
> index e003b41df..45181796a 100644
> --- a/ELF/Relocations.cpp
> +++ b/ELF/Relocations.cpp
> @@ -780,8 +780,8 @@ static RelExpr processRelocAux(InputSectionBase &Sec, RelExpr Expr,
> Addend, Expr, Type);
> return Expr;
> } else if (Target->isPicRel(Type)) {
> - InX::RelaDyn->addReloc(
> - {Target->getDynRel(Type), &Sec, Offset, false, &Sym, Addend});
> + InX::RelaDyn->addReloc(Target->getDynRel(Type), &Sec, Offset, false, &Sym,
> + Addend, R_ADDEND, Type);
>
> // MIPS ABI turns using of GOT and dynamic relocations inside out.
> // While regular ABI uses dynamic relocations to fill up GOT entries
> diff --git a/ELF/Relocations.h b/ELF/Relocations.h
> index 2cc8adfa5..7b142e15b 100644
> --- a/ELF/Relocations.h
> +++ b/ELF/Relocations.h
> @@ -32,6 +32,7 @@ typedef uint32_t RelType;
> enum RelExpr {
> R_INVALID,
> R_ABS,
> + R_ADDEND,
> R_ARM_SBREL,
> R_GOT,
> R_GOTONLY_PC,
> diff --git a/ELF/SyntheticSections.cpp b/ELF/SyntheticSections.cpp
> index ae4fd7fed..9bfb9b57d 100644
> --- a/ELF/SyntheticSections.cpp
> +++ b/ELF/SyntheticSections.cpp
> @@ -1212,11 +1212,9 @@ void RelocationBaseSection::addReloc(RelType DynType,
> 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)
> + // Write the addends to the relocated address if required. We skip
> + // it if the written value would be zero.
> + if (Config->WriteAddends && (UseSymVA || Addend != 0))
> InputSec->Relocations.push_back({Expr, Type, OffsetInSec, Addend, Sym});
> addReloc({DynType, InputSec, OffsetInSec, UseSymVA, Sym, Addend});
> }
> diff --git a/test/ELF/rel-addend-with-rela-input.s b/test/ELF/rel-addend-with-rela-input.s
> new file mode 100644
> index 000000000..11eac97ed
> --- /dev/null
> +++ b/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
>
>
> 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