Index: include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h =================================================================== --- include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h +++ include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h @@ -33,12 +33,14 @@ /// The integer depth of this DIE within the compile unit DIEs where the /// compile/type unit DIE has a depth of zero. - uint32_t Depth; + uint32_t Depth : 31; + /// If non-zero this DIE is the last child in a sibling chain. + uint32_t IsLastChild : 1; const DWARFAbbreviationDeclaration *AbbrevDecl; public: DWARFDebugInfoEntry() - : Offset(0), Depth(0), AbbrevDecl(nullptr) {} + : Offset(0), Depth(0), IsLastChild(false), AbbrevDecl(nullptr) {} /// Extracts a debug info entry, which is a child of a given unit, /// starting at a given offset. If DIE can't be extracted, returns false and @@ -52,6 +54,8 @@ uint32_t getOffset() const { return Offset; } uint32_t getDepth() const { return Depth; } + bool isLastChild() const { return IsLastChild; } + void setIsLastChild(bool B) { IsLastChild = B; } dwarf::Tag getTag() const { return AbbrevDecl ? AbbrevDecl->getTag() : dwarf::DW_TAG_null; } Index: include/llvm/DebugInfo/DWARF/DWARFDie.h =================================================================== --- include/llvm/DebugInfo/DWARF/DWARFDie.h +++ include/llvm/DebugInfo/DWARF/DWARFDie.h @@ -75,10 +75,6 @@ return Die->hasChildren(); } - /// Returns true for a valid DIE that terminates a sibling chain. - bool isNULL() const { - return getAbbreviationDeclarationPtr() == nullptr; - } /// Returns true if DIE represents a subprogram (not inlined). bool isSubprogramDIE() const; @@ -113,7 +109,8 @@ /// \param recurseDepth the depth to recurse to when dumping this DIE and its /// children. /// \param indent the number of characters to indent each line that is output. - void dump(raw_ostream &OS, unsigned recurseDepth, unsigned indent = 0) const; + /// \returns the DIE offset that follows all of the DIEs that were dumped. + uint32_t dump(raw_ostream &OS, unsigned recurseDepth, unsigned indent = 0) const; /// Extract the specified attribute from this DIE. /// Index: lib/DebugInfo/DWARF/DWARFDie.cpp =================================================================== --- lib/DebugInfo/DWARF/DWARFDie.cpp +++ lib/DebugInfo/DWARF/DWARFDie.cpp @@ -273,8 +273,7 @@ DWARFAddressRangesVector DWARFDie::getAddressRanges() const { - if (isNULL()) - return DWARFAddressRangesVector(); + assert(isValid() && "DIE must be checked before calling this function"); // Single range specified by low/high PC. uint64_t LowPC, HighPC; if (getLowAndHighPC(LowPC, HighPC)) { @@ -292,8 +291,7 @@ void DWARFDie::collectChildrenAddressRanges(DWARFAddressRangesVector& Ranges) const { - if (isNULL()) - return; + assert(isValid() && "DIE must be checked before calling this function"); if (isSubprogramDIE()) { const auto &DIERanges = getAddressRanges(); Ranges.insert(Ranges.end(), DIERanges.begin(), DIERanges.end()); @@ -353,17 +351,16 @@ CallColumn = getAttributeValueAsUnsignedConstant(DW_AT_call_column, 0); } -void DWARFDie::dump(raw_ostream &OS, unsigned RecurseDepth, - unsigned Indent) const { - if (!isValid()) - return; +uint32_t DWARFDie::dump(raw_ostream &OS, unsigned RecurseDepth, + unsigned Indent) const { + assert(isValid() && "DIE must be checked before calling this function"); DataExtractor debug_info_data = U->getDebugInfoExtractor(); - const uint32_t Offset = getOffset(); - uint32_t offset = Offset; + const uint32_t DieOffset = getOffset(); + uint32_t Offset = DieOffset; - if (debug_info_data.isValidOffset(offset)) { - uint32_t abbrCode = debug_info_data.getULEB128(&offset); - WithColor(OS, syntax::Address).get() << format("\n0x%8.8x: ", Offset); + if (debug_info_data.isValidOffset(Offset)) { + uint32_t abbrCode = debug_info_data.getULEB128(&Offset); + WithColor(OS, syntax::Address).get() << format("\n0x%8.8x: ", DieOffset); if (abbrCode) { auto AbbrevDecl = getAbbreviationDeclarationPtr(); @@ -380,17 +377,26 @@ // Dump all data in the DIE for the attributes. for (const auto &AttrSpec : AbbrevDecl->attributes()) { - dumpAttribute(OS, *this, &offset, AttrSpec.Attr, AttrSpec.Form, + dumpAttribute(OS, *this, &Offset, AttrSpec.Attr, AttrSpec.Form, Indent); } DWARFDie child = getFirstChild(); if (RecurseDepth > 0 && child) { while (child) { - child.dump(OS, RecurseDepth-1, Indent+2); + Offset = child.dump(OS, RecurseDepth-1, Indent+2); child = child.getSibling(); } } + + // We don't have NULL DIEs in our DWARFUnit anymore so we have to + // conjure up the NULL DIE when printing. + if (Die->isLastChild()) { + WithColor(OS, syntax::Address).get() << format("\n0x%8.8x: ", Offset); + OS.indent(Indent) << "NULL\n"; + // Add 1 to the offset for the ULEB128 abbreviation code byte. + Offset += 1; + } } else { OS << "Abbreviation code not found in 'debug_abbrev' class for code: " << abbrCode << '\n'; @@ -399,13 +405,14 @@ OS.indent(Indent) << "NULL\n"; } } + // Return the offset of the next DIE in case we need to print a NULL DIE. + return Offset; } void DWARFDie::getInlinedChainForAddress( const uint64_t Address, SmallVectorImpl &InlinedChain) const { - if (isNULL()) - return; + assert(isValid() && "DIE must be checked before calling this function"); DWARFDie DIE(*this); while (DIE) { // Append current DIE to inlined chain only if it has correct tag Index: lib/DebugInfo/DWARF/DWARFUnit.cpp =================================================================== --- lib/DebugInfo/DWARF/DWARFUnit.cpp +++ lib/DebugInfo/DWARF/DWARFUnit.cpp @@ -172,9 +172,15 @@ DataExtractor DebugInfoData = getDebugInfoExtractor(); uint32_t Depth = 0; bool IsCUDie = true; + // We don't add NULL DIEs do the Dies array anymore so we need to keep track + // of the indexes for DIEs at each depth so we can modify DIEs to tell them + // that they have a NULL sibling DIE so we can dump the NULL dies correctly. + SmallVector DieIndexes; + DieIndexes.push_back(0); while (DIE.extractFast(*this, &DIEOffset, DebugInfoData, NextCUOffset, Depth)) { + auto AbbrDecl = DIE.getAbbreviationDeclarationPtr(); if (IsCUDie) { if (AppendCUDie) Dies.push_back(DIE); @@ -185,17 +191,31 @@ // our DIE entries accordingly. Dies.reserve(Dies.size() + getDebugInfoSize() / 14); IsCUDie = false; - } else { + } else if (AbbrDecl) { + // Only append non NULL DIEs to the array. Dies.push_back(DIE); } - if (const DWARFAbbreviationDeclaration *AbbrDecl = - DIE.getAbbreviationDeclarationPtr()) { + if (AbbrDecl) { // Normal DIE - if (AbbrDecl->hasChildren()) + + // Set the DIE index in the DIE index array so we can track which siblings + // would have had NULL DIEs. + DieIndexes.back() = Dies.size() - 1; + if (AbbrDecl->hasChildren()) { + // Add an extra level to the DieIndexes when we have a child DIE. We + // set the index to the size of the Dies array since that will be the + // next DIE index. + DieIndexes.push_back(Dies.size()); ++Depth; + } } else { // NULL DIE. + + // Tell the previous sibing that is has a NULL sibling so we can dump + // the DWARF correctly. + Dies[DieIndexes.back()].setIsLastChild(true); + DieIndexes.pop_back(); if (Depth > 0) --Depth; if (Depth == 0) @@ -412,8 +432,14 @@ // Find the next DIE whose depth is the same as the Die's depth. for (size_t I=getDIEIndex(Die)+1, EndIdx = DieArray.size(); IaddChild(Clone); Index: unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp =================================================================== --- unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp +++ unittests/DebugInfo/DWARF/DWARFDebugInfoTest.cpp @@ -468,13 +468,7 @@ EXPECT_EQ(ArgcDieDG.getTag(), DW_TAG_formal_parameter); // Verify our formal parameter has a NULL tag sibling. - auto NullDieDG = ArgcDieDG.getSibling(); - EXPECT_TRUE(NullDieDG.isValid()); - if (NullDieDG) { - EXPECT_EQ(NullDieDG.getTag(), DW_TAG_null); - EXPECT_TRUE(!NullDieDG.getSibling().isValid()); - EXPECT_TRUE(!NullDieDG.getFirstChild().isValid()); - } + EXPECT_FALSE(ArgcDieDG.getSibling().isValid()); // Verify the sibling of our subprogram is our integer base type. auto IntDieDG = SubprogramDieDG.getSibling(); @@ -482,13 +476,7 @@ EXPECT_EQ(IntDieDG.getTag(), DW_TAG_base_type); // Verify the sibling of our subprogram is our integer base is a NULL tag. - NullDieDG = IntDieDG.getSibling(); - EXPECT_TRUE(NullDieDG.isValid()); - if (NullDieDG) { - EXPECT_EQ(NullDieDG.getTag(), DW_TAG_null); - EXPECT_TRUE(!NullDieDG.getSibling().isValid()); - EXPECT_TRUE(!NullDieDG.getFirstChild().isValid()); - } + EXPECT_FALSE(IntDieDG.getSibling().isValid()); } TEST(DWARFDebugInfo, TestDWARF32Version2Addr4Children) { @@ -984,10 +972,11 @@ enum class Tag: uint16_t { A = dwarf::DW_TAG_lo_user, B, - B1, - B2, C, - C1 + C1, + C2, + D, + D1 }; // Scope to allow us to re-use the same DIE names @@ -996,18 +985,20 @@ // // CU // A - // B - // B1 - // B2 - // C - // C1 + // B + // C + // C1 + // C2 + // D + // D1 dwarfgen::DIE CUDie = CU.getUnitDIE(); - CUDie.addChild((dwarf::Tag)Tag::A); - dwarfgen::DIE B = CUDie.addChild((dwarf::Tag)Tag::B); - dwarfgen::DIE C = CUDie.addChild((dwarf::Tag)Tag::C); - B.addChild((dwarf::Tag)Tag::B1); - B.addChild((dwarf::Tag)Tag::B2); + dwarfgen::DIE A = CUDie.addChild((dwarf::Tag)Tag::A); + A.addChild((dwarf::Tag)Tag::B); + dwarfgen::DIE C = A.addChild((dwarf::Tag)Tag::C); + dwarfgen::DIE D = A.addChild((dwarf::Tag)Tag::D); C.addChild((dwarf::Tag)Tag::C1); + C.addChild((dwarf::Tag)Tag::C2); + D.addChild((dwarf::Tag)Tag::D1); } MemoryBufferRef FileBuffer(DG->generate(), "dwarf"); @@ -1023,7 +1014,7 @@ // Get the compile unit DIE is valid. auto CUDie = U->getUnitDIE(false); EXPECT_TRUE(CUDie.isValid()); - // DieDG.dump(llvm::outs(), U, UINT32_MAX); + // CUDie.dump(llvm::outs(), UINT32_MAX); // The compile unit doesn't have a parent or a sibling. auto ParentDie = CUDie.getParent(); @@ -1033,44 +1024,55 @@ // Get the children of the compile unit auto A = CUDie.getFirstChild(); - auto B = A.getSibling(); + auto B = A.getFirstChild(); auto C = B.getSibling(); - auto Null = C.getSibling(); + auto D = C.getSibling(); - // Verify NULL Die is NULL and has no children or siblings - EXPECT_TRUE(Null.isNULL()); - EXPECT_FALSE(Null.getSibling().isValid()); - EXPECT_FALSE(Null.getFirstChild().isValid()); + // Verify C's sibling is invalid as we no longer store NULL DIEs. + EXPECT_FALSE(D.getSibling().isValid()); // Verify all children of the compile unit DIE are correct. EXPECT_EQ(A.getTag(), (dwarf::Tag)Tag::A); EXPECT_EQ(B.getTag(), (dwarf::Tag)Tag::B); EXPECT_EQ(C.getTag(), (dwarf::Tag)Tag::C); + EXPECT_EQ(D.getTag(), (dwarf::Tag)Tag::D); // Verify who has children - EXPECT_FALSE(A.hasChildren()); - EXPECT_TRUE(B.hasChildren()); + EXPECT_TRUE(A.hasChildren()); + EXPECT_FALSE(B.hasChildren()); + EXPECT_TRUE(C.hasChildren()); + EXPECT_TRUE(D.hasChildren()); // Make sure the parent of all the children of the compile unit are the // compile unit. EXPECT_EQ(A.getParent(), CUDie); - EXPECT_EQ(B.getParent(), CUDie); - EXPECT_EQ(Null.getParent(), CUDie); + + // Make sure the parent of all the children of A are the A. + // B is the first child in A, so we need to verify we can get the previous + // DIE as the parent. + EXPECT_EQ(B.getParent(), A); + // C is the second child in A, so we need to make sure we can backup across + // other DIE (B) at the same level to get the correct parent. + EXPECT_EQ(C.getParent(), A); + // D is the third child of A. We need to verify we can backup across other DIE + // (B and C) including DIE that have children (D) to get the correct parent. + EXPECT_EQ(D.getParent(), A); - EXPECT_FALSE(A.getFirstChild().isValid()); + // Verify that a DIE with no children returns an invalid DWARFDie. + EXPECT_FALSE(B.getFirstChild().isValid()); // Verify the children of the B DIE - auto B1 = B.getFirstChild(); - auto B2 = B1.getSibling(); - EXPECT_TRUE(B2.getSibling().isNULL()); + auto C1 = C.getFirstChild(); + auto C2 = C1.getSibling(); + EXPECT_FALSE(C2.getSibling().isValid()); // Verify all children of the B DIE correctly valid or invalid. - EXPECT_EQ(B1.getTag(), (dwarf::Tag)Tag::B1); - EXPECT_EQ(B2.getTag(), (dwarf::Tag)Tag::B2); + EXPECT_EQ(C1.getTag(), (dwarf::Tag)Tag::C1); + EXPECT_EQ(C2.getTag(), (dwarf::Tag)Tag::C2); // Make sure the parent of all the children of the B are the B. - EXPECT_EQ(B1.getParent(), B); - EXPECT_EQ(B2.getParent(), B); + EXPECT_EQ(C1.getParent(), C); + EXPECT_EQ(C2.getParent(), C); } TEST(DWARFDebugInfo, TestDWARFDie) {