[Lldb-commits] [lldb] 77440d6 - [lldb][NFC] Allow range-based for loops over DWARFDIE's children

Raphael Isemann via lldb-commits lldb-commits at lists.llvm.org
Thu Jul 22 06:03:48 PDT 2021


Author: Raphael Isemann
Date: 2021-07-22T15:03:30+02:00
New Revision: 77440d644b3ba26443c1d14d04a4046fab07d731

URL: https://github.com/llvm/llvm-project/commit/77440d644b3ba26443c1d14d04a4046fab07d731
DIFF: https://github.com/llvm/llvm-project/commit/77440d644b3ba26443c1d14d04a4046fab07d731.diff

LOG: [lldb][NFC] Allow range-based for loops over DWARFDIE's children

This patch adds the ability to get a DWARFDIE's children as an LLVM range.

This way we can use for range loops to iterate over them and we can use LLVM's
algorithms like `llvm::all_of` to query all children.

The implementation has to do some small shenanigans as the iterator needs to
store a DWARFDIE, but a DWARFDIE container is also a DWARFDIE so it can't return
the iterator by value. I just made the `children` getter a templated function to
avoid the cyclic dependency.

Reviewed By: #lldb, werat, JDevlieghere

Differential Revision: https://reviews.llvm.org/D103172

Added: 
    lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp

Modified: 
    lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
    lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
    lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    lldb/unittests/SymbolFile/DWARF/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 4cf4ea6ed0f4c..46015f7b43b1c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -666,8 +666,7 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
         // Blocks have a __FuncPtr inside them which is a pointer to a
         // function of the proper type.
 
