[DWARF] [NFC] Move a couple of member functions to DWARFUnit (baseclass) from DWARFCompileUnit (derived class)

[wolfgangp]

Summary

Moving member functions addSectionDelta() and addSectionLabel() into DWARFUnit, which is the base class for DWARFCompileUnit and DWARFTypeUnit. This is to prepare for generating DWARF v5 string offsets tables. We need those 2 functions in both DWARFCompileUnit and DWARFTypeUnit. NFC.

[dblaikie]

This is for DW_AT_str_offsets_base in a type unit?

Is the delta version needed? (is that modifying the existing use in split DWARF to now require a tag for str_offsets_base when none was needed before) Or only the addSectionLabel?

[wolfgangp]

In D34765#794069, @dblaikie wrote:

This is for DW_AT_str_offsets_base in a type unit?

Yes.

Is the delta version needed? (is that modifying the existing use in split DWARF to now require a tag for str_offsets_base when none was needed before) Or only the addSectionLabel?

The delta version avoids generating relocations, which we can't have for split DWARF, so for version >=5 the plan is to generate the str_offsets_base tag using addSectionLabel() for when we can use relocations and addSectionDelta() for when we can't (such as split DWARF, or Mach-O).

[dblaikie]

In D34765#794516, @wolfgangp wrote:

In D34765#794069, @dblaikie wrote:

This is for DW_AT_str_offsets_base in a type unit?

Yes.

Is the delta version needed? (is that modifying the existing use in split DWARF to now require a tag for str_offsets_base when none was needed before) Or only the addSectionLabel?

The delta version avoids generating relocations, which we can't have for split DWARF, so for version >=5 the plan is to generate the str_offsets_base tag using addSectionLabel() for when we can use relocations and addSectionDelta() for when we can't (such as split DWARF, or Mach-O).

Right, sorry I wasn't very clear - what I was asking is: In DWARF5 do .dwo CUs have a str_offsets_base? They didn't in the pre-standard Fission that's implemented in LLVM at the moment - because the value was effectively zero always.

[wolfgangp]

In D34765#794773, @dblaikie wrote:

In D34765#794516, @wolfgangp wrote:

In D34765#794069, @dblaikie wrote:

This is for DW_AT_str_offsets_base in a type unit?

Yes.

Is the delta version needed? (is that modifying the existing use in split DWARF to now require a tag for str_offsets_base when none was needed before) Or only the addSectionLabel?

The delta version avoids generating relocations, which we can't have for split DWARF, so for version >=5 the plan is to generate the str_offsets_base tag using addSectionLabel() for when we can use relocations and addSectionDelta() for when we can't (such as split DWARF, or Mach-O).

Right, sorry I wasn't very clear - what I was asking is: In DWARF5 do .dwo CUs have a str_offsets_base? They didn't in the pre-standard Fission that's implemented in LLVM at the moment - because the value was effectively zero always.

