[BOLT][AArch64] Handle gold linker veneers

[yota9]

Summary

The gold linker veneers are written between functions without symbols,
so we to handle it specially in BOLT.

Vladislav Khmelevsky,
Advanced Software Technology Lab, Huawei

[yota9]

ADd test description

[yota9]

bolt/include/bolt/Core/BinaryContext.h

647

The list seems to be more suitable, then previously used set. Since we have to handle references one by one the insertion is faster here. But more important the newely-handled trampolines will create new references, that must be added to the end of the list while iterating it.

bolt/lib/Core/BinaryContext.cpp

1145

We might just create a BF here and call disassemble, but since we don't know veneer size it seems to be that a bit of code copy-paste is more proper solution here, what do you think?

bolt/lib/Core/BinaryFunction.cpp

1298

@maksfb I hope we could solve this problem, do you have ideas how to handle it properly? :)

[yota9]

Add test checks

[yota9]

Uncomment __builtin_unreachable() branch handling.
@maksfb as a tmp solution I purpose to add extra check in unreachable handling. I hope we will able to move elliminate this code here oneday,
but for now it should work fine. Also as I see there is some other code that I think we need to be refactore also, e.g. removeConditionalTailCalls
in postProcessCFG. Ideally we want to move all code transformations to the passes, since at least golang support needs it. Thank you!

[yota9]

rebase

[yota9]

@maksfb @rafauler gentle ping

[yota9]

Run veneer ellimination pass before instrumentation.
We would like to remove veneers asap and before instrumentation pass, since we want to record call to the real function, not it's veneer.
@maksfb @rafauler gentle ping

[yota9]

dbgs->outs in veneer pass

[maksfb]

bolt/test/AArch64/veneer.s

9

We don't enforce double dash long options (at least not yet), but encourage.

13

Why is it .align 2? I though AArch64 required 4 bytes.

32

Will be also good to check the input, to make sure the linker respects --no-relax. Alternatively, print the function in BOLT before and after the veneer elimination pass.

[yota9]

bolt/test/AArch64/veneer.s

13

It's 1 << 2 = 4 bytes

32

NP :)

[maksfb]

bolt/test/AArch64/veneer.s

13

Are you sure? From https://ftp.gnu.org/old-gnu/Manuals/gas-2.9.1/html_chapter/as_7.html:

The way the required alignment is specified varies from system to system. For the a29k, hppa, m68k, m88k, w65, sparc, and Hitachi SH, and i386 using ELF format, the first expression is the alignment request in bytes. For example `.align 8' advances the location counter until it is a multiple of 8. If the location counter is already a multiple of 8, no change is needed.

For other systems, including the i386 using a.out format, it is the number of low-order zero bits the location counter must have after advancement. For example `.align 3' advances the location counter until it a multiple of 8. If the location counter is already a multiple of 8, no change is needed.

I suggest using .p2align to avoid the confusion.

[yota9]

bolt/test/AArch64/veneer.s

13

In your example .align 3 -> until it a multiple of 8 , 1<<3 = 8
From arm docs: https://developer.arm.com/documentation/dui0473/m/directives-reference/align

is a numeric expression evaluating to any power of 2 from 2^0 to 2^31

Also I've checked it by my self now :) But we can use .p2align 2 or .balign 4 indeed, no problem :)

[maksfb]

bolt/test/AArch64/veneer.s

13

Also: https://github.com/llvm/llvm-project/blob/768216cac0242db609fef55ea98283f57801fd18/llvm/test/MC/AsmParser/align_invalid.s#L4

[yota9]

bolt/test/AArch64/veneer.s

13

It is i386 test, there is no reason for arm platform to make alignment non power of two :) Anyway to avoid confusion I've replaced it with .balign 4 :)

[maksfb]

bolt/test/AArch64/veneer.s

13

Thanks. On AArch64 .align 2 is converted into .p2align 2 automatically.

[yota9]

Address comments

[rafauler]

bolt/lib/Core/BinaryContext.cpp

1139

It's curious to see that we arrive at disassembly stage without having mapped this address to a given function. but I guess this might happen in this scenario if there are no symbols to cover the veneer region.

This function serves a dual purpose: both matching a veneer and asserting that there is a veneer at a given address, but also building a new binary function.

I would preferrably avoid functions that have a boolean param and try to break it in two:

1. BC::matchAArch64Veneer()
2. BC::createAArch64VeneerFunction()

And then try to write the code in a way that 2 calls 1, and the BF::disassemble() function calls 1, and RI::disassembleFunctions() calls 2. The "tryToHandle" name prefix here is unnecessary since we can just assume that if we have "bool" as the return type, it might fail because it returns false for some instances, so the naming becomes a bit redundant.

If breaking it up in two functions is too awkward, then I think we can at least rename this to "handleAArch64Veneer", but in that case make the comment in the header file more explicit by saying that this function serves both to match a veneer and, if MatchOnly is false, to create a new function to hold the unmapped veneer instructions.

[yota9]

bolt/lib/Core/BinaryContext.cpp

1139

