[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

271

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

Use a short description.

860

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

lld/test/ELF/avr-thunk.s

9

Where is 000110b4 referenced in the CHECK lines?

20

Is this intended to be a local symbol?

[benshi001]

lld/test/ELF/avr-thunk.s

9

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

9

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

[benshi001]

lld/ELF/Arch/AVR.cpp

113

The redundant assert is removed.

[benshi001]

lld/test/ELF/avr-thunk.s

20

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

9

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

9

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

9

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

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

[MaskRay]

lld/test/ELF/avr-thunk.s

2

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

9

; 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) {
default:
     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, 0x1fffe].
+    assert((s.getVA() & 1) == 0 && "function address must be aligned to 2.");
+    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);

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;
 };
 
+// AVR thunk
+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);
 }
 
+void AVRThunk::writeTo(uint8_t *buf) {
+  write32(buf, 0x940c); // Use the direct jump instruction.
+  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");
   }
 }

lld/test/ELF/avr-thunk.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 --mcpu=atmega2560 %t | FileCheck %s
+
+.section .LDI,"ax",@progbits
+
+; CHECK:       000110b4 <__AVRThunk_b>:
+; CHECK-NEXT:    jmp   0x20000
+; CHECK:       000110b8 <__init>:
+; CHECK-NEXT:    ldi     r30, 0xff
+; CHECK-NEXT:    ldi     r31, 0xff
+; CHECK-NEXT:    eicall
+; CHECK-NEXT:    ldi     r30, 0x5a
+; CHECK-NEXT:    ldi     r31, 0x88
+; CHECK-NEXT:    eicall
+; CHECK-NOT:   __AVRThunk_a
+
+__init:
+;; No thunk is needed, since the destination is in range [0, 0x1fffe].
+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