[PATCH] D32754: Add line table verification to lldb-dwarfdump --verify.

[Greg Clayton via Phabricator via llvm-commits]

clayborg marked 6 inline comments as done.
clayborg added a comment.

Marked things as done and commented where things weren't changed.



================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:404
+      if (auto StmtFormValue = CU->getUnitDIE().find(DW_AT_stmt_list)) {
+        if (auto DataOpt = toSectionOffset(StmtFormValue)) {
+          LineTableOffset = *DataOpt;
----------------
dblaikie wrote:
> FIXME to handle this as a verification failure too? (if stmt_list isn't usable as a section offset)
This is handled when verifying the .debug_info section. No need to duplicate this here.


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:406
+          LineTableOffset = *DataOpt;
+          if (LineTableOffset >= getLineSection().Data.size()) {
+            // Make sure we don't get a valid line table back if the offset
----------------
dblaikie wrote:
> another verification failure possibility?
This is handled when verifying the .debug_info section. No need to duplicate this here.


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:415
+      } else {
+        continue; // No line table for this compile unit.
+      }
----------------
Yeah, I thought of that but have been asked to do the "if (auto StmtFormValue" way many times. The way you did it above is the way I wanted to do it. I will change it.


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1984
+  strm.flush();
+  const char *err = "error: .debug_line[0x00000000] row[1] decreases in "
+      "address from previous row:";
----------------
dblaikie wrote:
> Probably make this a StringRef
We search the std::string named "str" for this below and use must be a C string as part of the EXPECT_TRUE. No need for the StringRef here? Unless I wrap "str" into a StringRef. Let me know your preference here.


https://reviews.llvm.org/D32754