[PATCH] D33184: [DWARF] - Make collectAddressRanges() return section index in addition to Low/High PC

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugRangeList.h:28
+  uint64_t HighPC;
+  uint64_t SecNdx;
+};
----------------
I'm assuming that this is the "official" name used in the specification? Then this name is fine, otherwise I would prefer to call this field SectionIndex.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugRangeList.h:48
     uint64_t EndAddress;
+    // A section index this range belongs to.
+    uint64_t SecNdx;
----------------
If you find the time, it would be nice to convert all these to /// in a separate commit.


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:949
       Ret += SectionLoadAddress - RSec->getAddress();
-  return Ret;
+  return std::make_pair(Ret, RSec->getIndex());
 }
----------------
`return {Ret, RSec->getIndex()};`


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:1124
+        llvm::RelocAddrEntry Rel = {(*SymInfoOrErr).second, R.Value};
+        Map->insert(std::make_pair(Address, Rel));
       }
----------------
{}


================
Comment at: lib/DebugInfo/DWARF/DWARFUnit.cpp:355
+      auto B = AddrDieMap.upper_bound(R.LowPC);
+      if (B != AddrDieMap.begin() && R.LowPC < (--B)->second.first) {
         // The range is a sub-range of existing ranges, we need to split the
----------------
(--B)->second.first:

Would it make sense to replace some std::pairs with structs for better readability?


================
Comment at: lib/Object/COFFObjectFile.cpp:298
+  // TODO: COFFObjectFile::getSectionIndex() not implemented
+  return -1LL;
+}
----------------
Why does this return -1 and the others 0 for not found? Should it return an optional?


https://reviews.llvm.org/D33184