[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]

@rafauler I've add isIgnored check, could you please verify that everything is OK now?
P.S. How do you think should we add a test case from your binary for future (out of scope of this patch). Thank you!

[rafauler]

Looks good, let me kick off tests

[rafauler]

There's a memory leak the two tests:

• TEST 'BOLT :: AArch64/veneer-gold.s' FAILED *******
• TEST 'BOLT :: AArch64/veneer.s' FAILED ****

This is detected by just running our regular test suite, but with bolt built with address sanitizer from llvm. If you are using cmake, just add -DLLVM_USE_SANITIZER=Address if you are building bolt with clang.

Here's the leak stack trace:

74287==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 176 byte(s) in 2 object(s) allocated from:

#0 in operator new(unsigned long) (build/bin/llvm-bolt+0x769ce7)
#1 in llvm::bolt::BinaryContext::registerNameAtAddress(llvm::StringRef, unsigned long, unsigned long, unsigned short, unsigned int) /bolt/lib/Core/BinaryContext.cpp:985:10
#2 in llvm::bolt::BinaryContext::getOrCreateGlobalSymbol(unsigned long, llvm::Twine, unsigned long, unsigned short, unsigned int) /bolt/lib/Core/BinaryContext.cpp:743:10
#3 in operator() /bolt/lib/Core/BinaryFunction.cpp:1095:23
#4 in llvm::bolt::BinaryFunction::disassemble() /bolt/lib/Core/BinaryFunction.cpp:1300:28
#5 in llvm::bolt::RewriteInstance::disassembleFunctions() /bolt/lib/Rewrite/RewriteInstance.cpp:2836:19
#6 in llvm::bolt::RewriteInstance::run() /bolt/lib/Rewrite/RewriteInstance.cpp:747:3
#7 in main /bolt/tools/driver/llvm-bolt.cpp:244:24
#8 in __libc_start_main 
#9 in _start

Indirect leak of 16 byte(s) in 2 object(s) allocated from:

#0 in operator new(unsigned long) 
#1 in allocate 
#2 in allocate
#3 in _M_allocate
#4 in _M_realloc_insert<llvm::MCSymbol *> 
#5 in emplace_back<llvm::MCSymbol *>
#6 in push_back /
#7 in llvm::bolt::BinaryData::BinaryData(llvm::MCSymbol&, unsigned long, unsigned long, unsigned short, llvm::bolt::BinarySection&, unsigned int) /bolt/lib/Core/BinaryData.cpp:136:11
#8 in llvm::bolt::BinaryContext::registerNameAtAddress(llvm::StringRef, unsigned long, unsigned long, unsigned short, unsigned int) /bolt/lib/Core/BinaryContext.cpp:985:14
#9 in llvm::bolt::BinaryContext::getOrCreateGlobalSymbol(unsigned long, llvm::Twine, unsigned long, unsigned short, unsigned int) /bolt/lib/Core/BinaryContext.cpp:743:10
#10 in operator() /bolt/lib/Core/BinaryFunction.cpp:1095:23
#11 in llvm::bolt::BinaryFunction::disassemble() /bolt/lib/Core/BinaryFunction.cpp:1300:28
#12 in llvm::bolt::RewriteInstance::disassembleFunctions() /bolt/lib/Rewrite/RewriteInstance.cpp:2836:19
#13 in llvm::bolt::RewriteInstance::run() /bolt/lib/Rewrite/RewriteInstance.cpp:747:3
#14 in main /bolt/tools/driver/llvm-bolt.cpp:244:24
#15 in __libc_start_main 
#16 in _start

[rafauler]

bolt/include/bolt/Core/BinaryContext.h

647

nit: I suspect btw that a regular "std::vector" makes more sense here (since you're not doing any operation that would be faster with a linked list)

[yota9]

@rafauler I'll try to check the cause of memory leak, probably it is connected to the VeneerElimination pass, since it is unusual case to create the new functions after disassemble but also remove them during passes, probably something is not handled properly..

bolt/include/bolt/Core/BinaryContext.h

647

On the previous review I wrote a comment about the reasons I've choosed to use list here, newely-handled veneers will create new external references, that must be added to the end of the list while iterating it. Also I don't think we want to reallocate and copy the old vector each time, so I'd prefer to stay with the list here to be honest

[yota9]

@rafauler Thanks for sanitizer logs! Although I didn't try it my self, but I've checked them and as I said it was a problem in veneer elimination pass, which was not delete the allocated binarydata struct previously. Now everything should be fine :)

[yota9]

bolt/lib/Passes/VeneerElimination.cpp

39

Maybe instead of erasing this function and all its symbols we can just set isPseudo flag for BF? @rafauler what do you think about it?

[yota9]

@rafauler I've decided to go with isPseudo() for veneers, I don't think that the pass is an appropriate place to remove functions/symbols. + A bit of code refactoring in pass :)

[yota9]

Use veneers symbols directly in pass.

[rafauler]

