[lld] [LLD][RISCV] Fix incorrect call relaxation when mixing +c and -c objects (PR #73977)
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 1 10:15:08 PST 2023
https://github.com/preames updated https://github.com/llvm/llvm-project/pull/73977
>From 8de61cc72f1948f34a19eca109d9137c46aeaf00 Mon Sep 17 00:00:00 2001
From: Philip Reames <preames at rivosinc.com>
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 000000000000000..61665c3ebd55298
--- /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: <foo>:
+# CHECK-NEXT: 1260: c.j 0x1260 <foo>
+# 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, at function
+foo:
+ tail foo
+ nop
>From 56e2160bff2d4eae6d9a5ef76077dbe8b3f132be Mon Sep 17 00:00:00 2001
From: Philip Reames <preames at rivosinc.com>
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 a556d89c36400d3..898e3e45b9e7240 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 61665c3ebd55298..88ee08572875088 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: <foo>:
-# CHECK-NEXT: 1260: c.j 0x1260 <foo>
-# CHECK-NEXT: 1262: addi zero, zero, 0
+# CHECK-NEXT: 1260: jal zero, 0x1260 <foo>
+# CHECK-NEXT: 1264: addi zero, zero, 0
# w/C
#--- a.s
>From 969b6b954e7a79928cb10bbd7c2cc57d31cbc434 Mon Sep 17 00:00:00 2001
From: Philip Reames <preames at rivosinc.com>
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 88ee08572875088..bd8bfcadb90b563 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: <foo>:
-# CHECK-NEXT: 1260: jal zero, 0x1260 <foo>
-# CHECK-NEXT: 1264: addi zero, zero, 0
+# CHECK-NEXT: 10000: jal zero, {{.*}} <foo>
+# CHECK-NEXT: 10004: sub zero, zero, zero
-# w/C
+# w/ C
#--- a.s
.text
.attribute 4, 16
@@ -27,4 +27,6 @@
.type foo, at 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
More information about the llvm-commits
mailing list