[ELF] Fix displacement computation for intra-section branch after D127611

D127611 computed st_value is inaccurate:

* For a backward branch, the destination address may be wrong if there is no
  relaxable relocation between it and the current location due to `if (remove)`.
  We may incorrectly relax a branch to c.j which ends up an overflow.
* For a forward branch, the destination address may be overestimated
  and lose relaxation opportunities.

To fix the issues,

* Don't reset st_value to the original value.
* Save the st_value delta from the previous iteration into valueDelta, and use
  `sa[0].d->value -= delta - valueDelta.find(sa[0].d)->second`.
This commit is contained in:
Fangrui Song 2022-07-13 00:17:17 -07:00
parent aabfaf901b
commit 6b1d151fe3
2 changed files with 55 additions and 11 deletions

View File

@ -572,18 +572,20 @@ static bool relax(InputSection &sec) {
auto &aux = *sec.relaxAux;
bool changed = false;
// Restore original st_value for symbols relative to this section.
// Get st_value delta for symbols relative to this section from the previous
// iteration.
DenseMap<const Defined *, uint64_t> valueDelta;
ArrayRef<SymbolAnchor> sa = makeArrayRef(aux.anchors);
uint32_t delta = 0;
for (auto it : llvm::enumerate(sec.relocations)) {
for (; sa.size() && sa[0].offset <= it.value().offset; sa = sa.slice(1))
if (!sa[0].end)
sa[0].d->value += delta;
valueDelta[sa[0].d] = delta;
delta = aux.relocDeltas[it.index()];
}
for (const SymbolAnchor &sa : sa)
if (!sa.end)
sa.d->value += delta;
valueDelta[sa.d] = delta;
sa = makeArrayRef(aux.anchors);
delta = 0;
@ -615,13 +617,11 @@ static bool relax(InputSection &sec) {
// For all anchors whose offsets are <= r.offset, they are preceded by
// the previous relocation whose `relocDeltas` value equals `delta`.
// Decrease their st_value and update their st_size.
if (remove) {
for (; sa.size() && sa[0].offset <= r.offset; sa = sa.slice(1)) {
if (sa[0].end)
sa[0].d->size = sa[0].offset - delta - sa[0].d->value;
else
sa[0].d->value -= delta;
}
for (; sa.size() && sa[0].offset <= r.offset; sa = sa.slice(1)) {
if (sa[0].end)
sa[0].d->size = sa[0].offset - delta - sa[0].d->value;
else
sa[0].d->value -= delta - valueDelta.find(sa[0].d)->second;
}
delta += remove;
if (delta != cur) {
@ -634,7 +634,7 @@ static bool relax(InputSection &sec) {
if (a.end)
a.d->size = a.offset - delta - a.d->value;
else
a.d->value -= delta;
a.d->value -= delta - valueDelta.find(a.d)->second;
}
// Inform assignAddresses that the size has changed.
if (!isUInt<16>(delta))

View File

@ -0,0 +1,44 @@
# REQUIRES: riscv
## Test R_RISCV_CALL referencing the current input section with the displacement
## close to the boundary.
# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+c,+relax %s -o %t.o
# RUN: ld.lld -Ttext=0x10000 %t.o -o %t
# RUN: llvm-objdump -d --no-show-raw-insn -M no-aliases %t | FileCheck %s
# CHECK-LABEL: <_start>:
# CHECK-NEXT: jal ra, {{.*}} <_start>
# CHECK-NEXT: jal ra, {{.*}} <_start>
# CHECK-EMPTY:
# CHECK-NEXT: <a>:
# CHECK-NEXT: c.jr ra
# CHECK-LABEL: <b>:
# CHECK: jal zero, {{.*}} <a>
# CHECK-NEXT: jal zero, {{.*}} <c>
# CHECK-NEXT: c.j {{.*}} <c>
# CHECK-LABEL: <c>:
# CHECK-NEXT: c.jr ra
#--- a.s
.global _start
_start:
call _start
call _start
a:
ret
b:
.space 2048
## Relaxed to jal. If we don't compute the precise value of a, we may consider
## a reachable by c.j.
tail a
## Relaxed to jal. c.j is unreachable.
tail c # c.j
## Relaxed to c.j. If we don't compute the precise value of c, we may consider
## c.j unreachable.
tail c # c.j
.space 2042
c:
ret