[llvm-dwp] Refactor handling of section identifiers. NFCI.

[ikudrin]

Summary

There is a number of places in llvm-dwp.cpp where a section identifier is translated into an index of an internal array of section contributions, and another place where the index is converted to an on-disk value. All these places use direct expressions like <id> - DW_SECT_INFO or <index> + DW_SECT_INFO, exploiting the fact that DW_SECT_INFO is the minimum valid value of that kind.

The patch adds distinct functions for that translation. The goal is to make the code more readable and to prepare it to support index sections of new versions, where the numeric scheme of section indexes is changed.

[dblaikie]

I don't /mind/ this refactor (though the SectionKindToIndex function should use DW_SECT_INFO rather than the magic number 1) - but I'm not sure it's needed/especially useful given my position (which aligns with @labath's) on the original review D75929. So let's shelve this while we continue the design discussion there & see how it shakes out.

[ikudrin]

• Use a named constant DW_SECT_INFO in the translation function instead of a numeric constant 1.
• Rename the functions.

It seems like we generally consent about the way proposed in D75929. I am updating this patch because other changes slightly depend on it.

[ikudrin]

Ping.

[dblaikie]

llvm/tools/llvm-dwp/llvm-dwp.cpp

258–259

LLVM doesn't generally use top level const on local variables - drop the "const" here.

llvm/tools/llvm-dwp/llvm-dwp.cpp

 
 struct UnitIndexEntry {
   DWARFUnitIndex::Entry::SectionContribution Contributions[8];
   std::string Name;
   std::string DWOName;
   StringRef DWPName;
 };
 
+// Convert a section identifier into the index to use with
+// UnitIndexEntry::Contributions.
+static unsigned SectionKindToIndex(DWARFSectionKind Kind) {
+  assert(static_cast<unsigned>(Kind) >= 1);
+  return static_cast<unsigned>(Kind) - 1;
+}
+
+// Convert a UnitIndexEntry::Contributions index to the corresponding on-disk
+// value of the section identifier.
+static unsigned IndexToOnDiskSectionId(unsigned Index) { return Index + 1; }
+
 static StringRef getSubsection(StringRef Section,
                                const DWARFUnitIndex::Entry &Entry,
                                DWARFSectionKind Kind) {
   const auto *Off = Entry.getOffset(Kind);
   if (!Off)
     return StringRef();
   return Section.substr(Off->Offset, Off->Length);
 }
if (!I)
       continue;
     auto P = TypeIndexEntries.insert(std::make_pair(E.getSignature(), TUEntry));
     if (!P.second)
       continue;
     auto &Entry = P.first->second;
     // Zero out the debug_info contribution
     Entry.Contributions[0] = {};
     for (auto Kind : TUIndex.getColumnKinds()) {
-      auto &C = Entry.Contributions[Kind - DW_SECT_INFO];
+      auto &C = Entry.Contributions[SectionKindToIndex(Kind)];
       C.Offset += I->Offset;
       C.Length = I->Length;
       ++I;
     }
