Calculate indexes of last child of each DWARF entry once during tryExtractDIEsIfNeeded.

[simon.giesecke]

Summary

This ensures that the last child indexes are calculated in linear time and
can later be queried in constant time by getLastChild.

The baseline situation was that individual calls to getLastChild were linear in the
size of DieArray. Calling getLastChild once for every DWARFDebugInfoEntry was
amortized quadratic in the size of DieArray.

Running "llvm-gsymutil --convert llvm-gsymutil --quiet" using a RelWithDebInfo build
of llvm-gsymutil (after recent other optimizations) and gathering a profile using
perf showed that llvm::DWARFUnit::getLastChild is the no. 1 function, accounting
for 9.9% of the CPU time, and llvm::DWARFUnit::getSibiling is the no. 4 function,
accounting for 4.76% of the CPU time.

[simon.giesecke]

One thing I don't know here is whether all relevant uses of DWARFUnit will end up iterating the children. If that's not the case, then this be done only optionally. We can't do it on-demand on the first call to getLastChild easily though, because then, e.g. GsymCreator, would need to synchronize accesses from multiple threads. But maybe this could be passed as an option to tryExtractDIEsIfNeeded?

If this is worthwhile, a similar thing could be done for the siblings.

[clayborg]

You should probably get Phabricator working: https://llvm.org/docs/Phabricator.html

This will ensure your patches have context. If you submitting patches manually, you need to specify more context lines:

git diff -U999999

The DWARFDebugInfoEntry in LLDB contains more data. See inlined comment on improving performance for the LLVM DWARF reader. But if we are going to fix perf issues with DWARFDie navigation, we should improve the DWARFDebugInfoEntry class to contain the information needed to navigate better. FYI: be very careful when trying to port any code from the LLDB DWARF parser as it actually removes the NULL terminator DIEs from its DWARFUnit vector of DWARFDebugInfoEntry objects as this saves a ton of memory and they aren't needed if you create your DWARFDebugInfoEntry with enough info.

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

833–842

This is a side affect of how the DWARFDebugInfoEntry class is created. LLDB has a more efficient way of doing things as each DWARFDebugInfoEntry knows its sibling index. Since all of the DIEs are contained in an std::vector inside of the DWARFUnit, we can just store the parent index, the sibling index. See the file lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h.

uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
uint32_t m_sibling_idx; // How many to add to "this" to get the sibling.

The current DWARFDebugInfoEntry just contains a offset and a depth and an abbrev pointer. This info doesn't make it easy to navigate the DWARFDie objects sibling, and parent. If the LLVM DWARF parser adopts this parent index and sibling index, then navigation can happen much quicker and this function can simply get the sibling index and subtract 1 as that will always the the NULL tag that terminates the previous DIE child chain.

[simon.giesecke]

In D102634#2764987, @clayborg wrote:

You should probably get Phabricator working: https://llvm.org/docs/Phabricator.html

This will ensure your patches have context. If you submitting patches manually, you need to specify more context lines:

git diff -U999999

I have set up arc now.

The DWARFDebugInfoEntry in LLDB contains more data. See inlined comment on improving performance for the LLVM DWARF reader. But if we are going to fix perf issues with DWARFDie navigation, we should improve the DWARFDebugInfoEntry class to contain the information needed to navigate better.

Ok, I added this separately first, because I wasn't sure if it should be optional, and not take up memory for use cases that might not need it. But from your comments I now understand that the extra memory usage should not be an issue.

FYI: be very careful when trying to port any code from the LLDB DWARF parser as it actually removes the NULL terminator DIEs from its DWARFUnit vector of DWARFDebugInfoEntry objects as this saves a ton of memory and they aren't needed if you create your DWARFDebugInfoEntry with enough info.

Should we remove the NULL terminator DIEs here as well, eventually? But I guess even if that should be done, this requires a lot of further adaptations. However, we would probably save more memory than we're adding through the extra sibling and parent index entries.

[simon.giesecke]

Update using arc

[simon.giesecke]

Update using arc

[dblaikie]

What's your use case such that this performance concern has come up for you?

[avl]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

833–842

uint32_t m_parent_idx; How many to subtract from "this" to get the parent.
uint32_t m_sibling_idx; How many to add to "this" to get the sibling.

