[PATCH] D77146: [DebugInfo] Fix reading location tables headers of v5 units in DWP.

Wed Apr 1 01:36:54 PDT 2020

ikudrin marked an inline comment as done.
ikudrin added a comment.

In D77146#1954186 <https://reviews.llvm.org/D77146#1954186>, @jhenderson wrote:

> Looks good to me, although I'm not fully up to speed on .debug_loclists to be 100% certain, so might be worth a second opinion.

Thanks!

> What did the code do before? Parse the first one repeatedly?

The code is OK for a DWO case, but is not ready for DWP. The support for DWARFv5 DWP files is added in D75929 <https://reviews.llvm.org/D75929>, so this code also needs to be updated.

================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:541-542
       Offset -= HeaderSize;
+      if (auto *IndexEntry = Header.getIndexEntry())
+        if (const auto *C = IndexEntry->getOffset(DW_SECT_LOCLISTS))
+          Offset += C->Offset;
----------------
jhenderson wrote:
> Maybe these shouldn't be `auto`? Certainly, `C` looks like it should be a number (e.g. uint64_t), but it obviously isn't.
`getOffset()` returns `const DWARFUnitIndex::Entry::SectionContribution *`. Not that obvious, I agree. Do you think it would be better to rename `C` to, say, `Contrib` or `Contribution`?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77146/new/

https://reviews.llvm.org/D77146