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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
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;
----------------
FIXME to handle this as a verification failure too? (if stmt_list isn't usable as a section offset)


================
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
----------------
another verification failure possibility?


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:414-415
+        }
+      } else {
+        continue; // No line table for this compile unit.
+      }
----------------
probably invert this to reduce indentation:

  auto StmtFormValue = CU->getUnitDIE().find(DW_AT_stmt_list);
  if (!StmtFormValue)
    continue;
  ...



================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:449-450
+          << "][" << RowIndex
+          << "] has invalid file index (valid values are 1 - "
+          << MaxFileIndex << "):\n";
+          DWARFDebugLine::Row::dumpTableHeader(OS);
----------------
maybe use set notation to be clear about open/closed range, etc: "[1,MaxFileIndex]"


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:453
+          Row.dump(OS);
+          OS << "\n";
+        }
----------------
Probably use '\n' rather than "\n" - so there's no need to go hunting for a null terminator, etc.


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1914
+  // the address decreases.
+  const char *yamldata = R"(
+    debug_str:
----------------
Declare as StringRef?


================
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:";
----------------
Probably make this a StringRef


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:1992
+  // an invalid file index.
+  const char *yamldata = R"(
+    debug_str:
----------------
StringRef?


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:2064-2065
+  strm.flush();
+  const char *err = "error: .debug_line[0x00000000][1] has invalid file index "
+      "(valid values are 1 - 1):";
+  EXPECT_TRUE(strm.str().find(err) != std::string::npos);
----------------
Perhaps it's worthwhile to mention what the file index was? (doesn't look like it's in this error message)


https://reviews.llvm.org/D32754