[PATCH] D69672: DWARFDebugLoclists: Move to a incremental parsing model

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:318
+  bool CanContinue = true;
+  while (CanContinue && Data.isValidOffset(Offset) && Offset < EndOffset) {
+    OS << Separator;
----------------
dblaikie wrote:
> isValidOffset might be unnecessary given "Offset < EndOffset"? (maybe check that EndOffset is smaller than Data's actual end ofset to start with, then skip the isValidOffset check? (maybe need a lower bound check too))
This is debatable, I think. In case the EndOffset is wrong (corrupted header?), it may make sense to still dump as much as we are able to do instead of bailing out straight away. However, that is probably not a likely scenario, and it does make things cleaner, so I've gone with your suggestion. I've also changed the EndOffset argument into a "Size", as that seemed a bit cleaner.

The thing I would really want to have some day is the ability to "slice" a data extractor. That would make a lot of these offset calculations simpler and/or safer. For instance, this code has the bug that if one location list crosses the "end offset" boundary (and there is more data after it), then we will only detect that once we've finished dumping that location list. If the size was encoded in the data extractor, then this could be detected immediately, as each contribution would be handled as if it was the last one in the section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69672