[dwarfdump] Make recursion affect the parent chain

[JDevlieghere]

Summary

Currently, -r (recursion depth) doesn't affect -p (show parents). This patch fixes that and adds a test case.

[aprantl]

Thanks!

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

559

if (DumpOpts.RecurseDepth && Depth >= DumpOpts.RecurseDepth)

562

Depth + 1?

[dblaikie]

Is this the desired behavior?

Currently, I assume, searching for a particular DIE offset dumps that DIE, its parent, and all its children. (& the recursion limit is entirely ignored?)

This patch, if I understand it, dumps from the CU to the target DIE until the recursion limit is reached. So you get some parent DIE of the one you searched for.

I would've probably thought that the entire parent chain would be dumped, and then the recursion limit would limit how deep into the children of the target DIE would be dumped.

Thoughts?

[JDevlieghere]

In D62359#1515107, @dblaikie wrote:

Is this the desired behavior?

Currently, I assume, searching for a particular DIE offset dumps that DIE, its parent, and all its children. (& the recursion limit is entirely ignored?)

Yup.

This patch, if I understand it, dumps from the CU to the target DIE until the recursion limit is reached. So you get some parent DIE of the one you searched for.

I would've probably thought that the entire parent chain would be dumped, and then the recursion limit would limit how deep into the children of the target DIE would be dumped.

Thoughts?

This is what happens currently (without the patch). The recursion depth only applies to children, and passing -p shows the whole parent chain.

I think there's a case to be made for both. With the patch, if you pass -p -c -r 2, you'll get two parents and two children, which is a little weird.

@aprantl since you requested this, WDYT?

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

559

The recursion depth can be negative, so that wouldn't work.

562

Makes sense!

[aprantl]

I perceived the current behavior as a bug in a situation where I was doing dwarfdump --name=foo in a deeply nested DIE tree with inlined functions and I was annoyed at all the repetitive parent context making it impossible to see more than one result on a page. It's true that there may be situation where you'd want to limit the parent and children recursion depth individually.

If we take grep as an analogy, this patch implements -C, which is useful in it's own right, but in addition there is also -B and -A.

I'd have no problem with adding even more fine-grained --limit-parents and --limit-children options, if someone finds them useful, but I would also like to have the option that does both.

[aprantl]

I'd have no problem with adding even more fine-grained --limit-parents and --limit-children options, if someone finds them useful, but I would also like to have the option that does both.

I take that back. I want some form of limiting both directions. I'd be fine with only adding a --limit-parents (or some other name) option. The combined option is redundant and probably not nearly as popular.

[JDevlieghere]

• Add new flag -parent-recurse-depth.
• Add new aliases -limit-children and -limit-parents.

[dblaikie]

I'll leave it to you two to - I just wanted to bring up the thought at least - thanks!

[aprantl]

llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp

202 ↗

(On Diff #201282)

To avoid an overproliferation of options, I think I'd be fine with just adding parent-recurse-depth and dropping the limit-* aliases.

209 ↗

(On Diff #201282)

why do we use -1U here and check for 0 in the code? I suppose this will also work fine, but it's inconsistent.

llvm/lib/DebugInfo/DWARF/DWARFDie.cpp

void DWARFDie::getCallerFrame(uint32_t &CallFile, uint32_t &CallLine,
   CallFile = toUnsigned(find(DW_AT_call_file), 0);
   CallLine = toUnsigned(find(DW_AT_call_line), 0);
   CallColumn = toUnsigned(find(DW_AT_call_column), 0);
   CallDiscriminator = toUnsigned(find(DW_AT_GNU_discriminator), 0);
 }
 
 /// Helper to dump a DIE with all of its parents, but no siblings.
 static unsigned dumpParentChain(DWARFDie Die, raw_ostream &OS, unsigned Indent,
-                                DIDumpOptions DumpOpts) {
+                                DIDumpOptions DumpOpts, unsigned Depth = 0) {
   if (!Die)
     return Indent;
-  Indent = dumpParentChain(Die.getParent(), OS, Indent, DumpOpts);
+  if (DumpOpts.RecurseDepth > 0 && Depth >= DumpOpts.RecurseDepth)
+    return Indent;
+  Depth += 1;
+  Indent = dumpParentChain(Die.getParent(), OS, Indent, DumpOpts, Depth);
   Die.dump(OS, Indent, DumpOpts);
   return Indent + 2;
 }
 
 void DWARFDie::dump(raw_ostream &OS, unsigned Indent,
                     DIDumpOptions DumpOpts) const {
   if (!isValid())
     return;

llvm/test/tools/llvm-dwarfdump/X86/enum.s

 # RUN: llvm-mc -triple x86_64-apple-darwin %s -filetype=obj -o %t
 # RUN: llvm-dwarfdump --debug-info=0x0000002a -p %t | FileCheck %s --check-prefix=PARENTS
 # RUN: llvm-dwarfdump --debug-info=0x0000002a -c %t | FileCheck %s --check-prefix=CHILDREN
 # RUN: llvm-dwarfdump --debug-info=0x0000002a -p -c %t | FileCheck %s --check-prefix=BOTH
+# RUN: llvm-dwarfdump  --debug-info=0x00000032 -p -r 1 -c %t | FileCheck %s --check-prefix=ONEPARENT
 
 # PARENTS: DW_TAG_compile_unit
 # PARENTS: DW_TAG_enumeration_type
 # PARENTS-NOT: DW_TAG_enumerator
 
+# ONEPARENT-NOT: DW_TAG_compile_unit
+# ONEPARENT: DW_TAG_enumeration_type
+# ONEPARENT: DW_TAG_enumerator
+
 # CHILDREN-NOT: DW_TAG_compile_unit
 # CHILDREN:   DW_TAG_enumerator
 # CHILDREN:     DW_AT_name  ("first")
 # CHILDREN:   DW_TAG_enumerator
 # CHILDREN:     DW_AT_name  ("second")
 
 # BOTH: DW_TAG_compile_unit
 # BOTH: DW_TAG_enumeration_type