[BOLT][DWARF] Add ability to insert new entries in to DIE

[ayermolo]

Summary

Added ability to append new entries to DIE. This is useful to standadize DWARF4
Split Dwarf, and simplify implementation of DWARF5.
Multiple DIEs can share an abbrev. So currently limitation is that only unique
Attributes can be added.

[ayermolo]

nit

[ayermolo]

Missed a place where we update range base.

[maksfb]

bolt/lib/Core/DebugData.cpp

902

nit: add braces for the if since the nested for is braced.

bolt/lib/Rewrite/DWARFRewriter.cpp

603

Accidental?

bolt/test/X86/insert-debug-info-entry.test

9

Could you add a description what the test is checking? If you are inserting a new attribute, perhaps you should check that it's introduced by BOLT and not by llc?

[ayermolo]

addressing comments

[ayermolo]

missed comment

[maksfb]

bolt/include/bolt/Core/DebugData.h

666

669

bolt/lib/Core/DebugData.cpp

422

We need a proper way to handle endianness.

[yota9]

bolt/lib/Core/DebugData.cpp

422

You can take a look to support::endian::Writer, here is an example I'm using in golang support:

template <typename T>
static T writeEndian(BinaryContext &BC, T Val) {
  T ret;
  SmallVector<char, sizeof(T)> Tmp;
  raw_svector_ostream OS(Tmp);

  if (BC.AsmInfo->isLittleEndian())
    support::endian::Writer<support::little>(OS).write<T>(Val);
  else
    support::endian::Writer<support::big>(OS).write<T>(Val);

  memcpy(&ret, OS.str().data(), sizeof(T));
  return ret;
}

[ayermolo]

bolt/lib/Core/DebugData.cpp

422

I think it should be part of a different commit thought.

[maksfb]

bolt/lib/Core/DebugData.cpp

422

Since it's moved code, it makes sense.

bolt/include/bolt/Core/DebugData.h

public:
   enum class DebugPatchKind {
     PatchBaseClass,
     PatchValue32,
     PatchValue64to32,
     PatchValue64,
     PatchValueVariable,
     ReferencePatchValue,
     DWARFUnitOffsetBaseLabel,
-    DestinationReferenceLabel
+    DestinationReferenceLabel,
+    NewDebugEntry
   };
 
   struct Patch {
     Patch(uint32_t O, DebugPatchKind K) : Offset(O), Kind(K) {}
     DebugPatchKind getKind() const { return Kind; }
 
     static bool classof(const Patch *Writer) {
       return Writer->getKind() == DebugPatchKind::PatchBaseClass;
struct DestinationReferenceLabel : public Patch {
     DestinationReferenceLabel(uint32_t O)
         : Patch(O, DebugPatchKind::DestinationReferenceLabel) {}
 
     static bool classof(const Patch *Writer) {
       return Writer->getKind() == DebugPatchKind::DestinationReferenceLabel;
     }
   };
 
+  struct NewDebugEntry : public Patch {
+    NewDebugEntry() = delete;
+    NewDebugEntry(uint32_t O, std::string &&V)
+        : Patch(O, DebugPatchKind::NewDebugEntry) {
+      CurrentOrder = NewDebugEntry::OrderCounter++;
+      Value = std::move(V);
+    }
+
+    static bool classof(const Patch *Writer) {
+      return Writer->getKind() == DebugPatchKind::NewDebugEntry;
+    }
+    static uint32_t OrderCounter;
+    uint32_t CurrentOrder;
+    std::string Value;
+  };
+
   virtual PatcherKind getKind() const override {
     return PatcherKind::DebugInfoBinaryPatcher;
   }
 
   static bool classof(const SimpleBinaryPatcher *Patcher) {
     return Patcher->getKind() == PatcherKind::DebugInfoBinaryPatcher;
   }
 
public:
   void addDestinationReferenceLabel(uint64_t Offset);
 
   /// Adds a reference at \p Offset to patch with new fully resolved \p
   /// DestinationOffset . The \p OldValueSize is the original size of entry in
   /// the DIE. The \p Form is the form of the entry.
   void addReferenceToPatch(uint64_t Offset, uint32_t DestinationOffset,
                            uint32_t OldValueSize, dwarf::Form Form);
 
+  /// Inserts a new uint32_t \p Value at the end of \p DIE .
+  void insertNewEntry(const DWARFDie &DIE, uint32_t);
+
+  /// Inserts a new encoded \p Value at the end of \p DIE .
+  void insertNewEntry(const DWARFDie &DIE, std::string &&Value);
+
   /// Clears unordered set for DestinationLabels.
   void clearDestinationLabels() { DestinationLabels.clear(); }
 
   /// Sets DWARF Units offset, \p  DWPOffset , within DWP file.
   void setDWPOffset(uint64_t DWPOffset) { DWPUnitOffset = DWPOffset; }
 
   /// When this function is invoked all of the DebugInfo Patches must be done.
   /// Returns a map of old CU offsets to new offsets and new sizes.
void operator()(Patch *P) const {
         delete reinterpret_cast<DebugPatchReference *>(P);
         break;
       case DebugPatchKind::DWARFUnitOffsetBaseLabel:
         delete reinterpret_cast<DWARFUnitOffsetBaseLabel *>(P);
         break;
       case DebugPatchKind::DestinationReferenceLabel:
         delete reinterpret_cast<DestinationReferenceLabel *>(P);
         break;
+      case DebugPatchKind::NewDebugEntry:
+        delete reinterpret_cast<NewDebugEntry *>(P);
+        break;
       }
     }
   };
   using UniquePatchPtrType = std::unique_ptr<Patch, PatchDeleter>;
 
   uint64_t DWPUnitOffset{0};
   int32_t ChangeInSize{0};
   std::vector<UniquePatchPtrType> DebugPatches;