-    auto &C = Entry.Contributions[DW_SECT_TYPES - DW_SECT_INFO];
+    const unsigned TypesIndex = SectionKindToIndex(DW_SECT_TYPES);
+    auto &C = Entry.Contributions[TypesIndex];
     Out.emitBytes(Types.substr(
-        C.Offset - TUEntry.Contributions[DW_SECT_TYPES - DW_SECT_INFO].Offset,
-        C.Length));
+        C.Offset - TUEntry.Contributions[TypesIndex].Offset, C.Length));
     C.Offset = TypesOffset;
     TypesOffset += C.Length;
   }
 }
 
 static void addAllTypes(MCStreamer &Out,
                         MapVector<uint64_t, UnitIndexEntry> &TypeIndexEntries,
                         MCSection *OutputTypes,
                         const std::vector<StringRef> &TypesSections,
                         const UnitIndexEntry &CUEntry, uint32_t &TypesOffset) {
   for (StringRef Types : TypesSections) {
     Out.SwitchSection(OutputTypes);
     uint64_t Offset = 0;
     DataExtractor Data(Types, true, 0);
     while (Data.isValidOffset(Offset)) {
       UnitIndexEntry Entry = CUEntry;
       // Zero out the debug_info contribution
       Entry.Contributions[0] = {};
-      auto &C = Entry.Contributions[DW_SECT_TYPES - DW_SECT_INFO];
+      auto &C = Entry.Contributions[SectionKindToIndex(DW_SECT_TYPES)];
       C.Offset = TypesOffset;
       auto PrevOffset = Offset;
       // Length of the unit, including the 4 byte length field.
       C.Length = Data.getU32(&Offset) + 4;
 
       Data.getU16(&Offset); // Version
       Data.getU32(&Offset); // Abbrev offset
       Data.getU8(&Offset);  // Address size
writeIndex(MCStreamer &Out, MCSection *Section,
 
   // Write the indexes.
   for (const auto &I : Buckets)
     Out.emitIntValue(I, 4);
 
   // Write the column headers (which sections will appear in the table)
   for (size_t i = 0; i != ContributionOffsets.size(); ++i)
     if (ContributionOffsets[i])
-      Out.emitIntValue(i + DW_SECT_INFO, 4);
+      Out.emitIntValue(IndexToOnDiskSectionId(i), 4);
 
   // Write the offsets.
   writeIndexTable(Out, ContributionOffsets, IndexEntries,
                   &DWARFUnitIndex::Entry::SectionContribution::Offset);
 
   // Write the lengths.
   writeIndexTable(Out, ContributionOffsets, IndexEntries,
                   &DWARFUnitIndex::Entry::SectionContribution::Length);
static Error handleSection(
 
   Name = Name.substr(Name.find_first_not_of("._"));
 
   auto SectionPair = KnownSections.find(Name);
   if (SectionPair == KnownSections.end())
     return Error::success();
 
   if (DWARFSectionKind Kind = SectionPair->second.second) {
-    auto Index = Kind - DW_SECT_INFO;
+    auto Index = SectionKindToIndex(Kind);
     if (Kind != DW_SECT_TYPES) {
       CurEntry.Contributions[Index].Offset = ContributionOffsets[Index];
       ContributionOffsets[Index] +=
           (CurEntry.Contributions[Index].Length = Contents.size());
     }
 
     switch (Kind) {
     case DW_SECT_INFO:
if (CurCUIndexSection.empty()) {
         return createFileError(Input, EID.takeError());
       const auto &ID = *EID;
       auto P = IndexEntries.insert(std::make_pair(ID.Signature, CurEntry));
       if (!P.second)
         return buildDuplicateError(*P.first, ID, "");
       P.first->second.Name = ID.Name;
       P.first->second.DWOName = ID.DWOName;
       addAllTypes(Out, TypeIndexEntries, TypesSection, CurTypesSection,
-                  CurEntry, ContributionOffsets[DW_SECT_TYPES - DW_SECT_INFO]);
+                  CurEntry,
+                  ContributionOffsets[SectionKindToIndex(DW_SECT_TYPES)]);
       continue;
     }
 
     DWARFUnitIndex CUIndex(DW_SECT_INFO);
     DataExtractor CUIndexData(CurCUIndexSection, Obj.isLittleEndian(), 0);
     if (!CUIndex.parse(CUIndexData))
       return make_error<DWPError>("Failed to parse cu_index");
 
for (const DWARFUnitIndex::Entry &E : CUIndex.getRows()) {
       const auto &ID = *EID;
       if (!P.second)
         return buildDuplicateError(*P.first, ID, Input);
       auto &NewEntry = P.first->second;
       NewEntry.Name = ID.Name;
       NewEntry.DWOName = ID.DWOName;
       NewEntry.DWPName = Input;
       for (auto Kind : CUIndex.getColumnKinds()) {
-        auto &C = NewEntry.Contributions[Kind - DW_SECT_INFO];
+        auto &C = NewEntry.Contributions[SectionKindToIndex(Kind)];
         C.Offset += I->Offset;
         C.Length = I->Length;
         ++I;
       }
     }
 
     if (!CurTypesSection.empty()) {
       if (CurTypesSection.size() != 1)
         return make_error<DWPError>("multiple type unit sections in .dwp file");
       DWARFUnitIndex TUIndex(DW_SECT_TYPES);
       DataExtractor TUIndexData(CurTUIndexSection, Obj.isLittleEndian(), 0);
       if (!TUIndex.parse(TUIndexData))
         return make_error<DWPError>("Failed to parse tu_index");
-      addAllTypesFromDWP(Out, TypeIndexEntries, TUIndex, TypesSection,
-                         CurTypesSection.front(), CurEntry,
-                         ContributionOffsets[DW_SECT_TYPES - DW_SECT_INFO]);
+      addAllTypesFromDWP(
+          Out, TypeIndexEntries, TUIndex, TypesSection, CurTypesSection.front(),
+          CurEntry, ContributionOffsets[SectionKindToIndex(DW_SECT_TYPES)]);
     }
   }
 
   // Lie about there being no info contributions so the TU index only includes
   // the type unit contribution
   ContributionOffsets[0] = 0;
   writeIndex(Out, MCOFI.getDwarfTUIndexSection(), ContributionOffsets,
              TypeIndexEntries);
 
   // Lie about the type contribution
-  ContributionOffsets[DW_SECT_TYPES - DW_SECT_INFO] = 0;
+  ContributionOffsets[SectionKindToIndex(DW_SECT_TYPES)] = 0;
   // Unlie about the info contribution
   ContributionOffsets[0] = 1;
 
   writeIndex(Out, MCOFI.getDwarfCUIndexSection(), ContributionOffsets,
              IndexEntries);
 
   return Error::success();
 }