-        for (DWARFDIE child_die = target_die.GetFirstChild();
-             child_die.IsValid(); child_die = child_die.GetSibling()) {
+        for (DWARFDIE child_die : target_die.children()) {
           if (!strcmp(child_die.GetAttributeValueAsString(DW_AT_name, ""),
                       "__FuncPtr")) {
             DWARFDIE function_pointer_type =
@@ -1827,8 +1826,7 @@ bool DWARFASTParserClang::ParseTemplateDIE(
   case DW_TAG_GNU_template_parameter_pack: {
     template_param_infos.packed_args =
         std::make_unique<TypeSystemClang::TemplateParameterInfos>();
-    for (DWARFDIE child_die = die.GetFirstChild(); child_die.IsValid();
-         child_die = child_die.GetSibling()) {
+    for (DWARFDIE child_die : die.children()) {
       if (!ParseTemplateDIE(child_die, *template_param_infos.packed_args))
         return false;
     }
@@ -1933,8 +1931,7 @@ bool DWARFASTParserClang::ParseTemplateParameterInfos(
   if (!parent_die)
     return false;
 
-  for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid();
-       die = die.GetSibling()) {
+  for (DWARFDIE die : parent_die.children()) {
     const dw_tag_t tag = die.Tag();
 
     switch (tag) {
@@ -2108,8 +2105,7 @@ void DWARFASTParserClang::EnsureAllDIEsInDeclContextHaveBeenParsed(
   for (auto it = m_decl_ctx_to_die.find(opaque_decl_ctx);
        it != m_decl_ctx_to_die.end() && it->first == opaque_decl_ctx;
        it = m_decl_ctx_to_die.erase(it))
-    for (DWARFDIE decl = it->second.GetFirstChild(); decl;
-         decl = decl.GetSibling())
+    for (DWARFDIE decl : it->second.children())
       GetClangDeclForDIE(decl);
 }
 
@@ -2145,8 +2141,7 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
 
   size_t enumerators_added = 0;
 
-  for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid();
-       die = die.GetSibling()) {
+  for (DWARFDIE die : parent_die.children()) {
     const dw_tag_t tag = die.Tag();
     if (tag == DW_TAG_enumerator) {
       DWARFAttributes attributes;
@@ -2751,8 +2746,7 @@ bool DWARFASTParserClang::ParseChildMembers(
   if (ast == nullptr)
     return false;
 
-  for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid();
-       die = die.GetSibling()) {
+  for (DWARFDIE die : parent_die.children()) {
     dw_tag_t tag = die.Tag();
 
     switch (tag) {
@@ -2898,8 +2892,7 @@ size_t DWARFASTParserClang::ParseChildParameters(
     return 0;
 
   size_t arg_idx = 0;
-  for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid();
-       die = die.GetSibling()) {
+  for (DWARFDIE die : parent_die.children()) {
     const dw_tag_t tag = die.Tag();
     switch (tag) {
     case DW_TAG_formal_parameter: {
@@ -3018,8 +3011,7 @@ DWARFASTParser::ParseChildArrayInfo(const DWARFDIE &parent_die,
   if (!parent_die)
     return llvm::None;
 
-  for (DWARFDIE die = parent_die.GetFirstChild(); die.IsValid();
-       die = die.GetSibling()) {
+  for (DWARFDIE die : parent_die.children()) {
     const dw_tag_t tag = die.Tag();
     if (tag != DW_TAG_subrange_type)
       continue;
@@ -3321,8 +3313,7 @@ static DWARFDIE GetContainingFunctionWithAbstractOrigin(const DWARFDIE &die) {
 }
 
 static DWARFDIE FindAnyChildWithAbstractOrigin(const DWARFDIE &context) {
-  for (DWARFDIE candidate = context.GetFirstChild(); candidate.IsValid();
-       candidate = candidate.GetSibling()) {
+  for (DWARFDIE candidate : context.children()) {
     if (candidate.GetReferencedDIE(DW_AT_abstract_origin)) {
       return candidate;
     }
@@ -3486,8 +3477,7 @@ bool DWARFASTParserClang::CopyUniqueClassMethodTypes(
   UniqueCStringMap<DWARFDIE> dst_name_to_die;
   UniqueCStringMap<DWARFDIE> src_name_to_die_artificial;
   UniqueCStringMap<DWARFDIE> dst_name_to_die_artificial;
-  for (src_die = src_class_die.GetFirstChild(); src_die.IsValid();
-       src_die = src_die.GetSibling()) {
+  for (DWARFDIE src_die : src_class_die.children()) {
     if (src_die.Tag() == DW_TAG_subprogram) {
       // Make sure this is a declaration and not a concrete instance by looking
       // for DW_AT_declaration set to 1. Sometimes concrete function instances
@@ -3505,8 +3495,7 @@ bool DWARFASTParserClang::CopyUniqueClassMethodTypes(
       }
     }
   }
-  for (dst_die = dst_class_die.GetFirstChild(); dst_die.IsValid();
-       dst_die = dst_die.GetSibling()) {
+  for (DWARFDIE dst_die : dst_class_die.children()) {
     if (dst_die.Tag() == DW_TAG_subprogram) {
       // Make sure this is a declaration and not a concrete instance by looking
       // for DW_AT_declaration set to 1. Sometimes concrete function instances

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index f887f774577e6..dda691eecaccc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -192,7 +192,7 @@ DWARFDIE::LookupDeepestBlock(lldb::addr_t address) const {
   }
 
   if (check_children) {
-    for (DWARFDIE child = GetFirstChild(); child; child = child.GetSibling()) {
+    for (DWARFDIE child : children()) {
       if (DWARFDIE child_result = child.LookupDeepestBlock(address))
         return child_result;
     }

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
index 13737280926cf..56154055c44dc 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h
@@ -11,9 +11,11 @@
 
 #include "DWARFBaseDIE.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/iterator_range.h"
 
 class DWARFDIE : public DWARFBaseDIE {
 public:
+  class child_iterator;
   using DWARFBaseDIE::DWARFBaseDIE;
 
   // Tests
@@ -88,6 +90,47 @@ class DWARFDIE : public DWARFBaseDIE {
                             int &decl_line, int &decl_column, int &call_file,
                             int &call_line, int &call_column,
                             lldb_private::DWARFExpression *frame_base) const;
+  /// The range of all the children of this DIE.
+  ///
+  /// This is a template just because child_iterator is not completely defined
+  /// at this point.
+  template <typename T = child_iterator>
+  llvm::iterator_range<T> children() const {
+    return llvm::make_range(T(*this), T());
+  }
+};
+
+class DWARFDIE::child_iterator
+    : public llvm::iterator_facade_base<DWARFDIE::child_iterator,
+                                        std::forward_iterator_tag, DWARFDIE> {
+  /// The current child or an invalid DWARFDie.
+  DWARFDIE m_die;
+
+public:
+  child_iterator() = default;
+  child_iterator(const DWARFDIE &parent) : m_die(parent.GetFirstChild()) {}
+  bool operator==(const child_iterator &it) const {
+    // DWARFDIE's operator== 
diff erentiates between an invalid DWARFDIE that
+    // has a CU but no DIE and one that has neither CU nor DIE. The 'end'
+    // iterator could be default constructed, so explicitly allow
+    // (CU, (DIE)nullptr) == (nullptr, nullptr) -> true
+    if (!m_die.IsValid() && !it.m_die.IsValid())
+      return true;
+    return m_die == it.m_die;
+  }
+  const DWARFDIE &operator*() const {
+    assert(m_die.IsValid() && "Derefencing invalid iterator?");
+    return m_die;
+  }
+  DWARFDIE &operator*() {
+    assert(m_die.IsValid() && "Derefencing invalid iterator?");
+    return m_die;
+  }
+  child_iterator &operator++() {
+    assert(m_die.IsValid() && "Incrementing invalid iterator?");
+    m_die = m_die.GetSibling();
+    return *this;
+  }
 };
 
 #endif // LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDIE_H

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index d1423ec0d1075..ccaf31317d750 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -363,8 +363,7 @@ void SymbolFileDWARF::GetTypes(const DWARFDIE &die, dw_offset_t min_die_offset,
       }
     }
 
-    for (DWARFDIE child_die = die.GetFirstChild(); child_die.IsValid();
-         child_die = child_die.GetSibling()) {
+    for (DWARFDIE child_die : die.children()) {
       GetTypes(child_die, min_die_offset, max_die_offset, type_mask, type_set);
     }
   }
@@ -987,8 +986,7 @@ bool SymbolFileDWARF::ParseImportedModules(
   if (!die)
     return false;
 
-  for (DWARFDIE child_die = die.GetFirstChild(); child_die;
-       child_die = child_die.GetSibling()) {
+  for (DWARFDIE child_die : die.children()) {
     if (child_die.Tag() != DW_TAG_imported_declaration)
       continue;
 
@@ -1247,8 +1245,7 @@ size_t SymbolFileDWARF::ParseBlocksRecursive(
 
 bool SymbolFileDWARF::ClassOrStructIsVirtual(const DWARFDIE &parent_die) {
   if (parent_die) {
-    for (DWARFDIE die = parent_die.GetFirstChild(); die;
-         die = die.GetSibling()) {
+    for (DWARFDIE die : parent_die.children()) {
       dw_tag_t tag = die.Tag();
       bool check_virtuality = false;
       switch (tag) {
@@ -3421,8 +3418,7 @@ SymbolFileDWARF::FindBlockContainingSpecification(
     // Give the concrete function die specified by "func_die_offset", find the
     // concrete block whose DW_AT_specification or DW_AT_abstract_origin points
     // to "spec_block_die_offset"
-    for (DWARFDIE child_die = die.GetFirstChild(); child_die;
-         child_die = child_die.GetSibling()) {
+    for (DWARFDIE child_die : die.children()) {
       DWARFDIE result_die =
           FindBlockContainingSpecification(child_die, spec_block_die_offset);
       if (result_die)
@@ -3551,8 +3547,7 @@ size_t SymbolFileDWARF::ParseVariables(const SymbolContext &sc,
 static CallSiteParameterArray
 CollectCallSiteParameters(ModuleSP module, DWARFDIE call_site_die) {
   CallSiteParameterArray parameters;
-  for (DWARFDIE child = call_site_die.GetFirstChild(); child.IsValid();
-       child = child.GetSibling()) {
+  for (DWARFDIE child : call_site_die.children()) {
     if (child.Tag() != DW_TAG_call_site_parameter &&
         child.Tag() != DW_TAG_GNU_call_site_parameter)
       continue;
@@ -3617,8 +3612,7 @@ SymbolFileDWARF::CollectCallEdges(ModuleSP module, DWARFDIE function_die) {
   // For now, assume that all entries are nested directly under the subprogram
   // (this is the kind of DWARF LLVM produces) and parse them eagerly.
   std::vector<std::unique_ptr<CallEdge>> call_edges;
-  for (DWARFDIE child = function_die.GetFirstChild(); child.IsValid();
-       child = child.GetSibling()) {
+  for (DWARFDIE child : function_die.children()) {
     if (child.Tag() != DW_TAG_call_site && child.Tag() != DW_TAG_GNU_call_site)
       continue;
 

diff  --git a/lldb/unittests/SymbolFile/DWARF/CMakeLists.txt b/lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
index 982ac935794f5..76215c31b2aab 100644
--- a/lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
+++ b/lldb/unittests/SymbolFile/DWARF/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(SymbolFileDWARFTests
   DWARFASTParserClangTests.cpp
+  DWARFDIETest.cpp
   DWARFUnitTest.cpp
   SymbolFileDWARFTests.cpp
   XcodeSDKModuleTests.cpp

diff  --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
new file mode 100644
index 0000000000000..7fc37cefea2f2
--- /dev/null
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -0,0 +1,105 @@
+//===-- DWARFDIETest.cpp ----------------------------------------------=---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Plugins/SymbolFile/DWARF/DWARFDIE.h"
+#include "TestingSupport/Symbol/YAMLModuleTester.h"
+#include "llvm/ADT/STLExtras.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(DWARFDIETest, ChildIteration) {
+  // Tests DWARFDIE::child_iterator.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+    - Table:
+        - Code:            0x00000001
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_yes
+          Attributes:
+            - Attribute:       DW_AT_language
+              Form:            DW_FORM_data2
+        - Code:            0x00000002
+          Tag:             DW_TAG_base_type
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_encoding
+              Form:            DW_FORM_data1
+            - Attribute:       DW_AT_byte_size
+              Form:            DW_FORM_data1
+  debug_info:
+    - Version:         4
+      AddrSize:        8
+      Entries:
+        - AbbrCode:        0x00000001
+          Values:
+            - Value:           0x000000000000000C
+        - AbbrCode:        0x00000002
+          Values:
+            - Value:           0x0000000000000007 # DW_ATE_unsigned
+            - Value:           0x0000000000000004
+        - AbbrCode:        0x00000002
+          Values:
+            - Value:           0x0000000000000007 # DW_ATE_unsigned
+            - Value:           0x0000000000000008
+        - AbbrCode:        0x00000002
+          Values:
+            - Value:           0x0000000000000005 # DW_ATE_signed
+            - Value:           0x0000000000000008
+        - AbbrCode:        0x00000000
+)";
+
+  YAMLModuleTester t(yamldata);
+  ASSERT_TRUE((bool)t.GetDwarfUnit());
+
+  DWARFUnit *unit = t.GetDwarfUnit();
+  const DWARFDebugInfoEntry *die_first = unit->DIE().GetDIE();
+
+  // Create a DWARFDIE that has three DW_TAG_base_type children.
+  DWARFDIE top_die(unit, die_first);
+
+  // Create the iterator range that has the three tags as elements.
+  llvm::iterator_range<DWARFDIE::child_iterator> children = top_die.children();
+
+  // Compare begin() to the first child DIE.
+  DWARFDIE::child_iterator child_iter = children.begin();
+  ASSERT_NE(child_iter, children.end());
+  const DWARFDebugInfoEntry *die_child0 = die_first->GetFirstChild();
+  EXPECT_EQ((*child_iter).GetDIE(), die_child0);
+
+  // Step to the second child DIE.
+  ++child_iter;
+  ASSERT_NE(child_iter, children.end());
+  const DWARFDebugInfoEntry *die_child1 = die_child0->GetSibling();
+  EXPECT_EQ((*child_iter).GetDIE(), die_child1);
+
+  // Step to the third child DIE.
+  ++child_iter;
+  ASSERT_NE(child_iter, children.end());
+  const DWARFDebugInfoEntry *die_child2 = die_child1->GetSibling();
+  EXPECT_EQ((*child_iter).GetDIE(), die_child2);
+
+  // Step to the end of the range.
+  ++child_iter;
+  EXPECT_EQ(child_iter, children.end());
+
+  // Take one of the DW_TAG_base_type DIEs (which has no children) and make
+  // sure the children range is now empty.
+  DWARFDIE no_children_die(unit, die_child0);
+  EXPECT_TRUE(no_children_die.children().empty());
+}


        


More information about the lldb-commits mailing list