Store TypeUnits in a SmallVector<DWARFUnitSection> instead of a single DWARFUnitSection.

[friss]

Summary

There will be multiple TypeUnits in an unlinked object that will be extracted
from different sections. Now that we have DWARFUnitSection that is supposed
to represent an input section, we need a DWARFUnitSection<TypeUnit> per
input .debug_types section.

Once this is done, the interface is homogenous and we can move the Section
parsing code into DWARFUnitSection.

[dblaikie]

Looks fine to me - some /totally optional/ comments, just if you think they're improvements.

lib/DebugInfo/DWARFContext.cpp

87 ↗

(On Diff #14042)

getNumTypeUnits is now going to be potentially misleading - it'll specify the number of type unit sections, not the number of type units.

That happens to be the same for type units (for well constructed DWARF - but again, we might be dealing with bad DWARF when using this tool, so it's interesting to consider the possibility of present though empty type_units sections, etc) but isn't for DWO type units (which, when I first implemented them, went in separate sections - but then I later fixed it per the DWARF Fission spec, to put them all in the same section).

Might be a place where the 'empty(range)' template in that patch I showed previously could be handy - then there's no need for the extra getNum* functions, simply size(dwo_type_unit_sections()) - same behavior, but not quite as misleading.

Honestly though, I'm not sure how much value the "getNum*" calls provide here, just printing the section header seems pretty benign/possibly desired.

Some food for thought.

383 ↗

(On Diff #14042)

I'm sure I've used this idiom before - I'm not sure if it's nicer than something more like:

Foo f;
...
v.push_back(std::move(f));

(which I'm sure I've used as well)

lib/DebugInfo/DWARFContext.h

33 ↗

(On Diff #14042)

If it's going to have a small size of 1, maybe just use std::vector?

(granted I'm not quite as averse to the standard containers as some other LLVM contributors/owners - so opinions certainly differ)

41 ↗

(On Diff #14042)

same here

[samsonov]

LGTM

[friss]

lib/DebugInfo/DWARFContext.cpp

87 ↗

(On Diff #14042)

Yeah I agree. I won't change it right now, because I think all the getNum methods will disappear anyway or get renamed(they are never used to actually get the number of units, but just to know if there is at least one).

383 ↗

(On Diff #14042)

I think I prefer the one I used. I think can be more efficient depending on the actual move semantics of the object, but I'm not able to come up with a simple example were it really would matter.

lib/DebugInfo/DWARFContext.h

33 ↗

(On Diff #14042)

I used the idiom that the Unit lists were using before we moved them into DWARFUnitSection. I might change that, we can't predict an actual average size for the vector, thus SmallVector won't bring us anything.

[dblaikie]

(Alexey's already signed off here - but thanks for following up on my comments, sounds fine)

[friss]

Closed by commit rL218513 (authored by @friss).

llvm/trunk/lib/DebugInfo/DWARFContext.h

 
 /// DWARFContext
 /// This data structure is the top level entity that deals with dwarf debug
 /// information parsing. The actual data is supplied through pure virtual
 /// methods that a concrete implementation provides.
 class DWARFContext : public DIContext {
 
   DWARFUnitSection<DWARFCompileUnit> CUs;
-  DWARFUnitSection<DWARFTypeUnit> TUs;
+  SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> TUs;
   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;
 
   DWARFUnitSection<DWARFCompileUnit> DWOCUs;
-  DWARFUnitSection<DWARFTypeUnit> DWOTUs;
+  SmallVector<DWARFUnitSection<DWARFTypeUnit>,1> DWOTUs;
   std::unique_ptr<DWARFDebugAbbrev> AbbrevDWO;
   std::unique_ptr<DWARFDebugLocDWO> LocDWO;
 
   DWARFContext(DWARFContext &) LLVM_DELETED_FUNCTION;
   DWARFContext &operator=(DWARFContext &) LLVM_DELETED_FUNCTION;
 
   /// Read compile units from the debug_info section (if necessary)
   /// and store them in CUs.
public:
   static bool classof(const DIContext *DICtx) {
     return DICtx->getKind() == CK_DWARF;
   }
 
   void dump(raw_ostream &OS, DIDumpType DumpType = DIDT_All) override;
 
   typedef DWARFUnitSection<DWARFCompileUnit>::iterator_range cu_iterator_range;
   typedef DWARFUnitSection<DWARFTypeUnit>::iterator_range tu_iterator_range;
+  typedef iterator_range<SmallVectorImpl<DWARFUnitSection<DWARFTypeUnit>>::iterator> tu_section_iterator_range;
 
   /// Get compile units in this context.
   cu_iterator_range compile_units() {
     parseCompileUnits();
     return cu_iterator_range(CUs.begin(), CUs.end());
   }
 
   /// Get type units in this context.
-  tu_iterator_range type_units() {
+  tu_section_iterator_range type_unit_sections() {
     parseTypeUnits();
-    return tu_iterator_range(TUs.begin(), TUs.end());
+    return tu_section_iterator_range(TUs.begin(), TUs.end());
   }
 
   /// Get compile units in the DWO context.
   cu_iterator_range dwo_compile_units() {
     parseDWOCompileUnits();
     return cu_iterator_range(DWOCUs.begin(), DWOCUs.end());
   }
 
   /// Get type units in the DWO context.
-  tu_iterator_range dwo_type_units() {
+  tu_section_iterator_range dwo_type_unit_sections() {
     parseDWOTypeUnits();
-    return tu_iterator_range(DWOTUs.begin(), DWOTUs.end());
+    return tu_section_iterator_range(DWOTUs.begin(), DWOTUs.end());
   }
 
   /// Get the number of compile units in this context.
   unsigned getNumCompileUnits() {
     parseCompileUnits();
     return CUs.size();
   }
 

llvm/trunk/lib/DebugInfo/DWARFContext.cpp

if ((DumpType == DIDT_All || DumpType == DIDT_InfoDwo) &&
       getNumDWOCompileUnits()) {
     OS << "\n.debug_info.dwo contents:\n";
     for (const auto &DWOCU : dwo_compile_units())
       DWOCU->dump(OS);
   }
 
   if ((DumpType == DIDT_All || DumpType == DIDT_Types) && getNumTypeUnits()) {
     OS << "\n.debug_types contents:\n";
-    for (const auto &TU : type_units())
+    for (const auto &TUS : type_unit_sections())
+      for (const auto &TU : TUS)
         TU->dump(OS);
   }
 
   if ((DumpType == DIDT_All || DumpType == DIDT_TypesDwo) &&
       getNumDWOTypeUnits()) {
     OS << "\n.debug_types.dwo contents:\n";
-    for (const auto &DWOTU : dwo_type_units())
+    for (const auto &DWOTUS : dwo_type_unit_sections())
+      for (const auto &DWOTU : DWOTUS)
         DWOTU->dump(OS);
   }
 
   if (DumpType == DIDT_All || DumpType == DIDT_Loc) {
     OS << "\n.debug_loc contents:\n";
     getDebugLoc()->dump(OS);
   }
 
   if (DumpType == DIDT_All || DumpType == DIDT_LocDwo) {

 
 void DWARFContext::parseTypeUnits() {
   if (!TUs.empty())
     return;
   for (const auto &I : getTypesSections()) {
     uint32_t offset = 0;
     const DataExtractor &DIData =
         DataExtractor(I.second.Data, isLittleEndian(), 0);
+    TUs.push_back(DWARFUnitSection<DWARFTypeUnit>());
+    auto &TUS = TUs.back();
     while (DIData.isValidOffset(offset)) {
       std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,
            getDebugAbbrev(), I.second.Data, getRangeSection(),
            getStringSection(), StringRef(), getAddrSection(),
-           &I.second.Relocs, isLittleEndian(), TUs));
+           &I.second.Relocs, isLittleEndian(), TUS));
       if (!TU->extract(DIData, &offset))
         break;
-      TUs.push_back(std::move(TU));
-      offset = TUs.back()->getNextUnitOffset();
+      TUS.push_back(std::move(TU));
+      offset = TUS.back()->getNextUnitOffset();
     }
   }
 }
 
 void DWARFContext::parseDWOCompileUnits() {
   if (!DWOCUs.empty())
     return;
   uint32_t offset = 0;

 
 void DWARFContext::parseDWOTypeUnits() {
   if (!DWOTUs.empty())
     return;
   for (const auto &I : getTypesDWOSections()) {
     uint32_t offset = 0;
     const DataExtractor &DIData =
         DataExtractor(I.second.Data, isLittleEndian(), 0);
+    DWOTUs.push_back(DWARFUnitSection<DWARFTypeUnit>());
+    auto &TUS = DWOTUs.back();
     while (DIData.isValidOffset(offset)) {
       std::unique_ptr<DWARFTypeUnit> TU(new DWARFTypeUnit(*this,
           getDebugAbbrevDWO(), I.second.Data, getRangeDWOSection(),
           getStringDWOSection(), getStringOffsetDWOSection(), getAddrSection(),
-          &I.second.Relocs, isLittleEndian(), DWOTUs));
+          &I.second.Relocs, isLittleEndian(), TUS));
       if (!TU->extract(DIData, &offset))
         break;
-      DWOTUs.push_back(std::move(TU));
-      offset = DWOTUs.back()->getNextUnitOffset();
+      TUS.push_back(std::move(TU));
+      offset = TUS.back()->getNextUnitOffset();
     }
   }
 }
 
 DWARFCompileUnit *DWARFContext::getCompileUnitForOffset(uint32_t Offset) {
   parseCompileUnits();
   return CUs.getUnitForOffset(Offset);
 }