[PATCH] D32821: Add DWARF verifiers to verify address ranges are correct and scoped correctly.

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:78
+  ///
+  /// @param Die          The DWARF DIE to check
+  void verifyDie(const DWARFDie &Die, DieRangeInfo &UnitRI, VerifyDieInfo &ParentVRI);
----------------
We usually use `\param` for consistency.


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:44
+      if (Ranges[I].first > Ranges[I].second) {
+        OS << "error: invalid range in DIE\n";
+        HasErrors = true;
----------------
It doesn't look like this is tested? (as far as I can tell by searching for the error message).


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:73
+        ++NumDebugInfoErrors;
+        OS << "error: DIEs have overlapping address ranges:\n";
+        Prev->Die.dump(OS, 0);
----------------
test?


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:129
+  case DW_TAG_null:
+      return;
+  case DW_TAG_compile_unit: {
----------------
This entire function has really weird indentation. Please clang-format.


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:149
+    // We need to keep track of all function ranges for
+    // .debug_aranges and .debug_info verifiers
+    AllFunctionDieRangeInfos.insert(VRI.RI);
----------------
`.` at the end


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:187
+  }
+  // Verify the attributes and forms
+  for (auto AttrValue : Die.attributes()) {
----------------
`.`


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:197
+  // Verify that any address ranges from this DIE's children don't overlap.
+  if (!VRI.ChildRangesCantOverlap.empty()) {
+    verifyNoRangesOverlap(VRI.ChildRangesCantOverlap);
----------------
remove extra `{`


https://reviews.llvm.org/D32821