Skip to content

Commit 8957e64

Browse files
authored
[ELF,RISCV] Fix oscillation due to call relaxation
The new test (derived from riscv32 openssl/test/cmp_msg_test.c) revealed oscillation in two R_RISCV_CALL_PLT jumps: - First jump (~2^11 bytes away): alternated between 4 and 8 bytes. - Second jump (~2^20 bytes away): alternated between 2 and 8 bytes. The issue is not related to alignment. In 2019, GNU ld addressed a similar problem by reducing the relaxation allowance for cross-section relaxation (https://sourceware.org/bugzilla/show_bug.cgi?id=25181). This approach would result in a suboptimal layout for the tight range tested by riscv-relax-call.s. This patch stabilizes the process by preventing `remove` increment after a few passes, similar to integrated assembler's fragment relaxation. (For the Android bit reproduce, `pass < 2` leads to non-optimal layout while `pass < 3` and `pass < 4` output is identical.) Fix #113838 Possibly fix #123248 (inputs are bitcode, subject to ever-changing code generation, not reproducible) Pull Request: #142899
1 parent 1cb906e commit 8957e64

File tree

2 files changed

+65
-6
lines changed

2 files changed

+65
-6
lines changed

lld/ELF/Arch/RISCV.cpp

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -740,19 +740,23 @@ static void relaxCall(Ctx &ctx, const InputSection &sec, size_t i, uint64_t loc,
740740
(r.expr == R_PLT_PC ? sym.getPltVA(ctx) : sym.getVA(ctx)) + r.addend;
741741
const int64_t displace = dest - loc;
742742

743-
if (rvc && isInt<12>(displace) && rd == 0) {
743+
// When the caller specifies the old value of `remove`, disallow its
744+
// increment.
745+
if (remove >= 6 && rvc && isInt<12>(displace) && rd == 0) {
744746
sec.relaxAux->relocTypes[i] = R_RISCV_RVC_JUMP;
745747
sec.relaxAux->writes.push_back(0xa001); // c.j
746748
remove = 6;
747-
} else if (rvc && isInt<12>(displace) && rd == X_RA &&
749+
} else if (remove >= 6 && rvc && isInt<12>(displace) && rd == X_RA &&
748750
!ctx.arg.is64) { // RV32C only
749751
sec.relaxAux->relocTypes[i] = R_RISCV_RVC_JUMP;
750752
sec.relaxAux->writes.push_back(0x2001); // c.jal
751753
remove = 6;
752-
} else if (isInt<21>(displace)) {
754+
} else if (remove >= 4 && isInt<21>(displace)) {
753755
sec.relaxAux->relocTypes[i] = R_RISCV_JAL;
754756
sec.relaxAux->writes.push_back(0x6f | rd << 7); // jal
755757
remove = 4;
758+
} else {
759+
remove = 0;
756760
}
757761
}
758762

@@ -828,7 +832,7 @@ static void relaxHi20Lo12(Ctx &ctx, const InputSection &sec, size_t i,
828832
}
829833
}
830834

831-
static bool relax(Ctx &ctx, InputSection &sec) {
835+
static bool relax(Ctx &ctx, int pass, InputSection &sec) {
832836
const uint64_t secAddr = sec.getVA();
833837
const MutableArrayRef<Relocation> relocs = sec.relocs();
834838
auto &aux = *sec.relaxAux;
@@ -862,8 +866,13 @@ static bool relax(Ctx &ctx, InputSection &sec) {
862866
}
863867
case R_RISCV_CALL:
864868
case R_RISCV_CALL_PLT:
865-
if (relaxable(relocs, i))
869+
// Prevent oscillation between states by disallowing the increment of
870+
// `remove` after a few passes. The previous `remove` value is
871+
// `cur-delta`.
872+
if (relaxable(relocs, i)) {
873+
remove = pass < 4 ? 6 : cur - delta;
866874
relaxCall(ctx, sec, i, loc, r, remove);
875+
}
867876
break;
868877
case R_RISCV_TPREL_HI20:
869878
case R_RISCV_TPREL_ADD:
@@ -945,7 +954,7 @@ bool RISCV::relaxOnce(int pass) const {
945954
if (!(osec->flags & SHF_EXECINSTR))
946955
continue;
947956
for (InputSection *sec : getInputSections(*osec, storage))
948-
changed |= relax(ctx, *sec);
957+
changed |= relax(ctx, pass, *sec);
949958
}
950959
return changed;
951960
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# REQUIRES: riscv
2+
## The regression test led to oscillation between two states ("address assignment did not converge" error).
3+
## First jump (~2^11 bytes away): alternated between 4 and 8 bytes.
4+
## Second jump (~2^20 bytes away): alternated between 2 and 8 bytes.
5+
6+
# RUN: rm -rf %t && split-file %s %t && cd %t
7+
# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+c,+relax a.s -o a.o
8+
# RUN: ld.lld -T lds a.o -o out
9+
# RUN: llvm-objdump -td --no-show-raw-insn -M no-aliases out | FileCheck %s
10+
11+
# CHECK-LABEL: <_start>:
12+
# CHECK-NEXT: jal zero, {{.*}} <low>
13+
# CHECK-EMPTY:
14+
# CHECK-NEXT: <jump_high>:
15+
# CHECK-NEXT: 1004: auipc t0, 0x100
16+
# CHECK-NEXT: jalr zero, -0x2(t0) <high>
17+
# CHECK-NEXT: ...
18+
# CHECK: <low>:
19+
# CHECK: 1802: c.jr ra
20+
21+
# CHECK: <high>:
22+
# CHECK-NEXT: 101002: jal ra, 0x1004 <jump_high>
23+
# CHECK-NEXT: auipc ra, 0xfff00
24+
# CHECK-NEXT: jalr ra, -0x2(ra) <jump_high>
25+
26+
#--- a.s
27+
## At the beginning of state1, low-_start = 0x800-2, reachable by a c.j.
28+
## This increases high-jump_high from 0x100000-2 to 0x100000, unreachable by a jal.
29+
## In the next iteration, low-_start increases to 0x800, unreachable by a c.j.
30+
.global _start
31+
_start:
32+
jump low, t0 # state0: 4 bytes; state1: 2 bytes
33+
jump_high:
34+
jump high, t0 # state0: 4 bytes; state1: 8 bytes
35+
36+
.space 0x800-10
37+
low:
38+
ret
39+
40+
.section .high,"ax",@progbits
41+
nop
42+
high:
43+
call jump_high
44+
call jump_high
45+
46+
#--- lds
47+
SECTIONS {
48+
.text 0x1000 : { *(.text) }
49+
.high 0x101000 : { *(.high) }
50+
}

0 commit comments

Comments
 (0)