[lld][ELF] Support relocations R_AVR_LO8_LDI_GS/R_AVR_HI8_LDI_GS

[benshi001]

Summary

Relocations R_AVR_LO8_LDI_GS/R_AVR_HI8_LDI_GS (indirect calls
via function pointers) only cover range 128KiB. They are
equivalent to R_AVR_LO8_LDI_PM/R_AVR_HI8_LDI_PM within this
range.

But for function addresses beyond this range, GNU-ld emits
trampolines. And this patch implements corresponding thunks
for them in lld.

[benshi001]

Here are how GNU-ld handles these two relocations

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elf32-avr.c;h=702719136d09acbc8c98ec49ab8129d0f33fffa8;hb=HEAD#l493
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elf32-avr.c;h=702719136d09acbc8c98ec49ab8129d0f33fffa8;hb=HEAD#l1242

[MaskRay]

Sorry for my belated review. I finished a long trip and had a very long task queue....

lld/ELF/Arch/AVR.cpp

113

If s.getVA() & 1 is not rejected by previous checks, we should not have this check.

Don't add a period for messages. See https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

lld/ELF/Thunks.cpp

295

Don't copy the MIPS LA25 thunk comment. It doesn't provide any value for a reader.

Use a short description.

906

Just write the instruction form, as an example for other architectures: // b .+4

lld/test/ELF/avr-thunk.s

8 ↗