The current DWARFDebugInfoEntry just contains a offset and a depth and an abbrev pointer. This info doesn't make it easy to navigate the DWARFDie objects sibling, and parent. If the LLVM DWARF parser adopts this parent index and sibling index, then navigation can happen much quicker and this function can simply get the sibling index and subtract 1 as that will always the the NULL tag that terminates the previous DIE child chain.

If the LLVM DWARF parser adopts above parent index and sibling index solution then it would also be useful for dsymutil. Currently it creates links to parents by separate pass.

[clayborg]

In D102634#2766582, @dblaikie wrote:

What's your use case such that this performance concern has come up for you?

I think anyone who goes to any DIE that contains a bunch of children and uses the DWARFDie::iterator (like for a CU Die) and iterates over its children is probably the worst case.

So any users of DWARFDie::end() like DWARFDie::children(). This is used in many places like in llvm/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp, llvm/lib/CodeGen/AsmPrinter/DIEHash.cpp, llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp, llvm/lib/DWARFLinker/DWARFLinker.cpp and llvm/unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp. There may be more and I just searched for "Die.children()".

Any of the DWARFUnit::getSibling(...), DWARFUnit::getParent(...), DWARFUnit::getLastChild(...) would be greatly sped up if we use m_parent_idx and m_sibling_idx in DWARFDebugInfoEntry.

In D102634#2765564, @simon.giesecke wrote:

In D102634#2764987, @clayborg wrote:

The DWARFDebugInfoEntry in LLDB contains more data. See inlined comment on improving performance for the LLVM DWARF reader. But if we are going to fix perf issues with DWARFDie navigation, we should improve the DWARFDebugInfoEntry class to contain the information needed to navigate better.

Ok, I added this separately first, because I wasn't sure if it should be optional, and not take up memory for use cases that might not need it. But from your comments I now understand that the extra memory usage should not be an issue.

The LLVM DWARFDebugInfoEntry contains:

class DWARFDebugInfoEntry {
  /// Offset within the .debug_info of the start of this entry.
  uint64_t Offset = 0;

  /// The integer depth of this DIE within the compile unit DIEs where the
  /// compile/type unit DIE has a depth of zero.
  uint32_t Depth = 0;

  const DWARFAbbreviationDeclaration *AbbrevDecl = nullptr;
};

This will end up being 24 bits with 4 bytes of padding after the Depth.

If we modified the LLDB DWARFDebugInfoEntry for LLVM it could contain:

class DWARFDebugInfoEntry {
  uint64_t Offset; // Offset within the .debug_info/.debug_types
  uint32_t ParentIdx; // How many to subtract from "this" to get the parent. If zero this die has no parent
  uint32_t SiblingIdx; // How many to add to "this" to get the sibling.
  const DWARFAbbreviationDeclaration *AbbrevDecl = nullptr;
};

This would be the same 24 byte size as the current version and it would improve many of the DWARFUnit::getXXX() calls that get siblings, parents, etc.

So this wouldn't take up any more memory.

FYI: be very careful when trying to port any code from the LLDB DWARF parser as it actually removes the NULL terminator DIEs from its DWARFUnit vector of DWARFDebugInfoEntry objects as this saves a ton of memory and they aren't needed if you create your DWARFDebugInfoEntry with enough info.

Should we remove the NULL terminator DIEs here as well, eventually? But I guess even if that should be done, this requires a lot of further adaptations. However, we would probably save more memory than we're adding through the extra sibling and parent index entries.

Again, there will be no memory differences between the old and new due to the padding that exists in the older DWARFDebugInfoEntry structs. I would avoid removing the NULL tags for now because DWARF dumping code relies on these being in there for DWARF printing. This is something we could do later though.

[simon.giesecke]

In D102634#2766582, @dblaikie wrote:

What's your use case such that this performance concern has come up for you?

It's a hotspot when running llvm-gsymutil --convert. I can provide some numbers if you want.

[dblaikie]

In D102634#2767781, @simon.giesecke wrote:

In D102634#2766582, @dblaikie wrote:

What's your use case such that this performance concern has come up for you?

It's a hotspot when running llvm-gsymutil --convert. I can provide some numbers if you want.

If you happen to have them, might be nice to have in the patch description/commit message.

(@clayborg's suggestion of replacing "Depth" with two smaller fields seems roughly plausible to me/reasonable thing to try, for what it's worth)

[simon.giesecke]

Updated patch description with numbers from perf profile.

[avl]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

833–842