Thanks for the review rafauler! I think I would stay with the handleAArch64Veneer single function variant, since it is a bit messy to pass the instruction parameters & etc.

[yota9]

Address comments

[rafauler]

LGTM

[rafauler]

Hi @yota9, I reverted this commit because it is causing BOLT to crash in one of your internal binaries, let me work a bit more on it to understand what is happening before you land it again. Thanks!

[yota9]

Hello @rafauler ! Let me know if I can help you with investigation!

[rafauler]

The cause of the crash caused by this diff is because this diff moves processing of interprocedural references outside the disassembly loop.

In the disassembly loop, we don't call "process interprocedural references" on every function. For non simple functions, we bail out early.

With this diff, we process all interprocedural references, even the ones coming from non-simple/ignored functions. We happen to have an internal binary with a non-simple function that has a very weird interprocedural reference that, when processed, causes BOLT to crash (it's an invalid address that can't be mapped to any function).

[rafauler]

bolt/lib/Rewrite/RewriteInstance.cpp

2889

Notice this "continue" here. It will cause us to skip calling "BC->processInterproceduralReferences(Function)" for an ignored function.

[yota9]

bolt/lib/Rewrite/RewriteInstance.cpp

2889

I'm starting to see now, thanks! Probably your CI uses processAllFunctions flag? What if I will re-open the review and will add isIgnored() check in BCs processInterproceduralReferences()?

[rafauler]

I think that works. Feel free to reopen or create a new review, whichever is easier. CI runs:

-reorder-blocks=cache -use-gnu-stack -jump-tables=move

[yota9]

@rafauler Already :) https://reviews.llvm.org/D129260
Also could you please approve this one: https://reviews.llvm.org/D129321 - it fixes CI. Interesting, that such thing does broken it, probably IR during compilation has some extra checks and does not only rely on the symbol name, but also on function argument types..

bolt/include/bolt/Core/BinaryContext.h

 #include "llvm/MC/MCSectionELF.h"
 #include "llvm/MC/MCSectionMachO.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/raw_ostream.h"
 #include <functional>
+#include <list>
 #include <map>
 #include <set>
 #include <shared_mutex>
 #include <string>
 #include <system_error>
 #include <type_traits>
 #include <unordered_map>
 #include <vector>
public:
 
   /// Map SDT locations to SDT markers info
   std::unordered_map<uint64_t, SDTMarkerInfo> SDTMarkers;
 
   /// Map linux kernel program locations/instructions to their pointers in
   /// special linux kernel sections
   std::unordered_map<uint64_t, std::vector<LKInstructionMarkerInfo>> LKMarkers;
 
+  /// List of external addresses in the code that are not a function start
+  /// and are referenced from BinaryFunction.
+  std::list<std::pair<BinaryFunction *, uint64_t>> InterproceduralReferences;
+
   /// PseudoProbe decoder
   MCPseudoProbeDecoder ProbeDecoder;
 
   /// DWARF encoding. Available encoding types defined in BinaryFormat/Dwarf.h
   /// enum Constants, e.g. DW_EH_PE_omit.
   unsigned TTypeEncoding = dwarf::DW_EH_PE_omit;
   unsigned LSDAEncoding = dwarf::DW_EH_PE_omit;
 
public:
 
   /// Register \p TargetFunction as a fragment of \p Function if checks pass:
   /// - if \p TargetFunction name matches \p Function name with a suffix:
   ///   fragment_name == parent_name.cold(.\d+)?
   /// True if the Function is registered, false if the check failed.
   bool registerFragment(BinaryFunction &TargetFunction,
                         BinaryFunction &Function) const;
 
-  /// Resolve inter-procedural dependencies from \p Function.
-  void processInterproceduralReferences(BinaryFunction &Function);
+  /// Add unterprocedural reference for \p Function to \p Address
+  void addInterproceduralReference(BinaryFunction *Function, uint64_t Address) {
+    InterproceduralReferences.push_back({Function, Address});
+  }
+
+  /// Used to fix the target of linker-generated AArch64 adrp + add
+  /// sequence with no relocation info.
+  void addAdrpAddRelocAArch64(BinaryFunction &BF, MCInst &LoadLowBits,
+                              MCInst &LoadHiBits, uint64_t Target);
+
+  /// Return true if AARch64 veneer was successfully matched at a given
+  /// \p Address and register veneer binary function if \p MatchOnly
+  /// argument is false.
+  bool handleAArch64Veneer(uint64_t Address, bool MatchOnly = false);
+
+  /// Resolve inter-procedural dependencies from
+  void processInterproceduralReferences();
 
   /// Skip functions with all parent and child fragments transitively.
   void skipMarkedFragments();
 
   /// Perform any necessary post processing on the symbol table after
   /// function disassembly is complete.  This processing fixes top
   /// level data holes and makes sure the symbol table is valid.
   /// It also assigns all memory profiling info to the appropriate

bolt/include/bolt/Core/BinaryFunction.h

private:
 
   const MCSymbol *PersonalityFunction{nullptr};
   uint8_t PersonalityEncoding{dwarf::DW_EH_PE_sdata4 | dwarf::DW_EH_PE_pcrel};
 
   BinaryContext &BC;
 
   std::unique_ptr<BinaryLoopInfo> BLI;
 
