[lld] 35c5e56 - [ELF] Check the Elf_Rel addends for dynamic relocations
Alex Richardson via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 9 02:41:56 PDT 2021
Author: Alex Richardson
Date: 2021-07-09T10:41:40+01:00
New Revision: 35c5e564e628666af7903f6525b0de1a5917f071
URL: https://github.com/llvm/llvm-project/commit/35c5e564e628666af7903f6525b0de1a5917f071
DIFF: https://github.com/llvm/llvm-project/commit/35c5e564e628666af7903f6525b0de1a5917f071.diff
LOG: [ELF] Check the Elf_Rel addends for dynamic relocations
There used to be many cases where addends for Elf_Rel were not emitted in
the final object file (mostly when building for MIPS64 since the input .o
files use RELA but the output uses REL). These cases have been fixed since,
but this patch adds a check to ensure that the written values are correct.
It is based on a previous patch that I added to the CHERI fork of LLD since
we were using MIPS64 as a baseline. The work has now almost entirely
shifted to RISC-V and Arm Morello (which use Elf_Rela), but I thought
it would be useful to upstream our local changes anyway.
This patch adds a (hidden) command line flag --check-dynamic-relocations
that can be used to enable these checks. It is also on by default in
assertions builds for targets that handle all dynamic relocations kinds
that LLD can emit in Target::getImplicitAddend(). Currently this is
enabled for ARM, MIPS, and I386.
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D101450
Added:
Modified:
lld/ELF/Arch/ARM.cpp
lld/ELF/Arch/Mips.cpp
lld/ELF/Arch/X86.cpp
lld/ELF/Config.h
lld/ELF/Driver.cpp
lld/ELF/Options.td
lld/ELF/OutputSections.cpp
lld/ELF/OutputSections.h
lld/ELF/SyntheticSections.h
lld/ELF/Target.cpp
lld/ELF/Writer.cpp
lld/test/ELF/invalid/broken-relaxation-x64.test
lld/test/ELF/reloc-sec-before-relocated.test
Removed:
################################################################################
diff --git a/lld/ELF/Arch/ARM.cpp b/lld/ELF/Arch/ARM.cpp
index 13a7001e2296..d909a3234c10 100644
--- a/lld/ELF/Arch/ARM.cpp
+++ b/lld/ELF/Arch/ARM.cpp
@@ -707,20 +707,29 @@ void ARM::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
int64_t ARM::getImplicitAddend(const uint8_t *buf, RelType type) const {
switch (type) {
default:
+ internalLinkerError(getErrorLocation(buf),
+ "cannot read addend for relocation " + toString(type));
return 0;
case R_ARM_ABS32:
case R_ARM_BASE_PREL:
+ case R_ARM_GLOB_DAT:
case R_ARM_GOTOFF32:
case R_ARM_GOT_BREL:
case R_ARM_GOT_PREL:
+ case R_ARM_IRELATIVE:
case R_ARM_REL32:
+ case R_ARM_RELATIVE:
+ case R_ARM_SBREL32:
case R_ARM_TARGET1:
case R_ARM_TARGET2:
+ case R_ARM_TLS_DTPMOD32:
+ case R_ARM_TLS_DTPOFF32:
case R_ARM_TLS_GD32:
- case R_ARM_TLS_LDM32:
- case R_ARM_TLS_LDO32:
case R_ARM_TLS_IE32:
+ case R_ARM_TLS_LDM32:
case R_ARM_TLS_LE32:
+ case R_ARM_TLS_LDO32:
+ case R_ARM_TLS_TPOFF32:
return SignExtend64<32>(read32le(buf));
case R_ARM_PREL31:
return SignExtend64<31>(read32le(buf));
@@ -828,6 +837,10 @@ int64_t ARM::getImplicitAddend(const uint8_t *buf, RelType type) const {
uint64_t imm12 = read16le(buf + 2) & 0x0fff;
return u ? imm12 : -imm12;
}
+ case R_ARM_NONE:
+ case R_ARM_JUMP_SLOT:
+ // These relocations are defined as not having an implicit addend.
+ return 0;
}
}
diff --git a/lld/ELF/Arch/Mips.cpp b/lld/ELF/Arch/Mips.cpp
index 5c6b3e2371ef..a233a01d5bba 100644
--- a/lld/ELF/Arch/Mips.cpp
+++ b/lld/ELF/Arch/Mips.cpp
@@ -385,8 +385,10 @@ int64_t MIPS<ELFT>::getImplicitAddend(const uint8_t *buf, RelType type) const {
const endianness e = ELFT::TargetEndianness;
switch (type) {
case R_MIPS_32:
+ case R_MIPS_REL32:
case R_MIPS_GPREL32:
case R_MIPS_TLS_DTPREL32:
+ case R_MIPS_TLS_DTPMOD32:
case R_MIPS_TLS_TPREL32:
return SignExtend64<32>(read32(buf));
case R_MIPS_26:
@@ -394,25 +396,37 @@ int64_t MIPS<ELFT>::getImplicitAddend(const uint8_t *buf, RelType type) const {
// we should use another expression for calculation:
// ((A << 2) | (P & 0xf0000000)) >> 2
return SignExtend64<28>(read32(buf) << 2);
+ case R_MIPS_CALL_HI16:
case R_MIPS_GOT16:
+ case R_MIPS_GOT_HI16:
case R_MIPS_HI16:
case R_MIPS_PCHI16:
return SignExtend64<16>(read32(buf)) << 16;
+ case R_MIPS_CALL16:
+ case R_MIPS_CALL_LO16:
+ case R_MIPS_GOT_LO16:
case R_MIPS_GPREL16:
case R_MIPS_LO16:
case R_MIPS_PCLO16:
case R_MIPS_TLS_DTPREL_HI16:
case R_MIPS_TLS_DTPREL_LO16:
+ case R_MIPS_TLS_GD:
+ case R_MIPS_TLS_GOTTPREL:
+ case R_MIPS_TLS_LDM:
case R_MIPS_TLS_TPREL_HI16:
case R_MIPS_TLS_TPREL_LO16:
return SignExtend64<16>(read32(buf));
case R_MICROMIPS_GOT16:
case R_MICROMIPS_HI16:
return SignExtend64<16>(readShuffle<e>(buf)) << 16;
+ case R_MICROMIPS_CALL16:
case R_MICROMIPS_GPREL16:
case R_MICROMIPS_LO16:
case R_MICROMIPS_TLS_DTPREL_HI16:
case R_MICROMIPS_TLS_DTPREL_LO16:
+ case R_MICROMIPS_TLS_GD:
+ case R_MICROMIPS_TLS_GOTTPREL:
+ case R_MICROMIPS_TLS_LDM:
case R_MICROMIPS_TLS_TPREL_HI16:
case R_MICROMIPS_TLS_TPREL_LO16:
return SignExtend64<16>(readShuffle<e>(buf));
@@ -446,7 +460,22 @@ int64_t MIPS<ELFT>::getImplicitAddend(const uint8_t *buf, RelType type) const {
return SignExtend64<25>(readShuffle<e>(buf) << 2);
case R_MICROMIPS_PC26_S1:
return SignExtend64<27>(readShuffle<e>(buf) << 1);
+ case R_MIPS_64:
+ case R_MIPS_TLS_DTPMOD64:
+ case R_MIPS_TLS_DTPREL64:
+ case R_MIPS_TLS_TPREL64:
+ case (R_MIPS_64 << 8) | R_MIPS_REL32:
+ return read64(buf);
+ case R_MIPS_COPY:
+ return config->is64 ? read64(buf) : read32(buf);
+ case R_MIPS_NONE:
+ case R_MIPS_JUMP_SLOT:
+ case R_MIPS_JALR:
+ // These relocations are defined as not having an implicit addend.
+ return 0;
default:
+ internalLinkerError(getErrorLocation(buf),
+ "cannot read addend for relocation " + toString(type));
return 0;
}
}
diff --git a/lld/ELF/Arch/X86.cpp b/lld/ELF/Arch/X86.cpp
index f18fa0b179ab..df769f0a1c8b 100644
--- a/lld/ELF/Arch/X86.cpp
+++ b/lld/ELF/Arch/X86.cpp
@@ -250,16 +250,36 @@ int64_t X86::getImplicitAddend(const uint8_t *buf, RelType type) const {
case R_386_PC16:
return SignExtend64<16>(read16le(buf));
case R_386_32:
+ case R_386_GLOB_DAT:
case R_386_GOT32:
case R_386_GOT32X:
case R_386_GOTOFF:
case R_386_GOTPC:
+ case R_386_IRELATIVE:
case R_386_PC32:
case R_386_PLT32:
+ case R_386_RELATIVE:
+ case R_386_TLS_DTPMOD32:
+ case R_386_TLS_DTPOFF32:
case R_386_TLS_LDO_32:
+ case R_386_TLS_LDM:
+ case R_386_TLS_IE:
+ case R_386_TLS_IE_32:
case R_386_TLS_LE:
+ case R_386_TLS_LE_32:
+ case R_386_TLS_GD:
+ case R_386_TLS_GD_32:
+ case R_386_TLS_GOTIE:
+ case R_386_TLS_TPOFF:
+ case R_386_TLS_TPOFF32:
return SignExtend64<32>(read32le(buf));
+ case R_386_NONE:
+ case R_386_JUMP_SLOT:
+ // These relocations are defined as not having an implicit addend.
+ return 0;
default:
+ internalLinkerError(getErrorLocation(buf),
+ "cannot read addend for relocation " + toString(type));
return 0;
}
}
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 3b13ff773c71..9144347045b9 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -20,6 +20,7 @@
#include "llvm/Support/CodeGen.h"
#include "llvm/Support/Endian.h"
#include "llvm/Support/GlobPattern.h"
+#include "llvm/Support/PrettyStackTrace.h"
#include <atomic>
#include <vector>
@@ -147,6 +148,7 @@ struct Configuration {
bool bsymbolicFunctions = false;
bool callGraphProfileSort;
bool checkSections;
+ bool checkDynamicRelocs;
bool compressDebugSections;
bool cref;
std::vector<std::pair<llvm::GlobPattern, uint64_t>> deadRelocInNonAlloc;
@@ -356,6 +358,12 @@ static inline void errorOrWarn(const Twine &msg) {
else
warn(msg);
}
+
+static inline void internalLinkerError(StringRef loc, const Twine &msg) {
+ errorOrWarn(loc + "internal linker error: " + msg + "\n" +
+ llvm::getBugReportMsg());
+}
+
} // namespace elf
} // namespace lld
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index cb08516f72cc..defdf66cce69 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -1395,7 +1395,18 @@ static void setConfigs(opt::InputArgList &args) {
config->writeAddends = args.hasFlag(OPT_apply_dynamic_relocs,
OPT_no_apply_dynamic_relocs, false) ||
!config->isRela;
-
+ // Validation of dynamic relocation addends is on by default for assertions
+ // builds (for supported targets) and disabled otherwise. Ideally we would
+ // enable the debug checks for all targets, but currently not all targets
+ // have support for reading Elf_Rel addends, so we only enable for a subset.
+#ifndef NDEBUG
+ bool checkDynamicRelocsDefault = m == EM_ARM || m == EM_386 || m == EM_MIPS;
+#else
+ bool checkDynamicRelocsDefault = false;
+#endif
+ config->checkDynamicRelocs =
+ args.hasFlag(OPT_check_dynamic_relocations,
+ OPT_no_check_dynamic_relocations, checkDynamicRelocsDefault);
config->tocOptimize =
args.hasFlag(OPT_toc_optimize, OPT_no_toc_optimize, m == EM_PPC64);
config->pcRelOptimize =
diff --git a/lld/ELF/Options.td b/lld/ELF/Options.td
index 590a2ca61fa5..bedcf43bbe85 100644
--- a/lld/ELF/Options.td
+++ b/lld/ELF/Options.td
@@ -710,3 +710,9 @@ def: F<"Qy">;
defm mips_got_size:
Eq<"mips-got-size", "Max size of a single MIPS GOT. 0x10000 by default.">,
Flags<[HelpHidden]>;
+
+// Hidden option used to opt-in to additional output checks.
+defm check_dynamic_relocations: BB<"check-dynamic-relocations",
+ "Perform additional validation of the written dynamic relocations",
+ "Do not perform additional validation of the written dynamic relocations">,
+ Flags<[HelpHidden]>;
diff --git a/lld/ELF/OutputSections.cpp b/lld/ELF/OutputSections.cpp
index 9fd226fbba2c..088d1cdc65e4 100644
--- a/lld/ELF/OutputSections.cpp
+++ b/lld/ELF/OutputSections.cpp
@@ -539,6 +539,41 @@ std::array<uint8_t, 4> OutputSection::getFiller() {
return {0, 0, 0, 0};
}
+void OutputSection::checkDynRelAddends(const uint8_t *bufStart) {
+ assert(config->writeAddends && config->checkDynamicRelocs);
+ assert(type == SHT_REL || type == SHT_RELA);
+ std::vector<InputSection *> sections = getInputSections(this);
+ parallelForEachN(0, sections.size(), [&](size_t i) {
+ // When linking with -r or --emit-relocs we might also call this function
+ // for input .rel[a].<sec> sections which we simply pass through to the
+ // output. We skip over those and only look at the synthetic relocation
+ // sections created during linking.
+ const auto *sec = dyn_cast<RelocationBaseSection>(sections[i]);
+ if (!sec)
+ return;
+ for (const DynamicReloc &rel : sec->relocs) {
+ int64_t addend = rel.computeAddend();
+ const OutputSection *relOsec = rel.inputSec->getOutputSection();
+ assert(relOsec != nullptr && "missing output section for relocation");
+ const uint8_t *relocTarget =
+ bufStart + relOsec->offset + rel.inputSec->getOffset(rel.offsetInSec);
+ // For SHT_NOBITS the written addend is always zero.
+ int64_t writtenAddend =
+ relOsec->type == SHT_NOBITS
+ ? 0
+ : target->getImplicitAddend(relocTarget, rel.type);
+ if (addend != writtenAddend)
+ internalLinkerError(
+ getErrorLocation(relocTarget),
+ "wrote incorrect addend value 0x" + utohexstr(writtenAddend) +
+ " instead of 0x" + utohexstr(addend) +
+ " for dynamic relocation " + toString(rel.type) +
+ " at offset 0x" + utohexstr(rel.getOffset()) +
+ (rel.sym ? " against symbol " + toString(*rel.sym) : ""));
+ }
+ });
+}
+
template void OutputSection::writeHeaderTo<ELF32LE>(ELF32LE::Shdr *Shdr);
template void OutputSection::writeHeaderTo<ELF32BE>(ELF32BE::Shdr *Shdr);
template void OutputSection::writeHeaderTo<ELF64LE>(ELF64LE::Shdr *Shdr);
diff --git a/lld/ELF/OutputSections.h b/lld/ELF/OutputSections.h
index 69b7944d946a..a0f806614387 100644
--- a/lld/ELF/OutputSections.h
+++ b/lld/ELF/OutputSections.h
@@ -102,6 +102,8 @@ class OutputSection final : public BaseCommand, public SectionBase {
void finalize();
template <class ELFT> void writeTo(uint8_t *buf);
+ // Check that the addends for dynamic relocations were written correctly.
+ void checkDynRelAddends(const uint8_t *bufStart);
template <class ELFT> void maybeCompress();
void sort(llvm::function_ref<int(InputSectionBase *s)> order);
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index ef3588db6a63..ddf7e8daa458 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -538,6 +538,11 @@ class RelocationBaseSection : public SyntheticSection {
size_t getSize() const override { return relocs.size() * this->entsize; }
size_t getRelativeRelocCount() const { return numRelativeRelocs; }
void finalizeContents() override;
+ static bool classof(const SectionBase *d) {
+ return SyntheticSection::classof(d) &&
+ (d->type == llvm::ELF::SHT_RELA || d->type == llvm::ELF::SHT_REL ||
+ d->type == llvm::ELF::SHT_RELR);
+ }
int32_t dynamicTag, sizeDynamicTag;
std::vector<DynamicReloc> relocs;
diff --git a/lld/ELF/Target.cpp b/lld/ELF/Target.cpp
index cb816d8ed12d..d3e54f7387d7 100644
--- a/lld/ELF/Target.cpp
+++ b/lld/ELF/Target.cpp
@@ -130,6 +130,8 @@ ErrorPlace elf::getErrorPlace(const uint8_t *loc) {
TargetInfo::~TargetInfo() {}
int64_t TargetInfo::getImplicitAddend(const uint8_t *buf, RelType type) const {
+ internalLinkerError(getErrorLocation(buf),
+ "cannot read addend for relocation " + toString(type));
return 0;
}
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index 81ac28a91abc..e22568f53488 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2971,6 +2971,13 @@ template <class ELFT> void Writer<ELFT>::writeSections() {
for (OutputSection *sec : outputSections)
if (sec->type != SHT_REL && sec->type != SHT_RELA)
sec->writeTo<ELFT>(Out::bufferStart + sec->offset);
+
+ // Finally, check that all dynamic relocation addends were written correctly.
+ if (config->checkDynamicRelocs && config->writeAddends) {
+ for (OutputSection *sec : outputSections)
+ if (sec->type == SHT_REL || sec->type == SHT_RELA)
+ sec->checkDynRelAddends(Out::bufferStart);
+ }
}
// Computes a hash value of Data using a given hash function.
diff --git a/lld/test/ELF/invalid/broken-relaxation-x64.test b/lld/test/ELF/invalid/broken-relaxation-x64.test
index 97a977e2c03a..5e11eb682a46 100644
--- a/lld/test/ELF/invalid/broken-relaxation-x64.test
+++ b/lld/test/ELF/invalid/broken-relaxation-x64.test
@@ -1,4 +1,6 @@
# REQUIRES: x86
+# Needs https://reviews.llvm.org/D101451 to read the implicit addends.
+# XFAIL: *
# RUN: yaml2obj %s -o %t.o
# RUN: not ld.lld %t.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERR %s
diff --git a/lld/test/ELF/reloc-sec-before-relocated.test b/lld/test/ELF/reloc-sec-before-relocated.test
index a56231294a0c..76cd32bb858b 100644
--- a/lld/test/ELF/reloc-sec-before-relocated.test
+++ b/lld/test/ELF/reloc-sec-before-relocated.test
@@ -4,6 +4,9 @@
## want to use this feature, which is not restricted by ELF gABI.
## GNU ld supports this as well.
+# Needs https://reviews.llvm.org/D101451 to read the implicit addends.
+# XFAIL: *
+
# RUN: yaml2obj %s -DTYPE=SHT_RELA -o %t1.o
# RUN: ld.lld -shared %t1.o -o %t1
# RUN: llvm-readelf --relocs %t1 | FileCheck %s
More information about the llvm-commits
mailing list