[LLD][ELF][ARM] Refactor inBranchRange to use addend for PC Bias

[peter.smith]

Summary

In AArch32 ARM, the PC reads two instructions ahead of the currently executiing instruction. This evaluates to 8 in ARM state and 4 in Thumb state. Branch instructions on AArch32 compensate for this by subtracting the PC bias from the addend. For a branch to symbol this will result in an addend of -8 in ARM state and -4 in Thumb state.

The existing ARM Target::inBranchRange function accounted for this implict addend within the function meaning that if the addend were to be taken into account by the caller then it would be double counted. This complicates the interface for all Targets as callers wanting to account for addends had to account for the ARM PC-bias.

In certain situations such as: https://github.com/ClangBuiltLinux/linux/issues/1305 the PC-bias compensation code didn't match up. In particular normalizeExistingThunk() didn't put the PC-bias back in as Arm thunks did not store the addend.

The simplest fix for the problem is to add the PC bias in normalizeExistingThunk when restoring the addend. However I think it is worth refactoring the Arm inBranchRange implementation so that fewer calls to getPCBias are needed for other Targets. I wasn't able to remove getPCBias completely but hopefully the Relocations.cpp code is simpler now.

In principle a test could be written to replicate the linux kernel build failure but I wasn't able to reproduce with a small example that I could build up from scratch.

Fixes https://github.com/ClangBuiltLinux/linux/issues/1305

[MaskRay]

Thanks for working on this! inBranchRange looks simpler now.

lld/ELF/Arch/ARM.cpp

359

Nit: add a period after a complete sentence.

391

Thanks for the clean-up!

I stared at this piece code a couple of times and worried that this could be off by a few bytes but was not able to comprehend it.

lld/ELF/Relocations.cpp

1942

This probably needs a test (similar to D70637) to test sharing, even if it is difficult to construct a test exercising the range limit.

[MaskRay]

lld/ELF/Arch/ARM.cpp

301

IIUC a is usually -8, so dst+a-branchAddr is the value to be encoded. So this simplifies understanding. inBranchRange can just use the regular [-2**n, 2**n) range instead of doing some compensation.

lld/ELF/Relocations.cpp

1942

Perhaps the comment can say that keyAddend is usually 0, even on ARM.

[peter.smith]

Thanks, I'll upload another patch.

I can write a test case for thunk reuse. The linux kernel problem is complicated as it needs several passes to populate the table in such a way that an error message will be produced:
pass 0:

