From 3733d88e626f261192b78f1b40530c1544995d5a Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Wed, 24 Jan 2024 15:23:09 -0800 Subject: [PATCH 01/13] Not formatted, mostly to keep the current model easy to see --- llvm/include/llvm/DebugInfo/DIContext.h | 1 + .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 20 +++ llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | 1 + llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 151 +++++++++++++----- 4 files changed, 129 insertions(+), 44 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DIContext.h b/llvm/include/llvm/DebugInfo/DIContext.h index 78ac34e5f0d26c4..298628b551fb68d 100644 --- a/llvm/include/llvm/DebugInfo/DIContext.h +++ b/llvm/include/llvm/DebugInfo/DIContext.h @@ -205,6 +205,7 @@ struct DIDumpOptions { bool DisplayRawContents = false; bool IsEH = false; bool DumpNonSkeleton = false; + bool DumpAggregateErrors = true; std::function GetNameForDWARFReg; diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index e56d3781e824f34..b9a68217aebfce4 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -77,8 +77,23 @@ class DWARFVerifier { }; private: + class ErrorAggregator { + private: + DWARFVerifier &Verifier; + std::map UniqueErrors; + static std::string Clean(const char *s); + + public: + ErrorAggregator(DWARFVerifier &v) : Verifier(v) {} + raw_ostream &operator<<(const char *s); + void Collect(const std::string &s); + void Dump(); + }; + friend class ErrorAggregator; + raw_ostream &OS; DWARFContext &DCtx; + ErrorAggregator ErrAggregation; DIDumpOptions DumpOpts; uint32_t NumDebugLineErrors = 0; // Used to relax some checks that do not currently work portably @@ -86,6 +101,8 @@ class DWARFVerifier { bool IsMachOObject; using ReferenceMap = std::map>; + raw_ostream &aggregate(const char *); + ErrorAggregator &aggregate(); raw_ostream &error() const; raw_ostream &warn() const; raw_ostream ¬e() const; @@ -348,6 +365,9 @@ class DWARFVerifier { bool verifyDebugStrOffsets( StringRef SectionName, const DWARFSection &Section, StringRef StrData, void (DWARFObject::*)(function_ref) const); + + /// Emits any aggregate information collection, depending on the dump options + void finish(); }; static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS, diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp index 792df53d304aa77..aa294444b46803a 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp @@ -1408,6 +1408,7 @@ bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) { if (DumpOpts.DumpType & DIDT_DebugStrOffsets) Success &= verifier.handleDebugStrOffsets(); Success &= verifier.handleAccelTables(); + verifier.finish(); return Success; } diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index c4c14f5e2c9d360..e43f01fad3ce02d 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -240,20 +240,20 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit, DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false); if (!Die) { - error() << "Compilation unit without DIE.\n"; + aggregate() << "Compilation unit without DIE.\n"; NumUnitErrors++; return NumUnitErrors; } if (!dwarf::isUnitType(Die.getTag())) { - error() << "Compilation unit root DIE is not a unit DIE: " + aggregate() << "Compilation unit root DIE is not a unit DIE: " << dwarf::TagString(Die.getTag()) << ".\n"; NumUnitErrors++; } uint8_t UnitType = Unit.getUnitType(); if (!DWARFUnit::isMatchingUnitTypeAndTag(UnitType, Die.getTag())) { - error() << "Compilation unit type (" << dwarf::UnitTypeString(UnitType) + aggregate("Mismatched unit type") << "Compilation unit type (" << dwarf::UnitTypeString(UnitType) << ") and root DIE (" << dwarf::TagString(Die.getTag()) << ") do not match.\n"; NumUnitErrors++; @@ -263,7 +263,7 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit, // 3.1.2 Skeleton Compilation Unit Entries: // "A skeleton compilation unit has no children." if (Die.getTag() == dwarf::DW_TAG_skeleton_unit && Die.hasChildren()) { - error() << "Skeleton compilation unit has children.\n"; + aggregate() << "Skeleton compilation unit has children.\n"; NumUnitErrors++; } @@ -280,14 +280,14 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) { DWARFDie Curr = Die.getParent(); for (; Curr.isValid() && !Curr.isSubprogramDIE(); Curr = Die.getParent()) { if (Curr.getTag() == DW_TAG_inlined_subroutine) { - error() << "Call site entry nested within inlined subroutine:"; + aggregate() << "Call site entry nested within inlined subroutine:"; Curr.dump(OS); return 1; } } if (!Curr.isValid()) { - error() << "Call site entry not nested within a valid subprogram:"; + aggregate() << "Call site entry not nested within a valid subprogram:"; Die.dump(OS); return 1; } @@ -297,7 +297,7 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) { DW_AT_call_all_tail_calls, DW_AT_GNU_all_call_sites, DW_AT_GNU_all_source_call_sites, DW_AT_GNU_all_tail_call_sites}); if (!CallAttr) { - error() << "Subprogram with call site entry has no DW_AT_call attribute:"; + aggregate() << "Subprogram with call site entry has no DW_AT_call attribute:"; Curr.dump(OS); Die.dump(OS, /*indent*/ 1); return 1; @@ -313,7 +313,7 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) { Expected AbbrDeclsOrErr = Abbrev->getAbbreviationDeclarationSet(0); if (!AbbrDeclsOrErr) { - error() << toString(AbbrDeclsOrErr.takeError()) << "\n"; + aggregate("Abbreviateion Declaration error") << toString(AbbrDeclsOrErr.takeError()) << "\n"; return 1; } @@ -324,7 +324,7 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) { for (auto Attribute : AbbrDecl.attributes()) { auto Result = AttributeSet.insert(Attribute.Attr); if (!Result.second) { - error() << "Abbreviation declaration contains multiple " + aggregate("Abbreviation declartion contains multiple attributes") << "Abbreviation declaration contains multiple " << AttributeString(Attribute.Attr) << " attributes.\n"; AbbrDecl.dump(OS); ++NumErrors; @@ -440,7 +440,7 @@ unsigned DWARFVerifier::verifyIndex(StringRef Name, auto &M = *Sections[Col]; auto I = M.find(SC.getOffset()); if (I != M.end() && I.start() < (SC.getOffset() + SC.getLength())) { - error() << llvm::formatv( + aggregate("Overlapping index entries") << llvm::formatv( "overlapping index entries for entries {0:x16} " "and {1:x16} for column {2}\n", *I, Sig, toString(Index.getColumnKinds()[Col])); @@ -532,7 +532,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, for (const auto &Range : Ranges) { if (!Range.valid()) { ++NumErrors; - error() << "Invalid address range " << Range << "\n"; + aggregate() << "Invalid address range " << Range << "\n"; DumpDieAfterError = true; continue; } @@ -545,7 +545,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, // address: 0 or -1. if (auto PrevRange = RI.insert(Range)) { ++NumErrors; - error() << "DIE has overlapping ranges in DW_AT_ranges attribute: " + aggregate() << "DIE has overlapping ranges in DW_AT_ranges attribute: " << *PrevRange << " and " << Range << '\n'; DumpDieAfterError = true; } @@ -558,7 +558,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, const auto IntersectingChild = ParentRI.insert(RI); if (IntersectingChild != ParentRI.Children.end()) { ++NumErrors; - error() << "DIEs have overlapping address ranges:"; + aggregate() << "DIEs have overlapping address ranges:"; dump(Die); dump(IntersectingChild->Die) << '\n'; } @@ -569,7 +569,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, ParentRI.Die.getTag() == DW_TAG_subprogram); if (ShouldBeContained && !ParentRI.contains(RI)) { ++NumErrors; - error() << "DIE address ranges are not contained in its parent's ranges:"; + aggregate() << "DIE address ranges are not contained in its parent's ranges:"; dump(ParentRI.Die); dump(Die, 2) << '\n'; } @@ -754,7 +754,7 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die, auto CUOffset = AttrValue.Value.getRawUValue(); if (CUOffset >= CUSize) { ++NumErrors; - error() << FormEncodingString(Form) << " CU offset " + aggregate("Invalid CU offset") << FormEncodingString(Form) << " CU offset " << format("0x%08" PRIx64, CUOffset) << " is invalid (must be less than CU size of " << format("0x%08" PRIx64, CUSize) << "):\n"; @@ -776,7 +776,7 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die, if (RefVal) { if (*RefVal >= DieCU->getInfoSection().Data.size()) { ++NumErrors; - error() << "DW_FORM_ref_addr offset beyond .debug_info " + aggregate() << "DW_FORM_ref_addr offset beyond .debug_info " "bounds:\n"; dump(Die) << '\n'; } else { @@ -821,7 +821,7 @@ unsigned DWARFVerifier::verifyDebugInfoReferences( if (GetDIEForOffset(Pair.first)) continue; ++NumErrors; - error() << "invalid DIE reference " << format("0x%08" PRIx64, Pair.first) + aggregate() << "invalid DIE reference " << format("0x%08" PRIx64, Pair.first) << ". Offset is in between DIEs:\n"; for (auto Offset : Pair.second) dump(GetDIEForOffset(Offset)) << '\n'; @@ -845,7 +845,7 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() { if (LineTableOffset < DCtx.getDWARFObj().getLineSection().Data.size()) { if (!LineTable) { ++NumDebugLineErrors; - error() << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset) + aggregate("Unparseable .debug_line entry") << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset) << "] was not able to be parsed for CU:\n"; dump(Die) << '\n'; continue; @@ -860,7 +860,7 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() { auto Iter = StmtListToDie.find(LineTableOffset); if (Iter != StmtListToDie.end()) { ++NumDebugLineErrors; - error() << "two compile unit DIEs, " + aggregate("Identical DW_AT_stmt_list section offset") << "two compile unit DIEs, " << format("0x%08" PRIx64, Iter->second.getOffset()) << " and " << format("0x%08" PRIx64, Die.getOffset()) << ", have the same DW_AT_stmt_list section offset:\n"; @@ -892,7 +892,7 @@ void DWARFVerifier::verifyDebugLineRows() { // Verify directory index. if (FileName.DirIdx > MaxDirIndex) { ++NumDebugLineErrors; - error() << ".debug_line[" + aggregate("Invalid index in .debug_line->prologue.file_names->dir_idx") << ".debug_line[" << format("0x%08" PRIx64, *toSectionOffset(Die.find(DW_AT_stmt_list))) << "].prologue.file_names[" << FileIndex @@ -928,7 +928,7 @@ void DWARFVerifier::verifyDebugLineRows() { // Verify row address. if (Row.Address.Address < PrevAddress) { ++NumDebugLineErrors; - error() << ".debug_line[" + aggregate("decreasing address between debug_line rows") << ".debug_line[" << format("0x%08" PRIx64, *toSectionOffset(Die.find(DW_AT_stmt_list))) << "] row[" << RowIndex @@ -949,7 +949,7 @@ void DWARFVerifier::verifyDebugLineRows() { LineTable->Rows.size() != 1) && !LineTable->hasFileAtIndex(Row.File)) { ++NumDebugLineErrors; - error() << ".debug_line[" + aggregate("Invalid file index in debug_line") << ".debug_line[" << format("0x%08" PRIx64, *toSectionOffset(Die.find(DW_AT_stmt_list))) << "][" << RowIndex << "] has invalid file index " << Row.File @@ -971,8 +971,8 @@ void DWARFVerifier::verifyDebugLineRows() { DWARFVerifier::DWARFVerifier(raw_ostream &S, DWARFContext &D, DIDumpOptions DumpOpts) - : OS(S), DCtx(D), DumpOpts(std::move(DumpOpts)), IsObjectFile(false), - IsMachOObject(false) { + : OS(S), DCtx(D), ErrAggregation(*this), DumpOpts(std::move(DumpOpts)), + IsObjectFile(false), IsMachOObject(false) { if (const auto *F = DCtx.getDWARFObj().getFile()) { IsObjectFile = F->isRelocatableObject(); IsMachOObject = F->isMachO(); @@ -999,7 +999,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, // Verify that the fixed part of the header is not too short. if (!AccelSectionData.isValidOffset(AccelTable.getSizeHdr())) { - error() << "Section is too small to fit a section header.\n"; + aggregate() << "Section is too small to fit a section header.\n"; return 1; } @@ -1020,18 +1020,18 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, for (uint32_t BucketIdx = 0; BucketIdx < NumBuckets; ++BucketIdx) { uint32_t HashIdx = AccelSectionData.getU32(&BucketsOffset); if (HashIdx >= NumHashes && HashIdx != UINT32_MAX) { - error() << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx, + aggregate("Invalid hash index") << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx, HashIdx); ++NumErrors; } } uint32_t NumAtoms = AccelTable.getAtomsDesc().size(); if (NumAtoms == 0) { - error() << "No atoms: failed to read HashData.\n"; + aggregate() << "No atoms: failed to read HashData.\n"; return 1; } if (!AccelTable.validateForms()) { - error() << "Unsupported form: failed to read HashData.\n"; + aggregate() << "Unsupported form: failed to read HashData.\n"; return 1; } @@ -1042,7 +1042,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, uint64_t HashDataOffset = AccelSectionData.getU32(&DataOffset); if (!AccelSectionData.isValidOffsetForDataOfSize(HashDataOffset, sizeof(uint64_t))) { - error() << format("Hash[%d] has invalid HashData offset: " + aggregate("Invalid HashData offset") << format("Hash[%d] has invalid HashData offset: " "0x%08" PRIx64 ".\n", HashIdx, HashDataOffset); ++NumErrors; @@ -1068,7 +1068,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, if (!Name) Name = ""; - error() << format( + aggregate("Invalid DIE offset") << format( "%s Bucket[%d] Hash[%d] = 0x%08x " "Str[%u] = 0x%08" PRIx64 " DIE[%d] = 0x%08" PRIx64 " " "is not a valid DIE offset for \"%s\".\n", @@ -1079,7 +1079,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, continue; } if ((Tag != dwarf::DW_TAG_null) && (Die.getTag() != Tag)) { - error() << "Tag " << dwarf::TagString(Tag) + aggregate("Mismatched Tag in accellerator table") << "Tag " << dwarf::TagString(Tag) << " in accelerator table does not match Tag " << dwarf::TagString(Die.getTag()) << " of DIE[" << HashDataIdx << "].\n"; @@ -1106,7 +1106,7 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) { unsigned NumErrors = 0; for (const DWARFDebugNames::NameIndex &NI : AccelTable) { if (NI.getCUCount() == 0) { - error() << formatv("Name Index @ {0:x} does not index any CU\n", + aggregate("Name Index doesn't index any CU") << formatv("Name Index @ {0:x} does not index any CU\n", NI.getUnitOffset()); ++NumErrors; continue; @@ -1116,7 +1116,7 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) { auto Iter = CUMap.find(Offset); if (Iter == CUMap.end()) { - error() << formatv( + aggregate("Name Index references non-existing CU") << formatv( "Name Index @ {0:x} references a non-existing CU @ {1:x}\n", NI.getUnitOffset(), Offset); ++NumErrors; @@ -1124,7 +1124,7 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) { } if (Iter->second != NotIndexed) { - error() << formatv("Name Index @ {0:x} references a CU @ {1:x}, but " + aggregate("Duplicate Name Index") << formatv("Name Index @ {0:x} references a CU @ {1:x}, but " "this CU is already indexed by Name Index @ {2:x}\n", NI.getUnitOffset(), Offset, Iter->second); continue; @@ -1167,7 +1167,7 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI, for (uint32_t Bucket = 0, End = NI.getBucketCount(); Bucket < End; ++Bucket) { uint32_t Index = NI.getBucketArrayEntry(Bucket); if (Index > NI.getNameCount()) { - error() << formatv("Bucket {0} of Name Index @ {1:x} contains invalid " + aggregate("Name Index Bucket contains invalid value") << formatv("Bucket {0} of Name Index @ {1:x} contains invalid " "value {2}. Valid range is [0, {3}].\n", Bucket, NI.getUnitOffset(), Index, NI.getNameCount()); ++NumErrors; @@ -1202,7 +1202,7 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI, // will not match because we have already verified that the name's hash // puts it into the previous bucket.) if (B.Index > NextUncovered) { - error() << formatv("Name Index @ {0:x}: Name table entries [{1}, {2}] " + aggregate("Name table entries uncovered by hash table") << formatv("Name Index @ {0:x}: Name table entries [{1}, {2}] " "are not covered by the hash table.\n", NI.getUnitOffset(), NextUncovered, B.Index - 1); ++NumErrors; @@ -1220,7 +1220,7 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI, // bucket as empty. uint32_t FirstHash = NI.getHashArrayEntry(Idx); if (FirstHash % NI.getBucketCount() != B.Bucket) { - error() << formatv( + aggregate("Name Index point to mismatched hash value") << formatv( "Name Index @ {0:x}: Bucket {1} is not empty but points to a " "mismatched hash value {2:x} (belonging to bucket {3}).\n", NI.getUnitOffset(), B.Bucket, FirstHash, @@ -1238,7 +1238,7 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI, const char *Str = NI.getNameTableEntry(Idx).getString(); if (caseFoldingDjbHash(Str) != Hash) { - error() << formatv("Name Index @ {0:x}: String ({1}) at index {2} " + aggregate("String hash doesn't match Name Index hash") << formatv("Name Index @ {0:x}: String ({1}) at index {2} " "hashes to {3:x}, but " "the Name Index hash is {4:x}\n", NI.getUnitOffset(), Str, Idx, @@ -1258,7 +1258,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( DWARFDebugNames::AttributeEncoding AttrEnc) { StringRef FormName = dwarf::FormEncodingString(AttrEnc.Form); if (FormName.empty()) { - error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an " + aggregate("Unknown NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an " "unknown form: {3}.\n", NI.getUnitOffset(), Abbr.Code, AttrEnc.Index, AttrEnc.Form); @@ -1267,7 +1267,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( if (AttrEnc.Index == DW_IDX_type_hash) { if (AttrEnc.Form != dwarf::DW_FORM_data8) { - error() << formatv( + aggregate("Unexpected NameIndex Abbreviation") << formatv( "NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_type_hash " "uses an unexpected form {2} (should be {3}).\n", NI.getUnitOffset(), Abbr.Code, AttrEnc.Form, dwarf::DW_FORM_data8); @@ -1280,7 +1280,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( constexpr static auto AllowedForms = {dwarf::Form::DW_FORM_flag_present, dwarf::Form::DW_FORM_ref4}; if (!is_contained(AllowedForms, AttrEnc.Form)) { - error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent " + aggregate("Unexpected NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent " "uses an unexpected form {2} (should be " "DW_FORM_ref4 or DW_FORM_flag_present).\n", NI.getUnitOffset(), Abbr.Code, AttrEnc.Form); @@ -1315,7 +1315,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( } if (!DWARFFormValue(AttrEnc.Form).isFormClass(Iter->Class)) { - error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an " + aggregate("Unexpected NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an " "unexpected form {3} (expected form class {4}).\n", NI.getUnitOffset(), Abbr.Code, AttrEnc.Index, AttrEnc.Form, Iter->ClassName); @@ -1344,7 +1344,7 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) { SmallSet Attributes; for (const auto &AttrEnc : Abbrev.Attributes) { if (!Attributes.insert(AttrEnc.Index).second) { - error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x} contains " + aggregate("NameIndex Abbreviateion contains multiple attributes") << formatv("NameIndex @ {0:x}: Abbreviation {1:x} contains " "multiple {2} attributes.\n", NI.getUnitOffset(), Abbrev.Code, AttrEnc.Index); ++NumErrors; @@ -1354,7 +1354,7 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) { } if (NI.getCUCount() > 1 && !Attributes.count(dwarf::DW_IDX_compile_unit)) { - error() << formatv("NameIndex @ {0:x}: Indexing multiple compile units " + aggregate("Abbreviation contains no attribute") << formatv("NameIndex @ {0:x}: Indexing multiple compile units " "and abbreviation {1:x} has no {2} attribute.\n", NI.getUnitOffset(), Abbrev.Code, dwarf::DW_IDX_compile_unit); @@ -1803,6 +1803,69 @@ bool DWARFVerifier::verifyDebugStrOffsets( } return Success; } +/* + raw_ostream & operator <<(const char * s) { + collect(s); + out << "From inside the class: " << s; + return out; + } + void collect(const char * s) { + counters[s]++; + } + void dump() { + for (auto && [name, count]: counters) { + out << "counter: " << name << ", count: " << count << "\n"; + } + } + + ErrorAggregator collectErrors(); +*/ + +std::string DWARFVerifier::ErrorAggregator::Clean(const char *s) { + // Find the position of the first alphabetic character + const char *start = s; + while (!std::isalpha(*start)) { + ++start; + } + + // Find the position of the last alphabetic character + const char *end = &start[std::strlen(start) - 1]; + while (end >= start && !std::isalpha(*end)) { + --end; + } + + // Create a std::string from the trimmed portion of the const char* + return std::string(start, end - start + 1); +} + +raw_ostream &DWARFVerifier::ErrorAggregator::operator<<(const char *s) { + Collect(Clean(s)); + return Verifier.error() << s; +} + +void DWARFVerifier::ErrorAggregator::Collect(const std::string &s) { + UniqueErrors[s]++; +} + +void DWARFVerifier::ErrorAggregator::Dump() { + for (auto &&[name, count] : UniqueErrors) { + Verifier.error() << "counter: " << name << ", count: " << count << "\n"; + } +} + +raw_ostream &DWARFVerifier::aggregate(const char *name) { + ErrAggregation.Collect(name); + return error(); +} + +DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate() { + return ErrAggregation; +} + +void DWARFVerifier::finish() { + if (DumpOpts.DumpAggregateErrors) + ErrAggregation.Dump(); +} raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); } From 2d717a7e22d8c8e52e6875bc47bf48a405081303 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Wed, 24 Jan 2024 15:33:04 -0800 Subject: [PATCH 02/13] Added usage comments --- llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index b9a68217aebfce4..d55709675cbd69f 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -101,8 +101,14 @@ class DWARFVerifier { bool IsMachOObject; using ReferenceMap = std::map>; - raw_ostream &aggregate(const char *); + // For errors that have sensible names as the first (or only) string + // you can just use aggregate() << "DIE looks stupid: " << details... + // and it will get aggregated as "DIE looks stupid". ErrorAggregator &aggregate(); + // For errors that have details before good 'categories', you'll need + // to provide the aggregated name, like this: + // aggregate("CU index is busted") << "CU index [" << n << "] is busted" + raw_ostream &aggregate(const char *); raw_ostream &error() const; raw_ostream &warn() const; raw_ostream ¬e() const; From 01c52275e9addf168e61e046c8f565856b43bdab Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Wed, 24 Jan 2024 16:11:12 -0800 Subject: [PATCH 03/13] Finished first pass over the error()'s (inserted some TODO's) --- .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 24 +++-- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 93 ++++++++++--------- 2 files changed, 66 insertions(+), 51 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index d55709675cbd69f..ae2aa62e28f921e 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -81,12 +81,15 @@ class DWARFVerifier { private: DWARFVerifier &Verifier; std::map UniqueErrors; + raw_ostream *NextLineOverride; + static std::string Clean(const char *s); public: - ErrorAggregator(DWARFVerifier &v) : Verifier(v) {} + ErrorAggregator(DWARFVerifier &v) : Verifier(v), NextLineOverride(nullptr) {} raw_ostream &operator<<(const char *s); - void Collect(const std::string &s); + ErrorAggregator &NextLineStreamOverride(raw_ostream &o); + raw_ostream &Collect(const std::string &s); void Dump(); }; friend class ErrorAggregator; @@ -102,13 +105,20 @@ class DWARFVerifier { using ReferenceMap = std::map>; // For errors that have sensible names as the first (or only) string - // you can just use aggregate() << "DIE looks stupid: " << details... - // and it will get aggregated as "DIE looks stupid". + // you can just use aggregate() << "DIE is damaged: " << details... + // and it will get aggregated as "DIE is damaged". ErrorAggregator &aggregate(); - // For errors that have details before good 'categories', you'll need - // to provide the aggregated name, like this: - // aggregate("CU index is busted") << "CU index [" << n << "] is busted" + // For errors that have the useful aggregated category in a non-error stream + // you can do things like this (aggregated as 'Die size is wrong') + // aggregate(note()) << "DIE size is wrong.\n" + ErrorAggregator &aggregate(raw_ostream &); + + // For errors that have valuable detail before text that is the appropriate + // aggregate category, you can provide the aggregated name for the error like + // this: + // aggregate("CU index is broken") << "CU index [" << n << "] is broken" raw_ostream &aggregate(const char *); + raw_ostream &error() const; raw_ostream &warn() const; raw_ostream ¬e() const; diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index e43f01fad3ce02d..fcfef5904d55ed8 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -170,17 +170,19 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData, error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", UnitIndex, OffsetStart); if (!ValidLength) - note() << "The length for this unit is too " + aggregate(note()) << "The length for this unit is too " "large for the .debug_info provided.\n"; - if (!ValidVersion) - note() << "The 16 bit unit header version is not valid.\n"; - if (!ValidType) - note() << "The unit type encoding is not valid.\n"; + if (!ValidVersion) { + aggregate(note()) << "The 16 bit unit header version is not valid.\n"; + } + if (!ValidType){ + aggregate(note()) << "The unit type encoding is not valid.\n"; + } if (!ValidAbbrevOffset) - note() << "The offset into the .debug_abbrev section is " + aggregate(note()) << "The offset into the .debug_abbrev section is " "not valid.\n"; if (!ValidAddrSize) - note() << "The address size is unsupported.\n"; + aggregate(note()) << "The address size is unsupported.\n"; } *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4); return Success; @@ -198,7 +200,7 @@ bool DWARFVerifier::verifyName(const DWARFDie &Die) { if (OriginalFullName.empty() || OriginalFullName == ReconstructedName) return false; - error() << "Simplified template DW_AT_name could not be reconstituted:\n" + aggregate() << "Simplified template DW_AT_name could not be reconstituted:\n" << formatv(" original: {0}\n" " reconstituted: {1}\n", OriginalFullName, ReconstructedName); @@ -581,6 +583,7 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, return NumErrors; } +// Aggregation TODO: unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, DWARFAttribute &AttrValue) { unsigned NumErrors = 0; @@ -796,6 +799,7 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die, case DW_FORM_line_strp: { if (Error E = AttrValue.Value.getAsCString().takeError()) { ++NumErrors; + // Aggregation TODO: error() << toString(std::move(E)) << ":\n"; dump(Die) << '\n'; } @@ -1005,6 +1009,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, // Verify that the section is not too short. if (Error E = AccelTable.extract()) { + // Aggregation TODO: error() << toString(std::move(E)) << '\n'; return 1; } @@ -1361,7 +1366,7 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) { ++NumErrors; } if (!Attributes.count(dwarf::DW_IDX_die_offset)) { - error() << formatv( + aggregate("Abbreviate in NameIndex missing attribute") << formatv( "NameIndex @ {0:x}: Abbreviation {1:x} has no {2} attribute.\n", NI.getUnitOffset(), Abbrev.Code, dwarf::DW_IDX_die_offset); ++NumErrors; @@ -1417,7 +1422,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries( const char *CStr = NTE.getString(); if (!CStr) { - error() << formatv( + aggregate("Unable to get string associated with name") << formatv( "Name Index @ {0:x}: Unable to get string associated with name {1}.\n", NI.getUnitOffset(), NTE.getIndex()); return 1; @@ -1433,7 +1438,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries( EntryOr = NI.getEntry(&NextEntryID)) { uint32_t CUIndex = *EntryOr->getCUIndex(); if (CUIndex > NI.getCUCount()) { - error() << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an " + aggregate("Name Index entry contains invalid CU index") << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an " "invalid CU index ({2}).\n", NI.getUnitOffset(), EntryID, CUIndex); ++NumErrors; @@ -1443,21 +1448,21 @@ unsigned DWARFVerifier::verifyNameIndexEntries( uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset(); DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset); if (!DIE) { - error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a " + aggregate("NameIndex references nonexisten DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x} references a " "non-existing DIE @ {2:x}.\n", NI.getUnitOffset(), EntryID, DIEOffset); ++NumErrors; continue; } if (DIE.getDwarfUnit()->getOffset() != CUOffset) { - error() << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of " - "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n", + aggregate("Name index contains mismatched CU of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of " + "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n", NI.getUnitOffset(), EntryID, DIEOffset, CUOffset, DIE.getDwarfUnit()->getOffset()); ++NumErrors; } if (DIE.getTag() != EntryOr->tag()) { - error() << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of " + aggregate("Name Index contains mismatched Tag of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of " "DIE @ {2:x}: index - {3}; debug_info - {4}.\n", NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(), DIE.getTag()); @@ -1471,7 +1476,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries( DIE.getTag() == DW_TAG_inlined_subroutine; auto EntryNames = getNames(DIE, IncludeStrippedTemplateNames); if (!is_contained(EntryNames, Str)) { - error() << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name " + aggregate("Name Index contains mismatched name of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name " "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n", NI.getUnitOffset(), EntryID, DIEOffset, Str, make_range(EntryNames.begin(), EntryNames.end())); @@ -1482,12 +1487,13 @@ unsigned DWARFVerifier::verifyNameIndexEntries( [&](const DWARFDebugNames::SentinelError &) { if (NumEntries > 0) return; - error() << formatv("Name Index @ {0:x}: Name {1} ({2}) is " + aggregate("NameIndex Name is not associated with any entries") << formatv("Name Index @ {0:x}: Name {1} ({2}) is " "not associated with any entries.\n", NI.getUnitOffset(), NTE.getIndex(), Str); ++NumErrors; }, [&](const ErrorInfoBase &Info) { + // Aggregation TODO: error() << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n", NI.getUnitOffset(), NTE.getIndex(), Str, @@ -1619,7 +1625,7 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness( if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) { return E.getDIEUnitOffset() == DieUnitOffset; })) { - error() << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with " + aggregate("Name Index DIE entry missing name") << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with " "name {3} missing.\n", NI.getUnitOffset(), Die.getOffset(), Die.getTag(), Name); @@ -1641,6 +1647,7 @@ unsigned DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection, // This verifies that we can read individual name indices and their // abbreviation tables. if (Error E = AccelTable.extract()) { + // Aggregate TODO: error() << toString(std::move(E)) << '\n'; return 1; } @@ -1741,7 +1748,7 @@ bool DWARFVerifier::verifyDebugStrOffsets( if (!C) break; if (C.tell() + Length > DA.getData().size()) { - error() << formatv( + aggregate("Section contribution length exceeds available space") << formatv( "{0}: contribution {1:X}: length exceeds available space " "(contribution " "offset ({1:X}) + length field space ({2:X}) + length ({3:X}) == " @@ -1755,7 +1762,7 @@ bool DWARFVerifier::verifyDebugStrOffsets( NextUnit = C.tell() + Length; uint8_t Version = DA.getU16(C); if (C && Version != 5) { - error() << formatv("{0}: contribution {1:X}: invalid version {2}\n", + aggregate("Invalid Section version") << formatv("{0}: contribution {1:X}: invalid version {2}\n", SectionName, StartOffset, Version); Success = false; // Can't parse the rest of this contribution, since we don't know the @@ -1768,7 +1775,7 @@ bool DWARFVerifier::verifyDebugStrOffsets( DA.setAddressSize(OffsetByteSize); uint64_t Remainder = (Length - 4) % OffsetByteSize; if (Remainder != 0) { - error() << formatv( + aggregate("Invalid section contribution length") << formatv( "{0}: contribution {1:X}: invalid length ((length ({2:X}) " "- header (0x4)) % offset size {3:X} == {4:X} != 0)\n", SectionName, StartOffset, Length, OffsetByteSize, Remainder); @@ -1789,7 +1796,7 @@ bool DWARFVerifier::verifyDebugStrOffsets( } if (StrData[StrOff - 1] == '\0') continue; - error() << formatv("{0}: contribution {1:X}: index {2:X}: invalid string " + aggregate("Section contribution contains invalid string offset") << formatv("{0}: contribution {1:X}: index {2:X}: invalid string " "offset *{3:X} == {4:X}, is neither zero nor " "immediately following a null character\n", SectionName, StartOffset, Index, OffOff, StrOff); @@ -1798,28 +1805,12 @@ bool DWARFVerifier::verifyDebugStrOffsets( } if (Error E = C.takeError()) { + // Aggregate TODO: error() << SectionName << ": " << toString(std::move(E)) << '\n'; return false; } return Success; } -/* - raw_ostream & operator <<(const char * s) { - collect(s); - out << "From inside the class: " << s; - return out; - } - void collect(const char * s) { - counters[s]++; - } - void dump() { - for (auto && [name, count]: counters) { - out << "counter: " << name << ", count: " << count << "\n"; - } - } - - ErrorAggregator collectErrors(); -*/ std::string DWARFVerifier::ErrorAggregator::Clean(const char *s) { // Find the position of the first alphabetic character @@ -1839,12 +1830,18 @@ std::string DWARFVerifier::ErrorAggregator::Clean(const char *s) { } raw_ostream &DWARFVerifier::ErrorAggregator::operator<<(const char *s) { - Collect(Clean(s)); - return Verifier.error() << s; + return Collect(Clean(s)) << s; } -void DWARFVerifier::ErrorAggregator::Collect(const std::string &s) { +raw_ostream &DWARFVerifier::ErrorAggregator::Collect(const std::string &s) { + // Register the error in the aggregator UniqueErrors[s]++; + + // If we have an output stream override, return it and reset it back to + // the default + raw_ostream *o = NextLineOverride; + NextLineOverride = nullptr; + return (o == nullptr) ? Verifier.error() : *o; } void DWARFVerifier::ErrorAggregator::Dump() { @@ -1854,8 +1851,16 @@ void DWARFVerifier::ErrorAggregator::Dump() { } raw_ostream &DWARFVerifier::aggregate(const char *name) { - ErrAggregation.Collect(name); - return error(); + return ErrAggregation.Collect(name); +} + +DWARFVerifier::ErrorAggregator &DWARFVerifier::ErrorAggregator::NextLineStreamOverride(raw_ostream &o) { + NextLineOverride = &o; + return *this; +} + +DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate(raw_ostream &o) { + return ErrAggregation.NextLineStreamOverride(o); } DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate() { From b33a3c6c6fcf6c350e12854b1b3d05ccf8c370b0 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Wed, 24 Jan 2024 17:28:31 -0800 Subject: [PATCH 04/13] Updated with Greg's suggestion for the reporter --- .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 49 +--- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 264 +++++++++--------- 2 files changed, 151 insertions(+), 162 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index ae2aa62e28f921e..c2abc49c086947c 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -30,6 +30,20 @@ class DWARFDebugAbbrev; class DataExtractor; struct DWARFSection; +class OutputCategoryAggregator { +private: + std::map Aggregator; + bool Output; + +public: + OutputCategoryAggregator(bool DetailedOutput = false) + : Output(DetailedOutput) {} + void EnableDetail() { Output = true; } + void DisableDetail() { Output = false; } + void Report(StringRef s, std::function detailCallback); + void DumpAggregation(raw_ostream *o); +}; + /// A class that verifies DWARF debug information given a DWARF Context. class DWARFVerifier { public: @@ -76,27 +90,9 @@ class DWARFVerifier { bool intersects(const DieRangeInfo &RHS) const; }; -private: - class ErrorAggregator { - private: - DWARFVerifier &Verifier; - std::map UniqueErrors; - raw_ostream *NextLineOverride; - - static std::string Clean(const char *s); - - public: - ErrorAggregator(DWARFVerifier &v) : Verifier(v), NextLineOverride(nullptr) {} - raw_ostream &operator<<(const char *s); - ErrorAggregator &NextLineStreamOverride(raw_ostream &o); - raw_ostream &Collect(const std::string &s); - void Dump(); - }; - friend class ErrorAggregator; - raw_ostream &OS; DWARFContext &DCtx; - ErrorAggregator ErrAggregation; + OutputCategoryAggregator ErrorCategory; DIDumpOptions DumpOpts; uint32_t NumDebugLineErrors = 0; // Used to relax some checks that do not currently work portably @@ -104,21 +100,6 @@ class DWARFVerifier { bool IsMachOObject; using ReferenceMap = std::map>; - // For errors that have sensible names as the first (or only) string - // you can just use aggregate() << "DIE is damaged: " << details... - // and it will get aggregated as "DIE is damaged". - ErrorAggregator &aggregate(); - // For errors that have the useful aggregated category in a non-error stream - // you can do things like this (aggregated as 'Die size is wrong') - // aggregate(note()) << "DIE size is wrong.\n" - ErrorAggregator &aggregate(raw_ostream &); - - // For errors that have valuable detail before text that is the appropriate - // aggregate category, you can provide the aggregated name for the error like - // this: - // aggregate("CU index is broken") << "CU index [" << n << "] is broken" - raw_ostream &aggregate(const char *); - raw_ostream &error() const; raw_ostream &warn() const; raw_ostream ¬e() const; diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index fcfef5904d55ed8..db9b96d18476c8d 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -170,19 +170,29 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData, error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", UnitIndex, OffsetStart); if (!ValidLength) - aggregate(note()) << "The length for this unit is too " - "large for the .debug_info provided.\n"; + ErrorCategory.Report("Invalid Length in Unit Header", [&]() { + note() << "The length for this unit is too " + "large for the .debug_info provided.\n"; + }); if (!ValidVersion) { - aggregate(note()) << "The 16 bit unit header version is not valid.\n"; + ErrorCategory.Report("Invalid 16 bit unit header", [&]() { + note() << "The 16 bit unit header version is not valid.\n"; + }); } - if (!ValidType){ - aggregate(note()) << "The unit type encoding is not valid.\n"; + if (!ValidType) { + ErrorCategory.Report("Invalid unit type encoding", [&]() { + note() << "The unit type encoding is not valid.\n"; + }); } if (!ValidAbbrevOffset) - aggregate(note()) << "The offset into the .debug_abbrev section is " - "not valid.\n"; + ErrorCategory.Report("Invalid unit type encoding", [&]() { + note() << "The offset into the .debug_abbrev section is " + "not valid.\n"; + }); if (!ValidAddrSize) - aggregate(note()) << "The address size is unsupported.\n"; + ErrorCategory.Report("Invalid unit type encoding", [&]() { + note() << "The address size is unsupported.\n"; + }); } *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4); return Success; @@ -200,10 +210,14 @@ bool DWARFVerifier::verifyName(const DWARFDie &Die) { if (OriginalFullName.empty() || OriginalFullName == ReconstructedName) return false; - aggregate() << "Simplified template DW_AT_name could not be reconstituted:\n" - << formatv(" original: {0}\n" - " reconstituted: {1}\n", - OriginalFullName, ReconstructedName); + ErrorCategory.Report( + "Simplified template DW_AT_name could not be reconstituted", [&]() { + error() + << "Simplified template DW_AT_name could not be reconstituted:\n" + << formatv(" original: {0}\n" + " reconstituted: {1}\n", + OriginalFullName, ReconstructedName); + }); dump(Die) << '\n'; dump(Die.getDwarfUnit()->getUnitDIE()) << '\n'; return true; @@ -242,22 +256,28 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit, DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false); if (!Die) { - aggregate() << "Compilation unit without DIE.\n"; + ErrorCategory.Report("Compilation unity missing DIE", [&]() { + error() << "Compilation unit without DIE.\n"; + }); NumUnitErrors++; return NumUnitErrors; } if (!dwarf::isUnitType(Die.getTag())) { - aggregate() << "Compilation unit root DIE is not a unit DIE: " - << dwarf::TagString(Die.getTag()) << ".\n"; + ErrorCategory.Report("Compilation unit root DIE is not a unit DIE", [&]() { + error() << "Compilation unit root DIE is not a unit DIE: " + << dwarf::TagString(Die.getTag()) << ".\n"; + }); NumUnitErrors++; } uint8_t UnitType = Unit.getUnitType(); if (!DWARFUnit::isMatchingUnitTypeAndTag(UnitType, Die.getTag())) { - aggregate("Mismatched unit type") << "Compilation unit type (" << dwarf::UnitTypeString(UnitType) - << ") and root DIE (" << dwarf::TagString(Die.getTag()) - << ") do not match.\n"; + ErrorCategory.Report("Mismatched unit type", [&]() { + error() << "Compilation unit type (" << dwarf::UnitTypeString(UnitType) + << ") and root DIE (" << dwarf::TagString(Die.getTag()) + << ") do not match.\n"; + }); NumUnitErrors++; } @@ -265,7 +285,9 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit, // 3.1.2 Skeleton Compilation Unit Entries: // "A skeleton compilation unit has no children." if (Die.getTag() == dwarf::DW_TAG_skeleton_unit && Die.hasChildren()) { - aggregate() << "Skeleton compilation unit has children.\n"; + ErrorCategory.Report("Skeleton CU has children", [&]() { + error() << "Skeleton compilation unit has children.\n"; + }); NumUnitErrors++; } @@ -282,14 +304,20 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) { DWARFDie Curr = Die.getParent(); for (; Curr.isValid() && !Curr.isSubprogramDIE(); Curr = Die.getParent()) { if (Curr.getTag() == DW_TAG_inlined_subroutine) { - aggregate() << "Call site entry nested within inlined subroutine:"; + ErrorCategory.Report( + "Call site nested entry within inlined subroutine", [&]() { + error() << "Call site entry nested within inlined subroutine:"; + }); Curr.dump(OS); return 1; } } if (!Curr.isValid()) { - aggregate() << "Call site entry not nested within a valid subprogram:"; + ErrorCategory.Report( + "Call site entry not nexted within valid subprogram", [&]() { + error() << "Call site entry not nested within a valid subprogram:"; + }); Die.dump(OS); return 1; } @@ -299,7 +327,11 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) { DW_AT_call_all_tail_calls, DW_AT_GNU_all_call_sites, DW_AT_GNU_all_source_call_sites, DW_AT_GNU_all_tail_call_sites}); if (!CallAttr) { - aggregate() << "Subprogram with call site entry has no DW_AT_call attribute:"; + ErrorCategory.Report( + "Subprogram with call site entry has no DW_AT_call attribute", [&]() { + error() + << "Subprogram with call site entry has no DW_AT_call attribute:"; + }); Curr.dump(OS); Die.dump(OS, /*indent*/ 1); return 1; @@ -315,7 +347,9 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) { Expected AbbrDeclsOrErr = Abbrev->getAbbreviationDeclarationSet(0); if (!AbbrDeclsOrErr) { - aggregate("Abbreviateion Declaration error") << toString(AbbrDeclsOrErr.takeError()) << "\n"; + ErrorCategory.Report("Abbreviation Declaration error", [&]() { + error() << toString(AbbrDeclsOrErr.takeError()) << "\n"; + }); return 1; } @@ -326,9 +360,12 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) { for (auto Attribute : AbbrDecl.attributes()) { auto Result = AttributeSet.insert(Attribute.Attr); if (!Result.second) { - aggregate("Abbreviation declartion contains multiple attributes") << "Abbreviation declaration contains multiple " - << AttributeString(Attribute.Attr) << " attributes.\n"; - AbbrDecl.dump(OS); + ErrorCategory.Report( + "Abbreviation declartion contains multiple attributes", [&]() { + error() << "Abbreviation declaration contains multiple " + << AttributeString(Attribute.Attr) << " attributes.\n"; + AbbrDecl.dump(OS); + }); ++NumErrors; } } @@ -442,10 +479,12 @@ unsigned DWARFVerifier::verifyIndex(StringRef Name, auto &M = *Sections[Col]; auto I = M.find(SC.getOffset()); if (I != M.end() && I.start() < (SC.getOffset() + SC.getLength())) { - aggregate("Overlapping index entries") << llvm::formatv( - "overlapping index entries for entries {0:x16} " - "and {1:x16} for column {2}\n", - *I, Sig, toString(Index.getColumnKinds()[Col])); + ErrorCategory.Report("Overlapping index entries", [&]() { + error() << llvm::formatv( + "overlapping index entries for entries {0:x16} " + "and {1:x16} for column {2}\n", + *I, Sig, toString(Index.getColumnKinds()[Col])); + }); return 1; } M.insert(SC.getOffset(), SC.getOffset() + SC.getLength() - 1, Sig); @@ -529,6 +568,9 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, // For now, simply elide the range verification for the CU DIEs if we are // processing an object file. +// KBF TODO: Continue here tomorrow: +ErrorCategory.Report("", [&]() { + error if (!IsObjectFile || IsMachOObject || Die.getTag() != DW_TAG_compile_unit) { bool DumpDieAfterError = false; for (const auto &Range : Ranges) { @@ -1438,9 +1480,10 @@ unsigned DWARFVerifier::verifyNameIndexEntries( EntryOr = NI.getEntry(&NextEntryID)) { uint32_t CUIndex = *EntryOr->getCUIndex(); if (CUIndex > NI.getCUCount()) { - aggregate("Name Index entry contains invalid CU index") << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an " - "invalid CU index ({2}).\n", - NI.getUnitOffset(), EntryID, CUIndex); + aggregate("Name Index entry contains invalid CU index") + << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an " + "invalid CU index ({2}).\n", + NI.getUnitOffset(), EntryID, CUIndex); ++NumErrors; continue; } @@ -1448,24 +1491,26 @@ unsigned DWARFVerifier::verifyNameIndexEntries( uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset(); DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset); if (!DIE) { - aggregate("NameIndex references nonexisten DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x} references a " - "non-existing DIE @ {2:x}.\n", - NI.getUnitOffset(), EntryID, DIEOffset); + aggregate("NameIndex references nonexisten DIE") + << formatv("Name Index @ {0:x}: Entry @ {1:x} references a " + "non-existing DIE @ {2:x}.\n", + NI.getUnitOffset(), EntryID, DIEOffset); ++NumErrors; continue; } if (DIE.getDwarfUnit()->getOffset() != CUOffset) { - aggregate("Name index contains mismatched CU of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of " - "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n", - NI.getUnitOffset(), EntryID, DIEOffset, CUOffset, - DIE.getDwarfUnit()->getOffset()); + aggregate("Name index contains mismatched CU of DIE") + << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of " + "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n", + NI.getUnitOffset(), EntryID, DIEOffset, CUOffset, + DIE.getDwarfUnit()->getOffset()); ++NumErrors; } if (DIE.getTag() != EntryOr->tag()) { - aggregate("Name Index contains mismatched Tag of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of " - "DIE @ {2:x}: index - {3}; debug_info - {4}.\n", - NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(), - DIE.getTag()); + aggregate("Name Index contains mismatched Tag of DIE") << formatv( + "Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of " + "DIE @ {2:x}: index - {3}; debug_info - {4}.\n", + NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(), DIE.getTag()); ++NumErrors; } @@ -1476,10 +1521,11 @@ unsigned DWARFVerifier::verifyNameIndexEntries( DIE.getTag() == DW_TAG_inlined_subroutine; auto EntryNames = getNames(DIE, IncludeStrippedTemplateNames); if (!is_contained(EntryNames, Str)) { - aggregate("Name Index contains mismatched name of DIE") << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name " - "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n", - NI.getUnitOffset(), EntryID, DIEOffset, Str, - make_range(EntryNames.begin(), EntryNames.end())); + aggregate("Name Index contains mismatched name of DIE") + << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name " + "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n", + NI.getUnitOffset(), EntryID, DIEOffset, Str, + make_range(EntryNames.begin(), EntryNames.end())); ++NumErrors; } } @@ -1487,19 +1533,19 @@ unsigned DWARFVerifier::verifyNameIndexEntries( [&](const DWARFDebugNames::SentinelError &) { if (NumEntries > 0) return; - aggregate("NameIndex Name is not associated with any entries") << formatv("Name Index @ {0:x}: Name {1} ({2}) is " - "not associated with any entries.\n", - NI.getUnitOffset(), NTE.getIndex(), Str); - ++NumErrors; - }, - [&](const ErrorInfoBase &Info) { - // Aggregation TODO: - error() - << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n", - NI.getUnitOffset(), NTE.getIndex(), Str, - Info.message()); - ++NumErrors; - }); + aggregate("NameIndex Name is not associated with any entries") + << formatv("Name Index @ {0:x}: Name {1} ({2}) is " + "not associated with any entries.\n", + NI.getUnitOffset(), NTE.getIndex(), Str); + ++NumErrors; + }, + [&](const ErrorInfoBase &Info) { + // Aggregation TODO: + error() << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n", + NI.getUnitOffset(), NTE.getIndex(), Str, + Info.message()); + ++NumErrors; + }); return NumErrors; } @@ -1625,10 +1671,10 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness( if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) { return E.getDIEUnitOffset() == DieUnitOffset; })) { - aggregate("Name Index DIE entry missing name") << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with " - "name {3} missing.\n", - NI.getUnitOffset(), Die.getOffset(), Die.getTag(), - Name); + aggregate("Name Index DIE entry missing name") + << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with " + "name {3} missing.\n", + NI.getUnitOffset(), Die.getOffset(), Die.getTag(), Name); ++NumErrors; } } @@ -1748,13 +1794,15 @@ bool DWARFVerifier::verifyDebugStrOffsets( if (!C) break; if (C.tell() + Length > DA.getData().size()) { - aggregate("Section contribution length exceeds available space") << formatv( - "{0}: contribution {1:X}: length exceeds available space " - "(contribution " - "offset ({1:X}) + length field space ({2:X}) + length ({3:X}) == " - "{4:X} > section size {5:X})\n", - SectionName, StartOffset, C.tell() - StartOffset, Length, - C.tell() + Length, DA.getData().size()); + aggregate("Section contribution length exceeds available space") + << formatv( + "{0}: contribution {1:X}: length exceeds available space " + "(contribution " + "offset ({1:X}) + length field space ({2:X}) + length " + "({3:X}) == " + "{4:X} > section size {5:X})\n", + SectionName, StartOffset, C.tell() - StartOffset, Length, + C.tell() + Length, DA.getData().size()); Success = false; // Nothing more to do - no other contributions to try. break; @@ -1762,8 +1810,9 @@ bool DWARFVerifier::verifyDebugStrOffsets( NextUnit = C.tell() + Length; uint8_t Version = DA.getU16(C); if (C && Version != 5) { - aggregate("Invalid Section version") << formatv("{0}: contribution {1:X}: invalid version {2}\n", - SectionName, StartOffset, Version); + aggregate("Invalid Section version") + << formatv("{0}: contribution {1:X}: invalid version {2}\n", + SectionName, StartOffset, Version); Success = false; // Can't parse the rest of this contribution, since we don't know the // version, but we can pick up with the next contribution. @@ -1796,10 +1845,11 @@ bool DWARFVerifier::verifyDebugStrOffsets( } if (StrData[StrOff - 1] == '\0') continue; - aggregate("Section contribution contains invalid string offset") << formatv("{0}: contribution {1:X}: index {2:X}: invalid string " - "offset *{3:X} == {4:X}, is neither zero nor " - "immediately following a null character\n", - SectionName, StartOffset, Index, OffOff, StrOff); + aggregate("Section contribution contains invalid string offset") + << formatv("{0}: contribution {1:X}: index {2:X}: invalid string " + "offset *{3:X} == {4:X}, is neither zero nor " + "immediately following a null character\n", + SectionName, StartOffset, Index, OffOff, StrOff); Success = false; } } @@ -1812,64 +1862,22 @@ bool DWARFVerifier::verifyDebugStrOffsets( return Success; } -std::string DWARFVerifier::ErrorAggregator::Clean(const char *s) { - // Find the position of the first alphabetic character - const char *start = s; - while (!std::isalpha(*start)) { - ++start; - } - - // Find the position of the last alphabetic character - const char *end = &start[std::strlen(start) - 1]; - while (end >= start && !std::isalpha(*end)) { - --end; - } - - // Create a std::string from the trimmed portion of the const char* - return std::string(start, end - start + 1); -} - -raw_ostream &DWARFVerifier::ErrorAggregator::operator<<(const char *s) { - return Collect(Clean(s)) << s; -} - -raw_ostream &DWARFVerifier::ErrorAggregator::Collect(const std::string &s) { - // Register the error in the aggregator +void OutputCategoryAggregator::Report( + StringRef s, std::function detailCallback) { UniqueErrors[s]++; - - // If we have an output stream override, return it and reset it back to - // the default - raw_ostream *o = NextLineOverride; - NextLineOverride = nullptr; - return (o == nullptr) ? Verifier.error() : *o; + if (Output) + detailedCallback(*Output); } -void DWARFVerifier::ErrorAggregator::Dump() { - for (auto &&[name, count] : UniqueErrors) { - Verifier.error() << "counter: " << name << ", count: " << count << "\n"; +void OutputCategoryAggregator::DumpAggregation(raw_ostream *o) { + for (auto &&[name, count] : Aggregator) { + *o << name << ": seen " << count << " time(s)\n"; } } -raw_ostream &DWARFVerifier::aggregate(const char *name) { - return ErrAggregation.Collect(name); -} - -DWARFVerifier::ErrorAggregator &DWARFVerifier::ErrorAggregator::NextLineStreamOverride(raw_ostream &o) { - NextLineOverride = &o; - return *this; -} - -DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate(raw_ostream &o) { - return ErrAggregation.NextLineStreamOverride(o); -} - -DWARFVerifier::ErrorAggregator &DWARFVerifier::aggregate() { - return ErrAggregation; -} - void DWARFVerifier::finish() { if (DumpOpts.DumpAggregateErrors) - ErrAggregation.Dump(); + Category.Dump(error()); } raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); } From 2bd08dc6531117385a2f0f9ec35302560c191812 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Thu, 25 Jan 2024 11:41:14 -0800 Subject: [PATCH 05/13] Cleaned up: Non-verbose output is downright succinct! --- .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 13 +- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 682 +++++++++++------- llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp | 2 +- 3 files changed, 410 insertions(+), 287 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index c2abc49c086947c..d70175999e8df8b 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -32,16 +32,15 @@ struct DWARFSection; class OutputCategoryAggregator { private: - std::map Aggregator; - bool Output; + std::map Aggregation; + bool CallDetail; public: - OutputCategoryAggregator(bool DetailedOutput = false) - : Output(DetailedOutput) {} - void EnableDetail() { Output = true; } - void DisableDetail() { Output = false; } + OutputCategoryAggregator(bool callDetail = false) : CallDetail(callDetail) {} + void EnableDetail() { CallDetail = true; } + void DisableDetail() { CallDetail = false; } void Report(StringRef s, std::function detailCallback); - void DumpAggregation(raw_ostream *o); + void HandleAggregate(std::function handleCounts); }; /// A class that verifies DWARF debug information given a DWARF Context. diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index db9b96d18476c8d..88c0e4ab2c81042 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -167,32 +167,24 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData, if (!ValidLength || !ValidVersion || !ValidAddrSize || !ValidAbbrevOffset || !ValidType) { Success = false; - error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", UnitIndex, - OffsetStart); - if (!ValidLength) - ErrorCategory.Report("Invalid Length in Unit Header", [&]() { + ErrorCategory.Report("Invalid Length in Unit Header", [&]() { + error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", + UnitIndex, OffsetStart); + if (!ValidLength) note() << "The length for this unit is too " "large for the .debug_info provided.\n"; - }); - if (!ValidVersion) { - ErrorCategory.Report("Invalid 16 bit unit header", [&]() { + if (!ValidVersion) { note() << "The 16 bit unit header version is not valid.\n"; - }); - } - if (!ValidType) { - ErrorCategory.Report("Invalid unit type encoding", [&]() { + } + if (!ValidType) { note() << "The unit type encoding is not valid.\n"; - }); - } - if (!ValidAbbrevOffset) - ErrorCategory.Report("Invalid unit type encoding", [&]() { + } + if (!ValidAbbrevOffset) note() << "The offset into the .debug_abbrev section is " "not valid.\n"; - }); - if (!ValidAddrSize) - ErrorCategory.Report("Invalid unit type encoding", [&]() { + if (!ValidAddrSize) note() << "The address size is unsupported.\n"; - }); + }); } *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4); return Success; @@ -217,9 +209,9 @@ bool DWARFVerifier::verifyName(const DWARFDie &Die) { << formatv(" original: {0}\n" " reconstituted: {1}\n", OriginalFullName, ReconstructedName); + dump(Die) << '\n'; + dump(Die.getDwarfUnit()->getUnitDIE()) << '\n'; }); - dump(Die) << '\n'; - dump(Die.getDwarfUnit()->getUnitDIE()) << '\n'; return true; } @@ -256,7 +248,7 @@ unsigned DWARFVerifier::verifyUnitContents(DWARFUnit &Unit, DWARFDie Die = Unit.getUnitDIE(/* ExtractUnitDIEOnly = */ false); if (!Die) { - ErrorCategory.Report("Compilation unity missing DIE", [&]() { + ErrorCategory.Report("Compilation unit missing DIE", [&]() { error() << "Compilation unit without DIE.\n"; }); NumUnitErrors++; @@ -307,8 +299,8 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) { ErrorCategory.Report( "Call site nested entry within inlined subroutine", [&]() { error() << "Call site entry nested within inlined subroutine:"; + Curr.dump(OS); }); - Curr.dump(OS); return 1; } } @@ -317,8 +309,8 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) { ErrorCategory.Report( "Call site entry not nexted within valid subprogram", [&]() { error() << "Call site entry not nested within a valid subprogram:"; + Die.dump(OS); }); - Die.dump(OS); return 1; } @@ -331,9 +323,9 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) { "Subprogram with call site entry has no DW_AT_call attribute", [&]() { error() << "Subprogram with call site entry has no DW_AT_call attribute:"; + Curr.dump(OS); + Die.dump(OS, /*indent*/ 1); }); - Curr.dump(OS); - Die.dump(OS, /*indent*/ 1); return 1; } @@ -479,7 +471,10 @@ unsigned DWARFVerifier::verifyIndex(StringRef Name, auto &M = *Sections[Col]; auto I = M.find(SC.getOffset()); if (I != M.end() && I.start() < (SC.getOffset() + SC.getLength())) { - ErrorCategory.Report("Overlapping index entries", [&]() { + StringRef Category = InfoColumnKind == DWARFSectionKind::DW_SECT_INFO + ? "Overlapping CU index entries" + : "Overlapping TU index entries"; + ErrorCategory.Report(Category, [&]() { error() << llvm::formatv( "overlapping index entries for entries {0:x16} " "and {1:x16} for column {2}\n", @@ -568,15 +563,14 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, // For now, simply elide the range verification for the CU DIEs if we are // processing an object file. -// KBF TODO: Continue here tomorrow: -ErrorCategory.Report("", [&]() { - error if (!IsObjectFile || IsMachOObject || Die.getTag() != DW_TAG_compile_unit) { bool DumpDieAfterError = false; for (const auto &Range : Ranges) { if (!Range.valid()) { ++NumErrors; - aggregate() << "Invalid address range " << Range << "\n"; + ErrorCategory.Report("Invalid address range", [&]() { + error() << "Invalid address range " << Range << "\n"; + }); DumpDieAfterError = true; continue; } @@ -589,9 +583,11 @@ ErrorCategory.Report("", [&]() { // address: 0 or -1. if (auto PrevRange = RI.insert(Range)) { ++NumErrors; - aggregate() << "DIE has overlapping ranges in DW_AT_ranges attribute: " - << *PrevRange << " and " << Range << '\n'; - DumpDieAfterError = true; + ErrorCategory.Report("DIE has overlapping DW_AT_ranges", [&]() { + error() << "DIE has overlapping ranges in DW_AT_ranges attribute: " + << *PrevRange << " and " << Range << '\n'; + }); + DumpDieAfterError = DumpOpts.Verbose; } } if (DumpDieAfterError) @@ -602,9 +598,11 @@ ErrorCategory.Report("", [&]() { const auto IntersectingChild = ParentRI.insert(RI); if (IntersectingChild != ParentRI.Children.end()) { ++NumErrors; - aggregate() << "DIEs have overlapping address ranges:"; - dump(Die); - dump(IntersectingChild->Die) << '\n'; + ErrorCategory.Report("DIEs have overlapping address ranges", [&]() { + error() << "DIEs have overlapping address ranges:"; + dump(Die); + dump(IntersectingChild->Die) << '\n'; + }); } // Verify that ranges are contained within their parent. @@ -613,9 +611,13 @@ ErrorCategory.Report("", [&]() { ParentRI.Die.getTag() == DW_TAG_subprogram); if (ShouldBeContained && !ParentRI.contains(RI)) { ++NumErrors; - aggregate() << "DIE address ranges are not contained in its parent's ranges:"; - dump(ParentRI.Die); - dump(Die, 2) << '\n'; + ErrorCategory.Report( + "DIE address ranges are not contained by parent ranges", [&]() { + error() + << "DIE address ranges are not contained in its parent's ranges:"; + dump(ParentRI.Die); + dump(Die, 2) << '\n'; + }); } // Recursively check children. @@ -625,14 +627,15 @@ ErrorCategory.Report("", [&]() { return NumErrors; } -// Aggregation TODO: unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, DWARFAttribute &AttrValue) { unsigned NumErrors = 0; - auto ReportError = [&](const Twine &TitleMsg) { + auto ReportError = [&](StringRef category, const Twine &TitleMsg) { ++NumErrors; - error() << TitleMsg << '\n'; - dump(Die) << '\n'; + ErrorCategory.Report(category, [&]() { + error() << TitleMsg << '\n'; + dump(Die) << '\n'; + }); }; const DWARFObject &DObj = DCtx.getDWARFObj(); @@ -649,23 +652,27 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, if (U->isDWOUnit() && RangeSection.Data.empty()) break; if (*SectionOffset >= RangeSection.Data.size()) - ReportError( - "DW_AT_ranges offset is beyond " + - StringRef(DwarfVersion < 5 ? ".debug_ranges" : ".debug_rnglists") + - " bounds: " + llvm::formatv("{0:x8}", *SectionOffset)); + ReportError("DW_AT_ranges offset out of bounds", + "DW_AT_ranges offset is beyond " + + StringRef(DwarfVersion < 5 ? ".debug_ranges" + : ".debug_rnglists") + + " bounds: " + llvm::formatv("{0:x8}", *SectionOffset)); break; } - ReportError("DIE has invalid DW_AT_ranges encoding:"); + ReportError("Invalid DW_AT_ranges encoding", + "DIE has invalid DW_AT_ranges encoding:"); break; case DW_AT_stmt_list: // Make sure the offset in the DW_AT_stmt_list attribute is valid. if (auto SectionOffset = AttrValue.Value.getAsSectionOffset()) { if (*SectionOffset >= U->getLineSection().Data.size()) - ReportError("DW_AT_stmt_list offset is beyond .debug_line bounds: " + - llvm::formatv("{0:x8}", *SectionOffset)); + ReportError("DW_AT_stmt_list offset out of bounds", + "DW_AT_stmt_list offset is beyond .debug_line bounds: " + + llvm::formatv("{0:x8}", *SectionOffset)); break; } - ReportError("DIE has invalid DW_AT_stmt_list encoding:"); + ReportError("Invalid DW_AT_stmt_list encoding", + "DIE has invalid DW_AT_stmt_list encoding:"); break; case DW_AT_location: { // FIXME: It might be nice if there's a way to walk location expressions @@ -689,14 +696,15 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, return Op.isError(); }); if (Error || !Expression.verify(U)) - ReportError("DIE contains invalid DWARF expression:"); + ReportError("Invalid DWARF expressions", + "DIE contains invalid DWARF expression:"); } } else if (Error Err = handleErrors( Loc.takeError(), [&](std::unique_ptr E) { return U->isDWOUnit() ? Error::success() : Error(std::move(E)); })) - ReportError(toString(std::move(Err))); + ReportError("Invalid DW_AT_location", toString(std::move(Err))); break; } case DW_AT_specification: @@ -713,19 +721,21 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, // This might be reference to a function declaration. if (DieTag == DW_TAG_GNU_call_site && RefTag == DW_TAG_subprogram) break; - ReportError("DIE with tag " + TagString(DieTag) + " has " + - AttributeString(Attr) + - " that points to DIE with " - "incompatible tag " + - TagString(RefTag)); + ReportError("Incompatible DW_AT_abstract_origin tag reference", + "DIE with tag " + TagString(DieTag) + " has " + + AttributeString(Attr) + + " that points to DIE with " + "incompatible tag " + + TagString(RefTag)); } break; } case DW_AT_type: { DWARFDie TypeDie = Die.getAttributeValueAsReferencedDie(DW_AT_type); if (TypeDie && !isType(TypeDie.getTag())) { - ReportError("DIE has " + AttributeString(Attr) + - " with incompatible tag " + TagString(TypeDie.getTag())); + ReportError("Incompatible DW_AT_type attribute tag", + "DIE has " + AttributeString(Attr) + + " with incompatible tag " + TagString(TypeDie.getTag())); } break; } @@ -740,35 +750,43 @@ unsigned DWARFVerifier::verifyDebugInfoAttribute(const DWARFDie &Die, bool IsZeroIndexed = LT->Prologue.getVersion() >= 5; if (std::optional LastFileIdx = LT->getLastValidFileIndex()) { - ReportError("DIE has " + AttributeString(Attr) + - " with an invalid file index " + - llvm::formatv("{0}", *FileIdx) + - " (valid values are [" + (IsZeroIndexed ? "0-" : "1-") + - llvm::formatv("{0}", *LastFileIdx) + "])"); + ReportError("Invalid file index in DW_AT_decl_file", + "DIE has " + AttributeString(Attr) + + " with an invalid file index " + + llvm::formatv("{0}", *FileIdx) + + " (valid values are [" + + (IsZeroIndexed ? "0-" : "1-") + + llvm::formatv("{0}", *LastFileIdx) + "])"); } else { - ReportError("DIE has " + AttributeString(Attr) + - " with an invalid file index " + - llvm::formatv("{0}", *FileIdx) + - " (the file table in the prologue is empty)"); + ReportError("Invalid file index in DW_AT_decl_file", + "DIE has " + AttributeString(Attr) + + " with an invalid file index " + + llvm::formatv("{0}", *FileIdx) + + " (the file table in the prologue is empty)"); } } } else { - ReportError("DIE has " + AttributeString(Attr) + - " that references a file with index " + - llvm::formatv("{0}", *FileIdx) + - " and the compile unit has no line table"); + ReportError( + "File index in DW_AT_decl_file reference CU with no line table", + "DIE has " + AttributeString(Attr) + + " that references a file with index " + + llvm::formatv("{0}", *FileIdx) + + " and the compile unit has no line table"); } } else { - ReportError("DIE has " + AttributeString(Attr) + - " with invalid encoding"); + ReportError("Invalid encoding in DW_AT_decl_file", + "DIE has " + AttributeString(Attr) + + " with invalid encoding"); } break; } case DW_AT_call_line: case DW_AT_decl_line: { if (!AttrValue.Value.getAsUnsignedConstant()) { - ReportError("DIE has " + AttributeString(Attr) + - " with invalid encoding"); + ReportError( + Attr == DW_AT_call_line ? "Invalid file index in DW_AT_decl_line" + : "Invalid file index in DW_AT_call_line", + "DIE has " + AttributeString(Attr) + " with invalid encoding"); } break; } @@ -799,12 +817,14 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die, auto CUOffset = AttrValue.Value.getRawUValue(); if (CUOffset >= CUSize) { ++NumErrors; - aggregate("Invalid CU offset") << FormEncodingString(Form) << " CU offset " - << format("0x%08" PRIx64, CUOffset) - << " is invalid (must be less than CU size of " - << format("0x%08" PRIx64, CUSize) << "):\n"; - Die.dump(OS, 0, DumpOpts); - dump(Die) << '\n'; + ErrorCategory.Report("Invalid CU offset", [&]() { + error() << FormEncodingString(Form) << " CU offset " + << format("0x%08" PRIx64, CUOffset) + << " is invalid (must be less than CU size of " + << format("0x%08" PRIx64, CUSize) << "):\n"; + Die.dump(OS, 0, DumpOpts); + dump(Die) << '\n'; + }); } else { // Valid reference, but we will verify it points to an actual // DIE later. @@ -821,9 +841,11 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die, if (RefVal) { if (*RefVal >= DieCU->getInfoSection().Data.size()) { ++NumErrors; - aggregate() << "DW_FORM_ref_addr offset beyond .debug_info " - "bounds:\n"; - dump(Die) << '\n'; + ErrorCategory.Report("DW_FORM_ref_addr offset out of bounds", [&]() { + error() << "DW_FORM_ref_addr offset beyond .debug_info " + "bounds:\n"; + dump(Die) << '\n'; + }); } else { // Valid reference, but we will verify it points to an actual // DIE later. @@ -841,9 +863,10 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die, case DW_FORM_line_strp: { if (Error E = AttrValue.Value.getAsCString().takeError()) { ++NumErrors; - // Aggregation TODO: - error() << toString(std::move(E)) << ":\n"; - dump(Die) << '\n'; + ErrorCategory.Report("Invalid DW_FORM attribute", [&]() { + error() << toString(std::move(E)) << ":\n"; + dump(Die) << '\n'; + }); } break; } @@ -867,11 +890,13 @@ unsigned DWARFVerifier::verifyDebugInfoReferences( if (GetDIEForOffset(Pair.first)) continue; ++NumErrors; - aggregate() << "invalid DIE reference " << format("0x%08" PRIx64, Pair.first) - << ". Offset is in between DIEs:\n"; - for (auto Offset : Pair.second) - dump(GetDIEForOffset(Offset)) << '\n'; - OS << "\n"; + ErrorCategory.Report("Invalid DIE reference", [&]() { + error() << "invalid DIE reference " << format("0x%08" PRIx64, Pair.first) + << ". Offset is in between DIEs:\n"; + for (auto Offset : Pair.second) + dump(GetDIEForOffset(Offset)) << '\n'; + OS << "\n"; + }); } return NumErrors; } @@ -891,9 +916,11 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() { if (LineTableOffset < DCtx.getDWARFObj().getLineSection().Data.size()) { if (!LineTable) { ++NumDebugLineErrors; - aggregate("Unparseable .debug_line entry") << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset) - << "] was not able to be parsed for CU:\n"; - dump(Die) << '\n'; + ErrorCategory.Report("Unparseable .debug_line entry", [&]() { + error() << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset) + << "] was not able to be parsed for CU:\n"; + dump(Die) << '\n'; + }); continue; } } else { @@ -906,12 +933,14 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() { auto Iter = StmtListToDie.find(LineTableOffset); if (Iter != StmtListToDie.end()) { ++NumDebugLineErrors; - aggregate("Identical DW_AT_stmt_list section offset") << "two compile unit DIEs, " - << format("0x%08" PRIx64, Iter->second.getOffset()) << " and " - << format("0x%08" PRIx64, Die.getOffset()) - << ", have the same DW_AT_stmt_list section offset:\n"; - dump(Iter->second); - dump(Die) << '\n'; + ErrorCategory.Report("Identical DW_AT_stmt_list section offset", [&]() { + error() << "two compile unit DIEs, " + << format("0x%08" PRIx64, Iter->second.getOffset()) << " and " + << format("0x%08" PRIx64, Die.getOffset()) + << ", have the same DW_AT_stmt_list section offset:\n"; + dump(Iter->second); + dump(Die) << '\n'; + }); // Already verified this line table before, no need to do it again. continue; } @@ -938,12 +967,16 @@ void DWARFVerifier::verifyDebugLineRows() { // Verify directory index. if (FileName.DirIdx > MaxDirIndex) { ++NumDebugLineErrors; - aggregate("Invalid index in .debug_line->prologue.file_names->dir_idx") << ".debug_line[" - << format("0x%08" PRIx64, - *toSectionOffset(Die.find(DW_AT_stmt_list))) - << "].prologue.file_names[" << FileIndex - << "].dir_idx contains an invalid index: " << FileName.DirIdx - << "\n"; + ErrorCategory.Report( + "Invalid index in .debug_line->prologue.file_names->dir_idx", + [&]() { + error() << ".debug_line[" + << format("0x%08" PRIx64, + *toSectionOffset(Die.find(DW_AT_stmt_list))) + << "].prologue.file_names[" << FileIndex + << "].dir_idx contains an invalid index: " + << FileName.DirIdx << "\n"; + }); } // Check file paths for duplicates. @@ -956,7 +989,7 @@ void DWARFVerifier::verifyDebugLineRows() { auto It = FullPathMap.find(FullPath); if (It == FullPathMap.end()) FullPathMap[FullPath] = FileIndex; - else if (It->second != FileIndex) { + else if (It->second != FileIndex && DumpOpts.Verbose) { warn() << ".debug_line[" << format("0x%08" PRIx64, *toSectionOffset(Die.find(DW_AT_stmt_list))) @@ -974,17 +1007,20 @@ void DWARFVerifier::verifyDebugLineRows() { // Verify row address. if (Row.Address.Address < PrevAddress) { ++NumDebugLineErrors; - aggregate("decreasing address between debug_line rows") << ".debug_line[" - << format("0x%08" PRIx64, - *toSectionOffset(Die.find(DW_AT_stmt_list))) - << "] row[" << RowIndex - << "] decreases in address from previous row:\n"; - - DWARFDebugLine::Row::dumpTableHeader(OS, 0); - if (RowIndex > 0) - LineTable->Rows[RowIndex - 1].dump(OS); - Row.dump(OS); - OS << '\n'; + ErrorCategory.Report( + "decreasing address between debug_line rows", [&]() { + error() << ".debug_line[" + << format("0x%08" PRIx64, + *toSectionOffset(Die.find(DW_AT_stmt_list))) + << "] row[" << RowIndex + << "] decreases in address from previous row:\n"; + + DWARFDebugLine::Row::dumpTableHeader(OS, 0); + if (RowIndex > 0) + LineTable->Rows[RowIndex - 1].dump(OS); + Row.dump(OS); + OS << '\n'; + }); } // If the prologue contains no file names and the line table has only one @@ -995,16 +1031,18 @@ void DWARFVerifier::verifyDebugLineRows() { LineTable->Rows.size() != 1) && !LineTable->hasFileAtIndex(Row.File)) { ++NumDebugLineErrors; - aggregate("Invalid file index in debug_line") << ".debug_line[" - << format("0x%08" PRIx64, - *toSectionOffset(Die.find(DW_AT_stmt_list))) - << "][" << RowIndex << "] has invalid file index " << Row.File - << " (valid values are [" << MinFileIndex << ',' - << LineTable->Prologue.FileNames.size() - << (isDWARF5 ? ")" : "]") << "):\n"; - DWARFDebugLine::Row::dumpTableHeader(OS, 0); - Row.dump(OS); - OS << '\n'; + ErrorCategory.Report("Invalid file index in debug_line", [&]() { + error() << ".debug_line[" + << format("0x%08" PRIx64, + *toSectionOffset(Die.find(DW_AT_stmt_list))) + << "][" << RowIndex << "] has invalid file index " << Row.File + << " (valid values are [" << MinFileIndex << ',' + << LineTable->Prologue.FileNames.size() + << (isDWARF5 ? ")" : "]") << "):\n"; + DWARFDebugLine::Row::dumpTableHeader(OS, 0); + Row.dump(OS); + OS << '\n'; + }); } if (Row.EndSequence) PrevAddress = 0; @@ -1017,8 +1055,11 @@ void DWARFVerifier::verifyDebugLineRows() { DWARFVerifier::DWARFVerifier(raw_ostream &S, DWARFContext &D, DIDumpOptions DumpOpts) - : OS(S), DCtx(D), ErrAggregation(*this), DumpOpts(std::move(DumpOpts)), - IsObjectFile(false), IsMachOObject(false) { + : OS(S), DCtx(D), DumpOpts(std::move(DumpOpts)), IsObjectFile(false), + IsMachOObject(false) { + if (DumpOpts.Verbose) { + ErrorCategory.EnableDetail(); + } if (const auto *F = DCtx.getDWARFObj().getFile()) { IsObjectFile = F->isRelocatableObject(); IsMachOObject = F->isMachO(); @@ -1045,14 +1086,17 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, // Verify that the fixed part of the header is not too short. if (!AccelSectionData.isValidOffset(AccelTable.getSizeHdr())) { - aggregate() << "Section is too small to fit a section header.\n"; + ErrorCategory.Report("Section is too small to fit a section header", [&]() { + error() << "Section is too small to fit a section header.\n"; + }); return 1; } // Verify that the section is not too short. if (Error E = AccelTable.extract()) { - // Aggregation TODO: - error() << toString(std::move(E)) << '\n'; + std::string Msg = toString(std::move(E)); + ErrorCategory.Report("Section is too small to fit a section header", + [&]() { error() << Msg << '\n'; }); return 1; } @@ -1067,18 +1111,24 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, for (uint32_t BucketIdx = 0; BucketIdx < NumBuckets; ++BucketIdx) { uint32_t HashIdx = AccelSectionData.getU32(&BucketsOffset); if (HashIdx >= NumHashes && HashIdx != UINT32_MAX) { - aggregate("Invalid hash index") << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx, - HashIdx); + ErrorCategory.Report("Invalid hash index", [&]() { + error() << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx, + HashIdx); + }); ++NumErrors; } } uint32_t NumAtoms = AccelTable.getAtomsDesc().size(); if (NumAtoms == 0) { - aggregate() << "No atoms: failed to read HashData.\n"; + ErrorCategory.Report("No atoms", [&]() { + error() << "No atoms: failed to read HashData.\n"; + }); return 1; } if (!AccelTable.validateForms()) { - aggregate() << "Unsupported form: failed to read HashData.\n"; + ErrorCategory.Report("Unsupported form", [&]() { + error() << "Unsupported form: failed to read HashData.\n"; + }); return 1; } @@ -1089,9 +1139,11 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, uint64_t HashDataOffset = AccelSectionData.getU32(&DataOffset); if (!AccelSectionData.isValidOffsetForDataOfSize(HashDataOffset, sizeof(uint64_t))) { - aggregate("Invalid HashData offset") << format("Hash[%d] has invalid HashData offset: " - "0x%08" PRIx64 ".\n", - HashIdx, HashDataOffset); + ErrorCategory.Report("Invalid HashData offset", [&]() { + error() << format("Hash[%d] has invalid HashData offset: " + "0x%08" PRIx64 ".\n", + HashIdx, HashDataOffset); + }); ++NumErrors; } @@ -1115,21 +1167,25 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection, if (!Name) Name = ""; - aggregate("Invalid DIE offset") << format( - "%s Bucket[%d] Hash[%d] = 0x%08x " - "Str[%u] = 0x%08" PRIx64 " DIE[%d] = 0x%08" PRIx64 " " - "is not a valid DIE offset for \"%s\".\n", - SectionName, BucketIdx, HashIdx, Hash, StringCount, StrpOffset, - HashDataIdx, Offset, Name); + ErrorCategory.Report("Invalid DIE offset", [&]() { + error() << format( + "%s Bucket[%d] Hash[%d] = 0x%08x " + "Str[%u] = 0x%08" PRIx64 " DIE[%d] = 0x%08" PRIx64 " " + "is not a valid DIE offset for \"%s\".\n", + SectionName, BucketIdx, HashIdx, Hash, StringCount, StrpOffset, + HashDataIdx, Offset, Name); + }); ++NumErrors; continue; } if ((Tag != dwarf::DW_TAG_null) && (Die.getTag() != Tag)) { - aggregate("Mismatched Tag in accellerator table") << "Tag " << dwarf::TagString(Tag) - << " in accelerator table does not match Tag " - << dwarf::TagString(Die.getTag()) << " of DIE[" << HashDataIdx - << "].\n"; + ErrorCategory.Report("Mismatched Tag in accellerator table", [&]() { + error() << "Tag " << dwarf::TagString(Tag) + << " in accelerator table does not match Tag " + << dwarf::TagString(Die.getTag()) << " of DIE[" + << HashDataIdx << "].\n"; + }); ++NumErrors; } } @@ -1153,8 +1209,10 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) { unsigned NumErrors = 0; for (const DWARFDebugNames::NameIndex &NI : AccelTable) { if (NI.getCUCount() == 0) { - aggregate("Name Index doesn't index any CU") << formatv("Name Index @ {0:x} does not index any CU\n", - NI.getUnitOffset()); + ErrorCategory.Report("Name Index doesn't index any CU", [&]() { + error() << formatv("Name Index @ {0:x} does not index any CU\n", + NI.getUnitOffset()); + }); ++NumErrors; continue; } @@ -1163,17 +1221,22 @@ DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) { auto Iter = CUMap.find(Offset); if (Iter == CUMap.end()) { - aggregate("Name Index references non-existing CU") << formatv( - "Name Index @ {0:x} references a non-existing CU @ {1:x}\n", - NI.getUnitOffset(), Offset); + ErrorCategory.Report("Name Index references non-existing CU", [&]() { + error() << formatv( + "Name Index @ {0:x} references a non-existing CU @ {1:x}\n", + NI.getUnitOffset(), Offset); + }); ++NumErrors; continue; } if (Iter->second != NotIndexed) { - aggregate("Duplicate Name Index") << formatv("Name Index @ {0:x} references a CU @ {1:x}, but " - "this CU is already indexed by Name Index @ {2:x}\n", - NI.getUnitOffset(), Offset, Iter->second); + ErrorCategory.Report("Duplicate Name Index", [&]() { + error() << formatv( + "Name Index @ {0:x} references a CU @ {1:x}, but " + "this CU is already indexed by Name Index @ {2:x}\n", + NI.getUnitOffset(), Offset, Iter->second); + }); continue; } Iter->second = NI.getUnitOffset(); @@ -1214,9 +1277,12 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI, for (uint32_t Bucket = 0, End = NI.getBucketCount(); Bucket < End; ++Bucket) { uint32_t Index = NI.getBucketArrayEntry(Bucket); if (Index > NI.getNameCount()) { - aggregate("Name Index Bucket contains invalid value") << formatv("Bucket {0} of Name Index @ {1:x} contains invalid " - "value {2}. Valid range is [0, {3}].\n", - Bucket, NI.getUnitOffset(), Index, NI.getNameCount()); + ErrorCategory.Report("Name Index Bucket contains invalid value", [&]() { + error() << formatv("Bucket {0} of Name Index @ {1:x} contains invalid " + "value {2}. Valid range is [0, {3}].\n", + Bucket, NI.getUnitOffset(), Index, + NI.getNameCount()); + }); ++NumErrors; continue; } @@ -1249,9 +1315,11 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI, // will not match because we have already verified that the name's hash // puts it into the previous bucket.) if (B.Index > NextUncovered) { - aggregate("Name table entries uncovered by hash table") << formatv("Name Index @ {0:x}: Name table entries [{1}, {2}] " - "are not covered by the hash table.\n", - NI.getUnitOffset(), NextUncovered, B.Index - 1); + ErrorCategory.Report("Name table entries uncovered by hash table", [&]() { + error() << formatv("Name Index @ {0:x}: Name table entries [{1}, {2}] " + "are not covered by the hash table.\n", + NI.getUnitOffset(), NextUncovered, B.Index - 1); + }); ++NumErrors; } uint32_t Idx = B.Index; @@ -1267,11 +1335,13 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI, // bucket as empty. uint32_t FirstHash = NI.getHashArrayEntry(Idx); if (FirstHash % NI.getBucketCount() != B.Bucket) { - aggregate("Name Index point to mismatched hash value") << formatv( - "Name Index @ {0:x}: Bucket {1} is not empty but points to a " - "mismatched hash value {2:x} (belonging to bucket {3}).\n", - NI.getUnitOffset(), B.Bucket, FirstHash, - FirstHash % NI.getBucketCount()); + ErrorCategory.Report("Name Index point to mismatched hash value", [&]() { + error() << formatv( + "Name Index @ {0:x}: Bucket {1} is not empty but points to a " + "mismatched hash value {2:x} (belonging to bucket {3}).\n", + NI.getUnitOffset(), B.Bucket, FirstHash, + FirstHash % NI.getBucketCount()); + }); ++NumErrors; } @@ -1285,11 +1355,14 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI, const char *Str = NI.getNameTableEntry(Idx).getString(); if (caseFoldingDjbHash(Str) != Hash) { - aggregate("String hash doesn't match Name Index hash") << formatv("Name Index @ {0:x}: String ({1}) at index {2} " - "hashes to {3:x}, but " - "the Name Index hash is {4:x}\n", - NI.getUnitOffset(), Str, Idx, - caseFoldingDjbHash(Str), Hash); + ErrorCategory.Report( + "String hash doesn't match Name Index hash", [&]() { + error() << formatv( + "Name Index @ {0:x}: String ({1}) at index {2} " + "hashes to {3:x}, but " + "the Name Index hash is {4:x}\n", + NI.getUnitOffset(), Str, Idx, caseFoldingDjbHash(Str), Hash); + }); ++NumErrors; } @@ -1305,19 +1378,23 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( DWARFDebugNames::AttributeEncoding AttrEnc) { StringRef FormName = dwarf::FormEncodingString(AttrEnc.Form); if (FormName.empty()) { - aggregate("Unknown NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an " - "unknown form: {3}.\n", - NI.getUnitOffset(), Abbr.Code, AttrEnc.Index, - AttrEnc.Form); + ErrorCategory.Report("Unknown NameIndex Abbreviation", [&]() { + error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an " + "unknown form: {3}.\n", + NI.getUnitOffset(), Abbr.Code, AttrEnc.Index, + AttrEnc.Form); + }); return 1; } if (AttrEnc.Index == DW_IDX_type_hash) { if (AttrEnc.Form != dwarf::DW_FORM_data8) { - aggregate("Unexpected NameIndex Abbreviation") << formatv( - "NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_type_hash " - "uses an unexpected form {2} (should be {3}).\n", - NI.getUnitOffset(), Abbr.Code, AttrEnc.Form, dwarf::DW_FORM_data8); + ErrorCategory.Report("Unexpected NameIndex Abbreviation", [&]() { + error() << formatv( + "NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_type_hash " + "uses an unexpected form {2} (should be {3}).\n", + NI.getUnitOffset(), Abbr.Code, AttrEnc.Form, dwarf::DW_FORM_data8); + }); return 1; } return 0; @@ -1327,10 +1404,13 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( constexpr static auto AllowedForms = {dwarf::Form::DW_FORM_flag_present, dwarf::Form::DW_FORM_ref4}; if (!is_contained(AllowedForms, AttrEnc.Form)) { - aggregate("Unexpected NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent " - "uses an unexpected form {2} (should be " - "DW_FORM_ref4 or DW_FORM_flag_present).\n", - NI.getUnitOffset(), Abbr.Code, AttrEnc.Form); + ErrorCategory.Report("Unexpected NameIndex Abbreviation", [&]() { + error() << formatv( + "NameIndex @ {0:x}: Abbreviation {1:x}: DW_IDX_parent " + "uses an unexpected form {2} (should be " + "DW_FORM_ref4 or DW_FORM_flag_present).\n", + NI.getUnitOffset(), Abbr.Code, AttrEnc.Form); + }); return 1; } return 0; @@ -1362,10 +1442,12 @@ unsigned DWARFVerifier::verifyNameIndexAttribute( } if (!DWARFFormValue(AttrEnc.Form).isFormClass(Iter->Class)) { - aggregate("Unexpected NameIndex Abbreviation") << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an " - "unexpected form {3} (expected form class {4}).\n", - NI.getUnitOffset(), Abbr.Code, AttrEnc.Index, - AttrEnc.Form, Iter->ClassName); + ErrorCategory.Report("Unexpected NameIndex Abbreviation", [&]() { + error() << formatv("NameIndex @ {0:x}: Abbreviation {1:x}: {2} uses an " + "unexpected form {3} (expected form class {4}).\n", + NI.getUnitOffset(), Abbr.Code, AttrEnc.Index, + AttrEnc.Form, Iter->ClassName); + }); return 1; } return 0; @@ -1391,9 +1473,13 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) { SmallSet Attributes; for (const auto &AttrEnc : Abbrev.Attributes) { if (!Attributes.insert(AttrEnc.Index).second) { - aggregate("NameIndex Abbreviateion contains multiple attributes") << formatv("NameIndex @ {0:x}: Abbreviation {1:x} contains " - "multiple {2} attributes.\n", - NI.getUnitOffset(), Abbrev.Code, AttrEnc.Index); + ErrorCategory.Report( + "NameIndex Abbreviateion contains multiple attributes", [&]() { + error() << formatv( + "NameIndex @ {0:x}: Abbreviation {1:x} contains " + "multiple {2} attributes.\n", + NI.getUnitOffset(), Abbrev.Code, AttrEnc.Index); + }); ++NumErrors; continue; } @@ -1401,16 +1487,20 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) { } if (NI.getCUCount() > 1 && !Attributes.count(dwarf::DW_IDX_compile_unit)) { - aggregate("Abbreviation contains no attribute") << formatv("NameIndex @ {0:x}: Indexing multiple compile units " - "and abbreviation {1:x} has no {2} attribute.\n", - NI.getUnitOffset(), Abbrev.Code, - dwarf::DW_IDX_compile_unit); + ErrorCategory.Report("Abbreviation contains no attribute", [&]() { + error() << formatv("NameIndex @ {0:x}: Indexing multiple compile units " + "and abbreviation {1:x} has no {2} attribute.\n", + NI.getUnitOffset(), Abbrev.Code, + dwarf::DW_IDX_compile_unit); + }); ++NumErrors; } if (!Attributes.count(dwarf::DW_IDX_die_offset)) { - aggregate("Abbreviate in NameIndex missing attribute") << formatv( - "NameIndex @ {0:x}: Abbreviation {1:x} has no {2} attribute.\n", - NI.getUnitOffset(), Abbrev.Code, dwarf::DW_IDX_die_offset); + ErrorCategory.Report("Abbreviate in NameIndex missing attribute", [&]() { + error() << formatv( + "NameIndex @ {0:x}: Abbreviation {1:x} has no {2} attribute.\n", + NI.getUnitOffset(), Abbrev.Code, dwarf::DW_IDX_die_offset); + }); ++NumErrors; } } @@ -1464,9 +1554,11 @@ unsigned DWARFVerifier::verifyNameIndexEntries( const char *CStr = NTE.getString(); if (!CStr) { - aggregate("Unable to get string associated with name") << formatv( - "Name Index @ {0:x}: Unable to get string associated with name {1}.\n", - NI.getUnitOffset(), NTE.getIndex()); + ErrorCategory.Report("Unable to get string associated with name", [&]() { + error() << formatv("Name Index @ {0:x}: Unable to get string associated " + "with name {1}.\n", + NI.getUnitOffset(), NTE.getIndex()); + }); return 1; } StringRef Str(CStr); @@ -1480,10 +1572,11 @@ unsigned DWARFVerifier::verifyNameIndexEntries( EntryOr = NI.getEntry(&NextEntryID)) { uint32_t CUIndex = *EntryOr->getCUIndex(); if (CUIndex > NI.getCUCount()) { - aggregate("Name Index entry contains invalid CU index") - << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an " - "invalid CU index ({2}).\n", - NI.getUnitOffset(), EntryID, CUIndex); + ErrorCategory.Report("Name Index entry contains invalid CU index", [&]() { + error() << formatv("Name Index @ {0:x}: Entry @ {1:x} contains an " + "invalid CU index ({2}).\n", + NI.getUnitOffset(), EntryID, CUIndex); + }); ++NumErrors; continue; } @@ -1491,26 +1584,32 @@ unsigned DWARFVerifier::verifyNameIndexEntries( uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset(); DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset); if (!DIE) { - aggregate("NameIndex references nonexisten DIE") - << formatv("Name Index @ {0:x}: Entry @ {1:x} references a " - "non-existing DIE @ {2:x}.\n", - NI.getUnitOffset(), EntryID, DIEOffset); + ErrorCategory.Report("NameIndex references nonexisten DIE", [&]() { + error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a " + "non-existing DIE @ {2:x}.\n", + NI.getUnitOffset(), EntryID, DIEOffset); + }); ++NumErrors; continue; } if (DIE.getDwarfUnit()->getOffset() != CUOffset) { - aggregate("Name index contains mismatched CU of DIE") - << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of " - "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n", - NI.getUnitOffset(), EntryID, DIEOffset, CUOffset, - DIE.getDwarfUnit()->getOffset()); + ErrorCategory.Report("Name index contains mismatched CU of DIE", [&]() { + error() << formatv( + "Name Index @ {0:x}: Entry @ {1:x}: mismatched CU of " + "DIE @ {2:x}: index - {3:x}; debug_info - {4:x}.\n", + NI.getUnitOffset(), EntryID, DIEOffset, CUOffset, + DIE.getDwarfUnit()->getOffset()); + }); ++NumErrors; } if (DIE.getTag() != EntryOr->tag()) { - aggregate("Name Index contains mismatched Tag of DIE") << formatv( - "Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of " - "DIE @ {2:x}: index - {3}; debug_info - {4}.\n", - NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(), DIE.getTag()); + ErrorCategory.Report("Name Index contains mismatched Tag of DIE", [&]() { + error() << formatv( + "Name Index @ {0:x}: Entry @ {1:x}: mismatched Tag of " + "DIE @ {2:x}: index - {3}; debug_info - {4}.\n", + NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(), + DIE.getTag()); + }); ++NumErrors; } @@ -1521,11 +1620,12 @@ unsigned DWARFVerifier::verifyNameIndexEntries( DIE.getTag() == DW_TAG_inlined_subroutine; auto EntryNames = getNames(DIE, IncludeStrippedTemplateNames); if (!is_contained(EntryNames, Str)) { - aggregate("Name Index contains mismatched name of DIE") - << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name " - "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n", - NI.getUnitOffset(), EntryID, DIEOffset, Str, - make_range(EntryNames.begin(), EntryNames.end())); + ErrorCategory.Report("Name Index contains mismatched name of DIE", [&]() { + error() << formatv("Name Index @ {0:x}: Entry @ {1:x}: mismatched Name " + "of DIE @ {2:x}: index - {3}; debug_info - {4}.\n", + NI.getUnitOffset(), EntryID, DIEOffset, Str, + make_range(EntryNames.begin(), EntryNames.end())); + }); ++NumErrors; } } @@ -1533,17 +1633,20 @@ unsigned DWARFVerifier::verifyNameIndexEntries( [&](const DWARFDebugNames::SentinelError &) { if (NumEntries > 0) return; - aggregate("NameIndex Name is not associated with any entries") - << formatv("Name Index @ {0:x}: Name {1} ({2}) is " - "not associated with any entries.\n", - NI.getUnitOffset(), NTE.getIndex(), Str); + ErrorCategory.Report( + "NameIndex Name is not associated with any entries", [&]() { + error() << formatv("Name Index @ {0:x}: Name {1} ({2}) is " + "not associated with any entries.\n", + NI.getUnitOffset(), NTE.getIndex(), Str); + }); ++NumErrors; }, [&](const ErrorInfoBase &Info) { - // Aggregation TODO: - error() << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n", - NI.getUnitOffset(), NTE.getIndex(), Str, - Info.message()); + ErrorCategory.Report("Uncategorized NameIndex error", [&]() { + error() << formatv("Name Index @ {0:x}: Name {1} ({2}): {3}\n", + NI.getUnitOffset(), NTE.getIndex(), Str, + Info.message()); + }); ++NumErrors; }); return NumErrors; @@ -1671,10 +1774,12 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness( if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) { return E.getDIEUnitOffset() == DieUnitOffset; })) { - aggregate("Name Index DIE entry missing name") - << formatv("Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with " - "name {3} missing.\n", - NI.getUnitOffset(), Die.getOffset(), Die.getTag(), Name); + ErrorCategory.Report("Name Index DIE entry missing name", [&]() { + error() << formatv( + "Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with " + "name {3} missing.\n", + NI.getUnitOffset(), Die.getOffset(), Die.getTag(), Name); + }); ++NumErrors; } } @@ -1693,8 +1798,9 @@ unsigned DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection, // This verifies that we can read individual name indices and their // abbreviation tables. if (Error E = AccelTable.extract()) { - // Aggregate TODO: - error() << toString(std::move(E)) << '\n'; + std::string Msg = toString(std::move(E)); + ErrorCategory.Report("Accelerator Table Error", + [&]() { error() << Msg << '\n'; }); return 1; } @@ -1794,15 +1900,17 @@ bool DWARFVerifier::verifyDebugStrOffsets( if (!C) break; if (C.tell() + Length > DA.getData().size()) { - aggregate("Section contribution length exceeds available space") - << formatv( - "{0}: contribution {1:X}: length exceeds available space " - "(contribution " - "offset ({1:X}) + length field space ({2:X}) + length " - "({3:X}) == " - "{4:X} > section size {5:X})\n", - SectionName, StartOffset, C.tell() - StartOffset, Length, - C.tell() + Length, DA.getData().size()); + ErrorCategory.Report( + "Section contribution length exceeds available space", [&]() { + error() << formatv( + "{0}: contribution {1:X}: length exceeds available space " + "(contribution " + "offset ({1:X}) + length field space ({2:X}) + length " + "({3:X}) == " + "{4:X} > section size {5:X})\n", + SectionName, StartOffset, C.tell() - StartOffset, Length, + C.tell() + Length, DA.getData().size()); + }); Success = false; // Nothing more to do - no other contributions to try. break; @@ -1810,9 +1918,10 @@ bool DWARFVerifier::verifyDebugStrOffsets( NextUnit = C.tell() + Length; uint8_t Version = DA.getU16(C); if (C && Version != 5) { - aggregate("Invalid Section version") - << formatv("{0}: contribution {1:X}: invalid version {2}\n", - SectionName, StartOffset, Version); + ErrorCategory.Report("Invalid Section version", [&]() { + error() << formatv("{0}: contribution {1:X}: invalid version {2}\n", + SectionName, StartOffset, Version); + }); Success = false; // Can't parse the rest of this contribution, since we don't know the // version, but we can pick up with the next contribution. @@ -1824,10 +1933,12 @@ bool DWARFVerifier::verifyDebugStrOffsets( DA.setAddressSize(OffsetByteSize); uint64_t Remainder = (Length - 4) % OffsetByteSize; if (Remainder != 0) { - aggregate("Invalid section contribution length") << formatv( - "{0}: contribution {1:X}: invalid length ((length ({2:X}) " - "- header (0x4)) % offset size {3:X} == {4:X} != 0)\n", - SectionName, StartOffset, Length, OffsetByteSize, Remainder); + ErrorCategory.Report("Invalid section contribution length", [&]() { + error() << formatv( + "{0}: contribution {1:X}: invalid length ((length ({2:X}) " + "- header (0x4)) % offset size {3:X} == {4:X} != 0)\n", + SectionName, StartOffset, Length, OffsetByteSize, Remainder); + }); Success = false; } for (uint64_t Index = 0; C && C.tell() + OffsetByteSize <= NextUnit; ++Index) { @@ -1837,47 +1948,60 @@ bool DWARFVerifier::verifyDebugStrOffsets( if (StrOff == 0) continue; if (StrData.size() <= StrOff) { - error() << formatv( - "{0}: contribution {1:X}: index {2:X}: invalid string " - "offset *{3:X} == {4:X}, is beyond the bounds of the string section of length {5:X}\n", - SectionName, StartOffset, Index, OffOff, StrOff, StrData.size()); + ErrorCategory.Report( + "String offset out of bounds of string section", [&]() { + error() << formatv( + "{0}: contribution {1:X}: index {2:X}: invalid string " + "offset *{3:X} == {4:X}, is beyond the bounds of the string " + "section of length {5:X}\n", + SectionName, StartOffset, Index, OffOff, StrOff, + StrData.size()); + }); continue; } if (StrData[StrOff - 1] == '\0') continue; - aggregate("Section contribution contains invalid string offset") - << formatv("{0}: contribution {1:X}: index {2:X}: invalid string " - "offset *{3:X} == {4:X}, is neither zero nor " - "immediately following a null character\n", - SectionName, StartOffset, Index, OffOff, StrOff); + ErrorCategory.Report( + "Section contribution contains invalid string offset", [&]() { + error() << formatv( + "{0}: contribution {1:X}: index {2:X}: invalid string " + "offset *{3:X} == {4:X}, is neither zero nor " + "immediately following a null character\n", + SectionName, StartOffset, Index, OffOff, StrOff); + }); Success = false; } } if (Error E = C.takeError()) { - // Aggregate TODO: - error() << SectionName << ": " << toString(std::move(E)) << '\n'; - return false; + std::string Msg = toString(std::move(E)); + ErrorCategory.Report("String offset error", [&]() { + error() << SectionName << ": " << Msg << '\n'; + return false; + }); } return Success; } void OutputCategoryAggregator::Report( - StringRef s, std::function detailCallback) { - UniqueErrors[s]++; - if (Output) - detailedCallback(*Output); + StringRef s, std::function detailCallback) { + Aggregation[std::string(s)]++; + if (CallDetail) + detailCallback(); } -void OutputCategoryAggregator::DumpAggregation(raw_ostream *o) { - for (auto &&[name, count] : Aggregator) { - *o << name << ": seen " << count << " time(s)\n"; +void OutputCategoryAggregator::HandleAggregate( + std::function handleCounts) { + for (auto &&[name, count] : Aggregation) { + handleCounts(name, count); } } void DWARFVerifier::finish() { if (DumpOpts.DumpAggregateErrors) - Category.Dump(error()); + ErrorCategory.HandleAggregate([&](StringRef s, unsigned count) { + error() << s << ": " << count << '\n'; + }); } raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); } diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp index 941df4eb184457a..09eb659d4241b9b 100644 --- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp +++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp @@ -326,7 +326,7 @@ static DIDumpOptions getDumpOpts(DWARFContext &C) { DumpOpts.RecoverableErrorHandler = C.getRecoverableErrorHandler(); // In -verify mode, print DIEs without children in error messages. if (Verify) { - DumpOpts.Verbose = true; + // DumpOpts.Verbose = true; return DumpOpts.noImplicitRecursion(); } return DumpOpts; From a6613aca24c86c5ac325dbf2250e77cd79aa3225 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Thu, 25 Jan 2024 13:30:14 -0800 Subject: [PATCH 06/13] Set up the flags in a reasonable way --- llvm/include/llvm/DebugInfo/DIContext.h | 2 +- .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 2 +- llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | 2 +- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 9 ++++++--- llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp | 17 ++++++++++++++++- 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DIContext.h b/llvm/include/llvm/DebugInfo/DIContext.h index 298628b551fb68d..288ddf77bdfda78 100644 --- a/llvm/include/llvm/DebugInfo/DIContext.h +++ b/llvm/include/llvm/DebugInfo/DIContext.h @@ -205,7 +205,7 @@ struct DIDumpOptions { bool DisplayRawContents = false; bool IsEH = false; bool DumpNonSkeleton = false; - bool DumpAggregateErrors = true; + bool ShowAggregateErrors = false; std::function GetNameForDWARFReg; diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index d70175999e8df8b..728eb869ff01b1b 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -363,7 +363,7 @@ class DWARFVerifier { void (DWARFObject::*)(function_ref) const); /// Emits any aggregate information collection, depending on the dump options - void finish(); + void finish(bool Success); }; static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS, diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp index aa294444b46803a..13d9103a76ec2a6 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp @@ -1408,7 +1408,7 @@ bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) { if (DumpOpts.DumpType & DIDT_DebugStrOffsets) Success &= verifier.handleDebugStrOffsets(); Success &= verifier.handleAccelTables(); - verifier.finish(); + verifier.finish(Success); return Success; } diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index 88c0e4ab2c81042..56dfeef1f135088 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -1997,11 +1997,14 @@ void OutputCategoryAggregator::HandleAggregate( } } -void DWARFVerifier::finish() { - if (DumpOpts.DumpAggregateErrors) +void DWARFVerifier::finish(bool Success) { + if (!Success && DumpOpts.ShowAggregateErrors) { + error() << "Aggregated error category counts:\n"; ErrorCategory.HandleAggregate([&](StringRef s, unsigned count) { - error() << s << ": " << count << '\n'; + error() << "Error category '" << s << "' occurred " << count + << " time(s).\n"; }); + } } raw_ostream &DWARFVerifier::error() const { return WithColor::error(OS); } diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp index 09eb659d4241b9b..55d484f9136315b 100644 --- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp +++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp @@ -276,6 +276,14 @@ static cl::opt cat(DwarfDumpCategory)); static opt Verify("verify", desc("Verify the DWARF debug info."), cat(DwarfDumpCategory)); +static opt OnlyAggregateErrors( + "only-aggregate-errors", + desc("Only display the aggregate errors when verifying."), + cat(DwarfDumpCategory)); +static opt NoAggregateErrors( + "no-aggregate-errors", + desc("Do not display the aggregate errors when verifying."), + cat(DwarfDumpCategory)); static opt Quiet("quiet", desc("Use with -verify to not emit to STDOUT."), cat(DwarfDumpCategory)); static opt DumpUUID("uuid", desc("Show the UUID for each architecture."), @@ -326,7 +334,8 @@ static DIDumpOptions getDumpOpts(DWARFContext &C) { DumpOpts.RecoverableErrorHandler = C.getRecoverableErrorHandler(); // In -verify mode, print DIEs without children in error messages. if (Verify) { - // DumpOpts.Verbose = true; + DumpOpts.Verbose = !OnlyAggregateErrors; + DumpOpts.ShowAggregateErrors = !NoAggregateErrors; return DumpOpts.noImplicitRecursion(); } return DumpOpts; @@ -812,6 +821,12 @@ int main(int argc, char **argv) { "-verbose is currently not supported"; return 1; } + if (Verify && NoAggregateErrors && OnlyAggregateErrors) { + WithColor::error() << "incompatible arguments: specifying both " + " -no-aggregate-errors, and -only-aggregate-errors " + "is not supported"; + return 1; + } std::error_code EC; ToolOutputFile OutputFile(OutputFilename, EC, sys::fs::OF_TextWithCRLF); From c171a9582c076e8619538aafde43268dd1a79254 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Thu, 25 Jan 2024 14:05:58 -0800 Subject: [PATCH 07/13] A few final touches before tests --- .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 18 ++++++++------- llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | 2 +- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 23 ++++++++++--------- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index 728eb869ff01b1b..ad50fa92665ce54 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -33,14 +33,15 @@ struct DWARFSection; class OutputCategoryAggregator { private: std::map Aggregation; - bool CallDetail; + bool IncludeDetail; public: - OutputCategoryAggregator(bool callDetail = false) : CallDetail(callDetail) {} - void EnableDetail() { CallDetail = true; } - void DisableDetail() { CallDetail = false; } + OutputCategoryAggregator(bool includeDetail = false) + : IncludeDetail(includeDetail) {} + void EnableDetail() { IncludeDetail = true; } + void DisableDetail() { IncludeDetail = false; } void Report(StringRef s, std::function detailCallback); - void HandleAggregate(std::function handleCounts); + void EnumerateResults(std::function handleCounts); }; /// A class that verifies DWARF debug information given a DWARF Context. @@ -89,11 +90,12 @@ class DWARFVerifier { bool intersects(const DieRangeInfo &RHS) const; }; +private: raw_ostream &OS; DWARFContext &DCtx; - OutputCategoryAggregator ErrorCategory; DIDumpOptions DumpOpts; uint32_t NumDebugLineErrors = 0; + OutputCategoryAggregator ErrorCategory; // Used to relax some checks that do not currently work portably bool IsObjectFile; bool IsMachOObject; @@ -362,8 +364,8 @@ class DWARFVerifier { StringRef SectionName, const DWARFSection &Section, StringRef StrData, void (DWARFObject::*)(function_ref) const); - /// Emits any aggregate information collection, depending on the dump options - void finish(bool Success); + /// Emits any aggregate information collected, depending on the dump options + void summarize(bool Success); }; static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS, diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp index 13d9103a76ec2a6..32a7fec0860be34 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp @@ -1408,7 +1408,7 @@ bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) { if (DumpOpts.DumpType & DIDT_DebugStrOffsets) Success &= verifier.handleDebugStrOffsets(); Success &= verifier.handleAccelTables(); - verifier.finish(Success); + verifier.summarize(Success); return Success; } diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index 56dfeef1f135088..01278a83f3c5827 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -307,7 +307,7 @@ unsigned DWARFVerifier::verifyDebugInfoCallSite(const DWARFDie &Die) { if (!Curr.isValid()) { ErrorCategory.Report( - "Call site entry not nexted within valid subprogram", [&]() { + "Call site entry not nested within valid subprogram", [&]() { error() << "Call site entry not nested within a valid subprogram:"; Die.dump(OS); }); @@ -339,9 +339,9 @@ unsigned DWARFVerifier::verifyAbbrevSection(const DWARFDebugAbbrev *Abbrev) { Expected AbbrDeclsOrErr = Abbrev->getAbbreviationDeclarationSet(0); if (!AbbrDeclsOrErr) { - ErrorCategory.Report("Abbreviation Declaration error", [&]() { - error() << toString(AbbrDeclsOrErr.takeError()) << "\n"; - }); + std::string ErrMsg = toString(AbbrDeclsOrErr.takeError()); + ErrorCategory.Report("Abbreviation Declaration error", + [&]() { error() << ErrMsg << "\n"; }); return 1; } @@ -863,8 +863,9 @@ unsigned DWARFVerifier::verifyDebugInfoForm(const DWARFDie &Die, case DW_FORM_line_strp: { if (Error E = AttrValue.Value.getAsCString().takeError()) { ++NumErrors; + std::string ErrMsg = toString(std::move(E)); ErrorCategory.Report("Invalid DW_FORM attribute", [&]() { - error() << toString(std::move(E)) << ":\n"; + error() << ErrMsg << ":\n"; dump(Die) << '\n'; }); } @@ -916,7 +917,7 @@ void DWARFVerifier::verifyDebugLineStmtOffsets() { if (LineTableOffset < DCtx.getDWARFObj().getLineSection().Data.size()) { if (!LineTable) { ++NumDebugLineErrors; - ErrorCategory.Report("Unparseable .debug_line entry", [&]() { + ErrorCategory.Report("Unparsable .debug_line entry", [&]() { error() << ".debug_line[" << format("0x%08" PRIx64, LineTableOffset) << "] was not able to be parsed for CU:\n"; dump(Die) << '\n'; @@ -1584,7 +1585,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries( uint64_t DIEOffset = CUOffset + *EntryOr->getDIEUnitOffset(); DWARFDie DIE = DCtx.getDIEForOffset(DIEOffset); if (!DIE) { - ErrorCategory.Report("NameIndex references nonexisten DIE", [&]() { + ErrorCategory.Report("NameIndex references nonexistent DIE", [&]() { error() << formatv("Name Index @ {0:x}: Entry @ {1:x} references a " "non-existing DIE @ {2:x}.\n", NI.getUnitOffset(), EntryID, DIEOffset); @@ -1986,21 +1987,21 @@ bool DWARFVerifier::verifyDebugStrOffsets( void OutputCategoryAggregator::Report( StringRef s, std::function detailCallback) { Aggregation[std::string(s)]++; - if (CallDetail) + if (IncludeDetail) detailCallback(); } -void OutputCategoryAggregator::HandleAggregate( +void OutputCategoryAggregator::EnumerateResults( std::function handleCounts) { for (auto &&[name, count] : Aggregation) { handleCounts(name, count); } } -void DWARFVerifier::finish(bool Success) { +void DWARFVerifier::summarize(bool Success) { if (!Success && DumpOpts.ShowAggregateErrors) { error() << "Aggregated error category counts:\n"; - ErrorCategory.HandleAggregate([&](StringRef s, unsigned count) { + ErrorCategory.EnumerateResults([&](StringRef s, unsigned count) { error() << "Error category '" << s << "' occurred " << count << " time(s).\n"; }); From e3176b4fe80b82b4236305a26efb305ddb9d02b7 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Fri, 26 Jan 2024 08:31:35 -0800 Subject: [PATCH 08/13] Adding flags to make output the same as before for tests --- llvm/test/DebugInfo/X86/skeleton-unit-verify.s | 4 +--- llvm/test/DebugInfo/dwarfdump-accel.test | 2 +- .../tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml | 2 +- .../llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml | 2 +- llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml | 2 +- .../llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml | 2 +- .../tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml | 2 +- llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s | 2 +- 8 files changed, 8 insertions(+), 10 deletions(-) diff --git a/llvm/test/DebugInfo/X86/skeleton-unit-verify.s b/llvm/test/DebugInfo/X86/skeleton-unit-verify.s index d9c7436d1c750c5..28d287d20b13474 100644 --- a/llvm/test/DebugInfo/X86/skeleton-unit-verify.s +++ b/llvm/test/DebugInfo/X86/skeleton-unit-verify.s @@ -1,5 +1,5 @@ # RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o -# RUN: not llvm-dwarfdump --verify %t.o | FileCheck %s +# RUN: not llvm-dwarfdump --no-aggregate-errors --verify %t.o | FileCheck %s # CHECK: Verifying .debug_abbrev... # CHECK-NEXT: Verifying .debug_info Unit Header Chain... @@ -51,5 +51,3 @@ .byte 2 # Abbrev [2] .byte 0 .Lcu_end1: - - diff --git a/llvm/test/DebugInfo/dwarfdump-accel.test b/llvm/test/DebugInfo/dwarfdump-accel.test index 27720b6c9b42a80..cda8e3f599970c2 100644 --- a/llvm/test/DebugInfo/dwarfdump-accel.test +++ b/llvm/test/DebugInfo/dwarfdump-accel.test @@ -1,5 +1,5 @@ RUN: llvm-dwarfdump -v %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s -RUN: not llvm-dwarfdump -verify %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s --check-prefix=VERIFY +RUN: not llvm-dwarfdump -no-aggregate-errors -verify %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s --check-prefix=VERIFY Gather some DIE indexes to verify the accelerator table contents. CHECK: .debug_info contents diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml index b86623dd011d429..cadc35a09f06c30 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml @@ -1,4 +1,4 @@ -# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: # CHECK: error: DIE has DW_AT_decl_file with an invalid file index 2 (valid values are [1-1]){{[[:space:]]}} # CHECK-NEXT: 0x0000001e: DW_TAG_subprogram diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml index 3b56dca1bb090b3..655a81aecc0ab7c 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml @@ -1,4 +1,4 @@ -# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: # CHECK: error: DIE has DW_AT_decl_file with an invalid file index 2 (the file table in the prologue is empty){{[[:space:]]}} # CHECK-NEXT: 0x0000001e: DW_TAG_subprogram diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml index af55a3a7d103449..2d7b853871678d3 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml @@ -1,4 +1,4 @@ -# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: # CHECK: error: DIE has DW_AT_decl_file with invalid encoding{{[[:space:]]}} # CHECK-NEXT: 0x0000001a: DW_TAG_subprogram diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml index a40959f4d0ded6c..4fc99a04cf762c2 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml @@ -39,7 +39,7 @@ # # 0x00000066: NULL -# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: # CHECK: error: DIE has overlapping ranges in DW_AT_ranges attribute: [0x0000000000000000, 0x0000000000000020) and [0x0000000000000000, 0x0000000000000030) diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml index 5188ac5a6d407f6..bb61f431ff2da7d 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml @@ -30,7 +30,7 @@ # 0x00000056: NULL -# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: # CHECK: Verifying -: file format Mach-O 64-bit x86-64 # CHECK: Verifying .debug_abbrev... diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s index 3941d9f1a7a57fc..eb8dbd60538538b 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s @@ -1,5 +1,5 @@ # RUN: llvm-mc %s -filetype obj -triple x86_64-linux-gnu -o - \ -# RUN: | not llvm-dwarfdump -v -verify - 2>&1 \ +# RUN: | not llvm-dwarfdump --no-aggregate-errors -v -verify - 2>&1 \ # RUN: | FileCheck %s --implicit-check-not=error --implicit-check-not=warning # CHECK: Verifying dwo Units... From 064bef5c6b61ef88da11ecc15182c68d20301755 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Fri, 26 Jan 2024 12:16:50 -0800 Subject: [PATCH 09/13] A couple minor tweaks to keep the previous behavior 'available' --- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index 01278a83f3c5827..b3b99bc8babcc6d 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -570,8 +570,8 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, ++NumErrors; ErrorCategory.Report("Invalid address range", [&]() { error() << "Invalid address range " << Range << "\n"; + DumpDieAfterError = true; }); - DumpDieAfterError = true; continue; } @@ -586,8 +586,8 @@ unsigned DWARFVerifier::verifyDieRanges(const DWARFDie &Die, ErrorCategory.Report("DIE has overlapping DW_AT_ranges", [&]() { error() << "DIE has overlapping ranges in DW_AT_ranges attribute: " << *PrevRange << " and " << Range << '\n'; + DumpDieAfterError = true; }); - DumpDieAfterError = DumpOpts.Verbose; } } if (DumpDieAfterError) @@ -1058,7 +1058,7 @@ DWARFVerifier::DWARFVerifier(raw_ostream &S, DWARFContext &D, DIDumpOptions DumpOpts) : OS(S), DCtx(D), DumpOpts(std::move(DumpOpts)), IsObjectFile(false), IsMachOObject(false) { - if (DumpOpts.Verbose) { + if (DumpOpts.Verbose || !DumpOpts.ShowAggregateErrors) { ErrorCategory.EnableDetail(); } if (const auto *F = DCtx.getDWARFObj().getFile()) { From c09abdc34b5bf90dc0d585926a3db04b2bb14f37 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Fri, 26 Jan 2024 12:56:31 -0800 Subject: [PATCH 10/13] Added checking for aggregate counts for a dwarfdump test --- .../tools/llvm-dwarfdump/X86/verify_file_encoding.yaml | 8 ++++++-- llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml index 2d7b853871678d3..6e1d751439d1ee4 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml @@ -1,4 +1,4 @@ -# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --verify - | FileCheck %s --implicit-check-not=error: # CHECK: error: DIE has DW_AT_decl_file with invalid encoding{{[[:space:]]}} # CHECK-NEXT: 0x0000001a: DW_TAG_subprogram @@ -50,7 +50,11 @@ # CHECK-NEXT: DW_AT_decl_line [DW_FORM_sdata] (3) # CHECK-NEXT: DW_AT_call_file [DW_FORM_sdata] (4) # CHECK-NEXT: DW_AT_call_line [DW_FORM_sdata] (5){{[[:space:]]}} - +# CHECK-NEXT: Verifying dwo Units... +# CHECK-NEXT: error: Aggregated error category counts: +# CHECK-NEXT: error: Error category 'Invalid encoding in DW_AT_decl_file' occurred 4 time(s). +# CHECK-NEXT: error: Error category 'Invalid file index in DW_AT_call_line' occurred 1 time(s). +# CHECK-NEXT: error: Error category 'Invalid file index in DW_AT_decl_line' occurred 1 time(s). --- !ELF FileHeader: Class: ELFCLASS64 diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s index eb8dbd60538538b..f6eea2d65ccd76e 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s @@ -1,5 +1,5 @@ # RUN: llvm-mc %s -filetype obj -triple x86_64-linux-gnu -o - \ -# RUN: | not llvm-dwarfdump --no-aggregate-errors -v -verify - 2>&1 \ +# RUN: | not llvm-dwarfdump -v -verify - 2>&1 \ # RUN: | FileCheck %s --implicit-check-not=error --implicit-check-not=warning # CHECK: Verifying dwo Units... @@ -8,6 +8,11 @@ # CHECK: error: Unsupported DW_AT_location encoding: DW_FORM_data1 # FIXME: This should read "type unit" or just "unit" to be correct for this case/in general # CHECK: error: DIE has DW_AT_decl_file that references a file with index 1 and the compile unit has no line table +# CHECK: error: Aggregated error category counts: +# CHECK: error: Error category 'Compilation unit root DIE is not a unit DIE' occurred 1 time(s). +# CHECK: error: Error category 'File index in DW_AT_decl_file reference CU with no line table' occurred 1 time(s). +# CHECK: error: Error category 'Invalid DW_AT_location' occurred 1 time(s). +# CHECK: error: Error category 'Mismatched unit type' occurred 1 time(s). # CHECK: Errors detected .section .debug_info.dwo,"e",@progbits .long .Ldebug_info_dwo_end1-.Ldebug_info_dwo_start1 # Length of Unit From 5a7d23ec3b5038e95a1589859c441740025bd0b7 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Fri, 26 Jan 2024 13:07:20 -0800 Subject: [PATCH 11/13] Added a warning for the new flags --- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 9 +++++---- llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp | 3 +++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index b3b99bc8babcc6d..a1f32c3e838d3cb 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -1630,10 +1630,11 @@ unsigned DWARFVerifier::verifyNameIndexEntries( ++NumErrors; } } - handleAllErrors(EntryOr.takeError(), - [&](const DWARFDebugNames::SentinelError &) { - if (NumEntries > 0) - return; + handleAllErrors( + EntryOr.takeError(), + [&](const DWARFDebugNames::SentinelError &) { + if (NumEntries > 0) + return; ErrorCategory.Report( "NameIndex Name is not associated with any entries", [&]() { error() << formatv("Name Index @ {0:x}: Name {1} ({2}) is " diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp index 55d484f9136315b..258f61f599acdf2 100644 --- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp +++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp @@ -827,6 +827,9 @@ int main(int argc, char **argv) { "is not supported"; return 1; } + if (!Verify && (NoAggregateErrors || OnlyAggregateErrors)) + WithColor::warning() << "-no-aggregate-errors and -only-aggregate-errors " + "have no effect without -verify"; std::error_code EC; ToolOutputFile OutputFile(OutputFilename, EC, sys::fs::OF_TextWithCRLF); From 52697102bb77800d53e44d59ee9d14b907dd5140 Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Mon, 29 Jan 2024 10:11:28 -0800 Subject: [PATCH 12/13] First chunk of PR feedback addressed --- .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 3 +- llvm/lib/DebugInfo/DWARF/DWARFContext.cpp | 2 +- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 82 +++++++++++++------ .../X86/verify_file_encoding.yaml | 8 +- .../llvm-dwarfdump/X86/verify_split_cu.s | 12 +-- 5 files changed, 72 insertions(+), 35 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index ad50fa92665ce54..c636da9a4c5217e 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -40,6 +40,7 @@ class OutputCategoryAggregator { : IncludeDetail(includeDetail) {} void EnableDetail() { IncludeDetail = true; } void DisableDetail() { IncludeDetail = false; } + size_t GetNumCategories() const { return Aggregation.size(); } void Report(StringRef s, std::function detailCallback); void EnumerateResults(std::function handleCounts); }; @@ -365,7 +366,7 @@ class DWARFVerifier { void (DWARFObject::*)(function_ref) const); /// Emits any aggregate information collected, depending on the dump options - void summarize(bool Success); + void summarize(); }; static inline bool operator<(const DWARFVerifier::DieRangeInfo &LHS, diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp index 32a7fec0860be34..b7297c18da7c995 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp @@ -1408,7 +1408,7 @@ bool DWARFContext::verify(raw_ostream &OS, DIDumpOptions DumpOpts) { if (DumpOpts.DumpType & DIDT_DebugStrOffsets) Success &= verifier.handleDebugStrOffsets(); Success &= verifier.handleAccelTables(); - verifier.summarize(Success); + verifier.summarize(); return Success; } diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index a1f32c3e838d3cb..6bb68b30946e04b 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -167,24 +167,61 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData, if (!ValidLength || !ValidVersion || !ValidAddrSize || !ValidAbbrevOffset || !ValidType) { Success = false; - ErrorCategory.Report("Invalid Length in Unit Header", [&]() { - error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", - UnitIndex, OffsetStart); - if (!ValidLength) - note() << "The length for this unit is too " - "large for the .debug_info provided.\n"; - if (!ValidVersion) { - note() << "The 16 bit unit header version is not valid.\n"; - } - if (!ValidType) { - note() << "The unit type encoding is not valid.\n"; - } - if (!ValidAbbrevOffset) - note() << "The offset into the .debug_abbrev section is " - "not valid.\n"; - if (!ValidAddrSize) - note() << "The address size is unsupported.\n"; - }); + bool headerShown = false; + if (!ValidLength) + ErrorCategory.Report( + "Unit Header Length: Unit too large for .debug_info provided", [&]() { + if (!headerShown) { + error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", + UnitIndex, OffsetStart); + headerShown = true; + } + note() << "The length for this unit is too " + "large for the .debug_info provided.\n"; + }); + if (!ValidVersion) + ErrorCategory.Report( + "Unit Header Length: 16 bit unit header version is not valid", [&]() { + if (!headerShown) { + error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", + UnitIndex, OffsetStart); + headerShown = true; + } + note() << "The 16 bit unit header version is not valid.\n"; + }); + if (!ValidType) + ErrorCategory.Report( + "Unit Header Length: Unit type encoding is not valid", [&]() { + if (!headerShown) { + error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", + UnitIndex, OffsetStart); + headerShown = true; + } + note() << "The unit type encoding is not valid.\n"; + }); + if (!ValidAbbrevOffset) + ErrorCategory.Report( + "Unit Header Length: Offset into the .debug_abbrev section is not " + "valid", + [&]() { + if (!headerShown) { + error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", + UnitIndex, OffsetStart); + headerShown = true; + } + note() << "The offset into the .debug_abbrev section is " + "not valid.\n"; + }); + if (!ValidAddrSize) + ErrorCategory.Report( + "Unit Header Length: Address size is unsupported", [&]() { + if (!headerShown) { + error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", + UnitIndex, OffsetStart); + headerShown = true; + } + note() << "The address size is unsupported.\n"; + }); } *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4); return Success; @@ -1999,12 +2036,11 @@ void OutputCategoryAggregator::EnumerateResults( } } -void DWARFVerifier::summarize(bool Success) { - if (!Success && DumpOpts.ShowAggregateErrors) { - error() << "Aggregated error category counts:\n"; +void DWARFVerifier::summarize() { + if (ErrorCategory.GetNumCategories() && DumpOpts.ShowAggregateErrors) { + error() << "Aggregated error counts:\n"; ErrorCategory.EnumerateResults([&](StringRef s, unsigned count) { - error() << "Error category '" << s << "' occurred " << count - << " time(s).\n"; + error() << s << " occurred " << count << " time(s).\n"; }); } } diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml index 6e1d751439d1ee4..fe31436e9f6e355 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_file_encoding.yaml @@ -51,10 +51,10 @@ # CHECK-NEXT: DW_AT_call_file [DW_FORM_sdata] (4) # CHECK-NEXT: DW_AT_call_line [DW_FORM_sdata] (5){{[[:space:]]}} # CHECK-NEXT: Verifying dwo Units... -# CHECK-NEXT: error: Aggregated error category counts: -# CHECK-NEXT: error: Error category 'Invalid encoding in DW_AT_decl_file' occurred 4 time(s). -# CHECK-NEXT: error: Error category 'Invalid file index in DW_AT_call_line' occurred 1 time(s). -# CHECK-NEXT: error: Error category 'Invalid file index in DW_AT_decl_line' occurred 1 time(s). +# CHECK-NEXT: error: Aggregated error counts: +# CHECK-NEXT: error: Invalid encoding in DW_AT_decl_file occurred 4 time(s). +# CHECK-NEXT: error: Invalid file index in DW_AT_call_line occurred 1 time(s). +# CHECK-NEXT: error: Invalid file index in DW_AT_decl_line occurred 1 time(s). --- !ELF FileHeader: Class: ELFCLASS64 diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s index f6eea2d65ccd76e..ebc1b923f1a3b2e 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_split_cu.s @@ -8,12 +8,12 @@ # CHECK: error: Unsupported DW_AT_location encoding: DW_FORM_data1 # FIXME: This should read "type unit" or just "unit" to be correct for this case/in general # CHECK: error: DIE has DW_AT_decl_file that references a file with index 1 and the compile unit has no line table -# CHECK: error: Aggregated error category counts: -# CHECK: error: Error category 'Compilation unit root DIE is not a unit DIE' occurred 1 time(s). -# CHECK: error: Error category 'File index in DW_AT_decl_file reference CU with no line table' occurred 1 time(s). -# CHECK: error: Error category 'Invalid DW_AT_location' occurred 1 time(s). -# CHECK: error: Error category 'Mismatched unit type' occurred 1 time(s). -# CHECK: Errors detected +# CHECK: error: Aggregated error counts: +# CHECK: error: Compilation unit root DIE is not a unit DIE occurred 1 time(s). +# CHECK: error: File index in DW_AT_decl_file reference CU with no line table occurred 1 time(s). +# CHECK: error: Invalid DW_AT_location occurred 1 time(s). +# CHECK: error: Mismatched unit type occurred 1 time(s). +# CHECK: Errors detected. .section .debug_info.dwo,"e",@progbits .long .Ldebug_info_dwo_end1-.Ldebug_info_dwo_start1 # Length of Unit .Ldebug_info_dwo_start1: From 617670b302d2c90439d2442748bf591bfd9e587c Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Tue, 30 Jan 2024 12:49:25 -0800 Subject: [PATCH 13/13] Response to PR feedback #2 --- .../llvm/DebugInfo/DWARF/DWARFVerifier.h | 3 +- llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 51 +++++++------------ .../test/DebugInfo/X86/skeleton-unit-verify.s | 2 +- llvm/test/DebugInfo/dwarfdump-accel.test | 2 +- .../X86/verify_attr_file_indexes.yaml | 2 +- .../verify_attr_file_indexes_no_files.yaml | 2 +- .../X86/verify_overlapping_cu_ranges.yaml | 2 +- .../X86/verify_parent_zero_length.yaml | 2 +- llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp | 42 ++++++++------- 9 files changed, 49 insertions(+), 59 deletions(-) diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h index c636da9a4c5217e..ea73664b1e46ca4 100644 --- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h +++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h @@ -38,8 +38,7 @@ class OutputCategoryAggregator { public: OutputCategoryAggregator(bool includeDetail = false) : IncludeDetail(includeDetail) {} - void EnableDetail() { IncludeDetail = true; } - void DisableDetail() { IncludeDetail = false; } + void ShowDetail(bool showDetail) { IncludeDetail = showDetail; } size_t GetNumCategories() const { return Aggregation.size(); } void Report(StringRef s, std::function detailCallback); void EnumerateResults(std::function handleCounts); diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp index 6bb68b30946e04b..2124ff835c5727f 100644 --- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp +++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp @@ -167,36 +167,31 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData, if (!ValidLength || !ValidVersion || !ValidAddrSize || !ValidAbbrevOffset || !ValidType) { Success = false; - bool headerShown = false; + bool HeaderShown = false; + auto ShowHeaderOnce = [&]() { + if (!HeaderShown) { + error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", + UnitIndex, OffsetStart); + HeaderShown = true; + } + }; if (!ValidLength) ErrorCategory.Report( "Unit Header Length: Unit too large for .debug_info provided", [&]() { - if (!headerShown) { - error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", - UnitIndex, OffsetStart); - headerShown = true; - } + ShowHeaderOnce(); note() << "The length for this unit is too " "large for the .debug_info provided.\n"; }); if (!ValidVersion) ErrorCategory.Report( "Unit Header Length: 16 bit unit header version is not valid", [&]() { - if (!headerShown) { - error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", - UnitIndex, OffsetStart); - headerShown = true; - } + ShowHeaderOnce(); note() << "The 16 bit unit header version is not valid.\n"; }); if (!ValidType) ErrorCategory.Report( "Unit Header Length: Unit type encoding is not valid", [&]() { - if (!headerShown) { - error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", - UnitIndex, OffsetStart); - headerShown = true; - } + ShowHeaderOnce(); note() << "The unit type encoding is not valid.\n"; }); if (!ValidAbbrevOffset) @@ -204,24 +199,16 @@ bool DWARFVerifier::verifyUnitHeader(const DWARFDataExtractor DebugInfoData, "Unit Header Length: Offset into the .debug_abbrev section is not " "valid", [&]() { - if (!headerShown) { - error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", - UnitIndex, OffsetStart); - headerShown = true; - } + ShowHeaderOnce(); note() << "The offset into the .debug_abbrev section is " "not valid.\n"; }); if (!ValidAddrSize) - ErrorCategory.Report( - "Unit Header Length: Address size is unsupported", [&]() { - if (!headerShown) { - error() << format("Units[%d] - start offset: 0x%08" PRIx64 " \n", - UnitIndex, OffsetStart); - headerShown = true; - } - note() << "The address size is unsupported.\n"; - }); + ErrorCategory.Report("Unit Header Length: Address size is unsupported", + [&]() { + ShowHeaderOnce(); + note() << "The address size is unsupported.\n"; + }); } *Offset = OffsetStart + Length + (isUnitDWARF64 ? 12 : 4); return Success; @@ -1095,9 +1082,7 @@ DWARFVerifier::DWARFVerifier(raw_ostream &S, DWARFContext &D, DIDumpOptions DumpOpts) : OS(S), DCtx(D), DumpOpts(std::move(DumpOpts)), IsObjectFile(false), IsMachOObject(false) { - if (DumpOpts.Verbose || !DumpOpts.ShowAggregateErrors) { - ErrorCategory.EnableDetail(); - } + ErrorCategory.ShowDetail(DumpOpts.Verbose || !DumpOpts.ShowAggregateErrors); if (const auto *F = DCtx.getDWARFObj().getFile()) { IsObjectFile = F->isRelocatableObject(); IsMachOObject = F->isMachO(); diff --git a/llvm/test/DebugInfo/X86/skeleton-unit-verify.s b/llvm/test/DebugInfo/X86/skeleton-unit-verify.s index 28d287d20b13474..92a3df486da39d3 100644 --- a/llvm/test/DebugInfo/X86/skeleton-unit-verify.s +++ b/llvm/test/DebugInfo/X86/skeleton-unit-verify.s @@ -1,5 +1,5 @@ # RUN: llvm-mc -triple x86_64-unknown-linux %s -filetype=obj -o %t.o -# RUN: not llvm-dwarfdump --no-aggregate-errors --verify %t.o | FileCheck %s +# RUN: not llvm-dwarfdump --error-display=details --verify %t.o | FileCheck %s # CHECK: Verifying .debug_abbrev... # CHECK-NEXT: Verifying .debug_info Unit Header Chain... diff --git a/llvm/test/DebugInfo/dwarfdump-accel.test b/llvm/test/DebugInfo/dwarfdump-accel.test index cda8e3f599970c2..d564a8576c3c3bd 100644 --- a/llvm/test/DebugInfo/dwarfdump-accel.test +++ b/llvm/test/DebugInfo/dwarfdump-accel.test @@ -1,5 +1,5 @@ RUN: llvm-dwarfdump -v %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s -RUN: not llvm-dwarfdump -no-aggregate-errors -verify %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s --check-prefix=VERIFY +RUN: not llvm-dwarfdump -error-display=details -verify %p/Inputs/dwarfdump-objc.x86_64.o | FileCheck %s --check-prefix=VERIFY Gather some DIE indexes to verify the accelerator table contents. CHECK: .debug_info contents diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml index cadc35a09f06c30..a05b6f0cef8d0a8 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes.yaml @@ -1,4 +1,4 @@ -# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --error-display=details --verify - | FileCheck %s --implicit-check-not=error: # CHECK: error: DIE has DW_AT_decl_file with an invalid file index 2 (valid values are [1-1]){{[[:space:]]}} # CHECK-NEXT: 0x0000001e: DW_TAG_subprogram diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml index 655a81aecc0ab7c..2ba71f521d05836 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_attr_file_indexes_no_files.yaml @@ -1,4 +1,4 @@ -# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --error-display=details --verify - | FileCheck %s --implicit-check-not=error: # CHECK: error: DIE has DW_AT_decl_file with an invalid file index 2 (the file table in the prologue is empty){{[[:space:]]}} # CHECK-NEXT: 0x0000001e: DW_TAG_subprogram diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml index 4fc99a04cf762c2..32b1c399985f1a4 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_overlapping_cu_ranges.yaml @@ -39,7 +39,7 @@ # # 0x00000066: NULL -# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --error-display=details --verify - | FileCheck %s --implicit-check-not=error: # CHECK: error: DIE has overlapping ranges in DW_AT_ranges attribute: [0x0000000000000000, 0x0000000000000020) and [0x0000000000000000, 0x0000000000000030) diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml b/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml index bb61f431ff2da7d..655819515f0ff25 100644 --- a/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml +++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_parent_zero_length.yaml @@ -30,7 +30,7 @@ # 0x00000056: NULL -# RUN: yaml2obj %s | not llvm-dwarfdump --no-aggregate-errors --verify - | FileCheck %s --implicit-check-not=error: +# RUN: yaml2obj %s | not llvm-dwarfdump --error-display=details --verify - | FileCheck %s --implicit-check-not=error: # CHECK: Verifying -: file format Mach-O 64-bit x86-64 # CHECK: Verifying .debug_abbrev... diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp index 258f61f599acdf2..559e7a604898344 100644 --- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp +++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp @@ -124,6 +124,14 @@ template <> class parser final : public basic_parser { namespace { using namespace cl; +enum ErrorDetailLevel { + OnlyDetailsNoSummary, + NoDetailsOnlySummary, + NoDetailsOrSummary, + BothDetailsAndSummary, + Unspecified +}; + OptionCategory DwarfDumpCategory("Specific Options"); static list InputFilenames(Positional, desc(""), @@ -276,13 +284,16 @@ static cl::opt cat(DwarfDumpCategory)); static opt Verify("verify", desc("Verify the DWARF debug info."), cat(DwarfDumpCategory)); -static opt OnlyAggregateErrors( - "only-aggregate-errors", - desc("Only display the aggregate errors when verifying."), - cat(DwarfDumpCategory)); -static opt NoAggregateErrors( - "no-aggregate-errors", - desc("Do not display the aggregate errors when verifying."), +static opt ErrorDetails( + "error-display", init(Unspecified), + values(clEnumValN(NoDetailsOrSummary, "quiet", + "Only display whether errors occurred."), + clEnumValN(NoDetailsOnlySummary, "summary", + "Display only a summary of the errors found."), + clEnumValN(OnlyDetailsNoSummary, "details", + "Display each error in detail but no summary."), + clEnumValN(BothDetailsAndSummary, "full", + "Display each error as well as a summary. [default]")), cat(DwarfDumpCategory)); static opt Quiet("quiet", desc("Use with -verify to not emit to STDOUT."), cat(DwarfDumpCategory)); @@ -334,8 +345,10 @@ static DIDumpOptions getDumpOpts(DWARFContext &C) { DumpOpts.RecoverableErrorHandler = C.getRecoverableErrorHandler(); // In -verify mode, print DIEs without children in error messages. if (Verify) { - DumpOpts.Verbose = !OnlyAggregateErrors; - DumpOpts.ShowAggregateErrors = !NoAggregateErrors; + DumpOpts.Verbose = ErrorDetails != NoDetailsOnlySummary && + ErrorDetails != NoDetailsOrSummary; + DumpOpts.ShowAggregateErrors = ErrorDetails != OnlyDetailsNoSummary && + ErrorDetails != NoDetailsOnlySummary; return DumpOpts.noImplicitRecursion(); } return DumpOpts; @@ -821,15 +834,8 @@ int main(int argc, char **argv) { "-verbose is currently not supported"; return 1; } - if (Verify && NoAggregateErrors && OnlyAggregateErrors) { - WithColor::error() << "incompatible arguments: specifying both " - " -no-aggregate-errors, and -only-aggregate-errors " - "is not supported"; - return 1; - } - if (!Verify && (NoAggregateErrors || OnlyAggregateErrors)) - WithColor::warning() << "-no-aggregate-errors and -only-aggregate-errors " - "have no effect without -verify"; + if (!Verify && ErrorDetails != Unspecified) + WithColor::warning() << "-error-detail has no affect without -verify"; std::error_code EC; ToolOutputFile OutputFile(OutputFilename, EC, sys::fs::OF_TextWithCRLF);