[PATCH] D37696: [dwarfdump] Add DWARF verifiers for address ranges

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Also - how does this work (what does it do, does it catch things correctly) when there are DIEs without ranges - such as compile_unit{namespace{subprogram}} - does it still check that the subprogram is contained within the compile_unit?



================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:338
+  // Verify that ranges are contained within their parent.
+  const bool ShouldBeContained = !Ranges.empty() && !ParentRI.Ranges.empty() &&
+                                 !(Die.getTag() == DW_TAG_subprogram &&
----------------
dblaikie wrote:
> Would this correctly check that the nested subprogram is contained within the CU, though? (looks like it just punts on the nested subprogram entirely?)
the LLVM style doesn't generally use top-level const on locals - probably best to drop that for consistency


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:338-341
+  const bool ShouldBeContained = !Ranges.empty() && !ParentRI.Ranges.empty() &&
+                                 !(Die.getTag() == DW_TAG_subprogram &&
+                                   ParentRI.Die.getTag() == DW_TAG_subprogram);
+  if (ShouldBeContained && !ParentRI.contains(RI)) {
----------------
Would this correctly check that the nested subprogram is contained within the CU, though? (looks like it just punts on the nested subprogram entirely?)


================
Comment at: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp:2618-2619
+              - Value:           0x0000000000002000
+          - AbbrCode:        0x00000000
+            Values:
+  )";
----------------
Wouldn't this need three zero entries? (one to finish each DIE's child list)


Repository:
  rL LLVM

https://reviews.llvm.org/D37696