• Add thunks round 1 (all thunks are added with rel.addend + getPCBias(rel.type) = 0

pass N:

• normalizeExistingThunk invalidates some calls to thunks, their rel.addend is set to 0
• Thunks are added again with rel.addend + getPCBias(rel.type) = -8. By definition no match with any existing thunk as key addend is -8 and all previous thunks will have key addend 0.

pass N + M:

• normalizeExistingThunk invalidates some calls to thunks, their rel.addend is set to 0
• One of the invalidated calls matches an existing thunk as we now have existing thunks added with key addend -8 (from normalizeExistingThunks), we match one of these existing thunks yet we can be up to 8 bytes out of range due to the rel.addend being 0 in the range check.

lld/ELF/Relocations.cpp

1942

I've made a specific test case that tests this line. There is an existing test that fails if the getPCBias(rel.type) is removed but it is in the middle of a larger test that is more difficult to diagnose the problem.

[peter.smith]

Changes:

• Clang format run over changed code
• Improved comment
• Added test case for specific Arm/Thumb reuse

lld/ELF/Arch/ARM.cpp

 
 void ARM::addPltSymbols(InputSection &isec, uint64_t off) const {
   addSyntheticLocal("$a", STT_NOTYPE, off, 0, isec);
   addSyntheticLocal("$d", STT_NOTYPE, off + 12, 0, isec);
 }
 
 bool ARM::needsThunk(RelExpr expr, RelType type, const InputFile *file,
                      uint64_t branchAddr, const Symbol &s,
-                     int64_t /*a*/) const {
+                     int64_t a) const {
   // If S is an undefined weak symbol and does not have a PLT entry then it
   // will be resolved as a branch to the next instruction.
   if (s.isUndefWeak() && !s.isInPlt())
     return false;
   // A state change from ARM to Thumb and vice versa must go through an
   // interworking thunk if the relocation type is not R_ARM_CALL or
   // R_ARM_THM_CALL.
   switch (type) {
   case R_ARM_PC24:
   case R_ARM_PLT32:
   case R_ARM_JUMP24:
     // Source is ARM, all PLT entries are ARM so no interworking required.
     // Otherwise we need to interwork if STT_FUNC Symbol has bit 0 set (Thumb).
     if (s.isFunc() && expr == R_PC && (s.getVA() & 1))
       return true;
     LLVM_FALLTHROUGH;
   case R_ARM_CALL: {
     uint64_t dst = (expr == R_PLT_PC) ? s.getPltVA() : s.getVA();
-    return !inBranchRange(type, branchAddr, dst);
+    return !inBranchRange(type, branchAddr, dst + a);
   }
   case R_ARM_THM_JUMP19:
   case R_ARM_THM_JUMP24:
     // Source is Thumb, all PLT entries are ARM so interworking is required.
     // Otherwise we need to interwork if STT_FUNC Symbol has bit 0 clear (ARM).
     if (expr == R_PLT_PC || (s.isFunc() && (s.getVA() & 1) == 0))
       return true;
     LLVM_FALLTHROUGH;
   case R_ARM_THM_CALL: {
     uint64_t dst = (expr == R_PLT_PC) ? s.getPltVA() : s.getVA();
-    return !inBranchRange(type, branchAddr, dst);
+    return !inBranchRange(type, branchAddr, dst + a);
   }
   }
   return false;
 }
 
 uint32_t ARM::getThunkSectionSpacing() const {
   // The placing of pre-created ThunkSections is controlled by the value
   // thunkSectionSpacing returned by getThunkSectionSpacing(). The aim is to
uint32_t ARM::getThunkSectionSpacing() const {
   // range. On earlier Architectures such as ARMv4, ARMv5 and ARMv6 (except
   // ARMv6T2) the range is +/- 4MiB.
 
   return (config->armJ1J2BranchEncoding) ? 0x1000000 - 0x30000
                                          : 0x400000 - 0x7500;
 }
 
 bool ARM::inBranchRange(RelType type, uint64_t src, uint64_t dst) const {
-  uint64_t range;
-  uint64_t instrSize;
+  if ((dst & 0x1) == 0)
+    // Destination is ARM, if ARM caller then Src is already 4-byte aligned.
+    // If Thumb Caller (BLX) the Src address has bottom 2 bits cleared to ensure
+    // destination will be 4 byte aligned.
+    src &= ~0x3;
+  else
+    // Bit 0 == 1 denotes Thumb state, it is not part of the range.
+    dst &= ~0x1;
 
+  int64_t offset = dst - src;
   switch (type) {
   case R_ARM_PC24:
   case R_ARM_PLT32:
   case R_ARM_JUMP24:
   case R_ARM_CALL:
-    range = 0x2000000;
-    instrSize = 4;
-    break;
+    return llvm::isInt<26>(offset);
   case R_ARM_THM_JUMP19:
-    range = 0x100000;
-    instrSize = 2;
-    break;
+    return llvm::isInt<21>(offset);
   case R_ARM_THM_JUMP24:
   case R_ARM_THM_CALL:
-    range = config->armJ1J2BranchEncoding ? 0x1000000 : 0x400000;
-    instrSize = 2;
-    break;
+    return config->armJ1J2BranchEncoding ? llvm::isInt<25>(offset)
+                                         : llvm::isInt<23>(offset);
   default:
     return true;
   }
-  // PC at Src is 2 instructions ahead, immediate of branch is signed
-  if (src > dst)
-    range -= 2 * instrSize;
-  else
-    range += instrSize;
-
-  if ((dst & 0x1) == 0)
-    // Destination is ARM, if ARM caller then Src is already 4-byte aligned.
-    // If Thumb Caller (BLX) the Src address has bottom 2 bits cleared to ensure
-    // destination will be 4 byte aligned.
-    src &= ~0x3;
-  else
-    // Bit 0 == 1 denotes Thumb state, it is not part of the range
-    dst &= ~0x1;
-
-  uint64_t distance = (src > dst) ? src - dst : dst - src;
-  return distance <= range;
 }
 
 // Helper to produce message text when LLD detects that a CALL relocation to
 // a non STT_FUNC symbol that may result in incorrect interworking between ARM
 // or Thumb.
 static void stateChangeWarning(uint8_t *loc, RelType relt, const Symbol &s) {
   assert(!s.isFunc());
   if (s.isSection()) {

lld/ELF/Relocations.h

public:
   // to do one time initialization on Pass 0 and put a limit on the
   // number of times it can be called to prevent infinite loops.
   uint32_t pass = 0;
 
 private:
   void mergeThunks(ArrayRef<OutputSection *> outputSections);
 
   ThunkSection *getISDThunkSec(OutputSection *os, InputSection *isec,
-                               InputSectionDescription *isd, uint32_t type,
-                               uint64_t src);
+                               InputSectionDescription *isd,
+                               const Relocation &rel, uint64_t src);
 
   ThunkSection *getISThunkSec(InputSection *isec);
 
   void createInitialThunkSections(ArrayRef<OutputSection *> outputSections);
 
   std::pair<Thunk *, bool> getThunk(InputSection *isec, Relocation &rel,
                                     uint64_t src);
 

lld/ELF/Relocations.cpp

forEachInputSectionDescription(
 
         isd->sections = std::move(tmp);
       });
 }
 
 // Find or create a ThunkSection within the InputSectionDescription (ISD) that
 // is in range of Src. An ISD maps to a range of InputSections described by a
 // linker script section pattern such as { .text .text.* }.
-ThunkSection *ThunkCreator::getISDThunkSec(OutputSection *os, InputSection *isec,
+ThunkSection *ThunkCreator::getISDThunkSec(OutputSection *os,
+                                           InputSection *isec,
                                            InputSectionDescription *isd,
-                                           uint32_t type, uint64_t src) {
+                                           const Relocation &rel,
+                                           uint64_t src) {
   for (std::pair<ThunkSection *, uint32_t> tp : isd->thunkSections) {
     ThunkSection *ts = tp.first;
-    uint64_t tsBase = os->addr + ts->outSecOff;
-    uint64_t tsLimit = tsBase + ts->getSize();
-    if (target->inBranchRange(type, src, (src > tsLimit) ? tsBase : tsLimit))
+    uint64_t tsBase = os->addr + ts->outSecOff + rel.addend;
+    uint64_t tsLimit = tsBase + ts->getSize() + rel.addend;
+    if (target->inBranchRange(rel.type, src,
+                              (src > tsLimit) ? tsBase : tsLimit))
       return ts;
   }
 
   // No suitable ThunkSection exists. This can happen when there is a branch
   // with lower range than the ThunkSection spacing or when there are too
   // many Thunks. Create a new ThunkSection as close to the InputSection as
   // possible. Error if InputSection is so large we cannot place ThunkSection
   // anywhere in Range.
   uint64_t thunkSecOff = isec->outSecOff;
-  if (!target->inBranchRange(type, src, os->addr + thunkSecOff)) {
+  if (!target->inBranchRange(rel.type, src,
+                             os->addr + thunkSecOff + rel.addend)) {
     thunkSecOff = isec->outSecOff + isec->getSize();
-    if (!target->inBranchRange(type, src, os->addr + thunkSecOff))
+    if (!target->inBranchRange(rel.type, src,
+                               os->addr + thunkSecOff + rel.addend))
       fatal("InputSection too large for range extension thunk " +
             isec->getObjMsg(src - (os->addr + isec->outSecOff)));
   }
   return addThunkSection(os, isd, thunkSecOff);
 }
 
 // Add a Thunk that needs to be placed in a ThunkSection that immediately
 // precedes its Target.
static int64_t getPCBias(RelType type) {
   default:
     return 8;
   }
 }
 
 std::pair<Thunk *, bool> ThunkCreator::getThunk(InputSection *isec,
                                                 Relocation &rel, uint64_t src) {
   std::vector<Thunk *> *thunkVec = nullptr;
-  int64_t addend = rel.addend + getPCBias(rel.type);
+  // Arm and Thumb have a PC Bias of 8 and 4 respectively, this is cancelled
+  // out in the relocation addend. We compensate for the PC bias so that
+  // an Arm and Thumb relocation to the same destination get the same keyAddend,
+  // which is usually 0.
+  int64_t keyAddend = rel.addend + getPCBias(rel.type);
 
   // We use a ((section, offset), addend) pair to find the thunk position if
   // possible so that we create only one thunk for aliased symbols or ICFed
   // sections. There may be multiple relocations sharing the same (section,
   // offset + addend) pair. We may revert the relocation back to its original
   // non-Thunk target, so we cannot fold offset + addend.
   if (auto *d = dyn_cast<Defined>(rel.sym))
     if (!d->isInPlt() && d->section)
       thunkVec = &thunkedSymbolsBySectionAndAddend[{
-          {d->section->repl, d->value}, addend}];
+          {d->section->repl, d->value}, keyAddend}];
   if (!thunkVec)
-    thunkVec = &thunkedSymbols[{rel.sym, addend}];
+    thunkVec = &thunkedSymbols[{rel.sym, keyAddend}];
 
   // Check existing Thunks for Sym to see if they can be reused
   for (Thunk *t : *thunkVec)
     if (isThunkSectionCompatible(isec, t->getThunkTargetSym()->section) &&
         t->isCompatibleWith(*isec, rel) &&
         target->inBranchRange(rel.type, src,
-                              t->getThunkTargetSym()->getVA(rel.addend) +
-                                  getPCBias(rel.type)))
+                              t->getThunkTargetSym()->getVA(rel.addend)))
       return std::make_pair(t, false);
 
   // No existing compatible Thunk in range, create a new one
   Thunk *t = addThunk(*isec, rel);
   thunkVec->push_back(t);
   return std::make_pair(t, true);
 }
 
 // Return true if the relocation target is an in range Thunk.
 // Return false if the relocation is not to a Thunk. If the relocation target
 // was originally to a Thunk, but is no longer in range we revert the
 // relocation back to its original non-Thunk target.
 bool ThunkCreator::normalizeExistingThunk(Relocation &rel, uint64_t src) {
   if (Thunk *t = thunks.lookup(rel.sym)) {
-    if (target->inBranchRange(rel.type, src,
-                              rel.sym->getVA(rel.addend) + getPCBias(rel.type)))
+    if (target->inBranchRange(rel.type, src, rel.sym->getVA(rel.addend)))
       return true;
     rel.sym = &t->destination;
     rel.addend = t->addend;
     if (rel.sym->isInPlt())
       rel.expr = toPlt(rel.expr);
   }
   return false;
 }
