[lld] [LLD][RISCV] Error on PCREL_LO referencing other Section (PR #107558)
Sam Elliott via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 7 10:58:06 PDT 2024
https://github.com/lenary updated https://github.com/llvm/llvm-project/pull/107558
>From a7813118a79301f7203f7ef48b8bd7bf5c206813 Mon Sep 17 00:00:00 2001
From: Sam Elliott <quic_aelliott at quicinc.com>
Date: Fri, 6 Sep 2024 03:10:57 -0700
Subject: [PATCH] [LLD][RISCV] Error on PCREL_LO referencing other Section
The RISC-V psABI states that "The `R_RISCV_PCREL_LO12_I` or
`R_RISCV_PCREL_LO12_S` relocations contain a label pointing to an
instruction in the same section with an `R_RISCV_PCREL_HI20` relocation
entry that points to the target symbol."
In this case, GNU ld errors, but LLD does not -- I think because LLD is
doing the right thing, certainly in the testcase provided.
Nonetheless, I think an error is good here to bring LLD in line with
what GNU ld is doing in showing that the object they are using is not
following the psABI as written.
Fixes #107304
---
lld/ELF/InputSection.cpp | 42 ++++++++++++-------
.../ELF/riscv-pcrel-hilo-error-sections.s | 32 ++++++++++++++
lld/test/ELF/riscv-pcrel-hilo-error.s | 2 +-
3 files changed, 61 insertions(+), 15 deletions(-)
create mode 100644 lld/test/ELF/riscv-pcrel-hilo-error-sections.s
diff --git a/lld/ELF/InputSection.cpp b/lld/ELF/InputSection.cpp
index 68cce62188b0fe..0885815a22a14a 100644
--- a/lld/ELF/InputSection.cpp
+++ b/lld/ELF/InputSection.cpp
@@ -625,27 +625,40 @@ static uint64_t getARMStaticBase(const Symbol &sym) {
// points the corresponding R_RISCV_PCREL_HI20 relocation, and the target VA
// is calculated using PCREL_HI20's symbol.
//
-// This function returns the R_RISCV_PCREL_HI20 relocation from
-// R_RISCV_PCREL_LO12's symbol and addend.
-static Relocation *getRISCVPCRelHi20(const Symbol *sym, uint64_t addend) {
+// This function returns the R_RISCV_PCREL_HI20 relocation from the
+// R_RISCV_PCREL_LO12 relocation.
+static Relocation *getRISCVPCRelHi20(const InputSectionBase *loSec,
+ const Relocation &loReloc) {
+ uint64_t addend = loReloc.addend;
+ Symbol *sym = loReloc.sym;
+
const Defined *d = cast<Defined>(sym);
if (!d->section) {
- errorOrWarn("R_RISCV_PCREL_LO12 relocation points to an absolute symbol: " +
- sym->getName());
+ errorOrWarn(
+ loSec->getLocation(loReloc.offset) +
+ ": R_RISCV_PCREL_LO12 relocation points to an absolute symbol: " +
+ sym->getName());
return nullptr;
}
- InputSection *isec = cast<InputSection>(d->section);
+ InputSection *hiSec = cast<InputSection>(d->section);
+
+ if (hiSec != loSec)
+ errorOrWarn(loSec->getLocation(loReloc.offset) +
+ ": R_RISCV_PCREL_LO12 relocation points to a symbol '" +
+ sym->getName() + "' in a different section '" + hiSec->name +
+ "'");
if (addend != 0)
- warn("non-zero addend in R_RISCV_PCREL_LO12 relocation to " +
- isec->getObjMsg(d->value) + " is ignored");
+ warn(loSec->getLocation(loReloc.offset) +
+ ": non-zero addend in R_RISCV_PCREL_LO12 relocation to " +
+ hiSec->getObjMsg(d->value) + " is ignored");
// Relocations are sorted by offset, so we can use std::equal_range to do
// binary search.
- Relocation r;
- r.offset = d->value;
+ Relocation hiReloc;
+ hiReloc.offset = d->value;
auto range =
- std::equal_range(isec->relocs().begin(), isec->relocs().end(), r,
+ std::equal_range(hiSec->relocs().begin(), hiSec->relocs().end(), hiReloc,
[](const Relocation &lhs, const Relocation &rhs) {
return lhs.offset < rhs.offset;
});
@@ -655,8 +668,9 @@ static Relocation *getRISCVPCRelHi20(const Symbol *sym, uint64_t addend) {
it->type == R_RISCV_TLS_GD_HI20 || it->type == R_RISCV_TLS_GOT_HI20)
return &*it;
- errorOrWarn("R_RISCV_PCREL_LO12 relocation points to " +
- isec->getObjMsg(d->value) +
+ errorOrWarn(loSec->getLocation(loReloc.offset) +
+ ": R_RISCV_PCREL_LO12 relocation points to " +
+ hiSec->getObjMsg(d->value) +
" without an associated R_RISCV_PCREL_HI20 relocation");
return nullptr;
}
@@ -825,7 +839,7 @@ uint64_t InputSectionBase::getRelocTargetVA(Ctx &ctx, const Relocation &r,
return getAArch64Page(val) - getAArch64Page(p);
}
case R_RISCV_PC_INDIRECT: {
- if (const Relocation *hiRel = getRISCVPCRelHi20(r.sym, a))
+ if (const Relocation *hiRel = getRISCVPCRelHi20(this, r))
return getRelocTargetVA(ctx, *hiRel, r.sym->getVA());
return 0;
}
diff --git a/lld/test/ELF/riscv-pcrel-hilo-error-sections.s b/lld/test/ELF/riscv-pcrel-hilo-error-sections.s
new file mode 100644
index 00000000000000..ebdefd357d722d
--- /dev/null
+++ b/lld/test/ELF/riscv-pcrel-hilo-error-sections.s
@@ -0,0 +1,32 @@
+# REQUIRES: riscv
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.o
+# RUN: not ld.lld %t.o 2>&1 | FileCheck %s
+
+# CHECK: error: {{.*}}:(.text.sec_one+0x0): R_RISCV_PCREL_LO12 relocation points to a symbol '.Lpcrel_hi0' in a different section '.text.sec_two'
+# CHECK: error: {{.*}}:(.text.sec_one+0x4): R_RISCV_PCREL_LO12 relocation points to a symbol '.Lpcrel_hi1' in a different section '.text.sec_two'
+# CHECK-NOT: R_RISCV_PCREL_LO12 relocation points to a symbol '.Lpcrel_hi2'
+
+## This test is checking that we warn the user when the relocations in their
+## object don't follow the RISC-V psABI. In particular, the psABI requires
+## that PCREL_LO12 relocations are in the same section as the pcrel_hi
+## instruction they point to.
+
+ .section .text.sec_one,"ax"
+ addi a0, a0, %pcrel_lo(.Lpcrel_hi0)
+ sw a0, %pcrel_lo(.Lpcrel_hi1)(a1)
+
+ .section .text.sec_two,"ax"
+.Lpcrel_hi0:
+ auipc a0, %pcrel_hi(a)
+.Lpcrel_hi1:
+ auipc a1, %pcrel_hi(a)
+
+.Lpcrel_hi2:
+ auipc a2, %pcrel_hi(a)
+ addi a2, a2, %pcrel_lo(.Lpcrel_hi2)
+
+ .data
+ .global a
+a:
+ .word 50
diff --git a/lld/test/ELF/riscv-pcrel-hilo-error.s b/lld/test/ELF/riscv-pcrel-hilo-error.s
index 1557ac77bb7ed5..4dc80b5a5e716b 100644
--- a/lld/test/ELF/riscv-pcrel-hilo-error.s
+++ b/lld/test/ELF/riscv-pcrel-hilo-error.s
@@ -2,7 +2,7 @@
# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.o
# RUN: not ld.lld %t.o --defsym external=0 2>&1 | FileCheck %s
-# CHECK: error: R_RISCV_PCREL_LO12 relocation points to an absolute symbol: external
+# CHECK: error: {{.*}}:(.text+0x4): R_RISCV_PCREL_LO12 relocation points to an absolute symbol: external
# We provide a dummy %pcrel_hi referred to by external to appease the
# assembler, but make external weak so --defsym can still override it at link
More information about the llvm-commits
mailing list