class DebugAbbrevWriter {
 
   /// Attributes substitution (patch) information.
   struct PatchInfo {
     dwarf::Attribute OldAttr;
     dwarf::Attribute NewAttr;
     uint8_t NewAttrForm;
   };
 
+  struct AbbrevEntry {
+    dwarf::Attribute Attr;
+    dwarf::Form Form;
+  };
+
   using PatchesTy = std::unordered_map<const DWARFAbbreviationDeclaration *,
                                        SmallVector<PatchInfo, 2>>;
   std::unordered_map<const DWARFUnit *, PatchesTy> Patches;
 
+  using AbbrevEntryTy = std::unordered_map<const DWARFAbbreviationDeclaration *,
+                                           SmallVector<AbbrevEntry, 2>>;
+  std::unordered_map<const DWARFUnit *, AbbrevEntryTy> NewAbbrevEntries;
+
   /// DWARF context containing abbreviations.
   DWARFContext &Context;
 
   /// DWO ID used to identify unit contribution in DWP.
   Optional<uint64_t> DWOId;
 
   /// Add abbreviations from compile/type \p Unit to the writer.
   void addUnitAbbreviations(DWARFUnit &Unit);
void addAttributePatch(const DWARFUnit &Unit,
                          uint8_t NewAttrForm) {
     assert(&Unit.getContext() == &Context &&
            "cannot update attribute from a different DWARF context");
     std::lock_guard<std::mutex> Lock(WriterMutex);
     Patches[&Unit][Abbrev].emplace_back(
         PatchInfo{AttrTag, NewAttrTag, NewAttrForm});
   }
 
+  /// Adds attribute \p AttrTag and \p NewAttrForm in abbreviation declaration
+  /// \p Abbrev belonging to CU \p Unit .
+  void addAttribute(const DWARFUnit &Unit,
+                    const DWARFAbbreviationDeclaration *Abbrev,
+                    dwarf::Attribute AttrTag, dwarf::Form AttrForm) {
+    assert(&Unit.getContext() == &Context &&
+           "cannot update attribute from a different DWARF context");
+    std::lock_guard<std::mutex> Lock(WriterMutex);
+    bool AlreadyAdded = false;
+    for (AbbrevEntry &E : NewAbbrevEntries[&Unit][Abbrev])
+      if (E.Attr == AttrTag) {
+        AlreadyAdded = true;
+        break;
+      }
+
+    if (AlreadyAdded)
+      return;
+    NewAbbrevEntries[&Unit][Abbrev].emplace_back(
+        AbbrevEntry{AttrTag, AttrForm});
+  }
+
   /// Return a buffer with concatenated abbrev sections for all CUs and TUs
   /// in the associated DWARF context. Section offsets could be queried using
   /// getAbbreviationsOffsetForUnit() interface. For DWP, we are using DWOId
   /// to return only that unit's contribution to abbrev section.
   std::unique_ptr<DebugBufferVector> finalize();
 
