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

[teemperor]

Summary

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.

[shafik]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h

117

I think:

bool operator==(const DWARFBaseDIE &lhs, const DWARFBaseDIE &rhs) {
  return lhs.GetDIE() == rhs.GetDIE() && lhs.GetCU() == rhs.GetCU();
}

will do the same thing but maybe I am confused.

Maybe we should have a unit test?

[teemperor]

• Added a unit test (thanks Shafik!)

[teemperor]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h

117

That's the DWARFDIE::operator== implementation called below and this special case is because of the comment one line above (we need to match the end iterator if it's default constructed when GetSibling() returns a DWARFDIE that has a valid CU but a null DIE).

[teemperor]

ping.

[JDevlieghere]

LGTM

[dblaikie]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h

97–105

rather than a temeplate, what if it were code something like this: https://godbolt.org/z/3abeK3EPx ?

[wallace]

This breaks debugging in many cases :( Many of my users started reporting crashes in LLDB when setting source line breakpoints or printing out variables.

For example, a simple command like (lldb) b TransAttr.cpp:758 causes a crash and has the following stack trace

thread #1, name = 'lldb', stop reason = signal SIGSEGV: invalid address (fault address: 0x10)
 * frame #0: 0x00007f1c2b642caa liblldb.so.14`DWARFUnit::GetSymbolFileDWARF(this=0x0000000000000000) const at DWARFUnit.h:194:56
   frame #1: 0x00007f1c2bd46ce4 liblldb.so.14`SymbolFileDWARF::GetTypeSystem(unit=0x0000000000000000) at SymbolFileDWARF.cpp:3880:15
   frame #2: 0x00007f1c2bd3b29b liblldb.so.14`SymbolFileDWARF::GetDWARFParser(unit=0x0000000000000000) at SymbolFileDWARF.cpp:3884:29
   frame #3: 0x00007f1c2bd8cb87 liblldb.so.14`DWARFASTParserClang::CopyUniqueClassMethodTypes(this=0x0000000001eaa920, src_class_die=0x00007ffe9ff0ff38, dst_class_die=0x00007ffe9ff0fe48, class_type=0x0000000001d42c10, failures=size=0) at DWARFASTParserClang.cpp:3572:11
   frame #4: 0x00007f1c2bd891fd liblldb.so.14`DWARFASTParserClang::ParseSubroutine(this=0x0000000001eaa920, die=0x00007ffe9ff10778, attrs=0x00007ffe9ff101b0) at DWARFASTParserClang.cpp:1025:15
   frame #5: 0x00007f1c2bd85080 liblldb.so.14`DWARFASTParserClang::ParseTypeFromDWARF(this=0x0000000001eaa920, sc=0x00007ffe9ff105d0, die=0x00007ffe9ff10778, type_is_new_ptr=0x0000000000000000) at DWARFASTParserClang.cpp:522:15
   frame #6: 0x00007f1c2bd401fb liblldb.so.14`SymbolFileDWARF::ParseType(this=0x00000000004610a0, sc=0x00007ffe9ff105d0, die=0x00007ffe9ff10778, type_is_new_ptr=0x0000000000000000) at SymbolFileDWARF.cpp:2946:31
   frame #7: 0x00007f1c2bd3b70f liblldb.so.14`SymbolFileDWARF::GetTypeForDIE(this=0x00000000004610a0, die=0x00007ffe9ff10778, resolve_function_context=false) at SymbolFileDWARF.cpp:2560:17
   frame #8: 0x00007f1c2bd3a4bd liblldb.so.14`SymbolFileDWARF::ResolveType(this=0x00000000004610a0, die=0x00007ffe9ff10778, assert_not_being_parsed=true, resolve_function_context=false) at SymbolFileDWARF.cpp:1568:18
   frame #9: 0x00007f1c2bd17500 liblldb.so.14`DWARFDIE::ResolveType(this=0x00007ffe9ff10778) const at DWARFDIE.cpp:350:24
   frame #10: 0x00007f1c2bd3a365 liblldb.so.14`SymbolFileDWARF::ResolveTypeUID(this=0x00000000004610a0, type_uid=57283337) at SymbolFileDWARF.cpp:1437:21
   frame #11: 0x00007f1c2b73037e liblldb.so.14`lldb_private::Function::GetType(this=0x0000000001cd5ed0) at Function.cpp:515:24
   frame #12: 0x00007f1c2b73017a liblldb.so.14`lldb_private::Function::GetStartLineSourceInfo(this=0x0000000001cd5ed0, source_file=0x00007ffe9ff109e0, line_no=0x00007ffe9ff109dc) at Function.cpp:252:3
   frame #13: 0x00007f1c2b515a5e liblldb.so.14`lldb_private::BreakpointResolverFileLine::FilterContexts(this=0x0000000001f61c00, sc_list=0x00007ffe9ff10d00, is_relative=true) at BreakpointResolverFileLine.cpp:162:20
   frame #14: 0x00007f1c2b5163d3 liblldb.so.14`lldb_private::BreakpointResolverFileLine::SearchCallback(this=0x0000000001f61c00, filter=0x0000000001fab770, context=0x00007ffe9ff10d68, addr=0x0000000000000000) at BreakpointResolverFileLine.cpp:254:3
   frame #15: 0x00007f1c2b5cae2a liblldb.so.14`lldb_private::SearchFilter::DoModuleIteration(this=0x0000000001fab770, context=0x00007ffe9ff10f28, searcher=0x0000000001f61c00) at SearchFilter.cpp:271:20
   frame #16: 0x00007f1c2b5cabb9 liblldb.so.14`lldb_private::SearchFilter::Search(this=0x0000000001fab770, searcher=0x0000000001f61c00) at SearchFilter.cpp:216:3
   frame #17: 0x00007f1c2b50c24e liblldb.so.14`lldb_private::BreakpointResolver::ResolveBreakpoint(this=0x0000000001f61c00, filter=0x0000000001fab770) at BreakpointResolver.cpp:177:10
   frame #18: 0x00007f1c2b4e93e6 liblldb.so.14`lldb_private::Breakpoint::ResolveBreakpoint(this=0x00000000003c76d0) at Breakpoint.cpp:443:20
   frame #19: 0x00007f1c2b851596 liblldb.so.14`lldb_private::Target::AddBreakpoint(this=0x000000000044dad0, bp_sp=std::__shared_ptr<lldb_private::Breakpoint, __gnu_cxx::_S_atomic>::element_type @ 0x00000000003c76d0, internal=false) at Target.cpp:662:10
   frame #20: 0x00007f1c2b852f64 liblldb.so.14`lldb_private::Target::CreateBreakpoint(this=0x000000000044dad0, filter_sp=std::__shared_ptr<lldb_private::SearchFilter, __gnu_cxx::_S_atomic>::element_type @ 0x0000000001fab770, resolver_sp=std::__shared_ptr<lldb_private::BreakpointResolver, __gnu_cxx::_S_atomic>::element_type @ 0x0000000001f61c00, internal=false, request_hardware=false, resolve_indirect_symbols=true) at Target.cpp:641:5
   frame #21: 0x00007f1c2b85342f liblldb.so.14`lldb_private::Target::CreateBreakpoint(this=0x000000000044dad0, containingModules=0x0000000000348b80, file=0x00007ffe9ff11768, line_no=758, column=0, offset=0, check_inlines=eLazyBoolYes, skip_prologue=eLazyBoolYes, internal=false, hardware=false, move_to_nearest_code=eLazyBoolYes) at Target.cpp:384:10
   frame #22: 0x00007f1c2d6895f0 liblldb.so.14`CommandObjectBreakpointSet::DoExecute(this=0x0000000000348750, command=0x00007ffe9ff11908, result=0x00007ffe9ff123f8) at CommandObjectBreakpoint.cpp:581:22
   frame #23: 0x00007f1c2b6cfed4 liblldb.so.14`lldb_private::CommandObjectParsed::Execute(this=0x0000000000348750, args_string="--file 'TransAttr.cpp' --line 758", result=0x00007ffe9ff123f8) at CommandObject.cpp:995:19
   frame #24: 0x00007f1c2b6a5062 liblldb.so.14`lldb_private::CommandInterpreter::HandleCommand(this=0x0000000000333650, command_line="breakpoint set --file 'TransAttr.cpp' --line 758", lazy_add_to_history=eLazyBoolCalculate, result=0x00007ffe9ff123f8) at CommandInterpreter.cpp:1804:14
   frame #25: 0x00007f1c2d700897 liblldb.so.14`lldb_private::CommandObjectRegexCommand::DoExecute(this=0x000000000038ab60, command=(Data = "TransAttr.cpp:758", Length = 17), result=0x00007ffe9ff123f8) at CommandObjectRegexCommand.cpp:56:28
   frame #26: 0x00007f1c2b6d0125 liblldb.so.14`lldb_private::CommandObjectRaw::Execute(this=0x000000000038ab60, args_string="TransAttr.cpp:758", result=0x00007ffe9ff123f8) at CommandObject.cpp:1017:17
   frame #27: 0x00007f1c2b6a5062 liblldb.so.14`lldb_private::CommandInterpreter::HandleCommand(this=0x0000000000333650, command_line="b TransAttr.cpp:758", lazy_add_to_history=eLazyBoolCalculate, result=0x00007ffe9ff123f8) at CommandInterpreter.cpp:1804:14
   frame #28: 0x00007f1c2b6a9a7f liblldb.so.14`lldb_private::CommandInterpreter::IOHandlerInputComplete(this=0x0000000000333650, io_handler=0x0000000001f44ee0, line=error: summary string parsing error) at CommandInterpreter.cpp:2848:3
   frame #29: 0x00007f1c2b5799de liblldb.so.14`lldb_private::IOHandlerEditline::Run(this=0x0000000001f44ee0) at IOHandler.cpp:579:22
   frame #30: 0x00007f1c2b53c323 liblldb.so.14`lldb_private::Debugger::RunIOHandlers(this=0x0000000000331ca0) at Debugger.cpp:877:16
   frame #31: 0x00007f1c2b6ab353 liblldb.so.14`lldb_private::CommandInterpreter::RunCommandInterpreter(this=0x0000000000333650, options=0x00007ffe9ff12750) at CommandInterpreter.cpp:3094:16
   frame #32: 0x00007f1c2aea5281 liblldb.so.14`lldb::SBDebugger::RunCommandInterpreter(this=0x00007ffe9ff12bd0, auto_handle_events=true, spawn_thread=false) at SBDebugger.cpp:1214:42
   frame #33: 0x000000000022c48e lldb`Driver::MainLoop(this=0x00007ffe9ff12bb0) at Driver.cpp:676:18
   frame #34: 0x000000000022d1c1 lldb`main(argc=2, argv=0x00007ffe9ff131c8) at Driver.cpp:944:26
   frame #35: 0x00007f1c266ddd95 libc.so.6`__libc_start_main(main=(lldb`main at Driver.cpp:870), argc=2, argv=0x00007ffe9ff131c8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007ffe9ff131b8) at libc-start.c:308:16
   frame #36: 0x000000000022894a lldb`_start at start.S:120

An interesting part is frame 3:

(lldb) frame select 3
frame #3: 0x00007f1c2bd8cb87 liblldb.so.14`DWARFASTParserClang::CopyUniqueClassMethodTypes(this=0x0000000001eaa920, src_class_die=0x00007ffe9ff0ff38, dst_class_die=0x00007ffe9ff0fe48, class_type=0x0000000001d42c10, failures=size=0) at DWARFASTParserClang.cpp:3572:11
   3569
   3570   DWARFASTParserClang *src_dwarf_ast_parser =
   3571       static_cast<DWARFASTParserClang *>(
-> 3572           SymbolFileDWARF::GetDWARFParser(*src_die.GetCU()));
   3573   DWARFASTParserClang *dst_dwarf_ast_parser =
   3574       static_cast<DWARFASTParserClang *>(
   3575           SymbolFileDWARF::GetDWARFParser(*dst_die.GetCU()));
(lldb) p src_die
(DWARFDIE) $0 = {
  DWARFBaseDIE = {
    m_cu = nullptr
    m_die = nullptr
  }
}

This makes me think that at some points the iterator is leaking invalid DIEs. Reverting this diff fixes the issues locally. So I think we should revert this until we can make a more stable patch.

cc: @clayborg

[wallace]

I found the issue: https://reviews.llvm.org/D107434

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

if (!clang_type && (encoding_data_type == Type::eEncodingIsPointerUID ||
                       encoding_data_type == Type::eEncodingIsTypedefUID)) {
     if (tag == DW_TAG_pointer_type) {
       DWARFDIE target_die = die.GetReferencedDIE(DW_AT_type);
 
       if (target_die.GetAttributeValueAsUnsigned(DW_AT_APPLE_block, 0)) {
         // 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 =
                 child_die.GetReferencedDIE(DW_AT_type);
 
             if (function_pointer_type) {
               DWARFDIE function_type =
                   function_pointer_type.GetReferencedDIE(DW_AT_type);
bool DWARFASTParserClang::ParseTemplateDIE(
     TypeSystemClang::TemplateParameterInfos &template_param_infos) {
   const dw_tag_t tag = die.Tag();
   bool is_template_template_argument = false;
 
   switch (tag) {
   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;
     }
     if (const char *name = die.GetName()) {
       template_param_infos.pack_name = name;
     }
     return true;
   }

 
 bool DWARFASTParserClang::ParseTemplateParameterInfos(
     const DWARFDIE &parent_die,
     TypeSystemClang::TemplateParameterInfos &template_param_infos) {
 
   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) {
     case DW_TAG_template_type_parameter:
     case DW_TAG_template_value_parameter:
     case DW_TAG_GNU_template_parameter_pack:
     case DW_TAG_GNU_template_template_param:
       ParseTemplateDIE(die, template_param_infos);

 
 void DWARFASTParserClang::EnsureAllDIEsInDeclContextHaveBeenParsed(
     lldb_private::CompilerDeclContext decl_context) {
   auto opaque_decl_ctx =
       (clang::DeclContext *)decl_context.GetOpaqueDeclContext();
   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);
 }
 
 CompilerDecl DWARFASTParserClang::GetDeclForUIDFromDWARF(const DWARFDIE &die) {
   clang::Decl *clang_decl = GetClangDeclForDIE(die);
   if (clang_decl != nullptr)
     return m_ast.GetCompilerDecl(clang_decl);
   return CompilerDecl();

 size_t DWARFASTParserClang::ParseChildEnumerators(
     lldb_private::CompilerType &clang_type, bool is_signed,
     uint32_t enumerator_byte_size, const DWARFDIE &parent_die) {
   if (!parent_die)
     return 0;
 
   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;
       const size_t num_child_attributes = die.GetAttributes(attributes);
       if (num_child_attributes > 0) {
         const char *name = nullptr;
         bool got_value = false;
         int64_t enum_value = 0;
bool DWARFASTParserClang::ParseChildMembers(
   FieldInfo last_field_info;
 
   ModuleSP module_sp = parent_die.GetDWARF()->GetObjectFile()->GetModule();
   TypeSystemClang *ast =
       llvm::dyn_cast_or_null<TypeSystemClang>(class_clang_type.GetTypeSystem());
   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) {
     case DW_TAG_member:
     case DW_TAG_APPLE_property:
       ParseSingleMember(die, parent_die, class_clang_type,
                         default_accessibility, delayed_properties, layout_info,
                         last_field_info);
size_t DWARFASTParserClang::ParseChildParameters(
     bool skip_artificial, bool &is_static, bool &is_variadic,
     bool &has_template_params, std::vector<CompilerType> &function_param_types,
     std::vector<clang::ParmVarDecl *> &function_param_decls,
     unsigned &type_quals) {
   if (!parent_die)
     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: {
       DWARFAttributes attributes;
       const size_t num_attributes = die.GetAttributes(attributes);
       if (num_attributes > 0) {
         const char *name = nullptr;
         DWARFFormValue param_type_die_form;

 
 llvm::Optional<SymbolFile::ArrayInfo>
 DWARFASTParser::ParseChildArrayInfo(const DWARFDIE &parent_die,
                                     const ExecutionContext *exe_ctx) {
   SymbolFile::ArrayInfo array_info;
   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;
 
     DWARFAttributes attributes;
     const size_t num_child_attributes = die.GetAttributes(attributes);
     if (num_child_attributes > 0) {
       uint64_t num_elements = 0;
for (DWARFDIE candidate = die; candidate; candidate = candidate.GetParent()) {
     }
   }
   assert(0 && "Shouldn't call GetContainingFunctionWithAbstractOrigin on "
               "something not in a function");
   return DWARFDIE();
 }
 
 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;
     }
   }
   return DWARFDIE();
 }
 
 static DWARFDIE FindFirstChildWithAbstractOrigin(const DWARFDIE &block,
bool DWARFASTParserClang::CopyUniqueClassMethodTypes(
   class_type->GetFullCompilerType();
 
   DWARFDIE src_die;
   DWARFDIE dst_die;
   UniqueCStringMap<DWARFDIE> src_name_to_die;
   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
       // are placed inside the class definitions and shouldn't be included in
       // the list of things are are tracking here.
       if (src_die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0) == 1) {
         const char *src_name = src_die.GetMangledName();
         if (src_name) {
           ConstString src_const_name(src_name);
           if (src_die.GetAttributeValueAsUnsigned(DW_AT_artificial, 0))
             src_name_to_die_artificial.Append(src_const_name, src_die);
           else
             src_name_to_die.Append(src_const_name, src_die);
         }
       }
     }
   }
-  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
       // are placed inside the class definitions and shouldn't be included in
       // the list of things are are tracking here.
       if (dst_die.GetAttributeValueAsUnsigned(DW_AT_declaration, 0) == 1) {
         const char *dst_name = dst_die.GetMangledName();
         if (dst_name) {

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.h

 //===-- DWARFDIE.h ----------------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDIE_H
 #define LLDB_SOURCE_PLUGINS_SYMBOLFILE_DWARF_DWARFDIE_H
 
 #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
   bool IsStructUnionOrClass() const;
 
   bool IsMethod() const;
 
   // Accessors
public:
   DWARFDIE
   GetAttributeValueAsReferenceDIE(const dw_attr_t attr) const;
 
   bool GetDIENamesAndRanges(const char *&name, const char *&mangled,
                             DWARFRangeList &ranges, int &decl_file,
                             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== differentiates 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

lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp

if (m_die->GetAttributeAddressRanges(m_cu, ranges,
         break;
       }
     } else {
       check_children = false;
     }
   }
 
   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;
     }
   }
   return result;
 }
 
 const char *DWARFDIE::GetMangledName() const {

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (die_offset >= min_die_offset) {
       if (add_type) {
         const bool assert_not_being_parsed = true;
         Type *type = ResolveTypeUID(die, assert_not_being_parsed);
         if (type)
           type_set.insert(type);
       }
     }
 
-    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);
     }
   }
 }
 
 void SymbolFileDWARF::GetTypes(SymbolContextScope *sc_scope,
                                TypeClass type_mask, TypeList &type_list)
 
if (!ClangModulesDeclVendor::LanguageSupportsClangModules(
           sc.comp_unit->GetLanguage()))
     return false;
   UpdateExternalModuleListIfNeeded();
 
   const DWARFDIE die = dwarf_cu->DIE();
   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;
 
     DWARFDIE module_die = child_die.GetReferencedDIE(DW_AT_import);
     if (module_die.Tag() != DW_TAG_module)
       continue;
 
     if (const char *name =
while (die) {
     else
       die = die.GetSibling();
   }
   return blocks_added;
 }
 
 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) {
       case DW_TAG_inheritance:
       case DW_TAG_subprogram:
         check_virtuality = true;
         break;
       default:
if (die) {
     } break;
     default:
       break;
     }
 
     // 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)
         return result_die;
     }
   }
 
   return DWARFDIE();
size_t SymbolFileDWARF::ParseVariables(const SymbolContext &sc,
   }
   return vars_added;
 }
 
 /// Collect call site parameters in a DW_TAG_call_site DIE.
 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;
 
     llvm::Optional<DWARFExpression> LocationInCallee;
     llvm::Optional<DWARFExpression> LocationInCaller;
 
     DWARFAttributes attributes;
LLDB_LOG(log, "CollectCallEdges: Found call site info in {0}",
            function_die.GetPubname());
 
   // Scan the DIE for TAG_call_site entries.
   // TODO: A recursive scan of all blocks in the subprogram is needed in order
   // to be DWARF5-compliant. This may need to be done lazily to be performant.
   // 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;
 
     llvm::Optional<DWARFDIE> call_origin;
     llvm::Optional<DWARFExpression> call_target;
     addr_t return_pc = LLDB_INVALID_ADDRESS;
     addr_t call_inst_pc = LLDB_INVALID_ADDRESS;
     addr_t low_pc = LLDB_INVALID_ADDRESS;

lldb/unittests/SymbolFile/DWARF/CMakeLists.txt

 add_lldb_unittest(SymbolFileDWARFTests
   DWARFASTParserClangTests.cpp
+  DWARFDIETest.cpp
   DWARFUnitTest.cpp
   SymbolFileDWARFTests.cpp
   XcodeSDKModuleTests.cpp
 
   LINK_LIBS
     lldbCore
     lldbHost
     lldbSymbol

lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp

+//===-- 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());
+}