From 8de61cc72f1948f34a19eca109d9137c46aeaf00 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Thu, 30 Nov 2023 12:00:12 -0800 Subject: [PATCH 1/3] [LLD][RISCV] Add test case showing incorrect call relaxation --- lld/test/ELF/riscv-relax-call-mixed-rvc.s | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 lld/test/ELF/riscv-relax-call-mixed-rvc.s diff --git a/lld/test/ELF/riscv-relax-call-mixed-rvc.s b/lld/test/ELF/riscv-relax-call-mixed-rvc.s new file mode 100644 index 0000000000000..61665c3ebd552 --- /dev/null +++ b/lld/test/ELF/riscv-relax-call-mixed-rvc.s @@ -0,0 +1,30 @@ +# 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 +# RUN: llvm-objdump -d --no-show-raw-insn -M no-aliases a | FileCheck %s + +## FIXME: The use of c.j is wrong here, because this is coming from a source +## file which does not use C. +# CHECK-LABEL: : +# CHECK-NEXT: 1260: c.j 0x1260 +# CHECK-NEXT: 1262: addi zero, zero, 0 + +# 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 + nop From 56e2160bff2d4eae6d9a5ef76077dbe8b3f132be Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Thu, 30 Nov 2023 12:04:22 -0800 Subject: [PATCH 2/3] [LLD][RISCV] Fix incorrect call relaxation when mixing +c and -c objects 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 https://github.com/llvm/llvm-project/issues/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. --- lld/ELF/Arch/RISCV.cpp | 2 +- lld/test/ELF/riscv-relax-call-mixed-rvc.s | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lld/ELF/Arch/RISCV.cpp b/lld/ELF/Arch/RISCV.cpp index a556d89c36400..898e3e45b9e72 100644 --- a/lld/ELF/Arch/RISCV.cpp +++ b/lld/ELF/Arch/RISCV.cpp @@ -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); diff --git a/lld/test/ELF/riscv-relax-call-mixed-rvc.s b/lld/test/ELF/riscv-relax-call-mixed-rvc.s index 61665c3ebd552..88ee085728750 100644 --- a/lld/test/ELF/riscv-relax-call-mixed-rvc.s +++ b/lld/test/ELF/riscv-relax-call-mixed-rvc.s @@ -6,11 +6,11 @@ # RUN: ld.lld a.o b.o --shared -o a # RUN: llvm-objdump -d --no-show-raw-insn -M no-aliases a | FileCheck %s -## FIXME: The use of c.j is wrong here, because this is coming from a source -## file which does not use C. +## This needs to be a *uncompressed* jal instruction since it came from the +## source file which does not enable C # CHECK-LABEL: : -# CHECK-NEXT: 1260: c.j 0x1260 -# CHECK-NEXT: 1262: addi zero, zero, 0 +# CHECK-NEXT: 1260: jal zero, 0x1260 +# CHECK-NEXT: 1264: addi zero, zero, 0 # w/C #--- a.s From 969b6b954e7a79928cb10bbd7c2cc57d31cbc434 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Fri, 1 Dec 2023 10:14:49 -0800 Subject: [PATCH 3/3] Address review comments on test --- lld/test/ELF/riscv-relax-call-mixed-rvc.s | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lld/test/ELF/riscv-relax-call-mixed-rvc.s b/lld/test/ELF/riscv-relax-call-mixed-rvc.s index 88ee085728750..bd8bfcadb90b5 100644 --- a/lld/test/ELF/riscv-relax-call-mixed-rvc.s +++ b/lld/test/ELF/riscv-relax-call-mixed-rvc.s @@ -3,16 +3,16 @@ # 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 +# 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: : -# CHECK-NEXT: 1260: jal zero, 0x1260 -# CHECK-NEXT: 1264: addi zero, zero, 0 +# CHECK-NEXT: 10000: jal zero, {{.*}} +# CHECK-NEXT: 10004: sub zero, zero, zero -# w/C +# w/ C #--- a.s .text .attribute 4, 16 @@ -27,4 +27,6 @@ .type foo,@function foo: tail foo - nop + # Pick a non-canonical nop to ensure test output can't be confused + # with riscv_align padding + sub zero, zero, zero