[DebugInfo/DWARF] [4/4] De-segregate type units and compile units. NFC

[probinson]

Summary

This is Patch 4 of 4 NFC refactorings to handle type units and compile
units more consistently and with less concern about the object-file
section that they came from.

Patch 1 replaces the templated DWARFUnitSection with a non-templated
version. That is, instead of being a SmallVector of pointers to a
specific unit kind, it is now a SmallVector of pointers to the base
class for both type and compile units. Virtual methods are magic.

Patch 2 takes the existing std::deque<DWARFUnitSection> for type units
and makes it a simple DWARFUnitSection, simplifying the handling of
type units and making it more consistent with compile units.

Patch 3 simply renames DWARFUnitSection to DWARFUnitVector, as the
object-file section of a unit is nearly irrelevant now.

Patch 4 combines separate DWARFUnitVectors for compile and type units
into a single DWARFUnitVector that contains both. For now the
implementation distinguishes compile units from type units by putting
all compile units at the front of the vector, reflecting the DWARF v4
distinction between .debug_info and .debug_types sections. A future
patch will change this to allow the free mixing of unit kinds, as is
specified by DWARF v5.

[aprantl]

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

107 ↗

(On Diff #157072)

Why is Units capitalized? It's neither a type name and or a concept in DWARF (a compile/type unit is).

126 ↗

(On Diff #157072)

Doxygen comment that explains that this returns all kinds of units?

[probinson]

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

107 ↗

(On Diff #157072)

Because the old comment had it capitalized. I will fix it.

126 ↗

(On Diff #157072)

This entire class hierarchy could use doxygen-izing.

[JDevlieghere]

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

126 ↗

(On Diff #157072)

I guess it's slightly more pressing here, as otherwise you'd be left to guess whether it includes the DWO kind?

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

555 ↗

(On Diff #157072)

I recently learned about this "trick" where you define an enum like enum { parseLazy = true };. Not sure if it's worth it here but I (personally) think it's nicer than the comment.

[probinson]

llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h

126 ↗

(On Diff #157072)

Will doxygen-ize all these new methods to clarify.

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

555 ↗

(On Diff #157072)

I can do something along those lines.

[probinson]

Rebase. Add to and improve commentary.

[probinson]

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

555 ↗

(On Diff #157072)

Oops forgot this one. Will try again before committing.

[aprantl]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

101 ↗

(On Diff #158304)

Side note:I find this loop to be a bit hard to follow... but it's been this way since before you touched it.

107 ↗

(On Diff #158304)

might be nice to comment what the break means (did the parsing fail, are we at the end?).

[probinson]

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

101 ↗

(On Diff #158304)

And it took me the longest time to work out what it was doing; I hope the comment helps. Coming up with a better name for I might aid readability too. Another one for The List.

107 ↗

(On Diff #158304)

Right, it means parsing failed. The isValidOffset at the top decides when we've reached the end of the section. I'll add another comment here.

llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFContext.h

 /// Callback should return Halt if client application wants to stop
 /// object parsing, or should return Continue otherwise.
 enum class ErrorPolicy { Halt, Continue };
 
 /// DWARFContext
 /// This data structure is the top level entity that deals with dwarf debug
 /// information parsing. The actual data is supplied through DWARFObj.
 class DWARFContext : public DIContext {
-  DWARFUnitVector CUs;
-  DWARFUnitVector TUs;
+  DWARFUnitVector NormalUnits;
   std::unique_ptr<DWARFUnitIndex> CUIndex;
   std::unique_ptr<DWARFGdbIndex> GdbIndex;
   std::unique_ptr<DWARFUnitIndex> TUIndex;
   std::unique_ptr<DWARFDebugAbbrev> Abbrev;
   std::unique_ptr<DWARFDebugLoc> Loc;
   std::unique_ptr<DWARFDebugAranges> Aranges;
   std::unique_ptr<DWARFDebugLine> Line;
   std::unique_ptr<DWARFDebugFrame> DebugFrame;
   std::unique_ptr<DWARFDebugFrame> EHFrame;
   std::unique_ptr<DWARFDebugMacro> Macro;
   std::unique_ptr<DWARFDebugNames> Names;
   std::unique_ptr<AppleAcceleratorTable> AppleNames;
   std::unique_ptr<AppleAcceleratorTable> AppleTypes;
   std::unique_ptr<AppleAcceleratorTable> AppleNamespaces;
   std::unique_ptr<AppleAcceleratorTable> AppleObjC;
 
-  DWARFUnitVector DWOCUs;
-  DWARFUnitVector DWOTUs;
+  DWARFUnitVector DWOUnits;
   std::unique_ptr<DWARFDebugAbbrev> AbbrevDWO;
   std::unique_ptr<DWARFDebugLocDWO> LocDWO;
 
   /// The maximum DWARF version of all units.
   unsigned MaxVersion = 0;
 
   struct DWOFile {
     object::OwningBinary<object::ObjectFile> File;
     std::unique_ptr<DWARFContext> Context;
   };
   StringMap<std::weak_ptr<DWOFile>> DWOFiles;
   std::weak_ptr<DWOFile> DWP;
   bool CheckedForDWP = false;
   std::string DWPName;
 
   std::unique_ptr<MCRegisterInfo> RegInfo;
 
   /// Read compile units from the debug_info section (if necessary)
-  /// and store them in CUs.
-  void parseCompileUnits();
-
-  /// Read type units from the debug_types sections (if necessary)
-  /// and store them in TUs.
-  void parseTypeUnits();
+  /// and type units from the debug_types sections (if necessary)
+  /// and store them in NormalUnits.
+  void parseNormalUnits();
 
   /// Read compile units from the debug_info.dwo section (if necessary)
-  /// and store them in DWOCUs.
-  void parseDWOCompileUnits();
-
-  /// Read type units from the debug_types.dwo section (if necessary)
-  /// and store them in DWOTUs.
-  void parseDWOTypeUnits();
+  /// and type units from the debug_types.dwo section (if necessary)
+  /// and store them in DWOUnits.
+  /// If \p Lazy is true, set up to parse but don't actually parse them.
+  enum { EagerParse = false, LazyParse = true };
+  void parseDWOUnits(bool Lazy = false);
 
-protected:
   std::unique_ptr<const DWARFObject> DObj;
 
 public:
   DWARFContext(std::unique_ptr<const DWARFObject> DObj,
                std::string DWPName = "");
   ~DWARFContext();
 
   DWARFContext(DWARFContext &) = delete;
void dump(raw_ostream &OS, DIDumpOptions DumpOpts) override {
     dump(OS, DumpOpts, DumpOffsets);
   }
 
   bool verify(raw_ostream &OS, DIDumpOptions DumpOpts = {}) override;
 
   using cu_iterator_range = DWARFUnitVector::iterator_range;
   using tu_iterator_range = DWARFUnitVector::iterator_range;
 
-  /// Get compile units in this context.
-  cu_iterator_range compile_units() {
-    parseCompileUnits();
-    return cu_iterator_range(CUs.begin(), CUs.end());
+  /// Get units from .debug_info in this context.
+  cu_iterator_range info_section_units() {
+    parseNormalUnits();
+    return cu_iterator_range(NormalUnits.begin(),
+                             NormalUnits.begin() +
+                                 NormalUnits.getNumInfoUnits());
   }
 
+  /// Get units from .debug_types in this context.
+  tu_iterator_range types_section_units() {
+    parseNormalUnits();
+    return tu_iterator_range(
+        NormalUnits.begin() + NormalUnits.getNumInfoUnits(), NormalUnits.end());
+  }
+
+  /// Get compile units in this context.
+  cu_iterator_range compile_units() { return info_section_units(); }
+
   /// Get type units in this context.
-  tu_iterator_range type_units() {
-    parseTypeUnits();
-    return tu_iterator_range(TUs.begin(), TUs.end());
+  tu_iterator_range type_units() { return types_section_units(); }
+
+  /// Get units from .debug_info..dwo in the DWO context.
+  cu_iterator_range dwo_info_section_units() {
+    parseDWOUnits();
+    return cu_iterator_range(DWOUnits.begin(),
+                             DWOUnits.begin() + DWOUnits.getNumInfoUnits());
   }
 
-  /// Get compile units in the DWO context.
-  cu_iterator_range dwo_compile_units() {
-    parseDWOCompileUnits();
-    return cu_iterator_range(DWOCUs.begin(), DWOCUs.end());
+  /// Get units from .debug_types.dwo in the DWO context.
+  tu_iterator_range dwo_types_section_units() {
+    parseDWOUnits();
+    return tu_iterator_range(DWOUnits.begin() + DWOUnits.getNumInfoUnits(),
+                             DWOUnits.end());
   }
 
+  /// Get compile units in the DWO context.
+  cu_iterator_range dwo_compile_units() { return dwo_info_section_units(); }
+
   /// Get type units in the DWO context.
-  tu_iterator_range dwo_type_units() {
-    parseDWOTypeUnits();
-    return tu_iterator_range(DWOTUs.begin(), DWOTUs.end());
-  }
+  tu_iterator_range dwo_type_units() { return dwo_types_section_units(); }
 
   /// Get the number of compile units in this context.
   unsigned getNumCompileUnits() {
-    parseCompileUnits();
-    return CUs.size();
+    parseNormalUnits();
+    return NormalUnits.getNumInfoUnits();
   }
 
-  /// Get the number of compile units in this context.
+  /// Get the number of type units in this context.
   unsigned getNumTypeUnits() {
-    parseTypeUnits();
-    return TUs.size();
+    parseNormalUnits();
+    return NormalUnits.getNumTypesUnits();
   }
 
   /// Get the number of compile units in the DWO context.
   unsigned getNumDWOCompileUnits() {
-    parseDWOCompileUnits();
-    return DWOCUs.size();
+    parseDWOUnits();
+    return DWOUnits.getNumInfoUnits();
   }
 
-  /// Get the number of compile units in the DWO context.
+  /// Get the number of type units in the DWO context.
   unsigned getNumDWOTypeUnits() {
-    parseDWOTypeUnits();
-    return DWOTUs.size();
+    parseDWOUnits();
+    return DWOUnits.getNumTypesUnits();
   }
 
   /// Get the unit at the specified index.
   DWARFUnit *getUnitAtIndex(unsigned index) {
-    parseCompileUnits();
-    return CUs[index].get();
+    parseNormalUnits();
+    return NormalUnits[index].get();
   }
 
   /// Get the unit at the specified index for the DWO units.
   DWARFUnit *getDWOUnitAtIndex(unsigned index) {
-    parseDWOCompileUnits();
-    return DWOCUs[index].get();
+    parseDWOUnits();
+    return DWOUnits[index].get();
   }
 
   DWARFCompileUnit *getDWOCompileUnitForHash(uint64_t Hash);
 
   /// Return the compile unit that includes an offset (relative to .debug_info).
   DWARFCompileUnit *getCompileUnitForOffset(uint32_t Offset);
 
   /// Get a DIE given an exact offset.

llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFUnit.h

public:
   uint8_t getSize() const { return Size; }
   // FIXME: Support DWARF64.
   uint32_t getNextUnitOffset() const { return Offset + Length + 4; }
 };
 
 const DWARFUnitIndex &getDWARFUnitIndex(DWARFContext &Context,
                                         DWARFSectionKind Kind);
 
-/// Describes one section's Units.
+/// Describe a collection of units. Intended to hold all units either from
+/// .debug_info and .debug_types, or from .debug_info.dwo and .debug_types.dwo.
 class DWARFUnitVector final : public SmallVector<std::unique_ptr<DWARFUnit>, 1> {
-  std::function<std::unique_ptr<DWARFUnit>(uint32_t, const DWARFSection *)>
+  std::function<std::unique_ptr<DWARFUnit>(uint32_t, DWARFSectionKind,
+                                           const DWARFSection *)>
       Parser;
+  unsigned NumInfoUnits = 0;
 
 public:
   using UnitVector = SmallVectorImpl<std::unique_ptr<DWARFUnit>>;
   using iterator = typename UnitVector::iterator;
   using iterator_range = llvm::iterator_range<typename UnitVector::iterator>;
 
   DWARFUnit *getUnitForOffset(uint32_t Offset) const;
   DWARFUnit *getUnitForIndexEntry(const DWARFUnitIndex::Entry &E);
+
+  /// Read units from a .debug_info or .debug_types section.  Calls made
+  /// before finishedInfoUnits() are assumed to be for .debug_info sections,
+  /// calls after finishedInfoUnits() are for .debug_types sections.  Caller
+  /// must not mix calls to addUnitsForSection and addUnitsForDWOSection.
   void addUnitsForSection(DWARFContext &C, const DWARFSection &Section,
                           DWARFSectionKind SectionKind);
+  /// Read units from a .debug_info.dwo or .debug_types.dwo section.  Calls
+  /// made before finishedInfoUnits() are assumed to be for .debug_info.dwo
+  /// sections, calls after finishedInfoUnits() are for .debug_types.dwo
+  /// sections.  Caller must not mix calls to addUnitsForSection and
+  /// addUnitsForDWOSection.
   void addUnitsForDWOSection(DWARFContext &C, const DWARFSection &DWOSection,
                              DWARFSectionKind SectionKind, bool Lazy = false);
 
+  /// Returns number of all units held by this instance.
+  unsigned getNumUnits() { return size(); }
+  /// Returns number of units from all .debug_info[.dwo] sections.
+  unsigned getNumInfoUnits() { return NumInfoUnits; }
+  /// Returns number of units from all .debug_types[.dwo] sections.
+  unsigned getNumTypesUnits() { return size() - NumInfoUnits; }
+  /// Indicate that parsing .debug_info[.dwo] is done, and remaining units
+  /// will be from .debug_types[.dwo].
+  void finishedInfoUnits() { NumInfoUnits = size(); }
+
 private:
   void addUnitsImpl(DWARFContext &Context, const DWARFObject &Obj,
                     const DWARFSection &Section, const DWARFDebugAbbrev *DA,
                     const DWARFSection *RS, StringRef SS,
                     const DWARFSection &SOS, const DWARFSection *AOS,
                     const DWARFSection &LS, bool LE, bool IsDWO, bool Lazy,
                     DWARFSectionKind SectionKind);
 };
DWARFUnit(DWARFContext &Context, const DWARFSection &Section,
             const DWARFDebugAbbrev *DA, const DWARFSection *RS, StringRef SS,
             const DWARFSection &SOS, const DWARFSection *AOS,
             const DWARFSection &LS, bool LE, bool IsDWO,
             const DWARFUnitVector &UnitVector);
 
   virtual ~DWARFUnit();
 
   DWARFContext& getContext() const { return Context; }
+  const DWARFSection &getInfoSection() const { return InfoSection; }
   uint32_t getOffset() const { return Header.getOffset(); }
   const dwarf::FormParams &getFormParams() const {
     return Header.getFormParams();
   }
   uint16_t getVersion() const { return Header.getVersion(); }
   uint8_t getAddressByteSize() const { return Header.getAddressByteSize(); }
   uint8_t getRefAddrByteSize() const { return Header.getRefAddrByteSize(); }
   uint8_t getDwarfOffsetByteSize() const {

llvm/trunk/lib/DebugInfo/DWARF/DWARFContext.cpp

if (shouldDump(IsExplicit, Name, DIDT_ID_DebugInfo, Section.Data)) {
         getDIEForOffset(DumpOffset.getValue())
             .dump(OS, 0, DumpOpts.noImplicitRecursion());
       else
         for (const auto &CU : CUs)
           CU->dump(OS, DumpOpts);
     }
   };
   dumpDebugInfo(Explicit, ".debug_info", DObj->getInfoSection(),
-                compile_units());
+                info_section_units());
   dumpDebugInfo(ExplicitDWO, ".debug_info.dwo", DObj->getInfoDWOSection(),
-                dwo_compile_units());
+                dwo_info_section_units());
 
   auto dumpDebugType = [&](const char *Name, tu_iterator_range TUs) {
     OS << '\n' << Name << " contents:\n";
     DumpOffset = DumpOffsets[DIDT_ID_DebugTypes];
     for (const auto &TU : TUs)
       if (DumpOffset)
         TU->getDIEForOffset(*DumpOffset)
             .dump(OS, 0, DumpOpts.noImplicitRecursion());
       else
         TU->dump(OS, DumpOpts);
   };
   if ((DumpType & DIDT_DebugTypes)) {
     if (Explicit || getNumTypeUnits())
-      dumpDebugType(".debug_types", type_units());
+      dumpDebugType(".debug_types", types_section_units());
     if (ExplicitDWO || getNumDWOTypeUnits())
-      dumpDebugType(".debug_types.dwo", dwo_type_units());
+      dumpDebugType(".debug_types.dwo", dwo_types_section_units());
   }
 
   if (shouldDump(Explicit, ".debug_loc", DIDT_ID_DebugLoc,
                  DObj->getLocSection().Data)) {
     getDebugLoc()->dump(OS, getRegisterInfo(), DumpOffset);
   }
   if (shouldDump(ExplicitDWO, ".debug_loc.dwo", DIDT_ID_DebugLoc,
                  DObj->getLocDWOSection().Data)) {
if (shouldDump(Explicit, ".apple_objc", DIDT_ID_AppleObjC,
                  DObj->getAppleObjCSection().Data))
     getAppleObjC().dump(OS);
   if (shouldDump(Explicit, ".debug_names", DIDT_ID_DebugNames,
                  DObj->getDebugNamesSection().Data))
     getDebugNames().dump(OS);
 }
 
 DWARFCompileUnit *DWARFContext::getDWOCompileUnitForHash(uint64_t Hash) {
-  if (DWOCUs.empty())
-    DWOCUs.addUnitsForDWOSection(*this, DObj->getInfoDWOSection(), DW_SECT_INFO,
-                                 true);
+  parseDWOUnits(LazyParse);
 
   if (const auto &CUI = getCUIndex()) {
     if (const auto *R = CUI.getFromHash(Hash))
-      return dyn_cast_or_null<DWARFCompileUnit>(DWOCUs.getUnitForIndexEntry(*R));
+      return dyn_cast_or_null<DWARFCompileUnit>(
+          DWOUnits.getUnitForIndexEntry(*R));
     return nullptr;
   }
 
   // If there's no index, just search through the CUs in the DWO - there's
   // probably only one unless this is something like LTO - though an in-process
   // built/cached lookup table could be used in that case to improve repeated
   // lookups of different CUs in the DWO.
   for (const auto &DWOCU : dwo_compile_units()) {
     // Might not have parsed DWO ID yet.
     if (!DWOCU->getDWOId()) {
       if (Optional<uint64_t> DWOId =
           toUnsigned(DWOCU->getUnitDIE().find(DW_AT_GNU_dwo_id)))
         DWOCU->setDWOId(*DWOId);
       else
         // No DWO ID?
         continue;
     }
     if (DWOCU->getDWOId() == Hash)
       return dyn_cast<DWARFCompileUnit>(DWOCU.get());
   }
   return nullptr;
 }
 
 DWARFDie DWARFContext::getDIEForOffset(uint32_t Offset) {
-  parseCompileUnits();
-  if (auto *CU = CUs.getUnitForOffset(Offset))
+  parseNormalUnits();
+  if (auto *CU = NormalUnits.getUnitForOffset(Offset))
     return CU->getDIEForOffset(Offset);
   return DWARFDie();
 }
 
 bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) {
   bool Success = true;
   DWARFVerifier verifier(OS, *this, DumpOpts);
 
Expected<const DWARFDebugLine::LineTable *> DWARFContext::getLineTableForUnit(
 
   // We have to parse it first.
   DWARFDataExtractor lineData(*DObj, U->getLineSection(), isLittleEndian(),
                               U->getAddressByteSize());
   return Line->getOrParseLineTable(lineData, stmtOffset, *this, U,
                                    RecoverableErrorCallback);
 }
 
-void DWARFContext::parseCompileUnits() {
-  if (!CUs.empty())
-    return;
-  CUs.addUnitsForSection(*this, DObj->getInfoSection(), DW_SECT_INFO);
-}
-
-void DWARFContext::parseTypeUnits() {
-  if (!TUs.empty())
+void DWARFContext::parseNormalUnits() {
+  if (!NormalUnits.empty())
     return;
+  NormalUnits.addUnitsForSection(*this, DObj->getInfoSection(), DW_SECT_INFO);
+  NormalUnits.finishedInfoUnits();
   DObj->forEachTypesSections([&](const DWARFSection &S) {
-    TUs.addUnitsForSection(*this, S, DW_SECT_TYPES);
+    NormalUnits.addUnitsForSection(*this, S, DW_SECT_TYPES);
   });
 }
 
-void DWARFContext::parseDWOCompileUnits() {
-  if (!DWOCUs.empty())
-    return;
-  DWOCUs.addUnitsForDWOSection(*this, DObj->getInfoDWOSection(), DW_SECT_INFO);
-}
-
-void DWARFContext::parseDWOTypeUnits() {
-  if (!DWOTUs.empty())
+void DWARFContext::parseDWOUnits(bool Lazy) {
+  if (!DWOUnits.empty())
     return;
+  DWOUnits.addUnitsForDWOSection(*this, DObj->getInfoDWOSection(), DW_SECT_INFO,
+                                 Lazy);
+  DWOUnits.finishedInfoUnits();
   DObj->forEachTypesDWOSections([&](const DWARFSection &S) {
-    DWOTUs.addUnitsForDWOSection(*this, S, DW_SECT_TYPES);
+    DWOUnits.addUnitsForDWOSection(*this, S, DW_SECT_TYPES, Lazy);
   });
 }
 
 DWARFCompileUnit *DWARFContext::getCompileUnitForOffset(uint32_t Offset) {
-  parseCompileUnits();
-  return dyn_cast_or_null<DWARFCompileUnit>(CUs.getUnitForOffset(Offset));
+  parseNormalUnits();
+  return dyn_cast_or_null<DWARFCompileUnit>(
+      NormalUnits.getUnitForOffset(Offset));
 }
 
 DWARFCompileUnit *DWARFContext::getCompileUnitForAddress(uint64_t Address) {
   // First, get the offset of the compile unit.
   uint32_t CUOffset = getDebugAranges()->findAddress(Address);
   // Retrieve the compile unit.
   return getCompileUnitForOffset(CUOffset);
 }

llvm/trunk/lib/DebugInfo/DWARF/DWARFUnit.cpp

 void DWARFUnitVector::addUnitsImpl(
     DWARFContext &Context, const DWARFObject &Obj, const DWARFSection &Section,
     const DWARFDebugAbbrev *DA, const DWARFSection *RS, StringRef SS,
     const DWARFSection &SOS, const DWARFSection *AOS, const DWARFSection &LS,
     bool LE, bool IsDWO, bool Lazy, DWARFSectionKind SectionKind) {
   DWARFDataExtractor Data(Obj, Section, LE, 0);
   // Lazy initialization of Parser, now that we have all section info.
   if (!Parser) {
-    const DWARFUnitIndex *Index = nullptr;
-    if (IsDWO)
-      Index = &getDWARFUnitIndex(Context, SectionKind);
     Parser = [=, &Context, &Obj, &Section, &SOS, &LS](
-                 uint32_t Offset,
+                 uint32_t Offset, DWARFSectionKind SectionKind,
                  const DWARFSection *CurSection) -> std::unique_ptr<DWARFUnit> {
       const DWARFSection &InfoSection = CurSection ? *CurSection : Section;
       DWARFDataExtractor Data(Obj, InfoSection, LE, 0);
       if (!Data.isValidOffset(Offset))
         return nullptr;
+      const DWARFUnitIndex *Index = nullptr;
+      if (IsDWO)
+        Index = &getDWARFUnitIndex(Context, SectionKind);
       DWARFUnitHeader Header;
       if (!Header.extract(Context, Data, &Offset, SectionKind, Index))
         return nullptr;
       std::unique_ptr<DWARFUnit> U;
       if (Header.isTypeUnit())
         U = llvm::make_unique<DWARFTypeUnit>(Context, InfoSection, Header, DA,
                                              RS, SS, SOS, AOS, LS, LE, IsDWO,
                                              *this);
       else
         U = llvm::make_unique<DWARFCompileUnit>(Context, InfoSection, Header,
                                                 DA, RS, 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
+  // within a section, although not necessarily within the object file,
+  // even if we do lazy parsing.
   auto I = this->begin();
   uint32_t Offset = 0;
   while (Data.isValidOffset(Offset)) {
-    if (I != this->end() && (*I)->getOffset() == Offset) {
+    if (I != this->end() &&
+        (&(*I)->getInfoSection() != &Section || (*I)->getOffset() == Offset)) {
       ++I;
       continue;
     }
-    auto U = Parser(Offset, &Section);
+    auto U = Parser(Offset, SectionKind, &Section);
+    // If parsing failed, we're done with this section.
     if (!U)
       break;
     Offset = U->getNextUnitOffset();
     I = std::next(this->insert(I, std::move(U)));
   }
 }
 
 DWARFUnit *DWARFUnitVector::getUnitForOffset(uint32_t Offset) const {
auto *CU = std::upper_bound(
     return LHS < RHS->getNextUnitOffset();
   });
   if (CU != this->end() && (*CU)->getOffset() <= Offset)
     return CU->get();
 
   if (!Parser)
     return nullptr;
 
-  auto U = Parser(Offset, nullptr);
+  auto U = Parser(Offset, DW_SECT_INFO, nullptr);
   if (!U)
     U = nullptr;
 
   auto *NewCU = U.get();
   this->insert(CU, std::move(U));
   return NewCU;
 }