-  /// Set of external addresses in the code that are not a function start
-  /// and are referenced from this function.
-  std::set<uint64_t> InterproceduralReferences;
-
   /// All labels in the function that are referenced via relocations from
   /// data objects. Typically these are jump table destinations and computed
   /// goto labels.
   std::set<uint64_t> ExternallyReferencedOffsets;
 
   /// Offsets of indirect branches with unknown destinations.
   std::set<uint64_t> UnknownIndirectBranchOffsets;
 

bolt/include/bolt/Core/MCPlusBuilder.h

virtual void createLongJmp(InstructionListType &Seq, const MCSymbol *Target,
     llvm_unreachable("not implemented");
   }
 
   virtual void createShortJmp(InstructionListType &Seq, const MCSymbol *Target,
                               MCContext *Ctx, bool IsTailCall = false) {
     llvm_unreachable("not implemented");
   }
 
-  /// Return true if the instruction CurInst, in combination with the recent
+  /// Return not 0 if the instruction CurInst, in combination with the recent
   /// history of disassembled instructions supplied by [Begin, End), is a linker
   /// generated veneer/stub that needs patching. This happens in AArch64 when
   /// the code is large and the linker needs to generate stubs, but it does
   /// not put any extra relocation information that could help us to easily
   /// extract the real target. This function identifies and extract the real
   /// target in Tgt. The instruction that loads the lower bits of the target
   /// is put in TgtLowBits, and its pair in TgtHiBits. If the instruction in
   /// TgtHiBits does not have an immediate operand, but an expression, then
   /// this expression is put in TgtHiSym and Tgt only contains the lower bits.
