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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 11 14:33:21 PDT 2017


dblaikie added a comment.

Does this handle the interesting cases (test cases are a bit verbose, so I admit I haven't looked through them carefully) that came up in the original review:

Nested functions (a function inside a function - where the inner function shouldn't be contained within the ranges of the outer function (I find this weird, but apparently this is what Adrian & Paul & others think is right, and I can sort of see it - LLVM doesn't produce anything like this))?

Maybe that was the only interesting one...



================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:46
+    DieRangeInfo() = default;
+    DieRangeInfo(DWARFDie Die) : Die(Die), Ranges(), Children() {}
+
----------------
Drop "Ranges(), Children()" from here - they're the default


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:50
+    DieRangeInfo(std::vector<DWARFAddressRange> Ranges)
+        : Die(), Ranges(Ranges), Children() {}
+
----------------
Drop the "Die()" and "Children()" from here - they're the default anyway?


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:50
+    DieRangeInfo(std::vector<DWARFAddressRange> Ranges)
+        : Die(), Ranges(Ranges), Children() {}
+
----------------
dblaikie wrote:
> Drop the "Die()" and "Children()" from here - they're the default anyway?
use "Ranges(std::move(Ranges))" to avoid an unnecessary copy of the Ranges vector


================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:35-36
+    if (Pos->intersects(R))
+      return Pos;
+    else if (Pos != Begin) {
+      auto Iter = Pos - 1;
----------------
drop else after return


Repository:
  rL LLVM

https://reviews.llvm.org/D37696





More information about the llvm-commits mailing list