[AutoBump] Merge with cc04bbb (Jun 11)

Author: mgehre-amd

bolt/include/bolt/Core/MCPlusBuilder.h

@@ -1412,13 +1412,14 @@ class MCPlusBuilder {
     return false;
   }
 
-  /// Modify a direct call instruction \p Inst with an indirect call taking
-  /// a destination from a memory location pointed by \p TargetLocation symbol.
-  virtual bool convertCallToIndirectCall(MCInst &Inst,
-                                         const MCSymbol *TargetLocation,
-                                         MCContext *Ctx) {
+  /// Creates an indirect call to the function within the \p DirectCall PLT
+  /// stub. The function's memory location is pointed by the \p TargetLocation
+  /// symbol.
+  virtual InstructionListType
+  createIndirectPltCall(const MCInst &DirectCall,
+                        const MCSymbol *TargetLocation, MCContext *Ctx) {
     llvm_unreachable("not implemented");
-    return false;
+    return {};
   }
 
   /// Morph an indirect call into a load where \p Reg holds the call target.

bolt/include/bolt/Rewrite/DWARFRewriter.h

@@ -150,9 +150,6 @@ class DWARFRewriter {
   /// blocks) to be updated.
   void updateDebugAddressRanges();
 
-  /// Rewrite .gdb_index section if present.
-  void updateGdbIndexSection(CUOffsetMap &CUMap, uint32_t NumCUs);
-
   /// DWARFDie contains a pointer to a DIE and hence gets invalidated once the
   /// embedded DIE is destroyed. This wrapper class stores a DIE internally and
   /// could be cast to a DWARFDie that is valid even after the initial DIE is
@@ -194,14 +191,6 @@ class DWARFRewriter {
     DwoRangesBase[DWOId] = RangesBase;
   }
 
-  /// Adds an GDBIndexTUEntry if .gdb_index seciton exists.
-  void addGDBTypeUnitEntry(const GDBIndexTUEntry &&Entry);
-
-  /// Returns all entries needed for Types CU list
-  const GDBIndexTUEntryType &getGDBIndexTUEntryVector() const {
-    return GDBIndexTUEntryVector;
-  }
-
   using OverriddenSectionsMap = std::unordered_map<DWARFSectionKind, StringRef>;
   /// Output .dwo files.
   void writeDWOFiles(DWARFUnit &, const OverriddenSectionsMap &,

bolt/lib/Passes/PLTCall.cpp

@@ -48,8 +48,8 @@ Error PLTCall::runOnFunctions(BinaryContext &BC) {
     return Error::success();
 
   uint64_t NumCallsOptimized = 0;
-  for (auto &It : BC.getBinaryFunctions()) {
-    BinaryFunction &Function = It.second;
+  for (auto &BFI : BC.getBinaryFunctions()) {
+    BinaryFunction &Function = BFI.second;
     if (!shouldOptimize(Function))
       continue;
 
@@ -61,18 +61,21 @@ Error PLTCall::runOnFunctions(BinaryContext &BC) {
       if (opts::PLT == OT_HOT && !BB.getKnownExecutionCount())
         continue;
 
-      for (MCInst &Instr : BB) {
-        if (!BC.MIB->isCall(Instr))
+      for (auto II = BB.begin(); II != BB.end(); II++) {
+        if (!BC.MIB->isCall(*II))
           continue;
-        const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(Instr);
+        const MCSymbol *CallSymbol = BC.MIB->getTargetSymbol(*II);
         if (!CallSymbol)
           continue;
         const BinaryFunction *CalleeBF = BC.getFunctionForSymbol(CallSymbol);
         if (!CalleeBF || !CalleeBF->isPLTFunction())
           continue;
-        BC.MIB->convertCallToIndirectCall(Instr, CalleeBF->getPLTSymbol(),
-                                          BC.Ctx.get());
-        BC.MIB->addAnnotation(Instr, "PLTCall", true);
+        const InstructionListType NewCode = BC.MIB->createIndirectPltCall(
+            *II, CalleeBF->getPLTSymbol(), BC.Ctx.get());
+        II = BB.replaceInstruction(II, NewCode);
+        assert(!NewCode.empty() && "PLT Call replacement must be non-empty");
+        std::advance(II, NewCode.size() - 1);
+        BC.MIB->addAnnotation(*II, "PLTCall", true);
         ++NumCallsOptimized;
       }
     }

bolt/lib/Rewrite/DWARFRewriter.cpp

@@ -2060,177 +2060,6 @@ void DWARFRewriter::writeDWOFiles(
   TempOut->keep();
 }
 
-void DWARFRewriter::addGDBTypeUnitEntry(const GDBIndexTUEntry &&Entry) {
-  std::lock_guard<std::mutex> Lock(DWARFRewriterMutex);
-  if (!BC.getGdbIndexSection())
-    return;
-  GDBIndexTUEntryVector.emplace_back(Entry);
-}
-
-void DWARFRewriter::updateGdbIndexSection(CUOffsetMap &CUMap, uint32_t NumCUs) {
-  if (!BC.getGdbIndexSection())
-    return;
-
-  // See https://sourceware.org/gdb/onlinedocs/gdb/Index-Section-Format.html
-  // for .gdb_index section format.
-
-  StringRef GdbIndexContents = BC.getGdbIndexSection()->getContents();
-
-  const char *Data = GdbIndexContents.data();
-
-  // Parse the header.
-  const uint32_t Version = read32le(Data);
-  if (Version != 7 && Version != 8) {
-    errs() << "BOLT-ERROR: can only process .gdb_index versions 7 and 8\n";
-    exit(1);
-  }
-
-  // Some .gdb_index generators use file offsets while others use section
-  // offsets. Hence we can only rely on offsets relative to each other,
-  // and ignore their absolute values.
-  const uint32_t CUListOffset = read32le(Data + 4);
-  const uint32_t CUTypesOffset = read32le(Data + 8);
-  const uint32_t AddressTableOffset = read32le(Data + 12);
-  const uint32_t SymbolTableOffset = read32le(Data + 16);
-  const uint32_t ConstantPoolOffset = read32le(Data + 20);
-  Data += 24;
-
-  // Map CUs offsets to indices and verify existing index table.
-  std::map<uint32_t, uint32_t> OffsetToIndexMap;
-  const uint32_t CUListSize = CUTypesOffset - CUListOffset;
-  const uint32_t TUListSize = AddressTableOffset - CUTypesOffset;
-  const unsigned NUmCUsEncoded = CUListSize / 16;
-  unsigned MaxDWARFVersion = BC.DwCtx->getMaxVersion();
-  unsigned NumDWARF5TUs =
-      getGDBIndexTUEntryVector().size() - BC.DwCtx->getNumTypeUnits();
-  bool SkipTypeUnits = false;
-  // For DWARF5 Types are in .debug_info.
-  // LLD doesn't generate Types CU List, and in CU list offset
-  // only includes CUs.
-  // GDB 11+ includes only CUs in CU list and generates Types
-  // list.
-  // GDB 9 includes CUs and TUs in CU list and generates TYpes
-  // list. The NumCUs is CUs + TUs, so need to modify the check.
-  // For split-dwarf
-  // GDB-11, DWARF5: TU units from dwo are not included.
-  // GDB-11, DWARF4: TU units from dwo are included.
-  if (MaxDWARFVersion >= 5)
-    SkipTypeUnits = !TUListSize ? true
-                                : ((NUmCUsEncoded + NumDWARF5TUs) ==
-                                   BC.DwCtx->getNumCompileUnits());
-
-  if (!((CUListSize == NumCUs * 16) ||
-        (CUListSize == (NumCUs + NumDWARF5TUs) * 16))) {
-    errs() << "BOLT-ERROR: .gdb_index: CU count mismatch\n";
-    exit(1);
-  }
-  DenseSet<uint64_t> OriginalOffsets;
-  for (unsigned Index = 0, Units = BC.DwCtx->getNumCompileUnits();
-       Index < Units; ++Index) {
-    const DWARFUnit *CU = BC.DwCtx->getUnitAtIndex(Index);
-    if (SkipTypeUnits && CU->isTypeUnit())
-      continue;
-    const uint64_t Offset = read64le(Data);
-    Data += 16;
-    if (CU->getOffset() != Offset) {
-      errs() << "BOLT-ERROR: .gdb_index CU offset mismatch\n";
-      exit(1);
-    }
-
-    OriginalOffsets.insert(Offset);
-    OffsetToIndexMap[Offset] = Index;
-  }
-
-  // Ignore old address table.
-  const uint32_t OldAddressTableSize = SymbolTableOffset - AddressTableOffset;
-  // Move Data to the beginning of symbol table.
-  Data += SymbolTableOffset - CUTypesOffset;
-
-  // Calculate the size of the new address table.
-  uint32_t NewAddressTableSize = 0;
-  for (const auto &CURangesPair : ARangesSectionWriter->getCUAddressRanges()) {
-    const SmallVector<DebugAddressRange, 2> &Ranges = CURangesPair.second;
-    NewAddressTableSize += Ranges.size() * 20;
-  }
-
-  // Difference between old and new table (and section) sizes.
-  // Could be negative.
-  int32_t Delta = NewAddressTableSize - OldAddressTableSize;
-
-  size_t NewGdbIndexSize = GdbIndexContents.size() + Delta;
-
-  // Free'd by ExecutableFileMemoryManager.
-  auto *NewGdbIndexContents = new uint8_t[NewGdbIndexSize];
-  uint8_t *Buffer = NewGdbIndexContents;
-
-  write32le(Buffer, Version);
-  write32le(Buffer + 4, CUListOffset);
-  write32le(Buffer + 8, CUTypesOffset);
-  write32le(Buffer + 12, AddressTableOffset);
-  write32le(Buffer + 16, SymbolTableOffset + Delta);
-  write32le(Buffer + 20, ConstantPoolOffset + Delta);
-  Buffer += 24;
-
-  using MapEntry = std::pair<uint32_t, CUInfo>;
-  std::vector<MapEntry> CUVector(CUMap.begin(), CUMap.end());
-  // Need to sort since we write out all of TUs in .debug_info before CUs.
-  std::sort(CUVector.begin(), CUVector.end(),
-            [](const MapEntry &E1, const MapEntry &E2) -> bool {
-              return E1.second.Offset < E2.second.Offset;
-            });
-  // Writing out CU List <Offset, Size>
-  for (auto &CUInfo : CUVector) {
-    // Skipping TU for DWARF5 when they are not included in CU list.
-    if (!OriginalOffsets.count(CUInfo.first))
-      continue;
-    write64le(Buffer, CUInfo.second.Offset);
-    // Length encoded in CU doesn't contain first 4 bytes that encode length.
-    write64le(Buffer + 8, CUInfo.second.Length + 4);
-    Buffer += 16;
-  }
-
-  // Rewrite TU CU List, since abbrevs can be different.
-  // Entry example:
-  // 0: offset = 0x00000000, type_offset = 0x0000001e, type_signature =
-  // 0x418503b8111e9a7b Spec says " triplet, the first value is the CU offset,
-  // the second value is the type offset in the CU, and the third value is the
-  // type signature" Looking at what is being generated by gdb-add-index. The
-  // first entry is TU offset, second entry is offset from it, and third entry
-  // is the type signature.
-  if (TUListSize)
-    for (const GDBIndexTUEntry &Entry : getGDBIndexTUEntryVector()) {
-      write64le(Buffer, Entry.UnitOffset);
-      write64le(Buffer + 8, Entry.TypeDIERelativeOffset);
-      write64le(Buffer + 16, Entry.TypeHash);
-      Buffer += sizeof(GDBIndexTUEntry);
-    }
-
-  // Generate new address table.
-  for (const std::pair<const uint64_t, DebugAddressRangesVector> &CURangesPair :
-       ARangesSectionWriter->getCUAddressRanges()) {
-    const uint32_t CUIndex = OffsetToIndexMap[CURangesPair.first];
-    const DebugAddressRangesVector &Ranges = CURangesPair.second;
-    for (const DebugAddressRange &Range : Ranges) {
-      write64le(Buffer, Range.LowPC);
-      write64le(Buffer + 8, Range.HighPC);
-      write32le(Buffer + 16, CUIndex);
-      Buffer += 20;
-    }
-  }
-
-  const size_t TrailingSize =
-      GdbIndexContents.data() + GdbIndexContents.size() - Data;
-  assert(Buffer + TrailingSize == NewGdbIndexContents + NewGdbIndexSize &&
-         "size calculation error");
-
-  // Copy over the rest of the original data.
-  memcpy(Buffer, Data, TrailingSize);
-
-  // Register the new section.
-  BC.registerOrUpdateNoteSection(".gdb_index", NewGdbIndexContents,
-                                 NewGdbIndexSize);
-}
-
 std::unique_ptr<DebugBufferVector>
 DWARFRewriter::makeFinalLocListsSection(DWARFVersion Version) {
   auto LocBuffer = std::make_unique<DebugBufferVector>();

bolt/lib/Rewrite/LinuxKernelRewriter.cpp

@@ -273,6 +273,8 @@ class LinuxKernelRewriter final : public MetadataRewriter {
 
   /// Handle alternative instruction info from .altinstructions.
   Error readAltInstructions();
+  Error tryReadAltInstructions(uint32_t AltInstFeatureSize,
+                               bool AltInstHasPadLen, bool ParseOnly);
   Error rewriteAltInstructions();
 
   /// Read .pci_fixup
@@ -1319,12 +1321,69 @@ Error LinuxKernelRewriter::rewriteBugTable() {
 ///	    u8  padlen;         // present in older kernels
 ///   } __packed;
 ///
-/// Note the structures is packed.
+/// Note that the structure is packed.
+///
+/// Since the size of the "feature" field could be either u16 or u32, and
+/// "padlen" presence is unknown, we attempt to parse .altinstructions section
+/// using all possible combinations (four at this time). Since we validate the
+/// contents of the section and its size, the detection works quite well.
+/// Still, we leave the user the opportunity to specify these features on the
+/// command line and skip the guesswork.
 Error LinuxKernelRewriter::readAltInstructions() {
   AltInstrSection = BC.getUniqueSectionByName(".altinstructions");
   if (!AltInstrSection)
     return Error::success();
 
+  // Presence of "padlen" field.
+  std::vector<bool> PadLenVariants;
+  if (opts::AltInstHasPadLen.getNumOccurrences())
+    PadLenVariants.push_back(opts::AltInstHasPadLen);
+  else
+    PadLenVariants = {false, true};
+
+  // Size (in bytes) variants of "feature" field.
+  std::vector<uint32_t> FeatureSizeVariants;
+  if (opts::AltInstFeatureSize.getNumOccurrences())
+    FeatureSizeVariants.push_back(opts::AltInstFeatureSize);
+  else
+    FeatureSizeVariants = {2, 4};
+
+  for (bool AltInstHasPadLen : PadLenVariants) {
+    for (uint32_t AltInstFeatureSize : FeatureSizeVariants) {
+      LLVM_DEBUG({
+        dbgs() << "BOLT-DEBUG: trying AltInstHasPadLen = " << AltInstHasPadLen
+               << "; AltInstFeatureSize = " << AltInstFeatureSize << ";\n";
+      });
+      if (Error E = tryReadAltInstructions(AltInstFeatureSize, AltInstHasPadLen,
+                                           /*ParseOnly*/ true)) {
+        consumeError(std::move(E));
+        continue;
+      }
+
+      LLVM_DEBUG(dbgs() << "Matched .altinstructions format\n");
+
+      if (!opts::AltInstHasPadLen.getNumOccurrences())
+        BC.outs() << "BOLT-INFO: setting --" << opts::AltInstHasPadLen.ArgStr
+                  << '=' << AltInstHasPadLen << '\n';
+
+      if (!opts::AltInstFeatureSize.getNumOccurrences())
+        BC.outs() << "BOLT-INFO: setting --" << opts::AltInstFeatureSize.ArgStr
+                  << '=' << AltInstFeatureSize << '\n';
+
+      return tryReadAltInstructions(AltInstFeatureSize, AltInstHasPadLen,
+                                    /*ParseOnly*/ false);
+    }
+  }
+
+  // We couldn't match the format. Read again to properly propagate the error
+  // to the user.
+  return tryReadAltInstructions(opts::AltInstFeatureSize,
+                                opts::AltInstHasPadLen, /*ParseOnly*/ false);
+}
+
+Error LinuxKernelRewriter::tryReadAltInstructions(uint32_t AltInstFeatureSize,
+                                                  bool AltInstHasPadLen,
+                                                  bool ParseOnly) {
   const uint64_t Address = AltInstrSection->getAddress();
   DataExtractor DE = DataExtractor(AltInstrSection->getContents(),
                                    BC.AsmInfo->isLittleEndian(),
@@ -1336,12 +1395,12 @@ Error LinuxKernelRewriter::readAltInstructions() {
         Address + Cursor.tell() + (int32_t)DE.getU32(Cursor);
     const uint64_t AltInstAddress =
         Address + Cursor.tell() + (int32_t)DE.getU32(Cursor);
-    const uint64_t Feature = DE.getUnsigned(Cursor, opts::AltInstFeatureSize);
+    const uint64_t Feature = DE.getUnsigned(Cursor, AltInstFeatureSize);
     const uint8_t OrgSize = DE.getU8(Cursor);
     const uint8_t AltSize = DE.getU8(Cursor);
 
     // Older kernels may have the padlen field.
-    const uint8_t PadLen = opts::AltInstHasPadLen ? DE.getU8(Cursor) : 0;
+    const uint8_t PadLen = AltInstHasPadLen ? DE.getU8(Cursor) : 0;
 
     if (!Cursor)
       return createStringError(
@@ -1358,7 +1417,7 @@ Error LinuxKernelRewriter::readAltInstructions() {
                 << "\n\tFeature: 0x" << Twine::utohexstr(Feature)
                 << "\n\tOrgSize: " << (int)OrgSize
                 << "\n\tAltSize: " << (int)AltSize << '\n';
-      if (opts::AltInstHasPadLen)
+      if (AltInstHasPadLen)
         BC.outs() << "\tPadLen:  " << (int)PadLen << '\n';
     }
 
@@ -1375,7 +1434,7 @@ Error LinuxKernelRewriter::readAltInstructions() {
 
     BinaryFunction *AltBF =
         BC.getBinaryFunctionContainingAddress(AltInstAddress);
-    if (AltBF && BC.shouldEmit(*AltBF)) {
+    if (!ParseOnly && AltBF && BC.shouldEmit(*AltBF)) {
       BC.errs()
           << "BOLT-WARNING: alternative instruction sequence found in function "
           << *AltBF << '\n';
@@ -1397,6 +1456,9 @@ Error LinuxKernelRewriter::readAltInstructions() {
                                " referenced by .altinstructions entry %d",
                                OrgInstAddress, EntryID);
 
+    if (ParseOnly)
+      continue;
+
     // There could be more than one alternative instruction sequences for the
     // same original instruction. Annotate each alternative separately.
     std::string AnnotationName = "AltInst";