[Lldb-commits] [PATCH] D72158: [DebugInfo] Make most debug line prologue errors non-fatal to parsing

Mon Feb 17 04:08:53 PST 2020

labath added inline comments.

================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:323-325
+    // Treat this error as unrecoverable - we cannot be sure what any of
+    // the data represents including the length field, so cannot skip it or make
+    // any reasonable assumptions.
----------------
jhenderson wrote:
> labath wrote:
> > BTW, I think this error should be recoverable too. I believe the reason why the length field comes *before* the version number is specifically so that one can skip over contributions with unsupported (future) version numbers.
> > 
> > While it's hard to say what the future versions of dwarf will look like, I would expect that the committee will try very hard to avoid  making changes in the length field. I think they'd use one of the DW_LENGTH_lo_reserved..DW_LENGTH_hi_reserved-1 constants for severely incompatible changes.
> "Unrecoverable" here means don't try to parse this table, but do allow parsing the next. I think the comment might be slightly misleading in this regard. FWIW, a version of 0 or 1 probably doesn't have a leading length, so it is definitely unrecoverable. For versions > 5, which are now checked, we don't know what the structure of the header is, so although we could take a guess, we'd almost certainly get it wrong and produce invalid (possibly very invalid) output. I don't have a strong opinion as to whether that should be an unrecoverable error or not (currently it is).
Ah, right, I see now what you mean. I agree it makes no sense to parse the contents of a contribution with an unrecognized version. The part about not being able to trust the length field threw me off, as the length of the contribution is the one thing we can expect to remain unchanged between dwarf versions.

Sorry about the false alarm.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72158