   /// Return an offset in the finalized abbrev section corresponding to CU/TU.
   uint64_t getAbbreviationsOffsetForUnit(const DWARFUnit &Unit) {
public:
 
   /// Indicate that for the unit we should emit specified contents instead of
   /// generating a new line info table.
   void addRawContents(StringRef DebugLineContents) {
     RawData = DebugLineContents;
   }
 };
 
+struct AttrInfo {
+  DWARFFormValue V;
+  uint64_t Offset;
+  uint32_t Size; // Size of the attribute.
+};
+
+Optional<AttrInfo>
+findAttributeInfo(const DWARFDie DIE,
+                  const DWARFAbbreviationDeclaration *AbbrevDecl,
+                  uint32_t Index);
+
 } // namespace bolt
 } // namespace llvm
 
 #endif

bolt/lib/Core/DebugData.cpp

 extern llvm::cl::opt<unsigned> Verbosity;
 } // namespace opts
 
 namespace llvm {
 class MCSymbol;
 
 namespace bolt {
 
+/// Finds attributes FormValue and Offset.
+///
+/// \param DIE die to look up in.
+/// \param Index the attribute index to extract.
+/// \return an optional AttrInfo with DWARFFormValue and Offset.
+Optional<AttrInfo>
+findAttributeInfo(const DWARFDie DIE,
+                  const DWARFAbbreviationDeclaration *AbbrevDecl,
+                  uint32_t Index) {
+  const DWARFUnit &U = *DIE.getDwarfUnit();
+  uint64_t Offset =
+      AbbrevDecl->getAttributeOffsetFromIndex(Index, DIE.getOffset(), U);
+  Optional<DWARFFormValue> Value =
+      AbbrevDecl->getAttributeValueFromOffset(Index, Offset, U);
+  if (!Value)
+    return None;
+  // AttributeSpec
+  const DWARFAbbreviationDeclaration::AttributeSpec *AttrVal =
+      AbbrevDecl->attributes().begin() + Index;
+  uint32_t ValSize = 0;
+  Optional<int64_t> ValSizeOpt = AttrVal->getByteSize(U);
+  if (ValSizeOpt) {
+    ValSize = static_cast<uint32_t>(*ValSizeOpt);
+  } else {
+    DWARFDataExtractor DebugInfoData = U.getDebugInfoExtractor();
+    uint64_t NewOffset = Offset;
+    DWARFFormValue::skipValue(Value->getForm(), DebugInfoData, &NewOffset,
+                              U.getFormParams());
+    // This includes entire size of the entry, which might not be just the
+    // encoding part. For example for DW_AT_loc it will include expression
+    // location.
+    ValSize = NewOffset - Offset;
+  }
+
+  return AttrInfo{*Value, Offset, ValSize};
+}
+
 const DebugLineTableRowRef DebugLineTableRowRef::NULL_ROW{0, 0};
 
 namespace {
 
 LLVM_ATTRIBUTE_UNUSED
 static void printLE64(const std::string &S) {
   for (uint32_t I = 0, Size = S.size(); I < Size; ++I) {
     errs() << Twine::utohexstr(S[I]);
void DebugInfoBinaryPatcher::addDestinationReferenceLabel(uint64_t Offset) {
   std::lock_guard<std::mutex> Lock(WriterMutex);
   auto RetVal = DestinationLabels.insert(Offset);
   if (!RetVal.second)
     return;
 
   DebugPatches.emplace_back(new DestinationReferenceLabel(Offset));
 }
 
+static std::string encodeLE(size_t ByteSize, uint64_t NewValue) {
+  std::string LE64(ByteSize, 0);
+  for (size_t I = 0; I < ByteSize; ++I) {
+    LE64[I] = NewValue & 0xff;
+    NewValue >>= 8;
+  }
+  return LE64;
+}
+
+void DebugInfoBinaryPatcher::insertNewEntry(const DWARFDie &DIE,
+                                            uint32_t Value) {
+  std::string StrValue = encodeLE(4, Value);
+  insertNewEntry(DIE, std::move(StrValue));
+}
+
+void DebugInfoBinaryPatcher::insertNewEntry(const DWARFDie &DIE,
+                                            std::string &&Value) {
+  const DWARFAbbreviationDeclaration *AbbrevDecl =
+      DIE.getAbbreviationDeclarationPtr();
+
+  // In case this DIE has no attributes.
+  uint32_t Offset = DIE.getOffset() + 1;
+  size_t NumOfAttributes = AbbrevDecl->getNumAttributes();
+  if (NumOfAttributes) {
+    Optional<AttrInfo> Val =
+        findAttributeInfo(DIE, AbbrevDecl, NumOfAttributes - 1);
+    assert(Val && "Invalid Value.");
+
+    Offset = Val->Offset + Val->Size - DWPUnitOffset;
+  }
+  std::lock_guard<std::mutex> Lock(WriterMutex);
+  DebugPatches.emplace_back(new NewDebugEntry(Offset, std::move(Value)));
+}
+
 void DebugInfoBinaryPatcher::addReferenceToPatch(uint64_t Offset,
                                                  uint32_t DestinationOffset,
                                                  uint32_t OldValueSize,
                                                  dwarf::Form Form) {
   Offset -= DWPUnitOffset;
   DestinationOffset -= DWPUnitOffset;
   std::lock_guard<std::mutex> Lock(WriterMutex);
   DebugPatches.emplace_back(
void SimpleBinaryPatcher::addBinaryPatch(uint64_t Offset,
   Patches.emplace_back(Offset, std::move(NewValue));
 }
 
 void SimpleBinaryPatcher::addBytePatch(uint64_t Offset, uint8_t Value) {
   auto Str = std::string(1, Value);
   Patches.emplace_back(Offset, std::move(Str));
 }
 
-static std::string encodeLE(size_t ByteSize, uint64_t NewValue) {
-  std::string LE64(ByteSize, 0);
-  for (size_t I = 0; I < ByteSize; ++I) {
-    LE64[I] = NewValue & 0xff;
-    NewValue >>= 8;
-  }
-  return LE64;
-}
-
 void SimpleBinaryPatcher::addLEPatch(uint64_t Offset, uint64_t NewValue,
                                      size_t ByteSize) {
   Patches.emplace_back(Offset, encodeLE(ByteSize, NewValue));
 }
 
 void SimpleBinaryPatcher::addUDataPatch(uint64_t Offset, uint64_t Value,
                                         uint32_t OldValueSize) {
   std::string Buff;
std::string SimpleBinaryPatcher::patchBinary(StringRef BinaryContents) {
   return BinaryContentsStr;
 }
 
 CUOffsetMap DebugInfoBinaryPatcher::computeNewOffsets(DWARFContext &DWCtx,
                                                       bool IsDWOContext) {
   CUOffsetMap CUMap;
   std::sort(DebugPatches.begin(), DebugPatches.end(),
             [](const UniquePatchPtrType &V1, const UniquePatchPtrType &V2) {
+              if (V1.get()->Offset == V2.get()->Offset) {
+                if (V1->Kind == DebugPatchKind::NewDebugEntry &&
+                    V2->Kind == DebugPatchKind::NewDebugEntry)
+                  return reinterpret_cast<const NewDebugEntry *>(V1.get())
+                             ->CurrentOrder <
+                         reinterpret_cast<const NewDebugEntry *>(V2.get())
+                             ->CurrentOrder;
+
+                // This is a case where we are modifying first entry of next
+                // DIE, and adding a new one.
+                return V1->Kind == DebugPatchKind::NewDebugEntry;
+              }
               return V1.get()->Offset < V2.get()->Offset;
             });
 
   DWARFUnitVector::compile_unit_range CompileUnits =
       IsDWOContext ? DWCtx.dwo_compile_units() : DWCtx.compile_units();
 
   for (const std::unique_ptr<DWARFUnit> &CU : CompileUnits)
     CUMap[CU->getOffset()] = {static_cast<uint32_t>(CU->getOffset()),
case DebugPatchKind::DWARFUnitOffsetBaseLabel: {
           reinterpret_cast<DWARFUnitOffsetBaseLabel *>(P);
       uint32_t CUOffset = BaseLabel->Offset;
       ChangeInSize += PreviousChangeInSize;
       uint32_t CUOffsetUpdate = CUOffset + ChangeInSize;
       CUMap[CUOffset].Offset = CUOffsetUpdate;
       CUMap[PreviousOffset].Length += PreviousChangeInSize;
       PreviousChangeInSize = 0;
       PreviousOffset = CUOffset;
+      break;
+    }
+    case DebugPatchKind::NewDebugEntry: {
+      NewDebugEntry *NDE = reinterpret_cast<NewDebugEntry *>(P);
+      PreviousChangeInSize += NDE->Value.size();
+      break;
     }
     }
   }
   CUMap[PreviousOffset].Length += PreviousChangeInSize;
   return CUMap;
 }
+uint32_t DebugInfoBinaryPatcher::NewDebugEntry::OrderCounter = 0;
 
 std::string DebugInfoBinaryPatcher::patchBinary(StringRef BinaryContents) {
   std::string NewBinaryContents;
   NewBinaryContents.reserve(BinaryContents.size() + ChangeInSize);
   uint32_t StartOffset = 0;
   uint32_t DwarfUnitBaseOffset = 0;
   uint32_t OldValueSize = 0;
   uint32_t Offset = 0;
case DebugPatchKind::DWARFUnitOffsetBaseLabel: {
       uint32_t NewCUOffset = NewBinaryContents.size() + RemainingLength;
       DwarfUnitBaseOffset = NewCUOffset;
       // Length of previous CU = This CU Offset - sizeof(length) - last CU
       // Offset.
       Patch.second = NewCUOffset - 4 - Patch.first;
       LengthPatches.push_back({NewCUOffset, 0});
       break;
     }
+    case DebugPatchKind::NewDebugEntry: {
+      NewDebugEntry *NDE = reinterpret_cast<NewDebugEntry *>(P);
+      Offset = NDE->Offset;
+      OldValueSize = 0;
+      ByteSequence = NDE->Value;
+      break;
+    }
     }
 
-    assert(Offset + ByteSequence.size() <= BinaryContents.size() &&
+    assert((P->Kind == DebugPatchKind::NewDebugEntry ||
+            Offset + ByteSequence.size() <= BinaryContents.size()) &&
            "Applied patch runs over binary size.");
     uint32_t Length = Offset - StartOffset;
     NewBinaryContents.append(BinaryContents.substr(StartOffset, Length).data(),
                              Length);
     NewBinaryContents.append(ByteSequence.data(), ByteSequence.size());
     StartOffset = Offset + OldValueSize;
   }
   uint32_t Length = BinaryContents.size() - StartOffset;

 }
 
 void DebugAbbrevWriter::addUnitAbbreviations(DWARFUnit &Unit) {
   const DWARFAbbreviationDeclarationSet *Abbrevs = Unit.getAbbreviations();
   if (!Abbrevs)
     return;
 
   const PatchesTy &UnitPatches = Patches[&Unit];
+  const AbbrevEntryTy &AbbrevEntries = NewAbbrevEntries[&Unit];
 
   // We are duplicating abbrev sections, to handle the case where for one CU we
   // modify it, but for another we don't.
   auto UnitDataPtr = std::make_unique<AbbrevData>();
   AbbrevData &UnitData = *UnitDataPtr.get();
   UnitData.Buffer = std::make_unique<DebugBufferVector>();
   UnitData.Stream = std::make_unique<raw_svector_ostream>(*UnitData.Buffer);
+
   raw_svector_ostream &OS = *UnitData.Stream.get();
 
   // Returns true if AbbrevData is re-used, false otherwise.
   auto hashAndAddAbbrev = [&](StringRef AbbrevData) -> bool {
     llvm::SHA1 Hasher;
     Hasher.update(AbbrevData);
     StringRef Key = Hasher.final();
     auto Iter = AbbrevDataCache.find(Key);
     if (Iter != AbbrevDataCache.end()) {
       UnitsAbbrevData[&Unit] = Iter->second.get();
       return true;
     }
     AbbrevDataCache[Key] = std::move(UnitDataPtr);
     UnitsAbbrevData[&Unit] = &UnitData;
     return false;
   };
   // Take a fast path if there are no patches to apply. Simply copy the original
   // contents.
-  if (UnitPatches.empty()) {
+  if (UnitPatches.empty() && AbbrevEntries.empty()) {
     StringRef AbbrevSectionContents =
         Unit.isDWOUnit() ? Unit.getContext().getDWARFObj().getAbbrevDWOSection()
                          : Unit.getContext().getDWARFObj().getAbbrevSection();
     StringRef AbbrevContents;
 
     const DWARFUnitIndex &CUIndex = Unit.getContext().getCUIndex();
     if (!CUIndex.getRows().empty()) {
       // Handle DWP section contribution.
for (const DWARFAbbreviationDeclaration::AttributeSpec &AttrSpec :
           continue;
       }
 
       encodeULEB128(AttrSpec.Attr, OS);
       encodeULEB128(AttrSpec.Form, OS);
       if (AttrSpec.isImplicitConst())
         encodeSLEB128(AttrSpec.getImplicitConstValue(), OS);
     }
-
+    const auto Entries = AbbrevEntries.find(&Abbrev);
+    // Adding new Abbrevs for inserted entries.
+    if (Entries != AbbrevEntries.end())
+      for (const AbbrevEntry &Entry : Entries->second) {
+        encodeULEB128(Entry.Attr, OS);
+        encodeULEB128(Entry.Form, OS);
+      }
     encodeULEB128(0, OS);
     encodeULEB128(0, OS);
   }
   encodeULEB128(0, OS);
 
   hashAndAddAbbrev(OS.str());
 }
 

bolt/lib/Rewrite/DWARFRewriter.cpp

static void printDie(const DWARFDie &DIE) {
   DIDumpOptions DumpOpts;
   DumpOpts.ShowForm = true;
   DumpOpts.Verbose = true;
   DumpOpts.ChildRecurseDepth = 0;
   DumpOpts.ShowChildren = 0;
   DIE.dump(dbgs(), 0, DumpOpts);
 }
 
-struct AttrInfo {
-  DWARFFormValue V;
-  uint64_t Offset;
-  uint32_t Size; // Size of the attribute.
-};
-
-/// Finds attributes FormValue and Offset.
-///
-/// \param DIE die to look up in.
-/// \param Index the attribute index to extract.
-/// \return an optional AttrInfo with DWARFFormValue and Offset.
-static Optional<AttrInfo>
-findAttributeInfo(const DWARFDie DIE,
-                  const DWARFAbbreviationDeclaration *AbbrevDecl,
-                  uint32_t Index) {
-  const DWARFUnit &U = *DIE.getDwarfUnit();
-  uint64_t Offset =
-      AbbrevDecl->getAttributeOffsetFromIndex(Index, DIE.getOffset(), U);
-  Optional<DWARFFormValue> Value =
-      AbbrevDecl->getAttributeValueFromOffset(Index, Offset, U);
-  if (!Value)
-    return None;
-  // AttributeSpec
-  const DWARFAbbreviationDeclaration::AttributeSpec *AttrVal =
-      AbbrevDecl->attributes().begin() + Index;
-  uint32_t ValSize = 0;
-  Optional<int64_t> ValSizeOpt = AttrVal->getByteSize(U);
-  if (ValSizeOpt) {
-    ValSize = static_cast<uint32_t>(*ValSizeOpt);
-  } else {
-    DWARFDataExtractor DebugInfoData = U.getDebugInfoExtractor();
-    uint64_t NewOffset = Offset;
-    DWARFFormValue::skipValue(Value->getForm(), DebugInfoData, &NewOffset,
-                              U.getFormParams());
-    // This includes entire size of the entry, which might not be just the
-    // encoding part. For example for DW_AT_loc it will include expression
-    // location.
-    ValSize = NewOffset - Offset;
-  }
-
-  return AttrInfo{*Value, Offset, ValSize};
-}
-
+namespace llvm {
+namespace bolt {
 /// Finds attributes FormValue and Offset.
 ///
 /// \param DIE die to look up in.
 /// \param Attr the attribute to extract.
 /// \return an optional AttrInfo with DWARFFormValue and Offset.
 static Optional<AttrInfo> findAttributeInfo(const DWARFDie DIE,
                                             dwarf::Attribute Attr) {
   if (!DIE.isValid())
     return None;
   const DWARFAbbreviationDeclaration *AbbrevDecl =
       DIE.getAbbreviationDeclarationPtr();
   if (!AbbrevDecl)
     return None;
   Optional<uint32_t> Index = AbbrevDecl->findAttributeIndex(Attr);
   if (!Index)
     return None;
   return findAttributeInfo(DIE, AbbrevDecl, *Index);
 }
+} // namespace bolt
+} // namespace llvm
 
 using namespace llvm;
 using namespace llvm::support::endian;
 using namespace object;
 using namespace bolt;
 
 namespace opts {
 
void DWARFRewriter::updateDWARFObjectAddressRanges(
 
   // Some objects don't have an associated DIE and cannot be updated (such as
   // compiler-generated functions).
   if (!DIE)
     return;
 
   const DWARFAbbreviationDeclaration *AbbreviationDecl =
       DIE.getAbbreviationDeclarationPtr();
+
   if (!AbbreviationDecl) {
     if (opts::Verbosity >= 1)
       errs() << "BOLT-WARNING: object's DIE doesn't have an abbreviation: "
              << "skipping update. DIE at offset 0x"
              << Twine::utohexstr(DIE.getOffset()) << '\n';
     return;
   }
 
if (AbbreviationDecl->findAttributeIndex(dwarf::DW_AT_ranges)) {
     if (!LowPCAttrInfo) {
       errs() << "BOLT-ERROR: skeleton CU at 0x"
              << Twine::utohexstr(DIE.getOffset())
              << " does not have DW_AT_GNU_ranges_base or DW_AT_low_pc to"
                 " convert to update ranges base\n";
       return;
     }
 
-    AbbrevWriter.addAttributePatch(
-        *DIE.getDwarfUnit(), AbbreviationDecl, dwarf::DW_AT_low_pc,
-        dwarf::DW_AT_GNU_ranges_base, dwarf::DW_FORM_indirect);
-    DebugInfoPatcher.addUDataPatch(LowPCAttrInfo->Offset, dwarf::DW_FORM_udata,
-                                   1);
-    DebugInfoPatcher.addUDataPatch(LowPCAttrInfo->Offset + 1, *RangesBase, 7);
+    AbbrevWriter.addAttribute(*DIE.getDwarfUnit(), AbbreviationDecl,
+                              dwarf::DW_AT_GNU_ranges_base,
+                              dwarf::DW_FORM_sec_offset);
+    reinterpret_cast<DebugInfoBinaryPatcher &>(DebugInfoPatcher).insertNewEntry(DIE, *RangesBase);
 
     return;
   }
 
   // Case 2: The object has both DW_AT_low_pc and DW_AT_high_pc emitted back
   // to back. Replace with new attributes and patch the DIE.
   Optional<AttrInfo> HighPCAttrInfo =
       findAttributeInfo(DIE, dwarf::DW_AT_high_pc);
auto getAttributeForm = [&Abbrev](const dwarf::Attribute Attr) {
     return Abbrev->getFormByIndex(*Index);
   };
   dwarf::Form LowPCForm = getAttributeForm(dwarf::DW_AT_low_pc);
 
   // DW_FORM_GNU_addr_index is already variable encoding so nothing to do
   // there.
   if (RangesBase) {
     assert(LowPCForm != dwarf::DW_FORM_GNU_addr_index);
-    AbbrevWriter.addAttributePatch(Unit, Abbrev, dwarf::DW_AT_low_pc,
-                                   dwarf::DW_AT_GNU_ranges_base,
-                                   dwarf::DW_FORM_sec_offset);
+    AbbrevWriter.addAttribute(Unit, Abbrev, dwarf::DW_AT_GNU_ranges_base,
   }
 
   AbbrevWriter.addAttributePatch(Unit, Abbrev, dwarf::DW_AT_high_pc,
                                  dwarf::DW_AT_ranges,
                                  dwarf::DW_FORM_sec_offset);
 }
 
 void DWARFRewriter::convertToRangesPatchDebugInfo(
void DWARFRewriter::convertToRangesPatchDebugInfo(
   uint32_t BaseOffset = 0;
   if (LowPCVal->V.getForm() == dwarf::DW_FORM_GNU_addr_index) {
     // Use ULEB128 for the value.
     DebugInfoPatcher.addUDataPatch(LowPCOffset, 0,
                                    std::abs(int(HighPCOffset - LowPCOffset)));
     // Ranges are relative to DW_AT_GNU_ranges_base.
     BaseOffset = DebugInfoPatcher.getRangeBase();
   } else {
+    DebugInfoPatcher.addLE64Patch(LowPCOffset, 0);
     // If case DW_AT_low_pc was converted into DW_AT_GNU_ranges_base
     if (RangesBase)
-      DebugInfoPatcher.addLE32Patch(LowPCOffset, *RangesBase, 8);
-    else
-      DebugInfoPatcher.addLE64Patch(LowPCOffset, 0);
+      reinterpret_cast<DebugInfoBinaryPatcher &>(DebugInfoPatcher)
+          .insertNewEntry(DIE, *RangesBase);
   }
   DebugInfoPatcher.addLE32Patch(HighPCOffset, RangesSectionOffset - BaseOffset,
                                 HighPCVal->Size);
 }

bolt/test/X86/insert-debug-info-entry.test

+; RUN: rm -rf %t
+; RUN: mkdir %t
+; RUN: cd %t
+; RUN: llc -split-dwarf-file=foo.dwo -split-dwarf-output=foo.dwo -O0 -mtriple=x86_64-unknown-linux-gnu -filetype=obj %s -o=foo.o
+; RUN: %clang %cflags foo.o -o foo.exe
+; RUN: llvm-bolt foo.exe -o foo.exe.bolt --update-debug-sections
+; RUN: llvm-dwarfdump --debug-info foo.exe.bolt | FileCheck %s
+
+
+; CHECK: DW_AT_GNU_addr_base
+; CHECK-NEXT: DW_AT_GNU_ranges_base
+
+; ModuleID = 'main.cpp'
+source_filename = "main.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: mustprogress noinline nounwind optnone uwtable
+define dso_local noundef i32 @_Z3foov() #0 !dbg !8 {
+entry:
+  ret i32 3, !dbg !13
+}
+
+; Function Attrs: mustprogress noinline norecurse nounwind optnone uwtable
+define dso_local noundef i32 @main() #1 !dbg !14 {
+entry:
+  %retval = alloca i32, align 4
+  store i32 0, i32* %retval, align 4
+  %call = call noundef i32 @_Z3foov(), !dbg !15
+  ret i32 %call, !dbg !16
+}
+
+attributes #0 = { mustprogress noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+attributes #1 = { mustprogress noinline norecurse nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2, !3, !4, !5, !6}
+!llvm.ident = !{!7}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, splitDebugFilename: "main.dwo", emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: GNU)
+!1 = !DIFile(filename: "main.cpp", directory: ".")
+!2 = !{i32 7, !"Dwarf Version", i32 4}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 7, !"uwtable", i32 1}
+!6 = !{i32 7, !"frame-pointer", i32 2}
+!7 = !{!"clang"}
+!8 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !1, file: !1, line: 1, type: !9, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !12)
+!9 = !DISubroutineType(types: !10)
+!10 = !{!11}
+!11 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!12 = !{}
+!13 = !DILocation(line: 2, column: 3, scope: !8)
+!14 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 5, type: !9, scopeLine: 5, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !12)
+!15 = !DILocation(line: 6, column: 10, scope: !14)
+!16 = !DILocation(line: 6, column: 3, scope: !14)