-  virtual bool matchLinkerVeneer(InstructionIterator Begin,
+  /// Return value is a total number of instructions that were used to create
+  /// a veneer.
+  virtual uint64_t matchLinkerVeneer(InstructionIterator Begin,
                                      InstructionIterator End, uint64_t Address,
-                                 const MCInst &CurInst, MCInst *&TargetHiBits,
+                                     const MCInst &CurInst,
+                                     MCInst *&TargetHiBits,
                                      MCInst *&TargetLowBits,
                                      uint64_t &Target) const {
     llvm_unreachable("not implemented");
   }
 
   virtual int getShortJmpEncodingSize() const {
     llvm_unreachable("not implemented");
   }
 
   virtual int getUncondBranchEncodingSize() const {

bolt/lib/Core/BinaryContext.cpp

if (BF.containsAddress(Address, /*UseMaxSize=*/isAArch64())) {
           outs() << "BOLT-INFO: potentially escaped address 0x"
                  << Twine::utohexstr(Address) << " in function " << BF << '\n';
         }
         BF.HasInternalLabelReference = true;
         return std::make_pair(
             BF.addEntryPointAtOffset(Address - BF.getAddress()), Addend);
       }
     } else {
-      BF.InterproceduralReferences.insert(Address);
+      addInterproceduralReference(&BF, Address);
     }
   }
 
   // With relocations, catch jump table references outside of the basic block
   // containing the indirect jump.
   if (HasRelocations) {
     const MemoryContentsType MemType = analyzeMemoryAt(Address, BF);
     if (MemType == MemoryContentsType::POSSIBLE_PIC_JUMP_TABLE && IsPCRel) {
bool BinaryContext::registerFragment(BinaryFunction &TargetFunction,
   }
   if (opts::Verbosity >= 1) {
     outs() << "BOLT-INFO: marking " << TargetFunction << " as a fragment of "
            << Function << '\n';
   }
   return true;
 }
 
-void BinaryContext::processInterproceduralReferences(BinaryFunction &Function) {
-  for (uint64_t Address : Function.InterproceduralReferences) {
+void BinaryContext::addAdrpAddRelocAArch64(BinaryFunction &BF,
+                                           MCInst &LoadLowBits,
+                                           MCInst &LoadHiBits,
+                                           uint64_t Target) {
+  const MCSymbol *TargetSymbol;
+  uint64_t Addend = 0;
+  std::tie(TargetSymbol, Addend) = handleAddressRef(Target, BF,
+                                                    /*IsPCRel*/ true);
+  int64_t Val;
+  MIB->replaceImmWithSymbolRef(LoadHiBits, TargetSymbol, Addend, Ctx.get(), Val,
+                               ELF::R_AARCH64_ADR_PREL_PG_HI21);
+  MIB->replaceImmWithSymbolRef(LoadLowBits, TargetSymbol, Addend, Ctx.get(),
+                               Val, ELF::R_AARCH64_ADD_ABS_LO12_NC);
+}
+
+bool BinaryContext::handleAArch64Veneer(uint64_t Address, bool MatchOnly) {
+  BinaryFunction *TargetFunction = getBinaryFunctionContainingAddress(Address);
+  if (TargetFunction)
+    return false;
+
+  ErrorOr<BinarySection &> Section = getSectionForAddress(Address);
+  assert(Section && "cannot get section for referenced address");
+  if (!Section->isText())
+    return false;
+
+  bool Ret = false;
+  StringRef SectionContents = Section->getContents();
+  uint64_t Offset = Address - Section->getAddress();
+  const uint64_t MaxSize = SectionContents.size() - Offset;
+  const uint8_t *Bytes =
+      reinterpret_cast<const uint8_t *>(SectionContents.data());
+  ArrayRef<uint8_t> Data(Bytes + Offset, MaxSize);
+
+  auto matchVeneer = [&](BinaryFunction::InstrMapType &Instructions,
+                         MCInst &Instruction, uint64_t Offset,
+                         uint64_t AbsoluteInstrAddr,
+                         uint64_t TotalSize) -> bool {
+    MCInst *TargetHiBits, *TargetLowBits;
+    uint64_t TargetAddress, Count;
+    Count = MIB->matchLinkerVeneer(Instructions.begin(), Instructions.end(),
+                                   AbsoluteInstrAddr, Instruction, TargetHiBits,
+                                   TargetLowBits, TargetAddress);
+    if (!Count)
+      return false;
+
+    if (MatchOnly)
+      return true;
+
+    // NOTE The target symbol was created during disassemble's
+    // handleExternalReference
+    const MCSymbol *VeneerSymbol = getOrCreateGlobalSymbol(Address, "FUNCat");
+    BinaryFunction *Veneer = createBinaryFunction(VeneerSymbol->getName().str(),
+                                                  *Section, Address, TotalSize);
+    addAdrpAddRelocAArch64(*Veneer, *TargetLowBits, *TargetHiBits,
+                           TargetAddress);
+    MIB->addAnnotation(Instruction, "AArch64Veneer", true);
+    Veneer->addInstruction(Offset, std::move(Instruction));
+    --Count;
+    for (auto It = std::prev(Instructions.end()); Count != 0;
+         It = std::prev(It), --Count) {
+      MIB->addAnnotation(It->second, "AArch64Veneer", true);
+      Veneer->addInstruction(It->first, std::move(It->second));
+    }
+
+    Veneer->getOrCreateLocalLabel(Address);
+    Veneer->setMaxSize(TotalSize);
+    Veneer->updateState(BinaryFunction::State::Disassembled);
+    LLVM_DEBUG(dbgs() << "BOLT-DEBUG: handling veneer function at 0x" << Address
+                      << "\n");
+    return true;
+  };
+
+  uint64_t Size = 0, TotalSize = 0;
+  BinaryFunction::InstrMapType VeneerInstructions;
+  for (Offset = 0; Offset < MaxSize; Offset += Size) {
+    MCInst Instruction;
+    const uint64_t AbsoluteInstrAddr = Address + Offset;
+    if (!SymbolicDisAsm->getInstruction(Instruction, Size, Data.slice(Offset),
+                                        AbsoluteInstrAddr, nulls()))
+      break;
+
+    TotalSize += Size;
+    if (MIB->isBranch(Instruction)) {
+      Ret = matchVeneer(VeneerInstructions, Instruction, Offset,
+                        AbsoluteInstrAddr, TotalSize);
+      break;
+    }
+
+    VeneerInstructions.emplace(Offset, std::move(Instruction));
+  }
+
+  return Ret;
+}
+
+void BinaryContext::processInterproceduralReferences() {
+  for (const std::pair<BinaryFunction *, uint64_t> &It :
+       InterproceduralReferences) {
+    BinaryFunction &Function = *It.first;
+    uint64_t Address = It.second;
     if (!Address)
       continue;
 
     BinaryFunction *TargetFunction =
         getBinaryFunctionContainingAddress(Address);
     if (&Function == TargetFunction)
       continue;
 
     if (TargetFunction) {
-      if (TargetFunction->IsFragment &&
+      if (TargetFunction->isFragment() &&
           !registerFragment(*TargetFunction, Function)) {
         errs() << "BOLT-WARNING: interprocedural reference between unrelated "
                   "fragments: "
                << Function.getPrintName() << " and "
                << TargetFunction->getPrintName() << '\n';
       }
       if (uint64_t Offset = Address - TargetFunction->getAddress())
         TargetFunction->addEntryPointAtOffset(Offset);
for (const std::pair<BinaryFunction *, uint64_t> &It :
     if (!Section->isText())
       continue;
 
     // PLT requires special handling and could be ignored in this context.
     StringRef SectionName = Section->getName();
     if (SectionName == ".plt" || SectionName == ".plt.got")
       continue;
 
+    // Check if it is aarch64 veneer written at Address
+    if (isAArch64() && handleAArch64Veneer(Address))
+      continue;
+
     if (opts::processAllFunctions()) {
       errs() << "BOLT-ERROR: cannot process binaries with unmarked "
              << "object in code at address 0x" << Twine::utohexstr(Address)
              << " belonging to section " << SectionName << " in current mode\n";
       exit(1);
     }
 
     TargetFunction = getBinaryFunctionContainingAddress(Address,
                                                         /*CheckPastEnd=*/false,
                                                         /*UseMaxSize=*/true);
     // We are not going to overwrite non-simple functions, but for simple
     // ones - adjust the padding size.
     if (TargetFunction && TargetFunction->isSimple()) {
       errs() << "BOLT-WARNING: function " << *TargetFunction
              << " has an object detected in a padding region at address 0x"
              << Twine::utohexstr(Address) << '\n';
       TargetFunction->setMaxSize(TargetFunction->getSize());
     }
   }
 
-  clearList(Function.InterproceduralReferences);
+  InterproceduralReferences.clear();
 }
 
 void BinaryContext::postProcessSymbolTable() {
   fixBinaryDataHoles();
   bool Valid = true;
   for (auto &Entry : BinaryDataMap) {
     BinaryData *BD = Entry.second;
     if ((BD->getName().startswith("SYMBOLat") ||

bolt/lib/Core/BinaryFunction.cpp

if (TargetOffset) {
           MCConstantExpr::create(TargetOffset, *BC.Ctx);
       Expr = MCBinaryExpr::createAdd(Expr, Offset, *BC.Ctx);
     }
     MIB->replaceMemOperandDisp(Instruction,
                                MCOperand::createExpr(BC.MIB->getTargetExprFor(
                                    Instruction, Expr, *BC.Ctx, 0)));
   };
 
-  // Used to fix the target of linker-generated AArch64 stubs with no relocation
-  // info
-  auto fixStubTarget = [&](MCInst &LoadLowBits, MCInst &LoadHiBits,
-                           uint64_t Target) {
-    const MCSymbol *TargetSymbol;
-    uint64_t Addend = 0;
-    std::tie(TargetSymbol, Addend) = BC.handleAddressRef(Target, *this, true);
-
-    int64_t Val;
-    MIB->replaceImmWithSymbolRef(LoadHiBits, TargetSymbol, Addend, Ctx.get(),
-                                 Val, ELF::R_AARCH64_ADR_PREL_PG_HI21);
-    MIB->replaceImmWithSymbolRef(LoadLowBits, TargetSymbol, Addend, Ctx.get(),
-                                 Val, ELF::R_AARCH64_ADD_ABS_LO12_NC);
-  };
-
   auto handleExternalReference = [&](MCInst &Instruction, uint64_t Size,
                                      uint64_t Offset, uint64_t TargetAddress,
                                      bool &IsCall) -> MCSymbol * {
     const uint64_t AbsoluteInstrAddr = getAddress() + Offset;
     MCSymbol *TargetSymbol = nullptr;
-    InterproceduralReferences.insert(TargetAddress);
+    BC.addInterproceduralReference(this, TargetAddress);
     if (opts::Verbosity >= 2 && !IsCall && Size == 2 && !BC.HasRelocations) {
       errs() << "BOLT-WARNING: relaxed tail call detected at 0x"
              << Twine::utohexstr(AbsoluteInstrAddr) << " in function " << *this
              << ". Code size will be increased.\n";
     }
 
     assert(!MIB->isTailCall(Instruction) &&
            "synthetic tail call instruction found");
case IndirectBranchType::POSSIBLE_FIXED_BRANCH: {
       if (containsAddress(IndirectTarget)) {
         const MCSymbol *TargetSymbol = getOrCreateLocalLabel(IndirectTarget);
         Instruction.clear();
         MIB->createUncondBranch(Instruction, TargetSymbol, BC.Ctx.get());
         TakenBranches.emplace_back(Offset, IndirectTarget - getAddress());
         HasFixedIndirectBranch = true;
       } else {
         MIB->convertJmpToTailCall(Instruction);
-        InterproceduralReferences.insert(IndirectTarget);
+        BC.addInterproceduralReference(this, IndirectTarget);
       }
       break;
     }
     case IndirectBranchType::UNKNOWN:
       // Keep processing. We'll do more checks and fixes in
       // postProcessIndirectBranches().
       UnknownIndirectBranchOffsets.emplace(Offset);
       break;
     }
   };
 
   // Check for linker veneers, which lack relocations and need manual
   // adjustments.
   auto handleAArch64IndirectCall = [&](MCInst &Instruction, uint64_t Offset) {
     const uint64_t AbsoluteInstrAddr = getAddress() + Offset;
     MCInst *TargetHiBits, *TargetLowBits;
-    uint64_t TargetAddress;
-    if (MIB->matchLinkerVeneer(Instructions.begin(), Instructions.end(),
+    uint64_t TargetAddress, Count;
+    Count = MIB->matchLinkerVeneer(Instructions.begin(), Instructions.end(),
                                    AbsoluteInstrAddr, Instruction, TargetHiBits,
-                               TargetLowBits, TargetAddress)) {
+                                   TargetLowBits, TargetAddress);
+    if (Count) {
       MIB->addAnnotation(Instruction, "AArch64Veneer", true);
-
-      uint8_t Counter = 0;
-      for (auto It = std::prev(Instructions.end()); Counter != 2;
-           --It, ++Counter) {
+      --Count;
+      for (auto It = std::prev(Instructions.end()); Count != 0;
+           It = std::prev(It), --Count) {
         MIB->addAnnotation(It->second, "AArch64Veneer", true);
       }
 
-      fixStubTarget(*TargetLowBits, *TargetHiBits, TargetAddress);
+      BC.addAdrpAddRelocAArch64(*this, *TargetLowBits, *TargetHiBits,
+                                TargetAddress);
     }
   };
 
   uint64_t Size = 0; // instruction size
   for (uint64_t Offset = 0; Offset < getSize(); Offset += Size) {
     MCInst Instruction;
     const uint64_t AbsoluteInstrAddr = getAddress() + Offset;
 
if (MIB->isBranch(Instruction) || MIB->isCall(Instruction)) {
         }
 
         if (!TargetSymbol) {
           // Create either local label or external symbol.
           if (containsAddress(TargetAddress)) {
             TargetSymbol = getOrCreateLocalLabel(TargetAddress);
           } else {
             if (TargetAddress == getAddress() + getSize() &&
-                TargetAddress < getAddress() + getMaxSize()) {
+                TargetAddress < getAddress() + getMaxSize() &&
+                !(BC.isAArch64() &&
+                  BC.handleAArch64Veneer(TargetAddress, /*MatchOnly*/ true))) {
               // Result of __builtin_unreachable().
               LLVM_DEBUG(dbgs() << "BOLT-DEBUG: jump past end detected at 0x"
                                 << Twine::utohexstr(AbsoluteInstrAddr)
                                 << " in function " << *this
                                 << " : replacing with nop.\n");
               BC.MIB->createNoop(Instruction);
               if (IsCondBranch) {
                 // Register branch offset for profile validation.
                 IgnoredBranches.emplace_back(Offset, Offset + Size);
               }
               goto add_instruction;
             }
             // May update Instruction and IsCall
             TargetSymbol = handleExternalReference(Instruction, Size, Offset,
                                                    TargetAddress, IsCall);
           }
         }
 
         if (!IsCall) {
           // Add taken branch info.
uint64_t BinaryFunction::getEditDistance() const {
   return ComputeEditDistance<BinaryBasicBlock *>(BasicBlocksPreviousLayout,
                                                  BasicBlocksLayout);
 }
 
 void BinaryFunction::clearDisasmState() {
   clearList(Instructions);
   clearList(IgnoredBranches);
   clearList(TakenBranches);
-  clearList(InterproceduralReferences);
 
   if (BC.HasRelocations) {
     for (std::pair<const uint32_t, MCSymbol *> &LI : Labels)
       BC.UndefinedSymbols.insert(LI.second);
     if (FunctionEndLabel)
       BC.UndefinedSymbols.insert(FunctionEndLabel);
   }
 }
void BinaryFunction::printLoopInfo(raw_ostream &OS) const {
   }
 
   OS << "Total number of loops: " << BLI->TotalLoops << "\n";
   OS << "Number of outer loops: " << BLI->OuterLoops << "\n";
   OS << "Maximum nested loop depth: " << BLI->MaximumDepth << "\n\n";
 }
 
 bool BinaryFunction::isAArch64Veneer() const {
-  if (BasicBlocks.size() != 1)
+  if (empty())
     return false;
 
   BinaryBasicBlock &BB = **BasicBlocks.begin();
-  if (BB.size() != 3)
-    return false;
-
   for (MCInst &Inst : BB)
     if (!BC.MIB->hasAnnotation(Inst, "AArch64Veneer"))
       return false;
 
+  for (auto I = BasicBlocks.begin() + 1, E = BasicBlocks.end(); I != E; ++I) {
+    for (MCInst &Inst : **I)
+      if (!BC.MIB->isNoop(Inst))
+        return false;
+  }
+
   return true;
 }
 
 } // namespace bolt
 } // namespace llvm

bolt/lib/Passes/BinaryPasses.cpp

 void InstructionLowering::runOnFunctions(BinaryContext &BC) {
   for (auto &BFI : BC.getBinaryFunctions())
     for (BinaryBasicBlock &BB : BFI.second)
       for (MCInst &Instruction : BB)
         BC.MIB->lowerTailCall(Instruction);
 }
 
 void StripRepRet::runOnFunctions(BinaryContext &BC) {
+  if (!BC.isX86())
+    return;
+
   uint64_t NumPrefixesRemoved = 0;
   uint64_t NumBytesSaved = 0;
   for (auto &BFI : BC.getBinaryFunctions()) {
     for (BinaryBasicBlock &BB : BFI.second) {
       auto LastInstRIter = BB.getLastNonPseudo();
       if (LastInstRIter == BB.rend() || !BC.MIB->isReturn(*LastInstRIter) ||
           !BC.MIB->deleteREPPrefix(*LastInstRIter))
         continue;

bolt/lib/Passes/VeneerElimination.cpp

if (CurrentIt->second.isAArch64Veneer()) {
         BC.SymbolToFunctionMap.erase(Symbol);
       }
 
       BC.BinaryDataMap.erase(VeneerFunction.getAddress());
       BFs.erase(CurrentIt);
     }
   }
 
-  LLVM_DEBUG(dbgs() << "BOLT-INFO: number of removed linker-inserted veneers :"
-                    << VeneersCount << "\n");
+  outs() << "BOLT-INFO: number of removed linker-inserted veneers :"
+         << VeneersCount << "\n";
 
   // Handle veneers to veneers in case they occur
   for (auto entry : VeneerDestinations) {
     const MCSymbol *src = entry.first;
     const MCSymbol *dest = entry.second;
     while (VeneerDestinations.find(dest) != VeneerDestinations.end()) {
       dest = VeneerDestinations[dest];
     }

bolt/lib/Rewrite/BinaryPassManager.cpp

 void BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   BinaryFunctionPassManager Manager(BC);
 
   const DynoStats InitialDynoStats = getDynoStats(BC.getBinaryFunctions());
 
   Manager.registerPass(std::make_unique<AsmDumpPass>(),
                        opts::AsmDump.getNumOccurrences());
 
+  if (BC.isAArch64())
+    Manager.registerPass(
+        std::make_unique<VeneerElimination>(PrintVeneerElimination));
+
   if (opts::Instrument)
     Manager.registerPass(std::make_unique<Instrumentation>(NeverPrint));
 
   // Here we manage dependencies/order manually, since passes are run in the
   // order they're registered.
 
   // Run this pass first to use stats for the original functions.
   Manager.registerPass(std::make_unique<PrintProgramStats>(NeverPrint));
void BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   Manager.registerPass(std::make_unique<NormalizeCFG>(PrintNormalized));
 
   Manager.registerPass(std::make_unique<StripRepRet>(NeverPrint),
                        opts::StripRepRet);
 
   Manager.registerPass(std::make_unique<IdenticalCodeFolding>(PrintICF),
                        opts::ICF);
 
-  if (BC.isAArch64())
-    Manager.registerPass(
-        std::make_unique<VeneerElimination>(PrintVeneerElimination));
-
   Manager.registerPass(
       std::make_unique<SpecializeMemcpy1>(NeverPrint, opts::SpecializeMemcpy1),
       !opts::SpecializeMemcpy1.empty());
 
   Manager.registerPass(std::make_unique<InlineMemcpy>(NeverPrint),
                        opts::StringOps);
 
   Manager.registerPass(std::make_unique<IndirectCallPromotion>(PrintICP));

bolt/lib/Rewrite/RewriteInstance.cpp

if (!Function.disassemble()) {
         BC->exitWithBugReport("function cannot be properly disassembled. "
                               "Unable to continue in relocation mode.",
                               Function);
       if (opts::Verbosity >= 1)
         outs() << "BOLT-INFO: could not disassemble function " << Function
                << ". Will ignore.\n";
       // Forcefully ignore the function.
       Function.setIgnored();
       continue;
     }
 
     if (opts::PrintAll || opts::PrintDisasm)
       Function.print(outs(), "after disassembly", true);
-
-    BC->processInterproceduralReferences(Function);
   }
 
+  BC->processInterproceduralReferences();
   BC->clearJumpTableOffsets();
   BC->populateJumpTables();
   BC->skipMarkedFragments();
 
   for (auto &BFI : BC->getBinaryFunctions()) {
     BinaryFunction &Function = BFI.second;
 
     if (!shouldDisassemble(Function))

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

public:
   }
 
   /// Matching pattern here is
   ///
   ///    ADRP  x16, imm
   ///    ADD   x16, x16, imm
   ///    BR    x16
   ///
-  bool matchLinkerVeneer(InstructionIterator Begin, InstructionIterator End,
+  uint64_t matchLinkerVeneer(InstructionIterator Begin, InstructionIterator End,
                              uint64_t Address, const MCInst &CurInst,
                              MCInst *&TargetHiBits, MCInst *&TargetLowBits,
                              uint64_t &Target) const override {
     if (CurInst.getOpcode() != AArch64::BR || !CurInst.getOperand(0).isReg() ||
         CurInst.getOperand(0).getReg() != AArch64::X16)
-      return false;
+      return 0;
 
     auto I = End;
     if (I == Begin)
-      return false;
+      return 0;
 
     --I;
     Address -= 4;
     if (I == Begin || I->getOpcode() != AArch64::ADDXri ||
         MCPlus::getNumPrimeOperands(*I) < 3 || !I->getOperand(0).isReg() ||
         !I->getOperand(1).isReg() ||
         I->getOperand(0).getReg() != AArch64::X16 ||
         I->getOperand(1).getReg() != AArch64::X16 || !I->getOperand(2).isImm())
-      return false;
+      return 0;
     TargetLowBits = &*I;
     uint64_t Addr = I->getOperand(2).getImm() & 0xFFF;
 
     --I;
     Address -= 4;
     if (I->getOpcode() != AArch64::ADRP ||
         MCPlus::getNumPrimeOperands(*I) < 2 || !I->getOperand(0).isReg() ||
         !I->getOperand(1).isImm() || I->getOperand(0).getReg() != AArch64::X16)
-      return false;
+      return 0;
     TargetHiBits = &*I;
     Addr |= (Address + ((int64_t)I->getOperand(1).getImm() << 12)) &
             0xFFFFFFFFFFFFF000ULL;
     Target = Addr;
-    return true;
+    return 3;
   }
 
   bool replaceImmWithSymbolRef(MCInst &Inst, const MCSymbol *Symbol,
                                int64_t Addend, MCContext *Ctx, int64_t &Value,
                                uint64_t RelType) const override {
     unsigned ImmOpNo = -1U;
     for (unsigned Index = 0; Index < MCPlus::getNumPrimeOperands(Inst);
          ++Index) {

   bool isMoveMem2Reg(const MCInst &Inst) const override { return false; }
 
   bool isLeave(const MCInst &Inst) const override { return false; }
 
   bool isPop(const MCInst &Inst) const override { return false; }
 
   bool isPrefix(const MCInst &Inst) const override { return false; }
 
-  bool deleteREPPrefix(MCInst &Inst) const override { return false; }
-
   bool createReturn(MCInst &Inst) const override {
     Inst.setOpcode(AArch64::RET);
     Inst.clear();
     Inst.addOperand(MCOperand::createReg(AArch64::LR));
     return true;
   }
 
   InstructionListType materializeAddress(const MCSymbol *Target, MCContext *Ctx,

bolt/test/AArch64/veneer-gold.s

+// This test checks that the gold linker style veneer are properly handled
+// by BOLT.
+// Strip .rela.mytext section to simulate inserted by a linker veneers
+// that does not contain relocations.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
+# RUN:   %s -o %t.o
+# RUN: %clang %cflags -fPIC -pie %t.o -o %t.exe \
+# RUN:    -nostartfiles -nodefaultlibs -Wl,-z,notext \
+# RUN:    -fuse-ld=lld -Wl,--no-relax -Wl,-q
+# RUN: llvm-objcopy --remove-section .rela.mytext %t.exe
+# RUN: llvm-objdump -d -j .mytext %t.exe | \
+# RUN:   FileCheck --check-prefix=CHECKVENEER %s
+# RUN: llvm-bolt %t.exe -o %t.bolt --elim-link-veneers=true \
+# RUN:   --lite=0 --use-old-text=0
+# RUN: llvm-objdump -d -j .text %t.bolt | FileCheck %s
+
+.text
+.balign 4
+.global dummy
+.type dummy, %function
+dummy:
+  adrp x1, dummy
+  ret
+.size dummy, .-dummy
+
+.section ".mytext", "ax"
+.balign 4
+.global foo
+.type foo, %function
+foo:
+# CHECK: <foo>:
+# CHECK-NEXT : {{.*}} bl {{.*}} <foo2>
+  bl .L2
+  ret
+.size foo, .-foo
+
+.global foo2
+.type foo2, %function
+foo2:
+# CHECK: <foo2>:
+# CHECK-NEXT : {{.*}} bl {{.*}} <foo2>
+  bl .L2
+  ret
+.size foo2, .-foo2
+
+.global _start
+.type _start, %function
+_start:
+# CHECK: <_start>:
+# CHECK-NEXT: {{.*}} bl {{.*}} <foo>
+  bl .L1
+  adr x0, .L0
+  ret
+.L0:
+  .xword 0
+.size _start, .-_start
+.L1:
+# CHECKVENEER: adrp
+# CHECKVENEER-NEXT: add
+# CHECKVENEER-NEXT: br
+# CHECKVENEER-NEXT: nop
+  adrp x16, foo
+  add x16, x16, #:lo12:foo
+  br x16
+  nop
+.L2:
+# CHECKVENEER-NEXT: adrp
+# CHECKVENEER-NEXT: add
+# CHECKVENEER-NEXT: br
+# CHECKVENEER-NEXT: nop
+  adrp x16, foo2
+  add x16, x16, #:lo12:foo2
+  br x16
+  nop

bolt/test/AArch64/veneer.s

+// This test checks that the veneer are properly handled by BOLT.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
+# RUN:   %s -o %t.o
+# RUN: %clang %cflags -fPIC -pie %t.o -o %t.exe \
+# RUN:    -nostartfiles -nodefaultlibs -Wl,-z,notext \
+# RUN:    -fuse-ld=lld -Wl,--no-relax
+# RUN: llvm-objdump -d --disassemble-symbols='myveneer' %t.exe | \
+# RUN:   FileCheck --check-prefix=CHECKVENEER %s
+# RUN: llvm-bolt %t.exe -o %t.bolt --elim-link-veneers=true --lite=0
+# RUN: llvm-objdump -d --disassemble-symbols='_start' %t.bolt | FileCheck %s
+
+.text
+.balign 4
+.global foo
+.type foo, %function
+foo:
+  ret
+.size foo, .-foo
+
+.global myveneer
+.type myveneer, %function
+myveneer:
+# CHECKVENEER: adrp
+# CHECKVENEER-NEXT: add
+  adrp x16, foo
+  add x16, x16, #:lo12:foo
+  br x16
+  nop
+.size myveneer, .-myveneer
+
+.global _start
+.type _start, %function
+_start:
+# CHECK: {{.*}} bl {{.*}} <foo>
+  bl myveneer
+  ret
+.size _start, .-_start