[LLD][ELF][RISCV] Add auipc->lui relaxation.

[dybv-sc]

Summary

This relaxation enables use of weak undefined symbols in
programms written with medany code model in mind if code
section happens to be placed far from zero address(>2GiB).

It performs following transormation:

auipc + add -> lui + add

It does it if symbol address is out of range for pc-relative
addressing mode (>2GiB far from pc) and within range for
zero-relative one.

[MaskRay]

This relaxation enables use of weak undefined symbols in programms written with medany code model in mind if code section happens to be placed far from zero address(>2GiB).

I think the motivation is not right. For such weak references, the compiler should generate GOT-generating relocations, not relying on linker relaxation (which is an optional feature) for correctness. https://maskray.me/blog/2021-04-25-weak-symbol

https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/201 "Not doing so is deprecated and a future version of this specification will require using the GOT, not just advise."

[jrtc27]

In D149751#4316073, @MaskRay wrote:

This relaxation enables use of weak undefined symbols in programms written with medany code model in mind if code section happens to be placed far from zero address(>2GiB).

I think the motivation is not right. For such weak references, the compiler should generate GOT-generating relocations, not relying on linker relaxation (which is an optional feature) for correctness.

https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/201 "Not doing so is deprecated and a future version of this specification will require using the GOT, not just advise."

I need to get back to my LLVM patch to do that :( if someone wants to commandeer the series that's fine by me otherwise I'll try and do it once I'm finished with what I'm currently preoccupied with

[jrtc27]

This comment was removed by jrtc27.

[dybv-sc]

In D149751#4316073, @MaskRay wrote:

This relaxation enables use of weak undefined symbols in programms written with medany code model in mind if code section happens to be placed far from zero address(>2GiB).

I think the motivation is not right. For such weak references, the compiler should generate GOT-generating relocations, not relying on linker relaxation (which is an optional feature) for correctness. https://maskray.me/blog/2021-04-25-weak-symbol

https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/201 "Not doing so is deprecated and a future version of this specification will require using the GOT, not just advise."

I understand that, but why not to do that linker relaxation too? That makes lld more compatible with gnu.ld which does that already. Besides, it enables linking with libraries that were compiled using old abi, so that change enhance backwards compatibility.

[MaskRay]

In D149751#4319058, @dybv-sc wrote:

In D149751#4316073, @MaskRay wrote:

This relaxation enables use of weak undefined symbols in programms written with medany code model in mind if code section happens to be placed far from zero address(>2GiB).

I think the motivation is not right. For such weak references, the compiler should generate GOT-generating relocations, not relying on linker relaxation (which is an optional feature) for correctness. https://maskray.me/blog/2021-04-25-weak-symbol

https://github.com/riscv-non-isa/riscv-elf-psabi-doc/pull/201 "Not doing so is deprecated and a future version of this specification will require using the GOT, not just advise."

I understand that, but why not to do that linker relaxation too? That makes lld more compatible with gnu.ld which does that already. Besides, it enables linking with libraries that were compiled using old abi, so that change enhance backwards compatibility.

This relaxation is not appropriate and it will be removed from a future psABI. Code that relies on the brittle assumption is super rare and if present, should be fixed, not relying on the linker to work around the user error.
I don't think the need is significant enough to justify the to-be-removed relaxation.

[asb]

Do recent GCC toolchains still produce code requiring this relaxation (as opposed to using the GOT)?

Outside of R_RISCV_ALIGN, is this the only case that requires relaxation implementation for correctness? I agree with @MaskRay that it is better for the RISC-V ecosystem if the set of required relaxation transformations is kept minimal (not least as it makes it easier to bring up a new linker). But I'm also wary about incompatibilities between lld and GNU ld leading to issues.

lld/ELF/Arch/RISCV.cpp

if (rvc && isInt<12>(displace) && rd == 0) {
     remove = 6;
   } else if (isInt<21>(displace)) {
     sec.relaxAux->relocTypes[i] = R_RISCV_JAL;
     sec.relaxAux->writes.push_back(0x6f | rd << 7); // jal
     remove = 4;
   }
 }
 
