[PATCH] [dwarfdump] Pretty print location expressions and location lists

Frederic Riss friss at apple.com
Wed Feb 4 10:13:39 PST 2015


Thanks. I'm testing an update addressing your feedback. Some comments bellow:


================
Comment at: include/llvm/DebugInfo/DWARFExpression.h:109
@@ +108,3 @@
+
+    bool operator==(const iterator &I) const {
+      return Expression == I.Expression && Offset == I.Offset;
----------------
dblaikie wrote:
> operator overloads that can be non-members generally should be (to allow symmetric conversions on LHS and RHS) - could define it inline as a friend if that's useful.
Yeah, but the iterator_facade that I'm using requires a member function to deriver != from ==. I added a couple of non-members operators that forward to the member ones.

================
Comment at: lib/DebugInfo/DWARFDebugLoc.cpp:34
@@ +33,3 @@
+                       Unit->getAddressByteSize());
+    DWARFExpression(Data, Unit).Print(OS, false);
+  }
----------------
dblaikie wrote:
> what's the boolean? ('false')
> 
> (could use a comment, maybe, or an enum, etc)
It decides wether to use the standard or the EH register mapping. We never use the EH mapping for now, but I thought I'd thread it through anyway. I've changed that to a enum with a default value.

================
Comment at: test/CodeGen/X86/2011-01-24-DbgValue-Before-Use.ll:12
@@ -11,2 +11,3 @@
 ; CHECK-NEXT:   DW_AT_location
-; CHECK-NEXT:   DW_AT_name {{.*}} "z_s"
+; CHECK-NOT: DW_TAG
+; CHECK:   DW_AT_name {{.*}} "z_s"
----------------
dblaikie wrote:
> Maybe CHECK-NOT: DW_{{TAG|AT}} ? I assume this was here so we didn't skip any extra attributes on this TAG.
I changed that to match the old test, although I'm not sure their is any value in checking the ordering of the attributes within a TAG.

================
Comment at: test/DebugInfo/X86/debug-loc-offset.ll:50
@@ -49,1 +49,3 @@
+; do not check the location stored here, as it will be offseted by the
+; compile unit's low_pc. The real check is below in the debug_loc section.
 ; CHECK-NOT: DW_TAG
----------------
dblaikie wrote:
> Not sure I follow why the low_pc offset would be problematic - it'll always be zero in a single-CU file, right?
This is the whole point of this test. It's a multi-cu test that verifies that the location entries are stored relatively to the low_pc of the unit. The test is that the location's base address should be 0. When it is displayed inline the offset is applied - which is a good thing - but it's not what we want to test. So I defer the test to the raw dump of the debug_lc section bellow.
The comment shouldn't read 'stored here' though, 'deployed here' would be more accurate.

http://reviews.llvm.org/D6771

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list