forEachInputSectionDescription(
             std::tie(t, isNew) = getThunk(isec, rel, src);
 
             if (isNew) {
               // Find or create a ThunkSection for the new Thunk
               ThunkSection *ts;
               if (auto *tis = t->getTargetInputSection())
                 ts = getISThunkSec(tis);
               else
-                ts = getISDThunkSec(os, isec, isd, rel.type, src);
+                ts = getISDThunkSec(os, isec, isd, rel, src);
               ts->addThunk(t);
               thunks[t->getThunkTargetSym()] = t;
             }
 
             // Redirect relocation to Thunk, we never go via the PLT to a Thunk
             rel.sym = t->getThunkTargetSym();
             rel.expr = fromPlt(rel.expr);
 

lld/ELF/Thunks.cpp

 // An ARM thunk may be either short or long. A short thunk is simply a branch
 // (B) instruction, and it may be used to call ARM functions when the distance
 // from the thunk to the target is less than 32MB. Long thunks can branch to any
 // virtual address and can switch between ARM and Thumb, and they are
 // implemented in the derived classes. This class tries to create a short thunk
 // if the target is in range, otherwise it creates a long thunk.
 class ARMThunk : public Thunk {
 public:
-  ARMThunk(Symbol &dest) : Thunk(dest, 0) {}
+  ARMThunk(Symbol &dest, int64_t addend) : Thunk(dest, addend) {}
 
   bool getMayUseShortThunk();
   uint32_t size() override { return getMayUseShortThunk() ? 4 : sizeLong(); }
   void writeTo(uint8_t *buf) override;
   bool isCompatibleWith(const InputSection &isec,
                         const Relocation &rel) const override;
 
   // Returns the size of a long thunk.

 };
 
 // Base class for Thumb-2 thunks.
 //
 // This class is similar to ARMThunk, but it uses the Thumb-2 B.W instruction
 // which has a range of 16MB.
 class ThumbThunk : public Thunk {
 public:
-  ThumbThunk(Symbol &dest) : Thunk(dest, 0) { alignment = 2; }
+  ThumbThunk(Symbol &dest, int64_t addend) : Thunk(dest, addend) {
+    alignment = 2;
+  }
 
   bool getMayUseShortThunk();
   uint32_t size() override { return getMayUseShortThunk() ? 4 : sizeLong(); }
   void writeTo(uint8_t *buf) override;
   bool isCompatibleWith(const InputSection &isec,
                         const Relocation &rel) const override;
 
   // Returns the size of a long thunk.
   virtual uint32_t sizeLong() = 0;
 
   // Writes a long thunk to Buf.
   virtual void writeLong(uint8_t *buf) = 0;
 
 private:
   // See comment in ARMThunk above.
   bool mayUseShortThunk = true;
 };
 
 // Specific ARM Thunk implementations. The naming convention is:
 // Source State, TargetState, Target Requirement, ABS or PI, Range
 class ARMV7ABSLongThunk final : public ARMThunk {
 public:
-  ARMV7ABSLongThunk(Symbol &dest) : ARMThunk(dest) {}
+  ARMV7ABSLongThunk(Symbol &dest, int64_t addend) : ARMThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 12; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
 };
 
 class ARMV7PILongThunk final : public ARMThunk {
 public:
-  ARMV7PILongThunk(Symbol &dest) : ARMThunk(dest) {}
+  ARMV7PILongThunk(Symbol &dest, int64_t addend) : ARMThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 16; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
 };
 
 class ThumbV7ABSLongThunk final : public ThumbThunk {
 public:
-  ThumbV7ABSLongThunk(Symbol &dest) : ThumbThunk(dest) {}
+  ThumbV7ABSLongThunk(Symbol &dest, int64_t addend)
+      : ThumbThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 10; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
 };
 
 class ThumbV7PILongThunk final : public ThumbThunk {
 public:
-  ThumbV7PILongThunk(Symbol &dest) : ThumbThunk(dest) {}
+  ThumbV7PILongThunk(Symbol &dest, int64_t addend) : ThumbThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 12; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
 };
 
 // Implementations of Thunks for older Arm architectures that do not support
 // the movt/movw instructions. These thunks require at least Architecture v5
 // as used on processors such as the Arm926ej-s. There are no Thumb entry
 // points as there is no Thumb branch instruction on these architecture that
 // can result in a thunk
 class ARMV5ABSLongThunk final : public ARMThunk {
 public:
-  ARMV5ABSLongThunk(Symbol &dest) : ARMThunk(dest) {}
+  ARMV5ABSLongThunk(Symbol &dest, int64_t addend) : ARMThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 8; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
   bool isCompatibleWith(const InputSection &isec,
                         const Relocation &rel) const override;
 };
 
 class ARMV5PILongThunk final : public ARMThunk {
 public:
-  ARMV5PILongThunk(Symbol &dest) : ARMThunk(dest) {}
+  ARMV5PILongThunk(Symbol &dest, int64_t addend) : ARMThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 16; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
   bool isCompatibleWith(const InputSection &isec,
                         const Relocation &rel) const override;
 };
 
 // Implementations of Thunks for Arm v6-M. Only Thumb instructions are permitted
 class ThumbV6MABSLongThunk final : public ThumbThunk {
 public:
-  ThumbV6MABSLongThunk(Symbol &dest) : ThumbThunk(dest) {}
+  ThumbV6MABSLongThunk(Symbol &dest, int64_t addend)
+      : ThumbThunk(dest, addend) {}
 
   uint32_t sizeLong() override { return 12; }
   void writeLong(uint8_t *buf) override;
   void addSymbols(ThunkSection &isec) override;
 };
 
 class ThumbV6MPILongThunk final : public ThumbThunk {
 public:
-  ThumbV6MPILongThunk(Symbol &dest) : ThumbThunk(dest) {}
+  ThumbV6MPILongThunk(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;
 };
 
 // MIPS LA25 thunk
 class MipsThunk final : public Thunk {
static Thunk *addThunkAArch64(RelType type, Symbol &s, int64_t a) {
   return make<AArch64ABSLongThunk>(s, a);
 }
 
 // Creates a thunk for Thumb-ARM interworking.
 // Arm Architectures v5 and v6 do not support Thumb2 technology. This means
 // - MOVT and MOVW instructions cannot be used
 // - Only Thumb relocation that can generate a Thunk is a BL, this can always
 //   be transformed into a BLX
-static Thunk *addThunkPreArmv7(RelType reloc, Symbol &s) {
+static Thunk *addThunkPreArmv7(RelType reloc, Symbol &s, int64_t a) {
   switch (reloc) {
   case R_ARM_PC24:
   case R_ARM_PLT32:
   case R_ARM_JUMP24:
   case R_ARM_CALL:
   case R_ARM_THM_CALL:
     if (config->picThunk)
-      return make<ARMV5PILongThunk>(s);
-    return make<ARMV5ABSLongThunk>(s);
+      return make<ARMV5PILongThunk>(s, a);
+    return make<ARMV5ABSLongThunk>(s, a);
   }
   fatal("relocation " + toString(reloc) + " to " + toString(s) +
         " not supported for Armv5 or Armv6 targets");
 }
 
 // Create a thunk for Thumb long branch on V6-M.
 // Arm Architecture v6-M only supports Thumb instructions. This means
 // - MOVT and MOVW instructions cannot be used.
 // - Only a limited number of instructions can access registers r8 and above
 // - No interworking support is needed (all Thumb).
-static Thunk *addThunkV6M(RelType reloc, Symbol &s) {
+static Thunk *addThunkV6M(RelType reloc, Symbol &s, int64_t a) {
   switch (reloc) {
   case R_ARM_THM_JUMP19:
   case R_ARM_THM_JUMP24:
   case R_ARM_THM_CALL:
     if (config->isPic)
-      return make<ThumbV6MPILongThunk>(s);
-    return make<ThumbV6MABSLongThunk>(s);
+      return make<ThumbV6MPILongThunk>(s, a);
+    return make<ThumbV6MABSLongThunk>(s, a);
   }
   fatal("relocation " + toString(reloc) + " to " + toString(s) +
         " not supported for Armv6-M targets");
 }
 
 // Creates a thunk for Thumb-ARM interworking or branch range extension.
-static Thunk *addThunkArm(RelType reloc, Symbol &s) {
+static Thunk *addThunkArm(RelType reloc, Symbol &s, int64_t a) {
   // Decide which Thunk is needed based on:
   // Available instruction set
   // - An Arm Thunk can only be used if Arm state is available.
   // - A Thumb Thunk can only be used if Thumb state is available.
   // - Can only use a Thunk if it uses instructions that the Target supports.
   // Relocation is branch or branch and link
   // - Branch instructions cannot change state, can only select Thunk that
   //   starts in the same state as the caller.
   // - Branch and link relocations can change state, can select Thunks from
   //   either Arm or Thumb.
   // Position independent Thunks if we require position independent code.
 
   // Handle architectures that have restrictions on the instructions that they
   // can use in Thunks. The flags below are set by reading the BuildAttributes
   // of the input objects. InputFiles.cpp contains the mapping from ARM
   // architecture to flag.
   if (!config->armHasMovtMovw) {
     if (!config->armJ1J2BranchEncoding)
-      return addThunkPreArmv7(reloc, s);
-    return addThunkV6M(reloc, s);
+      return addThunkPreArmv7(reloc, s, a);
+    return addThunkV6M(reloc, s, a);
   }
 
   switch (reloc) {
   case R_ARM_PC24:
   case R_ARM_PLT32:
   case R_ARM_JUMP24:
   case R_ARM_CALL:
     if (config->picThunk)
-      return make<ARMV7PILongThunk>(s);
-    return make<ARMV7ABSLongThunk>(s);
+      return make<ARMV7PILongThunk>(s, a);
+    return make<ARMV7ABSLongThunk>(s, a);
   case R_ARM_THM_JUMP19:
   case R_ARM_THM_JUMP24:
   case R_ARM_THM_CALL:
     if (config->picThunk)
-      return make<ThumbV7PILongThunk>(s);
-    return make<ThumbV7ABSLongThunk>(s);
+      return make<ThumbV7PILongThunk>(s, a);
+    return make<ThumbV7ABSLongThunk>(s, a);
   }
   fatal("unrecognized relocation 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)

 Thunk *elf::addThunk(const InputSection &isec, Relocation &rel) {
   Symbol &s = *rel.sym;
   int64_t a = rel.addend;
 
   if (config->emachine == EM_AARCH64)
     return addThunkAArch64(rel.type, s, a);
 
   if (config->emachine == EM_ARM)
-    return addThunkArm(rel.type, s);
+    return addThunkArm(rel.type, s, a);
 
   if (config->emachine == EM_MIPS)
     return addThunkMips(rel.type, s);
 
   if (config->emachine == EM_PPC)
     return addThunkPPC32(isec, rel, s);
 
   if (config->emachine == EM_PPC64)
     return addThunkPPC64(rel.type, s, a);
 
   llvm_unreachable("add Thunk only supported for ARM, Mips and PowerPC");
 }

lld/test/ELF/arm-thunk-arm-thumb-reuse.s

+// REQUIRES: arm
+// RUN: split-file %s %t
+// RUN: llvm-mc -arm-add-build-attributes -filetype=obj -triple=thumbv7a-none-linux-gnueabi %t/test.s -o %t.o
+// RUN: ld.lld --script %t/script %t.o -o %t2
+// RUN: llvm-objdump --no-show-raw-insn -d %t2 | FileCheck %s
+
+/// Test that we can reuse thunks between Arm and Thumb callers
+/// using a BL. Expect two thunks, one for far, one for far2.
+
+//--- script
+SECTIONS {
+        .text 0x10000 : { *(.text) }
+        .text.far 0x10000000 : AT (0x10000000) { *(.far) }
+}
+
+//--- test.s
+
+.syntax unified
+.text
+.globl _start
+.type _start, %function
+ .arm
+_start:
+ bl far
+ .thumb
+ bl far
+ bl far2
+ .arm
+ bl far2
+
+// CHECK:   00010000 <_start>:
+// CHECK-NEXT: 10000: bl      #8 <__ARMv7ABSLongThunk_far>
+// CHECK:   00010004 <$t.1>:
+// CHECK-NEXT: 10004: blx     #8
+// CHECK-NEXT: 10008: bl      #16
+// CHECK:   0001000c <$a.2>:
+// CHECK-NEXT: 1000c: blx     #8 <__Thumbv7ABSLongThunk_far2>
+// CHECK:   00010010 <__ARMv7ABSLongThunk_far>:
+// CHECK-NEXT: 10010: movw    r12, #0
+// CHECK-NEXT: 10014: movt    r12, #4096
+// CHECK-NEXT: 10018: bx      r12
+// CHECK:   0001001c <__Thumbv7ABSLongThunk_far2>:
+// CHECK-NEXT: 1001c: movw    r12, #4
+// CHECK-NEXT: 10020: movt    r12, #4096
+// CHECK-NEXT: 10024: bx      r12
+
+.section .text.far, "ax", %progbits
+.globl far
+.type far, %function
+far:
+ bx lr
+.globl far2
+.type far2, %function
+far2:
+ bx lr
+
+// CHECK: Disassembly of section .text.far:
+// CHECK:      10000000 <far>:
+// CHECK-NEXT: 10000000: bx      lr
+// CHECK:      10000004 <far2>:
+// CHECK-NEXT: 10000004: bx      lr