This is a side affect of how the DWARFDebugInfoEntry class is created. LLDB has a more efficient way of doing things as each DWARFDebugInfoEntry knows its sibling index. Since all of the DIEs are contained in an std::vector inside of the DWARFUnit, we can just store the parent index, the sibling index. See the file lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h.

uint32_t m_parent_idx; // How many to subtract from "this" to get the parent.
uint32_t m_sibling_idx; // How many to add to "this" to get the sibling.

The current DWARFDebugInfoEntry just contains a offset and a depth and an abbrev pointer. This info doesn't make it easy to navigate the DWARFDie objects sibling, and parent. If the LLVM DWARF parser adopts this parent index and sibling index, then navigation can happen much quicker and this function can simply get the sibling index and subtract 1 as that will always the the NULL tag that terminates the previous DIE child chain.

Since it was mentioned in this review - Is somebody going to implementing that idea(replacing "Depth" with two smaller fields)?

[simon.giesecke]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

833–842

Since it was mentioned in this review - Is somebody going to implementing that idea(replacing "Depth" with two smaller fields)?

I would consider that when working on this again. However, I am not sure when I will be able to get back to this. If someone else wants to pick it up, that would be great.

[dblaikie]

I at least added a comment to DWARFDebugInfoEntry to reflect the possible directions (such as adding parent/sibling index) and reference this review here: 821954f97c6b978cca72cb412e98d35caee4cac3 - so if/whenever someone feels like poking at this they might have a good chance of coming across some of this history/context.

[avl]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

833–842

Since it was mentioned in this review - Is somebody going to implementing that idea(replacing "Depth" with two smaller fields)?

I would consider that when working on this again. However, I am not sure when I will be able to get back to this. If someone else wants to pick it up, that would be great.

I am interested in picking up this. Will do this. Thanks.

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

 //===- DWARFUnit.cpp ------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 

 using namespace llvm;
 using namespace dwarf;
 
 void DWARFUnitVector::addUnitsForSection(DWARFContext &C,
                                          const DWARFSection &Section,
                                          DWARFSectionKind SectionKind) {
   const DWARFObject &D = C.getDWARFObj();
   addUnitsImpl(C, D, Section, C.getDebugAbbrev(), &D.getRangesSection(),
-               &D.getLocSection(), D.getStrSection(),
-               D.getStrOffsetsSection(), &D.getAddrSection(),
-               D.getLineSection(), D.isLittleEndian(), false, false,
-               SectionKind);
+               &D.getLocSection(), D.getStrSection(), D.getStrOffsetsSection(),
+               &D.getAddrSection(), D.getLineSection(), D.isLittleEndian(),
+               false, false, SectionKind);
 }
 
 void DWARFUnitVector::addUnitsForDWOSection(DWARFContext &C,
                                             const DWARFSection &DWOSection,
                                             DWARFSectionKind SectionKind,
                                             bool Lazy) {
   const DWARFObject &D = C.getDWARFObj();
-  addUnitsImpl(C, D, DWOSection, C.getDebugAbbrevDWO(), &D.getRangesDWOSection(),
-               &D.getLocDWOSection(), D.getStrDWOSection(),
-               D.getStrOffsetsDWOSection(), &D.getAddrSection(),
-               D.getLineDWOSection(), C.isLittleEndian(), true, Lazy,
-               SectionKind);
+  addUnitsImpl(C, D, DWOSection, C.getDebugAbbrevDWO(),
+               &D.getRangesDWOSection(), &D.getLocDWOSection(),
+               D.getStrDWOSection(), D.getStrOffsetsDWOSection(),
+               &D.getAddrSection(), D.getLineDWOSection(), C.isLittleEndian(),
+               true, Lazy, SectionKind);
 }
 
 void DWARFUnitVector::addUnitsImpl(
     DWARFContext &Context, const DWARFObject &Obj, const DWARFSection &Section,
     const DWARFDebugAbbrev *DA, const DWARFSection *RS,
     const DWARFSection *LocSection, StringRef SS, const DWARFSection &SOS,
     const DWARFSection *AOS, const DWARFSection &LS, bool LE, bool IsDWO,
     bool Lazy, DWARFSectionKind SectionKind) {
Parser = [=, &Context, &Obj, &Section, &SOS,
             Context, Header.isTypeUnit() ? DW_SECT_EXT_TYPES : DW_SECT_INFO);
         IndexEntry = Index.getFromOffset(Header.getOffset());
       }
       if (IndexEntry && !Header.applyIndexEntry(IndexEntry))
         return nullptr;
       std::unique_ptr<DWARFUnit> U;
       if (Header.isTypeUnit())
         U = std::make_unique<DWARFTypeUnit>(Context, InfoSection, Header, DA,
-                                             RS, LocSection, SS, SOS, AOS, LS,
-                                             LE, IsDWO, *this);
       else