I agree, erasing functions in that pass seems hacky. Thanks.

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) {
-    if (!Address)
+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 || Function.isIgnored())
       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.
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

 
 namespace llvm {
 namespace bolt {
 
 void VeneerElimination::runOnFunctions(BinaryContext &BC) {
   if (!opts::EliminateVeneers || !BC.isAArch64())
     return;
 
-  auto &BFs = BC.getBinaryFunctions();
+  std::map<uint64_t, BinaryFunction> &BFs = BC.getBinaryFunctions();
   std::unordered_map<const MCSymbol *, const MCSymbol *> VeneerDestinations;
   uint64_t VeneersCount = 0;
-  for (auto It = BFs.begin(); It != BFs.end();) {
-    auto CurrentIt = It;
-    ++It;
+  for (auto &It : BFs) {
+    BinaryFunction &VeneerFunction = It.second;
+    if (!VeneerFunction.isAArch64Veneer())
+      continue;
 
-    if (CurrentIt->second.isAArch64Veneer()) {
-      VeneersCount++;
-      BinaryFunction &VeneerFunction = CurrentIt->second;
-
-      MCInst &FirstInstruction = *(VeneerFunction.begin()->begin());
-      const MCSymbol *VeneerTargetSymbol =
-          BC.MIB->getTargetSymbol(FirstInstruction, 1);
-
-      // Functions can have multiple symbols
-      for (StringRef Name : VeneerFunction.getNames()) {
-        MCSymbol *Symbol = BC.Ctx->lookupSymbol(Name);
-        VeneerDestinations[Symbol] = VeneerTargetSymbol;
-        BC.SymbolToFunctionMap.erase(Symbol);
-      }
-
-      BC.BinaryDataMap.erase(VeneerFunction.getAddress());
-      BFs.erase(CurrentIt);
-    }
+    VeneerFunction.setPseudo(true);
+    assert(VeneerTargetSymbol && "Expecting target symbol for instruction");
+    for (const MCSymbol *Symbol : VeneerFunction.getSymbols())
   }
 
-  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];
-    }
-    VeneerDestinations[src] = dest;
+  for (auto &Entry : VeneerDestinations) {
+    const MCSymbol *Src = Entry.first;
+    const MCSymbol *Dest = Entry.second;
+    while (VeneerDestinations.find(Dest) != VeneerDestinations.end())
+      Dest = VeneerDestinations[Dest];
+
+    VeneerDestinations[Src] = Dest;
   }
 
   uint64_t VeneerCallers = 0;
   for (auto &It : BFs) {
     BinaryFunction &Function = It.second;
     for (BinaryBasicBlock &BB : Function) {
       for (MCInst &Instr : BB) {
         if (!BC.MIB->isCall(Instr) || BC.MIB->isIndirectCall(Instr))
           continue;
 
         const MCSymbol *TargetSymbol = BC.MIB->getTargetSymbol(Instr, 0);
         if (VeneerDestinations.find(TargetSymbol) == VeneerDestinations.end())
           continue;
 
         VeneerCallers++;
         if (!BC.MIB->replaceBranchTarget(
-                Instr, VeneerDestinations[TargetSymbol], BC.Ctx.get()))
-          assert(false && "updating veneer call destination failed");
+                Instr, VeneerDestinations[TargetSymbol], BC.Ctx.get())) {
+          errs() << "BOLT-ERROR: updating veneer call destination failed\n";
+          exit(1);
+        }
       }
     }
   }
 
   LLVM_DEBUG(
-      dbgs() << "BOLT-INFO: number of linker-inserted veneers call sites :"
+      dbgs() << "BOLT-INFO: number of linker-inserted veneers call sites: "
              << VeneerCallers << "\n");
 }
 
 } // namespace bolt
 } // namespace llvm

bolt/lib/Rewrite/BinaryPassManager.cpp

void BinaryFunctionPassManager::runAllPasses(BinaryContext &BC) {
   BinaryFunctionPassManager Manager(BC);
 
   const DynoStats InitialDynoStats =
       getDynoStats(BC.getBinaryFunctions(), BC.isAArch64());
 
   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()) {
                << ". 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 -nostdlib \
+# 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.
+// 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 -nostdlib \
+# RUN:    -fuse-ld=lld -Wl,--no-relax -Wl,-q
+# RUN: llvm-objdump -d --disassemble-symbols='myveneer' %t.exe | \
+# RUN:   FileCheck --check-prefix=CHECKVENEER %s
+# RUN: llvm-objcopy --remove-section .rela.mytext %t.exe
+# RUN: llvm-bolt %t.exe -o %t.bolt --elim-link-veneers=true --lite=0
+# RUN: llvm-objdump -d -j .text --disassemble-symbols='myveneer' %t.bolt | \
+# RUN:   FileCheck --check-prefix=CHECKOUTVENEER %s
+# RUN: llvm-objdump -d --disassemble-symbols='_start' %t.bolt | FileCheck %s
+
+.text
+.balign 4
+.global foo
+.type foo, %function
+foo:
+  adrp x1, foo
+  ret
+.size foo, .-foo
+
+.section ".mytext", "ax"
+.balign 4
+# CHECKOUTVENEER-NOT: {{.*}} <myveneer>:
+.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