No you're right, my bad. Units in the .dwo sections (both type and CU) don't have a str_offsets_base, which implies that the .debug_str_offsets.dwo section has to consist of a monolithic table of string offsets (without the 8 or 16-byte header that's specified in section 7.26 of the DWARF 5 standard). Section 7.26 seems to say the opposite, though. It seems I'll have to clarify this with the DWARF5 folks.

In any case, I think the Delta is still needed for str_offsets_base (and addr_base) in file formats that don't seem to support cross section relocations (like Mach-O).

[dblaikie]

Good point re: Mach0.

[probinson]

In D34765#795601, @wolfgangp wrote:

No you're right, my bad. Units in the .dwo sections (both type and CU) don't have a str_offsets_base, which implies that the .debug_str_offsets.dwo section has to consist of a monolithic table of string offsets (without the 8 or 16-byte header that's specified in section 7.26 of the DWARF 5 standard). Section 7.26 seems to say the opposite, though. It seems I'll have to clarify this with the DWARF5 folks.

Worth clarifying on the dwarf-discuss list but I believe the idea is that the .dwo would have a single .debug_str_offsets.dwo contribution (complete with header), corresponding to the .debug_str.dwo contribution, and all units in the compilation would share it just like they would ordinarily share the single .debug_str section in a non-split compilation.

[wolfgangp]

In D34765#795633, @probinson wrote:

In D34765#795601, @wolfgangp wrote:

No you're right, my bad. Units in the .dwo sections (both type and CU) don't have a str_offsets_base, which implies that the .debug_str_offsets.dwo section has to consist of a monolithic table of string offsets (without the 8 or 16-byte header that's specified in section 7.26 of the DWARF 5 standard). Section 7.26 seems to say the opposite, though. It seems I'll have to clarify this with the DWARF5 folks.

Worth clarifying on the dwarf-discuss list but I believe the idea is that the .dwo would have a single .debug_str_offsets.dwo contribution (complete with header), corresponding to the .debug_str.dwo contribution, and all units in the compilation would share it just like they would ordinarily share the single .debug_str section in a non-split compilation.

So then given the lack of a str_offsets_base attribute in the unit DIEs the string indices in the .dwo units would have to start with 2 instead of 0 in order to ignore the header. Or else the (absent) attribute would have an implicit value of 8 (or 16 for DWARF64). Seems a bit odd.

[probinson]

In D34765#795663, @wolfgangp wrote:

In D34765#795633, @probinson wrote:

In D34765#795601, @wolfgangp wrote:

No you're right, my bad. Units in the .dwo sections (both type and CU) don't have a str_offsets_base, which implies that the .debug_str_offsets.dwo section has to consist of a monolithic table of string offsets (without the 8 or 16-byte header that's specified in section 7.26 of the DWARF 5 standard). Section 7.26 seems to say the opposite, though. It seems I'll have to clarify this with the DWARF5 folks.

Worth clarifying on the dwarf-discuss list but I believe the idea is that the .dwo would have a single .debug_str_offsets.dwo contribution (complete with header), corresponding to the .debug_str.dwo contribution, and all units in the compilation would share it just like they would ordinarily share the single .debug_str section in a non-split compilation.

So then given the lack of a str_offsets_base attribute in the unit DIEs the string indices in the .dwo units would have to start with 2 instead of 0 in order to ignore the header. Or else the (absent) attribute would have an implicit value of 8 (or 16 for DWARF64). Seems a bit odd.

Right, the absent attribute is implicitly 8 or 16, and you need to parse the header anyway to know whether elements are 32-bit or 64-bit.

lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

public:
   void addLabelAddress(DIE &Die, dwarf::Attribute Attribute,
                        const MCSymbol *Label);
 
   /// addLocalLabelAddress - Add a dwarf label attribute data and value using
   /// DW_FORM_addr only.
   void addLocalLabelAddress(DIE &Die, dwarf::Attribute Attribute,
                             const MCSymbol *Label);
 
-  /// addSectionDelta - Add a label delta attribute data and value.
-  DIE::value_iterator addSectionDelta(DIE &Die, dwarf::Attribute Attribute,
-                                      const MCSymbol *Hi, const MCSymbol *Lo);
-
   DwarfCompileUnit &getCU() override { return *this; }
 
   unsigned getOrCreateSourceID(StringRef FileName, StringRef DirName) override;
 
   void addImportedEntity(const DIImportedEntity* IE) {
     DIScope *Scope = IE->getScope();
     assert(Scope && "Invalid Scope encoding!");
     if (!isa<DILocalScope>(Scope))
       // No need to add imported enities that are not local declaration.
       return;
 
     auto *LocalScope = cast<DILocalScope>(Scope)->getNonLexicalBlockFileScope();
     ImportedEntities[LocalScope].push_back(IE);
   }
 
   /// addRange - Add an address range to the list of ranges for this unit.
   void addRange(RangeSpan Range);
 
   void attachLowHighPC(DIE &D, const MCSymbol *Begin, const MCSymbol *End);
 
-  /// addSectionLabel - Add a Dwarf section label attribute data and value.
-  ///
-  DIE::value_iterator addSectionLabel(DIE &Die, dwarf::Attribute Attribute,
-                                      const MCSymbol *Label,
-                                      const MCSymbol *Sec);
-
   /// \brief Find DIE for the given subprogram and attach appropriate
   /// DW_AT_low_pc and DW_AT_high_pc attributes. If there are global
   /// variables in this scope then create and insert DIEs for these
   /// variables.
   DIE &updateSubprogramScopeDIE(const DISubprogram *SP);
 
   void constructScopeDIE(LexicalScope *Scope,
                          SmallVectorImpl<DIE *> &FinalChildren);

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

if (CURanges.empty() || !SameAsPrevCU ||
        &Range.getEnd()->getSection())) {
     CURanges.push_back(Range);
     return;
   }
 
   CURanges.back().setEnd(Range.getEnd());
 }
 
-DIE::value_iterator
-DwarfCompileUnit::addSectionLabel(DIE &Die, dwarf::Attribute Attribute,
-                                  const MCSymbol *Label, const MCSymbol *Sec) {
-  if (Asm->MAI->doesDwarfUseRelocationsAcrossSections())
-    return addLabel(Die, Attribute,
-                    DD->getDwarfVersion() >= 4 ? dwarf::DW_FORM_sec_offset
-                                               : dwarf::DW_FORM_data4,
-                    Label);
-  return addSectionDelta(Die, Attribute, Label, Sec);
-}
-
 void DwarfCompileUnit::initStmtList() {
   // Define start line table label for each Compile Unit.
   MCSymbol *LineTableStartSym =
       Asm->OutStreamer->getDwarfLineTableSymbol(getUniqueID());
 
   // DW_AT_stmt_list is a offset of line number information for this
   // compile unit in debug_line section. For split dwarf this is
   // left in the skeleton CU and so not included.
void DwarfCompileUnit::constructScopeDIE(
 
   // Add children
   for (auto &I : Children)
     ScopeDIE->addChild(std::move(I));
 
   FinalChildren.push_back(std::move(ScopeDIE));
 }
 
-DIE::value_iterator
-DwarfCompileUnit::addSectionDelta(DIE &Die, dwarf::Attribute Attribute,
-                                  const MCSymbol *Hi, const MCSymbol *Lo) {
-  return Die.addValue(DIEValueAllocator, Attribute,
-                      DD->getDwarfVersion() >= 4 ? dwarf::DW_FORM_sec_offset
-                                                 : dwarf::DW_FORM_data4,
-                      new (DIEValueAllocator) DIEDelta(Hi, Lo));
-}
-
 void DwarfCompileUnit::addScopeRangeList(DIE &ScopeDIE,
                                          SmallVector<RangeSpan, 2> Range) {
   const TargetLoweringObjectFile &TLOF = Asm->getObjFileLowering();
 
   // Emit offset in .debug_range as a relocatable label. emitDIE will handle
   // emitting it appropriately.
   const MCSymbol *RangeSectionSym =
       TLOF.getDwarfRangesSection()->getBeginSymbol();

lib/CodeGen/AsmPrinter/DwarfUnit.h

public:
 
   /// Emit the header for this unit, not including the initial length field.
   virtual void emitHeader(bool UseOffsets) = 0;
 
   virtual DwarfCompileUnit &getCU() = 0;
 
   void constructTypeDIE(DIE &Buffer, const DICompositeType *CTy);
 
+  /// addSectionDelta - Add a label delta attribute data and value.
+  DIE::value_iterator addSectionDelta(DIE &Die, dwarf::Attribute Attribute,
+                                      const MCSymbol *Hi, const MCSymbol *Lo);
+
+  /// Add a Dwarf section label attribute data and value.
+  DIE::value_iterator addSectionLabel(DIE &Die, dwarf::Attribute Attribute,
+                                      const MCSymbol *Label,
+                                      const MCSymbol *Sec);
+
 protected:
   ~DwarfUnit();
 
   /// Create new static data member DIE.
   DIE *getOrCreateStaticMemberDIE(const DIDerivedType *DT);
 
   /// Look up the source ID with the given directory and source file names. If
   /// none currently exists, create a new ID and insert it in the line table.

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

void DwarfTypeUnit::emitHeader(bool UseOffsets) {
   Asm->OutStreamer->AddComment("Type Signature");
   Asm->OutStreamer->EmitIntValue(TypeSignature, sizeof(TypeSignature));
   Asm->OutStreamer->AddComment("Type DIE Offset");
   // In a skeleton type unit there is no type DIE so emit a zero offset.
   Asm->OutStreamer->EmitIntValue(Ty ? Ty->getOffset() : 0,
                                  sizeof(Ty->getOffset()));
 }
 
+DIE::value_iterator
+DwarfUnit::addSectionDelta(DIE &Die, dwarf::Attribute Attribute,
+                           const MCSymbol *Hi, const MCSymbol *Lo) {
+  return Die.addValue(DIEValueAllocator, Attribute,
+                      DD->getDwarfVersion() >= 4 ? dwarf::DW_FORM_sec_offset
+                                                 : dwarf::DW_FORM_data4,
+                      new (DIEValueAllocator) DIEDelta(Hi, Lo));
+}
+
+DIE::value_iterator
+DwarfUnit::addSectionLabel(DIE &Die, dwarf::Attribute Attribute,
+                           const MCSymbol *Label, const MCSymbol *Sec) {
+  if (Asm->MAI->doesDwarfUseRelocationsAcrossSections())
+    return addLabel(Die, Attribute,
+                    DD->getDwarfVersion() >= 4 ? dwarf::DW_FORM_sec_offset
+                                               : dwarf::DW_FORM_data4,
+                    Label);
+  return addSectionDelta(Die, Attribute, Label, Sec);
+}
+
 bool DwarfTypeUnit::isDwoUnit() const {
   // Since there are no skeleton type units, all type units are dwo type units
   // when split DWARF is being used.
   return DD->useSplitDwarf();
 }
 
 void DwarfTypeUnit::addGlobalName(StringRef Name, const DIE &Die,
                                   const DIScope *Context) {