-        U = std::make_unique<DWARFCompileUnit>(Context, InfoSection, Header,
-                                                DA, RS, LocSection, SS, SOS,
-                                                AOS, LS, LE, IsDWO, *this);
+        U = std::make_unique<DWARFCompileUnit>(Context, InfoSection, Header, DA,
+                                               RS, LocSection, SS, SOS, AOS, LS,
+                                               LE, IsDWO, *this);
       return U;
     };
   }
   if (Lazy)
     return;
   // Find a reasonable insertion point within the vector.  We skip over
   // (a) units from a different section, (b) units from the same section
   // but with lower offset-within-section.  This keeps units in order
bool DWARFUnitHeader::applyIndexEntry(const DWARFUnitIndex::Entry *Entry) {
   if (!AbbrEntry)
     return false;
   AbbrOffset = AbbrEntry->Offset;
   return true;
 }
 
 // Parse the rangelist table header, including the optional array of offsets
 // following it (DWARF v5 and later).
-template<typename ListTableType>
-static Expected<ListTableType>
-parseListTableHeader(DWARFDataExtractor &DA, uint64_t Offset,
+template <typename ListTableType>
+static Expected<ListTableType> parseListTableHeader(DWARFDataExtractor &DA,
+                                                    uint64_t Offset,
                                                     DwarfFormat Format) {
   // We are expected to be called with Offset 0 or pointing just past the table
   // header. Correct Offset in the latter case so that it points to the start
   // of the header.
   if (Offset > 0) {
     uint64_t HeaderSize = DWARFListTableHeader::getHeaderSize(Format);
     if (Offset < HeaderSize)
-      return createStringError(errc::invalid_argument, "did not detect a valid"
+      return createStringError(errc::invalid_argument,
+                               "did not detect a valid"
                                " list table with base = 0x%" PRIx64 "\n",
                                Offset);
     Offset -= HeaderSize;
   }
   ListTableType Table;
   if (Error E = Table.extractHeaderAndOffsets(DA, &Offset))
     return std::move(E);
   return Table;
void DWARFUnit::extractDIEsToVector(
   // next compilation unit header.
   uint64_t DIEOffset = getOffset() + getHeaderSize();
   uint64_t NextCUOffset = getNextUnitOffset();
   DWARFDebugInfoEntry DIE;
   DWARFDataExtractor DebugInfoData = getDebugInfoExtractor();
   uint32_t Depth = 0;
   bool IsCUDie = true;
 
-  while (DIE.extractFast(*this, &DIEOffset, DebugInfoData, NextCUOffset,
-                         Depth)) {
+  while (
+      DIE.extractFast(*this, &DIEOffset, DebugInfoData, NextCUOffset, Depth)) {
     if (IsCUDie) {
       if (AppendCUDie)
         Dies.push_back(DIE);
       if (!AppendNonCUDies)
         break;
       // The average bytes per DIE entry has been seen to be
       // around 6-12 so let's pre-reserve the needed memory for
       // our DIE entries accordingly.
       Dies.reserve(Dies.size() + getDebugInfoSize() / 6);
       IsCUDie = false;
     } else {
       Dies.push_back(DIE);
     }
 
     if (const DWARFAbbreviationDeclaration *AbbrDecl =
             DIE.getAbbreviationDeclarationPtr()) {
       // Normal DIE
       if (AbbrDecl->hasChildren())
         ++Depth;
     } else {
       // NULL DIE.
       if (Depth > 0)
         --Depth;
       if (Depth == 0)
-        break;  // We are done with this compile unit!
+        break; // We are done with this compile unit!
     }
   }
 
   Dies.shrink_to_fit();
 
   // Give a little bit of info if we encounter corrupt DWARF (our offset
   // should always terminate at or before the start of the next compilation
   // unit header).
if (R == AddrDieMap.begin())
     return DWARFDie();
   // upper_bound's previous item contains Address.
   --R;
   if (Address >= R->second.first)
     return DWARFDie();
   return R->second.second;
 }
 
-void
-DWARFUnit::getInlinedChainForAddress(uint64_t Address,
-                                     SmallVectorImpl<DWARFDie> &InlinedChain) {
+void DWARFUnit::getInlinedChainForAddress(
+    uint64_t Address, SmallVectorImpl<DWARFDie> &InlinedChain) {
   assert(InlinedChain.empty());
   // Try to look for subprogram DIEs in the DWO file.
   parseDWO();
   // First, find the subroutine that contains the given address (the leaf
   // of inlined chain).
   DWARFDie SubroutineDIE =
       (DWO ? *DWO : *this).getSubroutineForAddress(Address);
 
   while (SubroutineDIE) {
     if (SubroutineDIE.isSubprogramDIE()) {
       InlinedChain.push_back(SubroutineDIE);
       return;
     }
     if (SubroutineDIE.getTag() == DW_TAG_inlined_subroutine)
       InlinedChain.push_back(SubroutineDIE);
-    SubroutineDIE  = SubroutineDIE.getParent();
+    SubroutineDIE = SubroutineDIE.getParent();
   }
 }
 
 const DWARFUnitIndex &llvm::getDWARFUnitIndex(DWARFContext &Context,
                                               DWARFSectionKind Kind) {
   if (Kind == DW_SECT_INFO)
     return Context.getCUIndex();
   assert(Kind == DW_SECT_EXT_TYPES);
DWARFDie DWARFUnit::getFirstChild(const DWARFDebugInfoEntry *Die) {
   return DWARFDie(this, &DieArray[I]);
 }
 
 DWARFDie DWARFUnit::getLastChild(const DWARFDebugInfoEntry *Die) {
   if (!Die->hasChildren())
     return DWARFDie();
 
   // We do not want access out of bounds when parsing corrupted debug data.
   size_t I = getDIEIndex(Die);
   if (I >= LastChildren.size())
     return DWARFDie();
   size_t LastChild = LastChildren[I];
   if (!LastChild) {
     // TODO Shouldn't Die->hasChildren() be false in that case?
     return DWARFDie();
   }
   return DWARFDie(this, &DieArray[LastChild]);
 }
 
 const DWARFAbbreviationDeclarationSet *DWARFUnit::getAbbreviations() const {
   if (!Abbrevs)
     Abbrevs = Abbrev->getAbbreviationDeclarationSet(Header.getAbbrOffset());
   return Abbrevs;
 }
 
 llvm::Optional<object::SectionedAddress> DWARFUnit::getBaseAddress() {
StrOffsetsContributionDescriptor::validateContributionSize(
   uint8_t EntrySize = getDwarfOffsetByteSize();
   // In order to ensure that we don't read a partial record at the end of
   // the section we validate for a multiple of the entry size.
   uint64_t ValidationSize = alignTo(Size, EntrySize);
   // Guard against overflow.
   if (ValidationSize >= Size)
     if (DA.isValidOffsetForDataOfSize((uint32_t)Base, ValidationSize))
       return *this;
-  return createStringError(errc::invalid_argument, "length exceeds section size");
+  return createStringError(errc::invalid_argument,
+                           "length exceeds section size");
 }
 
 // Look for a DWARF64-formatted contribution to the string offsets table
 // starting at a given offset and record it in a descriptor.
 static Expected<StrOffsetsContributionDescriptor>
 parseDWARF64StringOffsetsTableHeader(DWARFDataExtractor &DA, uint64_t Offset) {
   if (!DA.isValidOffsetForDataOfSize(Offset, 16))
-    return createStringError(errc::invalid_argument, "section offset exceeds section size");
+    return createStringError(errc::invalid_argument,
+                             "section offset exceeds section size");
 
   if (DA.getU32(&Offset) != dwarf::DW_LENGTH_DWARF64)
-    return createStringError(errc::invalid_argument, "32 bit contribution referenced from a 64 bit unit");
+    return createStringError(
+        errc::invalid_argument,
+        "32 bit contribution referenced from a 64 bit unit");
 
   uint64_t Size = DA.getU64(&Offset);
   uint8_t Version = DA.getU16(&Offset);
   (void)DA.getU16(&Offset); // padding
   // The encoded length includes the 2-byte version field and the 2-byte
   // padding, so we need to subtract them out when we populate the descriptor.
   return StrOffsetsContributionDescriptor(Offset, Size - 4, Version, DWARF64);
 }
 
 // Look for a DWARF32-formatted contribution to the string offsets table
 // starting at a given offset and record it in a descriptor.
 static Expected<StrOffsetsContributionDescriptor>
 parseDWARF32StringOffsetsTableHeader(DWARFDataExtractor &DA, uint64_t Offset) {
   if (!DA.isValidOffsetForDataOfSize(Offset, 8))
-    return createStringError(errc::invalid_argument, "section offset exceeds section size");
+    return createStringError(errc::invalid_argument,
+                             "section offset exceeds section size");
 
   uint32_t ContributionSize = DA.getU32(&Offset);
   if (ContributionSize >= dwarf::DW_LENGTH_lo_reserved)
     return createStringError(errc::invalid_argument, "invalid length");
 
   uint8_t Version = DA.getU16(&Offset);
   (void)DA.getU16(&Offset); // padding
   // The encoded length includes the 2-byte version field and the 2-byte
   // padding, so we need to subtract them out when we populate the descriptor.
   return StrOffsetsContributionDescriptor(Offset, ContributionSize - 4, Version,
                                           DWARF32);
 }
 
 static Expected<StrOffsetsContributionDescriptor>
 parseDWARFStringOffsetsTableHeader(DWARFDataExtractor &DA,
                                    llvm::dwarf::DwarfFormat Format,
                                    uint64_t Offset) {
   StrOffsetsContributionDescriptor Desc;
   switch (Format) {
   case dwarf::DwarfFormat::DWARF64: {
     if (Offset < 16)
-      return createStringError(errc::invalid_argument, "insufficient space for 64 bit header prefix");
+      return createStringError(errc::invalid_argument,
+                               "insufficient space for 64 bit header prefix");
     auto DescOrError = parseDWARF64StringOffsetsTableHeader(DA, Offset - 16);
     if (!DescOrError)
       return DescOrError.takeError();
     Desc = *DescOrError;
     break;
   }
   case dwarf::DwarfFormat::DWARF32: {
     if (Offset < 8)
-      return createStringError(errc::invalid_argument, "insufficient space for 32 bit header prefix");
+      return createStringError(errc::invalid_argument,
+                               "insufficient space for 32 bit header prefix");
     auto DescOrError = parseDWARF32StringOffsetsTableHeader(DA, Offset - 8);
     if (!DescOrError)
       return DescOrError.takeError();
     Desc = *DescOrError;
     break;
   }
   }
   return Desc.validateContributionSize(DA);
 }
 
 Expected<Optional<StrOffsetsContributionDescriptor>>
 DWARFUnit::determineStringOffsetsTableContribution(DWARFDataExtractor &DA) {
   assert(!IsDWO);
   auto OptOffset = toSectionOffset(getUnitDIE().find(DW_AT_str_offsets_base));
   if (!OptOffset)
     return None;
   auto DescOrError =
       parseDWARFStringOffsetsTableHeader(DA, Header.getFormat(), *OptOffset);
   if (!DescOrError)
     return DescOrError.takeError();
   return *DescOrError;
 }
 
 Expected<Optional<StrOffsetsContributionDescriptor>>
-DWARFUnit::determineStringOffsetsTableContributionDWO(DWARFDataExtractor & DA) {
+DWARFUnit::determineStringOffsetsTableContributionDWO(DWARFDataExtractor &DA) {
   assert(IsDWO);
   uint64_t Offset = 0;
   auto IndexEntry = Header.getIndexEntry();
   const auto *C =
       IndexEntry ? IndexEntry->getContribution(DW_SECT_STR_OFFSETS) : nullptr;
   if (C)
     Offset = C->Offset;
   if (getVersion() >= 5) {
     if (DA.getData().data() == nullptr)
       return None;
     Offset += Header.getFormat() == dwarf::DwarfFormat::DWARF32 ? 8 : 16;
     // Look for a valid contribution at the given offset.
-    auto DescOrError = parseDWARFStringOffsetsTableHeader(DA, Header.getFormat(), Offset);
+    auto DescOrError =
+        parseDWARFStringOffsetsTableHeader(DA, Header.getFormat(), Offset);
     if (!DescOrError)
       return DescOrError.takeError();
     return *DescOrError;
   }
   // Prior to DWARF v5, we derive the contribution size from the
   // index table (in a package file). In a .dwo file it is simply
   // the length of the string offsets section.
   StrOffsetsContributionDescriptor Desc;