Skip to content

Commit

Permalink
[LLD][RISCV] Fix incorrect call relaxation when mixing +c and -c obje…
Browse files Browse the repository at this point in the history
…cts (#73977)

This fixes a mis-link when mixing compressed and non-compressed input to
LLD. When relaxing calls, we must respect the source file that the
section came from when deciding whether it's legal to use compressed
instructions. If the call in question comes from a non-rvc source, then it will not
expect 2-byte alignments and cascading failures may result.

This fixes #63964. The symptom 
seen there is that a latter RISCV_ALIGN can't be satisfied and we either
fail an assert or produce a totally bogus link result. (It can be easily
reproduced by putting .p2align 5 right before the nop in the reduced
test case and running check-lld on an assertions enabled build.)  However,
it's important to note this is just one possible symptom of the problem.

If the resulting binary has a runtime switch between rvc and non-rvc
routines (via e.g. ifuncs), then even if we manage to link we may execute invalid
instructions on a machine which doesn't implement compressed instructions.
  • Loading branch information
preames authored Dec 1, 2023
1 parent e817966 commit 62213be
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lld/ELF/Arch/RISCV.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ static void initSymbolAnchors() {
// Relax R_RISCV_CALL/R_RISCV_CALL_PLT auipc+jalr to c.j, c.jal, or jal.
static void relaxCall(const InputSection &sec, size_t i, uint64_t loc,
Relocation &r, uint32_t &remove) {
const bool rvc = config->eflags & EF_RISCV_RVC;
const bool rvc = getEFlags(sec.file) & EF_RISCV_RVC;
const Symbol &sym = *r.sym;
const uint64_t insnPair = read64le(sec.content().data() + r.offset);
const uint32_t rd = extractBits(insnPair, 32 + 11, 32 + 7);
Expand Down
32 changes: 32 additions & 0 deletions lld/test/ELF/riscv-relax-call-mixed-rvc.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# REQUIRES: riscv
# RUN: rm -rf %t && split-file %s %t && cd %t
# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+c,+relax a.s -o a.o
# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=-c,+relax b.s -o b.o

# RUN: ld.lld a.o b.o --shared -o a -Ttext=0x10000
# RUN: llvm-objdump -d --no-show-raw-insn -M no-aliases a | FileCheck %s

## This needs to be a *uncompressed* jal instruction since it came from the
## source file which does not enable C
# CHECK-LABEL: <foo>:
# CHECK-NEXT: 10000: jal zero, {{.*}} <foo>
# CHECK-NEXT: 10004: sub zero, zero, zero

# w/ C
#--- a.s
.text
.attribute 4, 16
.attribute 5, "rv64i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0_zifencei2p0"

# w/o C
#--- b.s
.text
.attribute 4, 16
.attribute 5, "rv64i2p1_m2p0_a2p1_f2p2_d2p2_zicsr2p0_zifencei2p0"
.p2align 5
.type foo,@function
foo:
tail foo
# Pick a non-canonical nop to ensure test output can't be confused
# with riscv_align padding
sub zero, zero, zero

0 comments on commit 62213be

Please sign in to comment.