(On Diff #513054)

Where is 000110b4 referenced in the CHECK lines?

19 ↗

(On Diff #513054)

Is this intended to be a local symbol?

[benshi001]

lld/test/ELF/avr-thunk.s

8 ↗

(On Diff #513054)

000110b4 is reffered by

; CHECK-NEXT:    ldi     r30, 0x5a
; CHECK-NEXT:    ldi     r31, 0x88

that is ((0x88 << 8) + 0x5a) == 0x110b4

[benshi001]

lld/test/ELF/avr-thunk.s

8 ↗

(On Diff #513054)

((0x88 << 8) + 0x5a) << 1 == 0x110b4

[benshi001]

lld/ELF/Arch/AVR.cpp

113

The redundant assert is removed.

[benshi001]

lld/test/ELF/avr-thunk.s

19 ↗

(On Diff #513054)

this symbol is not important, I just want it to be a seperator between the original code and the auto generated thunk code.

[MaskRay]

lld/test/ELF/avr-thunk.s

8 ↗

(On Diff #513054)

I'd add a ;; CHECK-NEXT just before ; CHECK-NEXT: ldi r30, 0x5a. You can see my past updates to ppc-* or aarch64-* tests.

[benshi001]

lld/test/ELF/avr-thunk.s

8 ↗

(On Diff #513054)

I did not catch your point here, do you mean, it would be better to be

;; CHECK:       [[ADDR0:[0-9]+]] <__AVRThunk_b>:
;; CHECK-NEXT:    jmp   0x20000
;; CHECK-NEXT:    ldi     r30, [[ADDR0]]
;; CHECK-NEXT:    ldi     r31, [[ADDR0]]

But this does not work, since ADDR0 is broken to 0x88 and 0x5a.

[MaskRay]

lld/test/ELF/avr-thunk.s

8 ↗

(On Diff #513054)

Sorry, I mean you can add comments how 0x5a and 0x88 are computed

;; (0x20000 ... ) = 0x5a
;; (0x20000 ... ) = 0x88

[MaskRay]

lld/test/ELF/avr-thunk.s

1 ↗

(On Diff #515213)

If there can be other thunks, I'd suggest a filename with more information, e.g. avr-thunk-ldi-gs.s

8 ↗

(On Diff #515213)

; CHECK-LABEL: <__AVRThunk_b>:
; CHECK-NEXT:   000110b4 jmp 0x20000

(Then use llvm-objdump ... --no-show-raw-insn

This idea is that <__AVRThunk_b> will always match. If the address changes, the NEXT: pattern ensures that FileCheck can locate the exact failure line. Technically such issues can be fixed by a sufficiently smart auto tool.

000110b4 <__AVRThunk_b>: may not give FileCheck sufficient information to locate a line.

[benshi001]

I have renamed the file to avr-thunk-ldi-gs.s, Thanks!

lld/ELF/Arch/AVR.cpp

 // Note that the current AVR support is very preliminary so you can't
 // link any useful program yet, though.
 //
 //===----------------------------------------------------------------------===//
 
 #include "InputFiles.h"
 #include "Symbols.h"
 #include "Target.h"
+#include "Thunks.h"
 #include "lld/Common/ErrorHandler.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/Support/Endian.h"
 
 using namespace llvm;
 using namespace llvm::object;
 using namespace llvm::support::endian;
 using namespace llvm::ELF;
 using namespace lld;
 using namespace lld::elf;
 
 namespace {
 class AVR final : public TargetInfo {
 public:
+  AVR() { needsThunks = true; }
   uint32_t calcEFlags() const override;
   RelExpr getRelExpr(RelType type, const Symbol &s,
                      const uint8_t *loc) const override;
+  bool needsThunk(RelExpr expr, RelType type, const InputFile *file,
+                  uint64_t branchAddr, const Symbol &s,
+                  int64_t a) const override;
   void relocate(uint8_t *loc, const Relocation &rel,
                 uint64_t val) const override;
 };
 } // namespace
 
 RelExpr AVR::getRelExpr(RelType type, const Symbol &s,
                         const uint8_t *loc) const {
   switch (type) {
RelExpr AVR::getRelExpr(RelType type, const Symbol &s,
   case R_AVR_LO8_LDI:
   case R_AVR_LO8_LDI_NEG:
   case R_AVR_HI8_LDI:
   case R_AVR_HI8_LDI_NEG:
   case R_AVR_HH8_LDI_NEG:
   case R_AVR_HH8_LDI:
   case R_AVR_MS8_LDI_NEG:
   case R_AVR_MS8_LDI:
+  case R_AVR_LO8_LDI_GS:
   case R_AVR_LO8_LDI_PM:
   case R_AVR_LO8_LDI_PM_NEG:
+  case R_AVR_HI8_LDI_GS:
   case R_AVR_HI8_LDI_PM:
   case R_AVR_HI8_LDI_PM_NEG:
   case R_AVR_HH8_LDI_PM:
   case R_AVR_HH8_LDI_PM_NEG:
   case R_AVR_LDS_STS_16:
   case R_AVR_PORT5:
   case R_AVR_PORT6:
   case R_AVR_CALL:
     return R_ABS;
   case R_AVR_7_PCREL:
   case R_AVR_13_PCREL:
     return R_PC;
   default:
     error(getErrorLocation(loc) + "unknown relocation (" + Twine(type) +
           ") against symbol " + toString(s));
     return R_NONE;
   }
 }
 
 static void writeLDI(uint8_t *loc, uint64_t val) {
   write16le(loc, (read16le(loc) & 0xf0f0) | (val & 0xf0) << 4 | (val & 0x0f));
 }
 
+bool AVR::needsThunk(RelExpr expr, RelType type, const InputFile *file,
+                     uint64_t branchAddr, const Symbol &s, int64_t a) const {
+  switch (type) {
+  case R_AVR_LO8_LDI_GS:
+  case R_AVR_HI8_LDI_GS:
+    // A thunk is needed if the symbol's virtual address is out of range
+    // [0, 0x1ffff].
+    return s.getVA() >= 0x20000;
+  default:
+    return false;
+  }
+}
+
 void AVR::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {
   switch (rel.type) {
   case R_AVR_8:
     checkUInt(loc, val, 8, rel);
     *loc = val;
     break;
   case R_AVR_8_LO8:
     checkUInt(loc, val, 32, rel);
case R_AVR_HH8_LDI:
     break;
   case R_AVR_MS8_LDI_NEG:
     writeLDI(loc, (-val >> 24) & 0xff);
     break;
   case R_AVR_MS8_LDI:
     writeLDI(loc, (val >> 24) & 0xff);
     break;
 
+  case R_AVR_LO8_LDI_GS:
+    checkUInt(loc, val, 17, rel);
+    [[fallthrough]];
   case R_AVR_LO8_LDI_PM:
     checkAlignment(loc, val, 2, rel);
     writeLDI(loc, (val >> 1) & 0xff);
     break;
+  case R_AVR_HI8_LDI_GS:
+    checkUInt(loc, val, 17, rel);
+    [[fallthrough]];
   case R_AVR_HI8_LDI_PM:
     checkAlignment(loc, val, 2, rel);
     writeLDI(loc, (val >> 9) & 0xff);
     break;
   case R_AVR_HH8_LDI_PM:
     checkAlignment(loc, val, 2, rel);
     writeLDI(loc, (val >> 17) & 0xff);
     break;

lld/ELF/Thunks.cpp

public:
   ThumbV4ABSLongThunk(Symbol &dest, int64_t addend)
       : ThumbThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 16; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
 };
 
+// The AVR devices need thunks for R_AVR_LO8_LDI_GS/R_AVR_HI8_LDI_GS
+// when their destination is out of range [0, 0x1ffff].
+class AVRThunk : public Thunk {
+public:
+  AVRThunk(Symbol &dest, int64_t addend) : Thunk(dest, addend) {}
+  uint32_t size() override { return 4; }
+  void writeTo(uint8_t *buf) override;
+  void addSymbols(ThunkSection &isec) override;
+};
+
 // MIPS LA25 thunk
 class MipsThunk final : public Thunk {
 public:
   MipsThunk(Symbol &dest) : Thunk(dest, 0) {}
 
   uint32_t size() override { return 16; }
   void writeTo(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;

 void ThumbV4PILongThunk::addSymbols(ThunkSection &isec) {
   addSymbol(saver().save("__Thumbv4PILongThunk_" + destination.getName()),
             STT_FUNC, 1, isec);
   addSymbol("$t", STT_NOTYPE, 0, isec);
   addSymbol("$a", STT_NOTYPE, 4, isec);
   addSymbol("$d", STT_NOTYPE, 16, isec);
 }
 
+// Use the long jump which covers a range up to 8MiB.
+void AVRThunk::writeTo(uint8_t *buf) {
+  write32(buf, 0x940c); // jmp func
+  target->relocateNoSym(buf, R_AVR_CALL, destination.getVA());
+}
+
+void AVRThunk::addSymbols(ThunkSection &isec) {
+  addSymbol(saver().save("__AVRThunk_" + destination.getName()), STT_FUNC, 0,
+            isec);
+}
+
 // Write MIPS LA25 thunk code to call PIC function from the non-PIC one.
 void MipsThunk::writeTo(uint8_t *buf) {
   uint64_t s = destination.getVA();
   write32(buf, 0x3c190000); // lui   $25, %hi(func)
   write32(buf + 4, 0x08000000 | (s >> 2)); // j     func
   write32(buf + 8, 0x27390000); // addiu $25, $25, %lo(func)
   write32(buf + 12, 0x00000000); // nop
   target->relocateNoSym(buf, R_MIPS_HI16, s);
static Thunk *addThunkArm(RelType reloc, Symbol &s, int64_t a) {
   case R_ARM_THM_CALL:
     if (config->picThunk)
       return make<ThumbV7PILongThunk>(s, a);
     return make<ThumbV7ABSLongThunk>(s, a);
   }
   fatal("unrecognized relocation type");
 }
 
+static Thunk *addThunkAVR(RelType type, Symbol &s, int64_t a) {
+  switch (type) {
+  case R_AVR_LO8_LDI_GS:
+  case R_AVR_HI8_LDI_GS:
+    return make<AVRThunk>(s, a);
+  default:
+    fatal("unrecognized relocation type " + toString(type));
+  }
+}
+
 static Thunk *addThunkMips(RelType type, Symbol &s) {
   if ((s.stOther & STO_MIPS_MICROMIPS) && isMipsR6())
     return make<MicroMipsR6Thunk>(s);
   if (s.stOther & STO_MIPS_MICROMIPS)
     return make<MicroMipsThunk>(s);
   return make<MipsThunk>(s);
 }
 
Thunk *elf::addThunk(const InputSection &isec, Relocation &rel) {
   Symbol &s = *rel.sym;
   int64_t a = rel.addend;
 
   switch (config->emachine) {
   case EM_AARCH64:
     return addThunkAArch64(rel.type, s, a);
   case EM_ARM:
     return addThunkArm(rel.type, s, a);
+  case EM_AVR:
+    return addThunkAVR(rel.type, s, a);
   case EM_MIPS:
     return addThunkMips(rel.type, s);
   case EM_PPC:
     return addThunkPPC32(isec, rel, s);
   case EM_PPC64:
     return addThunkPPC64(rel.type, s, a);
   default:
-    llvm_unreachable("add Thunk only supported for ARM, Mips and PowerPC");
+    llvm_unreachable("add Thunk only supported for ARM, AVR, Mips and PowerPC");
   }
 }

lld/test/ELF/avr-reloc.s

 ; REQUIRES: avr
 ; RUN: llvm-mc -filetype=obj -triple=avr -mcpu=atmega328p %s -o %t0.o
-; RUN: ld.lld %t0.o --defsym=a=0x12345678 --defsym=b=30 -o %t0
+; RUN: ld.lld %t0.o --defsym=a=0x12345678 --defsym=b=30 --defsym=c=0x15554 -o %t0
 ; RUN: llvm-objdump -d --print-imm-hex --mcpu=atmega328p %t0 | \
 ; RUN:     FileCheck --check-prefixes=CHECK,AVR %s
 ; RUN: llvm-objdump -s --mcpu=atmega328p %t0 | \
 ; RUN:     FileCheck --check-prefixes=HEX,AVRHEX %s
 ; RUN: llvm-mc -filetype=obj -triple=avr -mcpu=attiny10 %s --defsym=TINY=1 -o %t1.o
-; RUN: ld.lld %t1.o --defsym=a=0x12345678 --defsym=b=30 -o %t1
+; RUN: ld.lld %t1.o --defsym=a=0x12345678 --defsym=b=30 --defsym=c=0x15554 -o %t1
 ; RUN: llvm-objdump -d --print-imm-hex --mcpu=attiny10 %t1 | FileCheck %s
 ; RUN: llvm-objdump -s --mcpu=attiny10 %t1 | \
 ; RUN:     FileCheck --check-prefixes=HEX,TINYHEX %s
 
 .section .LDI,"ax",@progbits
 ; CHECK-LABEL: section .LDI:
 ; CHECK:       ldi     r20, 0x78
 ; CHECK-NEXT:  ldi     r20, 0x56
 ; CHECK-NEXT:  ldi     r20, 0x34
 ; CHECK-NEXT:  ldi     r20, 0x12
 ; CHECK-NEXT:  ldi     r20, 0x3c
 ; CHECK-NEXT:  ldi     r20, 0x2b
 ; CHECK-NEXT:  ldi     r20, 0x1a
+; CHECK-NEXT:  ldi     r20, 0xaa
+; CHECK-NEXT:  ldi     r20, 0xaa
 ; CHECK-NEXT:  ldi     r20, 0xff
+
 ldi r20, lo8(a)     ; R_AVR_LO8_LDI
 ldi r20, hi8(a)     ; R_AVR_HI8_LDI
 ldi r20, hh8(a)     ; R_AVR_HH8_LDI
 ldi r20, hhi8(a)    ; R_AVR_MS8_LDI
 
 ldi r20, pm_lo8(a)  ; R_AVR_LO8_LDI_PM
 ldi r20, pm_hi8(a)  ; R_AVR_HI8_LDI_PM
 ldi r20, pm_hh8(a)  ; R_AVR_HH8_LDI_PM
 
+ldi r20, lo8_gs(c)  ; R_AVR_LO8_LDI_GS
+ldi r20, hi8_gs(c)  ; R_AVR_HI8_LDI_GS
+
 ldi r20, b+225
 
 .section .LDI_NEG,"ax",@progbits
 ; CHECK-LABEL: section .LDI_NEG:
 ; CHECK:       ldi     r20, 0x88
 ; CHECK-NEXT:  ldi     r20, 0xa9
 ; CHECK-NEXT:  ldi     r20, 0xcb
 ; CHECK-NEXT:  ldi     r20, 0xed

lld/test/ELF/avr-thunk-ldi-gs.s

+; REQUIRES: avr
+; RUN: llvm-mc -filetype=obj -triple=avr -mcpu=atmega2560 %s -o %t.o
+; RUN: ld.lld %t.o --defsym=a=0x1fffe --defsym=b=0x20000 -o %t
+; RUN: llvm-objdump -d --print-imm-hex --no-show-raw-insn --mcpu=atmega2560 %t \
+; RUN:     | FileCheck %s
+
+.section .LDI,"ax",@progbits
+
+;; CHECK-LABEL: <__AVRThunk_b>:
+;; CHECK-NEXT:  110b4: jmp  0x20000
+;; CHECK-LABEL: <__init>:
+;; CHECK-NEXT:  110b8:  ldi r30, 0xff
+;; CHECK-NEXT:  110ba:  ldi r31, 0xff
+;; CHECK-NEXT:  110bc:  eicall
+;; The destination of the following two LDI instructions is
+;; __AVRThunk_b == 0x110b4, so they actually are
+;;                      ldi r30, ((0x110b4) >> 1) & 0xff
+;;                      ldi r31, ((0x110b4) >> 9)
+;; CHECK-NEXT:  110be:  ldi r30, 0x5a
+;; CHECK-NEXT:  110c0:  ldi r31, 0x88
+;; CHECK-NEXT:  110c2:  eicall
+;; CHECK-NOT:   __AVRThunk_a
+
+.globl __init
+__init:
+;; No thunk is needed, since the destination is in range [0, 0x1ffff].
+ldi r30, lo8_gs(a)  ; R_AVR_LO8_LDI_GS
+ldi r31, hi8_gs(a)  ; R_AVR_HI8_LDI_GS
+eicall
+;; A thunk is needed, since the destination is out of range.
+ldi r30, lo8_gs(b)  ; R_AVR_LO8_LDI_GS
+ldi r31, hi8_gs(b)  ; R_AVR_HI8_LDI_GS
+eicall