Skip to content

Commit

Permalink
[LLD][RISCV] Error on PCREL_LO referencing other Section (#107558)
Browse files Browse the repository at this point in the history
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."

Without this patch, 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 the user provided is not
following the psABI as written.

Fixes #107304
  • Loading branch information
lenary authored Oct 8, 2024
1 parent f01364e commit db1a762
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 15 deletions.
42 changes: 28 additions & 14 deletions lld/ELF/InputSection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
32 changes: 32 additions & 0 deletions lld/test/ELF/riscv-pcrel-hilo-error-sections.s
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion lld/test/ELF/riscv-pcrel-hilo-error.s
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit db1a762

Please sign in to comment.