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

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2022-07-13T00:17:17-07:00
New Revision: 6b1d151fe3dc530195d8802f1ecc247c8235dd3a

URL: https://github.com/llvm/llvm-project/commit/6b1d151fe3dc530195d8802f1ecc247c8235dd3a
DIFF: https://github.com/llvm/llvm-project/commit/6b1d151fe3dc530195d8802f1ecc247c8235dd3a.diff

LOG: [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`.

Added: 
    lld/test/ELF/riscv-relax-call-intra-sec.s

Modified: 
    lld/ELF/Arch/RISCV.cpp

Removed: 
    


################################################################################
diff  --git a/lld/ELF/Arch/RISCV.cpp b/lld/ELF/Arch/RISCV.cpp
index 7553a53fce60..ffb68009a1c6 100644
--- a/lld/ELF/Arch/RISCV.cpp
+++ b/lld/ELF/Arch/RISCV.cpp
@@ -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))

diff  --git a/lld/test/ELF/riscv-relax-call-intra-sec.s b/lld/test/ELF/riscv-relax-call-intra-sec.s
new file mode 100644
index 000000000000..bedb8edf838d
--- /dev/null
+++ b/lld/test/ELF/riscv-relax-call-intra-sec.s
@@ -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