+// Relax R_RISCV_PCREL_HI20 and R_RISCV_PCREL_LO12_(I|S) pair form auipc+add to
+// lui+add.
+static void relaxPcrel(InputSection &sec, size_t i, uint64_t loc, Relocation &r,
+                       uint32_t &remove) {
+  const Symbol &sym = *r.sym;
+  const uint64_t dest =
+      (r.expr == R_PLT_PC ? sym.getPltVA() : sym.getVA()) + r.addend;
+  const int64_t displace = dest - loc;
+
+  // This relaxation is useless in general, because it does not lessen code
+  // size, so perform it only if pcrel is out of range.
+  if (isInt<32>(displace))
+    return;
+
+  // lui + add could only reference 2GiB from zero.
+  if (!isInt<32>(dest))
+    return;
+
+  // auipc rd, 0 => lui rd, 0
+  const uint64_t auipc = read32le(sec.content().data() + r.offset);
+  const uint32_t rd = extractBits(auipc, 11, 7);
+  sec.relaxAux->writes.push_back(0x37 | rd << 7); // lui
+  sec.relaxAux->relocTypes[i] = R_RISCV_HI20;
+
+  MutableArrayRef<Relocation> rels = sec.relocs();
+
+  // R_RISCV...LO12_(I|S) counterpart.
+  auto &LoRel = rels[i + 2];
+
+  //  R_RISCV_PCREL_HI20/R_RISCV_PCREL_LO12_(I|S) pair
+  //  is a bit weird, because lower part of relocation
+  //  references special symbol in location of auipc instruction
+  //  rather than the actual relocating symbol.
+  //  That's why symbol of lower relocation should be substituted
+  //  during relaxation process.
+  //
+  //  ...
+  //  .Lpcrel_hi0:
+  //      auipc   a0, %pcrel_hi(foo)
+  //      addi    a0, a0, %pcrel_lo(.Lpcrel_hi0)
+  //  ...
+  //
+  LoRel.sym = r.sym;
+
+  r.expr = R_ABS;
+  LoRel.expr = r.expr;
+
+  switch (LoRel.type) {
+  case R_RISCV_PCREL_LO12_I:
+    sec.relaxAux->relocTypes[i + 2] = R_RISCV_LO12_I;
+    break;
+  case R_RISCV_PCREL_LO12_S:
+    sec.relaxAux->relocTypes[i + 2] = R_RISCV_LO12_S;
+  }
+}
+
 // Relax local-exec TLS when hi20 is zero.
 static void relaxTlsLe(const InputSection &sec, size_t i, uint64_t loc,
                        Relocation &r, uint32_t &remove) {
   uint64_t val = r.sym->getVA(r.addend);
   if (hi20(val) != 0)
     return;
   uint32_t insn = read32le(sec.content().data() + r.offset);
   switch (r.type) {
case R_RISCV_ALIGN: {
       break;
     }
     case R_RISCV_CALL:
     case R_RISCV_CALL_PLT:
       if (i + 1 != sec.relocs().size() &&
           sec.relocs()[i + 1].type == R_RISCV_RELAX)
         relaxCall(sec, i, loc, r, remove);
       break;
+    case R_RISCV_PCREL_HI20:
+      if (i + 1 != sec.relocs().size() &&
+          sec.relocs()[i + 1].type == R_RISCV_RELAX)
+        relaxPcrel(sec, i, loc, r, remove);
+      break;
     case R_RISCV_TPREL_HI20:
     case R_RISCV_TPREL_ADD:
     case R_RISCV_TPREL_LO12_I:
     case R_RISCV_TPREL_LO12_S:
       if (i + 1 != sec.relocs().size() &&
           sec.relocs()[i + 1].type == R_RISCV_RELAX)
         relaxTlsLe(sec, i, loc, r, remove);
       break;
for (InputSection *sec : getInputSections(*osec, storage)) {
               write32le(p + j, 0x00000013); // nop
             if (j != skip) {
               assert(j + 2 == skip);
               write16le(p + j, 0x0001); // c.nop
             }
           }
         } else if (RelType newType = aux.relocTypes[i]) {
           switch (newType) {
+          case R_RISCV_HI20:
+            skip = 4;
+            write32le(p, aux.writes[writesIdx++]);
+            break;
+          case R_RISCV_LO12_I:
+          case R_RISCV_LO12_S:
           case INTERNAL_R_RISCV_GPREL_I:
           case INTERNAL_R_RISCV_GPREL_S:
             break;
           case R_RISCV_RELAX:
             // Used by relaxTlsLe to indicate the relocation is ignored.
             break;
           case R_RISCV_RVC_JUMP:
             skip = 2;

lld/test/ELF/riscv-relax-weak.s

+# REQUIRES: riscv
+
+## auipc -> lui relaxation
+##
+## This example uses weak undefined symbol 'foo'
+## which value would be 0 in final elf image.
+## Function '_start' that reference that symbol
+## was written/compiled with medany code model
+## in mind. That's why it uses pc-relative
+## addressing mode to access foo. However
+## if _start is put in distant address(over 2GiB from zero)
+## which is legal for medany model, foo relocation would be
+## out of range. To allow for weak references to work
+## linker should change addressing mode to absolute.
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax a.s -o a.o
+# RUN: ld.lld a.o -T a.ld -o a.elf
+# RUN: llvm-objdump -d a.elf | FileCheck %s --check-prefix=RELAX
+# RUN: not ld.lld a.o -T a.ld --no-relax |& FileCheck %s --check-prefix=NOREL
+
+# RELAX: 0000001000000000 <_start>:
+# RELAX-NEXT: 1000000000: 37 05 00 00 lui a0, 0
+# RELAX-NEXT: 1000000004: 13 05 05 00 mv  a0, a0
+
+# NOREL: relocation R_RISCV_PCREL_HI20 out of range: -16777216 is not in [-524288, 524287]; references 'foo'
+
+#--- a.s
+        .text
+        .section        .text._start,"ax",@progbits
+        .globl  _start
+        .p2align        1
+_start:
+.Lpcrel_hi0:
+        auipc   a0, %pcrel_hi(foo)
+        addi    a0, a0, %pcrel_lo(.Lpcrel_hi0)
+        beqz    a0, .LBB0_2
+        addi    sp, sp, -16
+        sd      ra, 8(sp)
+        call    foo
+        ld      ra, 8(sp)
+        addi    sp, sp, 16
+.LBB0_2:
+        ret
+        .weak   foo
+        .addrsig
+        .addrsig_sym foo
+#--- a.ld
+SECTIONS {
+  .text 0x1000000000 : { *(.text) }
+}
+