[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 ↗

(On Diff #201102)

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

562 ↗

(On Diff #201102)

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 ↗

(On Diff #201102)

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

562 ↗

(On Diff #201102)

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/trunk/include/llvm/DebugInfo/DIContext.h

public:
 
   uint32_t getNumberOfFrames() const {
     return Frames.size();
   }
 
   void addFrame(const DILineInfo &Frame) {
     Frames.push_back(Frame);
   }
-  
   void resize(unsigned i) {
     Frames.resize(i);
   }
-  
 };
 
 /// Container for description of a global variable.
 struct DIGlobal {
   std::string Name;
   uint64_t Start = 0;
   uint64_t Size = 0;
 

 #undef HANDLE_DWARF_SECTION
   DIDT_UUID = 1 << DIDT_ID_UUID,
 };
 
 /// Container for dump options that control which debug information will be
 /// dumped.
 struct DIDumpOptions {
   unsigned DumpType = DIDT_All;
-  unsigned RecurseDepth = -1U;
+  unsigned ChildRecurseDepth = -1U;
+  unsigned ParentRecurseDepth = -1U;
   uint16_t Version = 0; // DWARF version to assume when extracting.
   uint8_t AddrSize = 4; // Address byte size to assume when extracting.
   bool ShowAddresses = true;
   bool ShowChildren = false;
   bool ShowParents = false;
   bool ShowForm = false;
   bool SummarizeTypes = false;
   bool Verbose = false;
   bool DisplayRawContents = false;
 
   /// Return default option set for printing a single DIE without children.
   static DIDumpOptions getForSingleDIE() {
     DIDumpOptions Opts;
-    Opts.RecurseDepth = 0;
+    Opts.ChildRecurseDepth = 0;
+    Opts.ParentRecurseDepth = 0;
     return Opts;
   }
 
   /// Return the options with RecurseDepth set to 0 unless explicitly required.
   DIDumpOptions noImplicitRecursion() const {
     DIDumpOptions Opts = *this;
-    if (RecurseDepth == -1U && !ShowChildren)
-      Opts.RecurseDepth = 0;
+    if (ChildRecurseDepth == -1U && !ShowChildren)
+      Opts.ChildRecurseDepth = 0;
+    if (ParentRecurseDepth == -1U && !ShowParents)
+      Opts.ParentRecurseDepth = 0;
     return Opts;
   }
 };
 
 class DIContext {
 public:
   enum DIContextKind {
     CK_DWARF,

llvm/trunk/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.ParentRecurseDepth > 0 && Depth >= DumpOpts.ParentRecurseDepth)
+    return Indent;
+  Indent = dumpParentChain(Die.getParent(), OS, Indent, DumpOpts, Depth + 1);
   Die.dump(OS, Indent, DumpOpts);
   return Indent + 2;
 }
 
 void DWARFDie::dump(raw_ostream &OS, unsigned Indent,
                     DIDumpOptions DumpOpts) const {
   if (!isValid())
     return;
if (abbrCode) {
             // but in .debug_abbrev section. So we just skip such attrs.
             continue;
           }
           dumpAttribute(OS, *this, &offset, AttrSpec.Attr, AttrSpec.Form,
                         Indent, DumpOpts);
         }
 
         DWARFDie child = getFirstChild();
-        if (DumpOpts.ShowChildren && DumpOpts.RecurseDepth > 0 && child) {
-          DumpOpts.RecurseDepth--;
+        if (DumpOpts.ShowChildren && DumpOpts.ChildRecurseDepth > 0 && child) {
+          DumpOpts.ChildRecurseDepth--;
           DIDumpOptions ChildDumpOpts = DumpOpts;
           ChildDumpOpts.ShowParents = false;
           while (child) {
             child.dump(OS, Indent + 2, ChildDumpOpts);
             child = child.getSibling();
           }
         }
       } else {

llvm/trunk/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 -parent-recurse-depth 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

llvm/trunk/test/tools/llvm-dwarfdump/cmdline.test

 RUN: llvm-dwarfdump -h 2>&1 | FileCheck --check-prefix=HELP %s
 RUN: llvm-dwarfdump --help 2>&1 | FileCheck --check-prefix=HELP %s
 HELP: USAGE: llvm-dwarfdump{{[^ ]*}} [options] <input object files or .dSYM bundles>
 HELP-NOT: -reverse-iterate
 HELP: Color Options
 HELP: -color
 HELP: Section-specific Dump Options
 HELP: -debug-info            - Dump the .debug_info section
 HELP: -eh-frame
 HELP: Specific Options
 HELP: -diff
 HELP: -find
 HELP: -ignore-case
 HELP: -lookup
 HELP: -name
+HELP: -parent-recurse-depth=<N>
 HELP: -recurse-depth=<N>
 HELP: -regex
 HELP: -show-children
 HELP: -show-form
 HELP: -show-parents
 HELP: -statistics
 HELP: -summarize-types
 HELP-NOT: -reverse-iterate
 
 RUN: llvm-dwarfdump --version 2>&1 | FileCheck --check-prefix=VERSION %s
 VERSION: {{ version }}
 
 RUN: llvm-dwarfdump -diff -verbose 2>&1 | FileCheck --check-prefix=INCOMPATIBLE %s
 INCOMPATIBLE: error: incompatible arguments: specifying both -diff and -verbose is currently not supported

llvm/trunk/tools/dsymutil/DwarfLinker.cpp

void DwarfLinker::reportWarning(const Twine &Warning, const DebugMapObject &DMO,
                                 const DWARFDie *DIE) const {
   StringRef Context = DMO.getObjectFilename();
   warn(Warning, Context);
 
   if (!Options.Verbose || !DIE)
     return;
 
   DIDumpOptions DumpOpts;
-  DumpOpts.RecurseDepth = 0;
+  DumpOpts.ChildRecurseDepth = 0;
   DumpOpts.Verbose = Options.Verbose;
 
   WithColor::note() << "    in DIE:\n";
   DIE->dump(errs(), 6 /* Indent */, DumpOpts);
 }
 
 bool DwarfLinker::createStreamer(const Triple &TheTriple,
                                  raw_fd_ostream &OutFile) {
unsigned DwarfLinker::shouldKeepVariableDIE(RelocationManager &RelocMgr,
   // DIEInfo is filled. However, we don't want a static variable in a
   // function to force us to keep the enclosing function.
   if (!RelocMgr.hasValidRelocation(LocationOffset, LocationEndOffset, MyInfo) ||
       (Flags & TF_InFunctionScope))
     return Flags;
 
   if (Options.Verbose) {
     DIDumpOptions DumpOpts;
-    DumpOpts.RecurseDepth = 0;
+    DumpOpts.ChildRecurseDepth = 0;
     DumpOpts.Verbose = Options.Verbose;
     DIE.dump(outs(), 8 /* Indent */, DumpOpts);
   }
 
   return Flags | TF_Keep;
 }
 
 /// Check if a function describing DIE should be kept.
unsigned DwarfLinker::shouldKeepSubprogramDIE(
   auto LowPc = dwarf::toAddress(DIE.find(dwarf::DW_AT_low_pc));
   assert(LowPc.hasValue() && "low_pc attribute is not an address.");
   if (!LowPc ||
       !RelocMgr.hasValidRelocation(LowPcOffset, LowPcEndOffset, MyInfo))
     return Flags;
 
   if (Options.Verbose) {
     DIDumpOptions DumpOpts;
-    DumpOpts.RecurseDepth = 0;
+    DumpOpts.ChildRecurseDepth = 0;
     DumpOpts.Verbose = Options.Verbose;
     DIE.dump(outs(), 8 /* Indent */, DumpOpts);
   }
 
   if (DIE.getTag() == dwarf::DW_TAG_label) {
     if (Unit.hasLabelAt(*LowPc))
       return Flags;
     // FIXME: dsymutil-classic compat. dsymutil-classic doesn't consider labels
for (const auto &Attr : Abbrev.attributes()) {
     if (hasODR && isODRAttribute(Attr.Attr))
       Form = dwarf::DW_FORM_ref_addr;
     Copy.AddAttribute(dwarf::Attribute(Attr.Attr), dwarf::Form(Form));
   }
 
   Linker.AssignAbbrev(Copy);
 }
 
-uint32_t DwarfLinker::DIECloner::hashFullyQualifiedName(
-    DWARFDie DIE, CompileUnit &U, const DebugMapObject &DMO, int RecurseDepth) {
+uint32_t
+DwarfLinker::DIECloner::hashFullyQualifiedName(DWARFDie DIE, CompileUnit &U,
+                                               const DebugMapObject &DMO,
+                                               int ChildRecurseDepth) {
   const char *Name = nullptr;
   DWARFUnit *OrigUnit = &U.getOrigUnit();
   CompileUnit *CU = &U;
   Optional<DWARFFormValue> Ref;
 
   while (1) {
     if (const char *CurrentName = DIE.getName(DINameKind::ShortName))
       Name = CurrentName;
DwarfLinker::DIECloner::hashFullyQualifiedName(DWARFDie DIE, CompileUnit &U,
   unsigned Idx = OrigUnit->getDIEIndex(DIE);
   if (!Name && DIE.getTag() == dwarf::DW_TAG_namespace)
     Name = "(anonymous namespace)";
 
   if (CU->getInfo(Idx).ParentIdx == 0 ||
       // FIXME: dsymutil-classic compatibility. Ignore modules.
       CU->getOrigUnit().getDIEAtIndex(CU->getInfo(Idx).ParentIdx).getTag() ==
           dwarf::DW_TAG_module)
-    return djbHash(Name ? Name : "", djbHash(RecurseDepth ? "" : "::"));
+    return djbHash(Name ? Name : "", djbHash(ChildRecurseDepth ? "" : "::"));
 
   DWARFDie Die = OrigUnit->getDIEAtIndex(CU->getInfo(Idx).ParentIdx);
   return djbHash(
       (Name ? Name : ""),
       djbHash((Name ? "::" : ""),
-              hashFullyQualifiedName(Die, *CU, DMO, ++RecurseDepth)));
+              hashFullyQualifiedName(Die, *CU, DMO, ++ChildRecurseDepth)));
 }
 
 static uint64_t getDwoId(const DWARFDie &CUDie, const DWARFUnit &Unit) {
   auto DwoId = dwarf::toUnsigned(
       CUDie.find({dwarf::DW_AT_dwo_id, dwarf::DW_AT_GNU_dwo_id}));
   if (DwoId)
     return *DwoId;
   return 0;
LinkContext.CompileUnits.reserve(
         LinkContext.DwarfContext->getNumCompileUnits());
 
     for (const auto &CU : LinkContext.DwarfContext->compile_units()) {
       updateDwarfVersion(CU->getVersion());
       auto CUDie = CU->getUnitDIE(false);
       if (Options.Verbose) {
         outs() << "Input compilation unit:";
         DIDumpOptions DumpOpts;
-        DumpOpts.RecurseDepth = 0;
+        DumpOpts.ChildRecurseDepth = 0;
         DumpOpts.Verbose = Options.Verbose;
         CUDie.dump(outs(), 0, DumpOpts);
       }
       if (CUDie && !LLVM_UNLIKELY(Options.Update))
         registerModuleReference(CUDie, *CU, ModuleMap, LinkContext.DMO,
                                 LinkContext.Ranges, OffsetsStringPool,
                                 UniquingStringPool, ODRContexts, 0, UnitID,
                                 LinkContext.DwarfContext->isLittleEndian());

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

 static alias ShowParentsAlias("p", desc("Alias for -show-parents."),
                               aliasopt(ShowParents));
 static opt<bool>
     ShowForm("show-form",
              desc("Show DWARF form types after the DWARF attribute types."),
              cat(DwarfDumpCategory));
 static alias ShowFormAlias("F", desc("Alias for -show-form."),
                            aliasopt(ShowForm), cat(DwarfDumpCategory));
-static opt<unsigned> RecurseDepth(
-    "recurse-depth",
-    desc("Only recurse to a depth of N when displaying debug info entries."),
+static opt<unsigned>
+    ChildRecurseDepth("recurse-depth",
+                      desc("Only recurse to a depth of N when displaying "
+                           "children of debug info entries."),
+                      cat(DwarfDumpCategory), init(-1U), value_desc("N"));
+static alias ChildRecurseDepthAlias("r", desc("Alias for -recurse-depth."),
+                                    aliasopt(ChildRecurseDepth));
+static opt<unsigned>
+    ParentRecurseDepth("parent-recurse-depth",
+                       desc("Only recurse to a depth of N when displaying "
+                            "parents of debug info entries."),
                        cat(DwarfDumpCategory), init(-1U), value_desc("N"));
-static alias RecurseDepthAlias("r", desc("Alias for -recurse-depth."),
-                               aliasopt(RecurseDepth));
-
 static opt<bool>
     SummarizeTypes("summarize-types",
                    desc("Abbreviate the description of type unit entries."),
                    cat(DwarfDumpCategory));
 static cl::opt<bool>
     Statistics("statistics",
                cl::desc("Emit JSON-formatted debug info quality metrics."),
                cat(DwarfDumpCategory));
if (!EC)
     return;
   WithColor::error() << Prefix << ": " << EC.message() << "\n";
   exit(1);
 }
 
 static DIDumpOptions getDumpOpts() {
   DIDumpOptions DumpOpts;
   DumpOpts.DumpType = DumpType;
-  DumpOpts.RecurseDepth = RecurseDepth;
+  DumpOpts.ChildRecurseDepth = ChildRecurseDepth;
+  DumpOpts.ParentRecurseDepth = ParentRecurseDepth;
   DumpOpts.ShowAddresses = !Diff;
   DumpOpts.ShowChildren = ShowChildren;
   DumpOpts.ShowParents = ShowParents;
   DumpOpts.ShowForm = ShowForm;
   DumpOpts.SummarizeTypes = SummarizeTypes;
   DumpOpts.Verbose = Verbose;
   // In -verify mode, print DIEs without children in error messages.
   if (Verify)

 static bool lookup(ObjectFile &Obj, DWARFContext &DICtx, uint64_t Address,
                    raw_ostream &OS) {
   auto DIEsForAddr = DICtx.getDIEsForAddress(Lookup);
 
   if (!DIEsForAddr)
     return false;
 
   DIDumpOptions DumpOpts = getDumpOpts();
-  DumpOpts.RecurseDepth = 0;
+  DumpOpts.ChildRecurseDepth = 0;
   DIEsForAddr.CompileUnit->dump(OS, DumpOpts);
   if (DIEsForAddr.FunctionDIE) {
     DIEsForAddr.FunctionDIE.dump(OS, 2, DumpOpts);
     if (DIEsForAddr.BlockDIE)
       DIEsForAddr.BlockDIE.dump(OS, 4, DumpOpts);
   }
 
   // TODO: it is neccessary to set proper SectionIndex here.