[llvm] [llvm-dwarfdump] Make --verify for .debug_names multithreaded. (PR #127281)
Alexander Yermolovich via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 27 15:52:12 PST 2025
https://github.com/ayermolo updated https://github.com/llvm/llvm-project/pull/127281
>From 4b29a0487dd142d3979ad501564c1b8b8386e2e9 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Fri, 14 Feb 2025 15:51:53 -0800
Subject: [PATCH 1/6] [llvm-dwarf] Make --verify for .debug_names
multithreaded.
This PR makes verification of .debug_names acceleration table multithreaded.
In local testing it improves verification of clang .debug_names
from four minutes to under a minute.
This PR relies on a current mechanism of extracting DIEs into a vector.
Future improvements can include creating API to extract DIE at a time,
or grouping Entires into buckets by CUs and extracting before parallel
step.
Single Thread
4:12.37 real, 246.88 user, 3.54 sys, 0 amem,
10232004 mmem
0:49.40 real, 612.84 user, 515.73 sys, 0 amem, 11226292 mmem
---
.../DebugInfo/DWARF/DWARFAcceleratorTable.h | 11 +-
.../llvm/DebugInfo/DWARF/DWARFVerifier.h | 41 +--
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 288 +++++++++---------
.../X86/debug-names-verify-abbrev-forms.s | 2 +-
.../X86/debug-names-verify-completeness.s | 3 +-
.../X86/debug-names-verify-cu-lists.s | 2 +-
.../X86/debug-names-verify-entries.s | 2 +-
.../verify_split_dwarf_debug_names.test | 4 +-
.../verify_split_dwarf_debug_names_ftus.test | 4 +-
llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp | 8 +-
10 files changed, 194 insertions(+), 171 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 9d7ac12cefdc8..ccd8ac147a287 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -770,11 +770,12 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
}
public:
+ using size_type = size_t;
using iterator_category = std::input_iterator_tag;
using value_type = NameTableEntry;
using difference_type = uint32_t;
using pointer = NameTableEntry *;
- using reference = NameTableEntry; // We return entries by value.
+ using reference = NameTableEntry;
/// Creates an iterator whose initial position is name CurrentName in
/// CurrentIndex.
@@ -793,6 +794,14 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
next();
return I;
}
+ reference operator[](size_type idx) {
+ return CurrentIndex->getNameTableEntry(idx + 1);
+ }
+
+ difference_type operator-(const NameIterator &other) const {
+ assert(CurrentIndex == other.CurrentIndex);
+ return this->CurrentName - other.CurrentName;
+ }
friend bool operator==(const NameIterator &A, const NameIterator &B) {
return A.CurrentIndex == B.CurrentIndex && A.CurrentName == B.CurrentName;
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index 7b51bb63cd15b..27a790ac6bec4 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -16,6 +16,7 @@
#include "llvm/DebugInfo/DWARF/DWARFUnitIndex.h"
#include <cstdint>
#include <map>
+#include <mutex>
#include <set>
namespace llvm {
@@ -32,7 +33,9 @@ struct DWARFSection;
class OutputCategoryAggregator {
private:
+ std::mutex WriteMutex;
std::map<std::string, unsigned> Aggregation;
+ unsigned NumErrors = 0;
bool IncludeDetail;
public:
@@ -42,6 +45,8 @@ class OutputCategoryAggregator {
size_t GetNumCategories() const { return Aggregation.size(); }
void Report(StringRef s, std::function<void()> detailCallback);
void EnumerateResults(std::function<void(StringRef, unsigned)> handleCounts);
+ /// Return the number of errors that have been reported.
+ unsigned GetNumErrors() const { return NumErrors; }
};
/// A class that verifies DWARF debug information given a DWARF Context.
@@ -104,6 +109,7 @@ class DWARFVerifier {
bool IsObjectFile;
bool IsMachOObject;
using ReferenceMap = std::map<uint64_t, std::set<uint64_t>>;
+ std::mutex AccessMutex;
raw_ostream &error() const;
raw_ostream &warn() const;
@@ -264,21 +270,22 @@ class DWARFVerifier {
/// \param SectionName the name of the table we're verifying
///
/// \returns The number of errors occurred during verification
- unsigned verifyAppleAccelTable(const DWARFSection *AccelSection,
- DataExtractor *StrData,
- const char *SectionName);
-
- unsigned verifyDebugNamesCULists(const DWARFDebugNames &AccelTable);
- unsigned verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
- const DataExtractor &StrData);
- unsigned verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI);
- unsigned verifyNameIndexAttribute(const DWARFDebugNames::NameIndex &NI,
- const DWARFDebugNames::Abbrev &Abbr,
- DWARFDebugNames::AttributeEncoding AttrEnc);
- unsigned verifyNameIndexEntries(const DWARFDebugNames::NameIndex &NI,
- const DWARFDebugNames::NameTableEntry &NTE);
- unsigned verifyNameIndexCompleteness(const DWARFDie &Die,
- const DWARFDebugNames::NameIndex &NI);
+ void verifyAppleAccelTable(const DWARFSection *AccelSection,
+ DataExtractor *StrData, const char *SectionName);
+
+ void verifyDebugNamesCULists(const DWARFDebugNames &AccelTable);
+ void verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
+ const DataExtractor &StrData);
+ void verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI);
+ void verifyNameIndexAttribute(const DWARFDebugNames::NameIndex &NI,
+ const DWARFDebugNames::Abbrev &Abbr,
+ DWARFDebugNames::AttributeEncoding AttrEnc);
+ void verifyNameIndexEntries(
+ const DWARFDebugNames::NameIndex &NI,
+ const DWARFDebugNames::NameTableEntry &NTE,
+ const DenseMap<uint64_t, DWARFUnit *> &CUOffsetsToDUMap);
+ void verifyNameIndexCompleteness(const DWARFDie &Die,
+ const DWARFDebugNames::NameIndex &NI);
/// Verify that the DWARF v5 accelerator table is valid.
///
@@ -297,8 +304,8 @@ class DWARFVerifier {
/// \param StrData string section
///
/// \returns The number of errors occurred during verification
- unsigned verifyDebugNames(const DWARFSection &AccelSection,
- const DataExtractor &StrData);
+ void verifyDebugNames(const DWARFSection &AccelSection,
+ const DataExtractor &StrData);
public:
DWARFVerifier(raw_ostream &S, DWARFContext &D,
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 107e79cc5a05a..db7d159f492fc 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -33,10 +33,12 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/JSON.h"
+#include "llvm/Support/Parallel.h"
#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
#include <map>
#include <set>
+#include <unordered_set>
#include <vector>
using namespace llvm;
@@ -1106,10 +1108,9 @@ bool DWARFVerifier::handleDebugLine() {
return NumDebugLineErrors == 0;
}
-unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
- DataExtractor *StrData,
- const char *SectionName) {
- unsigned NumErrors = 0;
+void DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
+ DataExtractor *StrData,
+ const char *SectionName) {
DWARFDataExtractor AccelSectionData(DCtx.getDWARFObj(), *AccelSection,
DCtx.isLittleEndian(), 0);
AppleAcceleratorTable AccelTable(AccelSectionData, *StrData);
@@ -1121,7 +1122,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
ErrorCategory.Report("Section is too small to fit a section header", [&]() {
error() << "Section is too small to fit a section header.\n";
});
- return 1;
+ return;
}
// Verify that the section is not too short.
@@ -1129,7 +1130,7 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
std::string Msg = toString(std::move(E));
ErrorCategory.Report("Section is too small to fit a section header",
[&]() { error() << Msg << '\n'; });
- return 1;
+ return;
}
// Verify that all buckets have a valid hash index or are empty.
@@ -1147,7 +1148,6 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
error() << format("Bucket[%d] has invalid hash index: %u.\n", BucketIdx,
HashIdx);
});
- ++NumErrors;
}
}
uint32_t NumAtoms = AccelTable.getAtomsDesc().size();
@@ -1155,13 +1155,13 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
ErrorCategory.Report("No atoms", [&]() {
error() << "No atoms: failed to read HashData.\n";
});
- return 1;
+ return;
}
if (!AccelTable.validateForms()) {
ErrorCategory.Report("Unsupported form", [&]() {
error() << "Unsupported form: failed to read HashData.\n";
});
- return 1;
+ return;
}
for (uint32_t HashIdx = 0; HashIdx < NumHashes; ++HashIdx) {
@@ -1176,7 +1176,6 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
"0x%08" PRIx64 ".\n",
HashIdx, HashDataOffset);
});
- ++NumErrors;
}
uint64_t StrpOffset;
@@ -1207,8 +1206,6 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
SectionName, BucketIdx, HashIdx, Hash, StringCount, StrpOffset,
HashDataIdx, Offset, Name);
});
-
- ++NumErrors;
continue;
}
if ((Tag != dwarf::DW_TAG_null) && (Die.getTag() != Tag)) {
@@ -1218,74 +1215,70 @@ unsigned DWARFVerifier::verifyAppleAccelTable(const DWARFSection *AccelSection,
<< dwarf::TagString(Die.getTag()) << " of DIE["
<< HashDataIdx << "].\n";
});
- ++NumErrors;
}
}
- ++StringCount;
}
}
- return NumErrors;
}
-unsigned
-DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
+void DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
// A map from CU offset to the (first) Name Index offset which claims to index
// this CU.
DenseMap<uint64_t, uint64_t> CUMap;
- const uint64_t NotIndexed = std::numeric_limits<uint64_t>::max();
-
CUMap.reserve(DCtx.getNumCompileUnits());
+
+ std::unordered_set<uint64_t> CUOffsets;
for (const auto &CU : DCtx.compile_units())
- CUMap[CU->getOffset()] = NotIndexed;
+ CUOffsets.insert(CU->getOffset());
- unsigned NumErrors = 0;
- for (const DWARFDebugNames::NameIndex &NI : AccelTable) {
+ parallelForEach(AccelTable, [&](const DWARFDebugNames::NameIndex &NI) {
if (NI.getCUCount() == 0) {
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;
+ return;
}
for (uint32_t CU = 0, End = NI.getCUCount(); CU < End; ++CU) {
uint64_t Offset = NI.getCUOffset(CU);
- auto Iter = CUMap.find(Offset);
-
- if (Iter == CUMap.end()) {
+ if (!CUOffsets.count(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;
+ return;
}
-
- if (Iter->second != NotIndexed) {
+ uint64_t DuplicateCUOffset = 0;
+ {
+ std::lock_guard<std::mutex> Lock(AccessMutex);
+ auto Iter = CUMap.find(Offset);
+ if (Iter != CUMap.end())
+ DuplicateCUOffset = Iter->second;
+ else
+ CUMap[Offset] = NI.getUnitOffset();
+ }
+ if (DuplicateCUOffset) {
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);
+ NI.getUnitOffset(), Offset, DuplicateCUOffset);
});
- continue;
+ return;
}
- Iter->second = NI.getUnitOffset();
}
- }
+ });
- for (const auto &KV : CUMap) {
- if (KV.second == NotIndexed)
- warn() << formatv("CU @ {0:x} not covered by any Name Index\n", KV.first);
+ for (const auto &CU : DCtx.compile_units()) {
+ if (CUMap.count(CU->getOffset()) == 0)
+ warn() << formatv("CU @ {0:x} not covered by any Name Index\n",
+ CU->getOffset());
}
-
- return NumErrors;
}
-unsigned
-DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
- const DataExtractor &StrData) {
+void DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
+ const DataExtractor &StrData) {
struct BucketInfo {
uint32_t Bucket;
uint32_t Index;
@@ -1295,17 +1288,17 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
bool operator<(const BucketInfo &RHS) const { return Index < RHS.Index; }
};
- uint32_t NumErrors = 0;
if (NI.getBucketCount() == 0) {
warn() << formatv("Name Index @ {0:x} does not contain a hash table.\n",
NI.getUnitOffset());
- return NumErrors;
+ return;
}
// Build up a list of (Bucket, Index) pairs. We use this later to verify that
// each Name is reachable from the appropriate bucket.
std::vector<BucketInfo> BucketStarts;
BucketStarts.reserve(NI.getBucketCount() + 1);
+ const unsigned OrigNumberOfErrors = ErrorCategory.GetNumErrors();
for (uint32_t Bucket = 0, End = NI.getBucketCount(); Bucket < End; ++Bucket) {
uint32_t Index = NI.getBucketArrayEntry(Bucket);
if (Index > NI.getNameCount()) {
@@ -1315,7 +1308,6 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
Bucket, NI.getUnitOffset(), Index,
NI.getNameCount());
});
- ++NumErrors;
continue;
}
if (Index > 0)
@@ -1325,8 +1317,8 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
// If there were any buckets with invalid values, skip further checks as they
// will likely produce many errors which will only confuse the actual root
// problem.
- if (NumErrors > 0)
- return NumErrors;
+ if (OrigNumberOfErrors != ErrorCategory.GetNumErrors())
+ return;
// Sort the list in the order of increasing "Index" entries.
array_pod_sort(BucketStarts.begin(), BucketStarts.end());
@@ -1352,7 +1344,6 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
"are not covered by the hash table.\n",
NI.getUnitOffset(), NextUncovered, B.Index - 1);
});
- ++NumErrors;
}
uint32_t Idx = B.Index;
@@ -1374,7 +1365,6 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
NI.getUnitOffset(), B.Bucket, FirstHash,
FirstHash % NI.getBucketCount());
});
- ++NumErrors;
}
// This find the end of this bucket and also verifies that all the hashes in
@@ -1395,17 +1385,15 @@ DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
"the Name Index hash is {4:x}\n",
NI.getUnitOffset(), Str, Idx, caseFoldingDjbHash(Str), Hash);
});
- ++NumErrors;
}
-
++Idx;
}
NextUncovered = std::max(NextUncovered, Idx);
}
- return NumErrors;
+ return;
}
-unsigned DWARFVerifier::verifyNameIndexAttribute(
+void DWARFVerifier::verifyNameIndexAttribute(
const DWARFDebugNames::NameIndex &NI, const DWARFDebugNames::Abbrev &Abbr,
DWARFDebugNames::AttributeEncoding AttrEnc) {
StringRef FormName = dwarf::FormEncodingString(AttrEnc.Form);
@@ -1416,7 +1404,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
NI.getUnitOffset(), Abbr.Code, AttrEnc.Index,
AttrEnc.Form);
});
- return 1;
+ return;
}
if (AttrEnc.Index == DW_IDX_type_hash) {
@@ -1427,9 +1415,9 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
"uses an unexpected form {2} (should be {3}).\n",
NI.getUnitOffset(), Abbr.Code, AttrEnc.Form, dwarf::DW_FORM_data8);
});
- return 1;
+ return;
}
- return 0;
+ return;
}
if (AttrEnc.Index == dwarf::DW_IDX_parent) {
@@ -1443,9 +1431,9 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
"DW_FORM_ref4 or DW_FORM_flag_present).\n",
NI.getUnitOffset(), Abbr.Code, AttrEnc.Form);
});
- return 1;
+ return;
}
- return 0;
+ return;
}
// A list of known index attributes and their expected form classes.
@@ -1470,7 +1458,7 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
warn() << formatv("NameIndex @ {0:x}: Abbreviation {1:x} contains an "
"unknown index attribute: {2}.\n",
NI.getUnitOffset(), Abbr.Code, AttrEnc.Index);
- return 0;
+ return;
}
if (!DWARFFormValue(AttrEnc.Form).isFormClass(Iter->Class)) {
@@ -1480,14 +1468,13 @@ unsigned DWARFVerifier::verifyNameIndexAttribute(
NI.getUnitOffset(), Abbr.Code, AttrEnc.Index,
AttrEnc.Form, Iter->ClassName);
});
- return 1;
+ return;
}
- return 0;
+ return;
}
-unsigned
-DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
- unsigned NumErrors = 0;
+void DWARFVerifier::verifyNameIndexAbbrevs(
+ const DWARFDebugNames::NameIndex &NI) {
for (const auto &Abbrev : NI.getAbbrevs()) {
StringRef TagName = dwarf::TagString(Abbrev.Tag);
if (TagName.empty()) {
@@ -1505,10 +1492,9 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
"multiple {2} attributes.\n",
NI.getUnitOffset(), Abbrev.Code, AttrEnc.Index);
});
- ++NumErrors;
continue;
}
- NumErrors += verifyNameIndexAttribute(NI, Abbrev, AttrEnc);
+ verifyNameIndexAttribute(NI, Abbrev, AttrEnc);
}
if (NI.getCUCount() > 1 && !Attributes.count(dwarf::DW_IDX_compile_unit) &&
@@ -1519,7 +1505,6 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
"or DW_IDX_type_unit attribute.\n",
NI.getUnitOffset(), Abbrev.Code);
});
- ++NumErrors;
}
if (!Attributes.count(dwarf::DW_IDX_die_offset)) {
ErrorCategory.Report("Abbreviate in NameIndex missing attribute", [&]() {
@@ -1527,10 +1512,8 @@ DWARFVerifier::verifyNameIndexAbbrevs(const DWARFDebugNames::NameIndex &NI) {
"NameIndex @ {0:x}: Abbreviation {1:x} has no {2} attribute.\n",
NI.getUnitOffset(), Abbrev.Code, dwarf::DW_IDX_die_offset);
});
- ++NumErrors;
}
}
- return NumErrors;
}
static SmallVector<std::string, 3> getNames(const DWARFDie &DIE,
@@ -1571,9 +1554,10 @@ static SmallVector<std::string, 3> getNames(const DWARFDie &DIE,
return Result;
}
-unsigned DWARFVerifier::verifyNameIndexEntries(
+void DWARFVerifier::verifyNameIndexEntries(
const DWARFDebugNames::NameIndex &NI,
- const DWARFDebugNames::NameTableEntry &NTE) {
+ const DWARFDebugNames::NameTableEntry &NTE,
+ const DenseMap<uint64_t, DWARFUnit *> &CUOffsetsToDUMap) {
const char *CStr = NTE.getString();
if (!CStr) {
ErrorCategory.Report("Unable to get string associated with name", [&]() {
@@ -1581,11 +1565,9 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
"with name {1}.\n",
NI.getUnitOffset(), NTE.getIndex());
});
- return 1;
+ return;
}
StringRef Str(CStr);
-
- unsigned NumErrors = 0;
unsigned NumEntries = 0;
uint64_t EntryID = NTE.getEntryOffset();
uint64_t NextEntryID = EntryID;
@@ -1601,7 +1583,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
"invalid CU index ({2}).\n",
NI.getUnitOffset(), EntryID, *CUIndex);
});
- ++NumErrors;
continue;
}
const uint32_t NumLocalTUs = NI.getLocalTUCount();
@@ -1612,7 +1593,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
"invalid TU index ({2}).\n",
NI.getUnitOffset(), EntryID, *TUIndex);
});
- ++NumErrors;
continue;
}
std::optional<uint64_t> UnitOffset;
@@ -1640,7 +1620,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
"foreign TU index ({2}) with no CU index.\n",
NI.getUnitOffset(), EntryID, *TUIndex);
});
- ++NumErrors;
continue;
}
} else {
@@ -1668,7 +1647,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
"invalid CU or TU offset {2:x}.\n",
NI.getUnitOffset(), EntryID, *UnitOffset);
});
- ++NumErrors;
continue;
}
// This function will try to get the non skeleton unit DIE, but if it is
@@ -1682,9 +1660,15 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
// call to properly deal with it. It isn't clear that getNonSkeletonUnitDIE
// will return the unit DIE of DU if we aren't able to get the .dwo file,
// but that is what the function currently does.
+ DWARFUnit *NonSkeletonUnit = nullptr;
+ if (DU->getDWOId()) {
+ auto Iter = CUOffsetsToDUMap.find(DU->getOffset());
+ NonSkeletonUnit = Iter->second;
+ } else {
+ NonSkeletonUnit = DU;
+ }
DWARFDie UnitDie = DU->getUnitDIE();
- DWARFDie NonSkeletonUnitDie = DU->getNonSkeletonUnitDIE();
- if (DU->getDWOId() && UnitDie == NonSkeletonUnitDie) {
+ if (DU->getDWOId() && !NonSkeletonUnit->isDWOUnit()) {
ErrorCategory.Report("Unable to get load .dwo file", [&]() {
error() << formatv(
"Name Index @ {0:x}: Entry @ {1:x} unable to load "
@@ -1693,10 +1677,9 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
dwarf::toString(UnitDie.find({DW_AT_dwo_name, DW_AT_GNU_dwo_name})),
*UnitOffset);
});
- ++NumErrors;
continue;
}
- DWARFUnit *NonSkeletonUnit = nullptr;
+
if (TUIndex && *TUIndex >= NumLocalTUs) {
// We have a foreign TU index, which either means we have a .dwo file
// that has one or more type units, or we have a .dwp file with one or
@@ -1707,17 +1690,20 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
// above, so we know we have the right file.
const uint32_t ForeignTUIdx = *TUIndex - NumLocalTUs;
const uint64_t TypeSig = NI.getForeignTUSignature(ForeignTUIdx);
- llvm::DWARFContext &SkeletonDCtx =
- NonSkeletonUnitDie.getDwarfUnit()->getContext();
+ llvm::DWARFContext &NonSkeletonDCtx = NonSkeletonUnit->getContext();
// Now find the type unit from the type signature and then update the
// NonSkeletonUnitDie to point to the actual type unit in the .dwo/.dwp.
NonSkeletonUnit =
- SkeletonDCtx.getTypeUnitForHash(TypeSig, /*IsDWO=*/true);
- NonSkeletonUnitDie = NonSkeletonUnit->getUnitDIE(true);
+ NonSkeletonDCtx.getTypeUnitForHash(TypeSig, /*IsDWO=*/true);
+ DWARFDie NonSkeletonUnitDie = DWARFDie();
+ {
+ std::lock_guard<std::mutex> Lock(AccessMutex);
+ NonSkeletonUnitDie = NonSkeletonUnit->getUnitDIE(true);
+ }
// If we have foreign type unit in a DWP file, then we need to ignore
// any entries from type units that don't match the one that made it into
// the .dwp file.
- if (SkeletonDCtx.isDWP()) {
+ if (NonSkeletonDCtx.isDWP()) {
StringRef DUDwoName = dwarf::toStringRef(
UnitDie.find({DW_AT_dwo_name, DW_AT_GNU_dwo_name}));
StringRef TUDwoName = dwarf::toStringRef(
@@ -1725,8 +1711,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
if (DUDwoName != TUDwoName)
continue; // Skip this TU, it isn't the one in the .dwp file.
}
- } else {
- NonSkeletonUnit = NonSkeletonUnitDie.getDwarfUnit();
}
uint64_t DIEOffset =
NonSkeletonUnit->getOffset() + *EntryOr->getDIEUnitOffset();
@@ -1742,15 +1726,29 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
});
continue;
}
+ {
+ // TODO: Right now getDIEForOffset extracts everything into a DIE.
+ // Ideally we want another API that extracts only specfic DIE.
+ // Alternatively foreach NameIndex we can iterate over
+ // NameTableEntries/Entries, and put Entries into buckets based on CUs
+ // they access. Then extract all the DIEs for that CUs. For ThinLTO and
+ // BOLT there could be multiple CUs in NameIndex.
+ // This way we can remove the lock, and reduce memory foot print by
+ // releasing DIEs memory in CU after we are done.
+ std::lock_guard<std::mutex> Lock(AccessMutex);
+ if (Error E = NonSkeletonUnit->tryExtractDIEsIfNeeded(false)) {
+ std::string Msg = toString(std::move(E));
+ ErrorCategory.Report("Accelerator Table Error",
+ [&]() { error() << Msg << '\n'; });
+ }
+ }
DWARFDie DIE = NonSkeletonUnit->getDIEForOffset(DIEOffset);
-
if (!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);
});
- ++NumErrors;
continue;
}
// Only compare the DIE we found's DWARFUnit offset if the DIE lives in
@@ -1766,7 +1764,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
NI.getUnitOffset(), EntryID, DIEOffset, *UnitOffset,
DIE.getDwarfUnit()->getOffset());
});
- ++NumErrors;
}
if (DIE.getTag() != EntryOr->tag()) {
ErrorCategory.Report("Name Index contains mismatched Tag of DIE", [&]() {
@@ -1776,7 +1773,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
NI.getUnitOffset(), EntryID, DIEOffset, EntryOr->tag(),
DIE.getTag());
});
- ++NumErrors;
}
// We allow an extra name for functions: their name without any template
@@ -1792,7 +1788,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
NI.getUnitOffset(), EntryID, DIEOffset, Str,
make_range(EntryNames.begin(), EntryNames.end()));
});
- ++NumErrors;
}
}
handleAllErrors(
@@ -1806,7 +1801,6 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
"not associated with any entries.\n",
NI.getUnitOffset(), NTE.getIndex(), Str);
});
- ++NumErrors;
},
[&](const ErrorInfoBase &Info) {
ErrorCategory.Report("Uncategorized NameIndex error", [&]() {
@@ -1814,9 +1808,7 @@ unsigned DWARFVerifier::verifyNameIndexEntries(
NI.getUnitOffset(), NTE.getIndex(), Str,
Info.message());
});
- ++NumErrors;
});
- return NumErrors;
}
static bool isVariableIndexable(const DWARFDie &Die, DWARFContext &DCtx) {
@@ -1844,7 +1836,7 @@ static bool isVariableIndexable(const DWARFDie &Die, DWARFContext &DCtx) {
return false;
}
-unsigned DWARFVerifier::verifyNameIndexCompleteness(
+void DWARFVerifier::verifyNameIndexCompleteness(
const DWARFDie &Die, const DWARFDebugNames::NameIndex &NI) {
// First check, if the Die should be indexed. The code follows the DWARF v5
@@ -1853,7 +1845,7 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
// "All non-defining declarations (that is, debugging information entries
// with a DW_AT_declaration attribute) are excluded."
if (Die.find(DW_AT_declaration))
- return 0;
+ return;
// "DW_TAG_namespace debugging information entries without a DW_AT_name
// attribute are included with the name “(anonymous namespace)”.
@@ -1871,7 +1863,7 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
auto EntryNames = getNames(Die, IncludeStrippedTemplateNames,
IncludeObjCNames, IncludeLinkageName);
if (EntryNames.empty())
- return 0;
+ return;
// We deviate from the specification here, which says:
// "The name index must contain an entry for each debugging information entry
@@ -1882,7 +1874,7 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
// Compile units and modules have names but shouldn't be indexed.
case DW_TAG_compile_unit:
case DW_TAG_module:
- return 0;
+ return;
// Function and template parameters are not globally visible, so we shouldn't
// index them.
@@ -1891,22 +1883,22 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
case DW_TAG_template_type_parameter:
case DW_TAG_GNU_template_parameter_pack:
case DW_TAG_GNU_template_template_param:
- return 0;
+ return;
// Object members aren't globally visible.
case DW_TAG_member:
- return 0;
+ return;
// According to a strict reading of the specification, enumerators should not
// be indexed (and LLVM currently does not do that). However, this causes
// problems for the debuggers, so we may need to reconsider this.
case DW_TAG_enumerator:
- return 0;
+ return;
// Imported declarations should not be indexed according to the specification
// and LLVM currently does not do that.
case DW_TAG_imported_declaration:
- return 0;
+ return;
// "DW_TAG_subprogram, DW_TAG_inlined_subroutine, and DW_TAG_label debugging
// information entries without an address attribute (DW_AT_low_pc,
@@ -1917,7 +1909,7 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
if (Die.findRecursively(
{DW_AT_low_pc, DW_AT_high_pc, DW_AT_ranges, DW_AT_entry_pc}))
break;
- return 0;
+ return;
// "DW_TAG_variable debugging information entries with a DW_AT_location
// attribute that includes a DW_OP_addr or DW_OP_form_tls_address operator are
@@ -1927,7 +1919,7 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
case DW_TAG_variable:
if (isVariableIndexable(Die, DCtx))
break;
- return 0;
+ return;
default:
break;
@@ -1935,7 +1927,6 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
// Now we know that our Die should be present in the Index. Let's check if
// that's the case.
- unsigned NumErrors = 0;
uint64_t DieUnitOffset = Die.getOffset() - Die.getDwarfUnit()->getOffset();
for (StringRef Name : EntryNames) {
if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) {
@@ -1947,15 +1938,12 @@ unsigned DWARFVerifier::verifyNameIndexCompleteness(
"name {3} missing.\n",
NI.getUnitOffset(), Die.getOffset(), Die.getTag(), Name);
});
- ++NumErrors;
}
}
- return NumErrors;
}
-unsigned DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
- const DataExtractor &StrData) {
- unsigned NumErrors = 0;
+void DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
+ const DataExtractor &StrData) {
DWARFDataExtractor AccelSectionData(DCtx.getDWARFObj(), AccelSection,
DCtx.isLittleEndian(), 0);
DWARFDebugNames AccelTable(AccelSectionData, StrData);
@@ -1968,21 +1956,34 @@ unsigned DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
std::string Msg = toString(std::move(E));
ErrorCategory.Report("Accelerator Table Error",
[&]() { error() << Msg << '\n'; });
- return 1;
+ return;
}
-
- NumErrors += verifyDebugNamesCULists(AccelTable);
- for (const auto &NI : AccelTable)
- NumErrors += verifyNameIndexBuckets(NI, StrData);
+ const unsigned OriginalNumErrors = ErrorCategory.GetNumErrors();
+ verifyDebugNamesCULists(AccelTable);
for (const auto &NI : AccelTable)
- NumErrors += verifyNameIndexAbbrevs(NI);
+ verifyNameIndexBuckets(NI, StrData);
+ parallelForEach(AccelTable, [&](const DWARFDebugNames::NameIndex &NI) {
+ verifyNameIndexAbbrevs(NI);
+ });
// Don't attempt Entry validation if any of the previous checks found errors
- if (NumErrors > 0)
- return NumErrors;
- for (const auto &NI : AccelTable)
- for (const DWARFDebugNames::NameTableEntry &NTE : NI)
- NumErrors += verifyNameIndexEntries(NI, NTE);
+ if (OriginalNumErrors != ErrorCategory.GetNumErrors())
+ return;
+ DenseMap<uint64_t, DWARFUnit *> CUOffsetsToDUMap;
+ for (const auto &CU : DCtx.compile_units()) {
+ if (CU->getDWOId()) {
+ CUOffsetsToDUMap[CU->getOffset()] =
+ CU->getNonSkeletonUnitDIE().getDwarfUnit();
+ DWARFContext &Context =
+ CU->getNonSkeletonUnitDIE().getDwarfUnit()->getContext();
+ Context.getDWARFObj();
+ }
+ }
+ for (const DWARFDebugNames::NameIndex &NI : AccelTable) {
+ parallelForEach(NI, [&](DWARFDebugNames::NameTableEntry NTE) {
+ verifyNameIndexEntries(NI, NTE, CUOffsetsToDUMap);
+ });
+ }
for (const std::unique_ptr<DWARFUnit> &U : DCtx.info_section_units()) {
if (const DWARFDebugNames::NameIndex *NI =
@@ -1994,41 +1995,40 @@ unsigned DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
DWARFDie NonSkeletonUnitDie =
CUDie.getDwarfUnit()->getNonSkeletonUnitDIE(false);
if (CUDie != NonSkeletonUnitDie) {
- for (const DWARFDebugInfoEntry &Die :
- NonSkeletonUnitDie.getDwarfUnit()->dies())
- NumErrors += verifyNameIndexCompleteness(
- DWARFDie(NonSkeletonUnitDie.getDwarfUnit(), &Die), *NI);
+ parallelForEach(
+ NonSkeletonUnitDie.getDwarfUnit()->dies(),
+ [&](const DWARFDebugInfoEntry &Die) {
+ verifyNameIndexCompleteness(
+ DWARFDie(NonSkeletonUnitDie.getDwarfUnit(), &Die), *NI);
+ });
}
} else {
- for (const DWARFDebugInfoEntry &Die : CU->dies())
- NumErrors += verifyNameIndexCompleteness(DWARFDie(CU, &Die), *NI);
+ parallelForEach(CU->dies(), [&](const DWARFDebugInfoEntry &Die) {
+ verifyNameIndexCompleteness(DWARFDie(CU, &Die), *NI);
+ });
}
}
}
}
- return NumErrors;
+ return;
}
bool DWARFVerifier::handleAccelTables() {
const DWARFObject &D = DCtx.getDWARFObj();
DataExtractor StrData(D.getStrSection(), DCtx.isLittleEndian(), 0);
- unsigned NumErrors = 0;
if (!D.getAppleNamesSection().Data.empty())
- NumErrors += verifyAppleAccelTable(&D.getAppleNamesSection(), &StrData,
- ".apple_names");
+ verifyAppleAccelTable(&D.getAppleNamesSection(), &StrData, ".apple_names");
if (!D.getAppleTypesSection().Data.empty())
- NumErrors += verifyAppleAccelTable(&D.getAppleTypesSection(), &StrData,
- ".apple_types");
+ verifyAppleAccelTable(&D.getAppleTypesSection(), &StrData, ".apple_types");
if (!D.getAppleNamespacesSection().Data.empty())
- NumErrors += verifyAppleAccelTable(&D.getAppleNamespacesSection(), &StrData,
- ".apple_namespaces");
+ verifyAppleAccelTable(&D.getAppleNamespacesSection(), &StrData,
+ ".apple_namespaces");
if (!D.getAppleObjCSection().Data.empty())
- NumErrors += verifyAppleAccelTable(&D.getAppleObjCSection(), &StrData,
- ".apple_objc");
+ verifyAppleAccelTable(&D.getAppleObjCSection(), &StrData, ".apple_objc");
if (!D.getNamesSection().Data.empty())
- NumErrors += verifyDebugNames(D.getNamesSection(), StrData);
- return NumErrors == 0;
+ verifyDebugNames(D.getNamesSection(), StrData);
+ return ErrorCategory.GetNumErrors() == 0;
}
bool DWARFVerifier::handleDebugStrOffsets() {
@@ -2168,6 +2168,8 @@ bool DWARFVerifier::verifyDebugStrOffsets(
void OutputCategoryAggregator::Report(
StringRef s, std::function<void(void)> detailCallback) {
+ std::lock_guard<std::mutex> Lock(WriteMutex);
+ ++NumErrors;
Aggregation[std::string(s)]++;
if (IncludeDetail)
detailCallback();
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-abbrev-forms.s b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-abbrev-forms.s
index bc3dcfadc31bd..fd9fa9578db79 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-abbrev-forms.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-abbrev-forms.s
@@ -1,5 +1,5 @@
# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj | \
-# RUN: not llvm-dwarfdump -verify - | FileCheck %s
+# RUN: not llvm-dwarfdump -verify --verify-num-threads 1 - | FileCheck %s
# CHECK: error: NameIndex @ 0x0: Abbreviation 0x2: DW_IDX_compile_unit uses an unexpected form DW_FORM_ref1 (expected form class constant).
# CHECK: error: NameIndex @ 0x0: Abbreviation 0x2: DW_IDX_type_unit uses an unexpected form DW_FORM_ref1 (expected form class constant).
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-completeness.s b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-completeness.s
index b16f8658f87ec..585789cac7dc2 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-completeness.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-completeness.s
@@ -1,4 +1,4 @@
-# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj -o - | not llvm-dwarfdump -verify - | FileCheck %s
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj -o - | not llvm-dwarfdump -verify --verify-num-threads 1 - | FileCheck %s
# CHECK: error: Name Index @ 0x0: Entry for DIE @ {{.*}} (DW_TAG_namespace) with name namesp missing.
# CHECK: error: Name Index @ 0x0: Entry for DIE @ {{.*}} (DW_TAG_variable) with name var_block_addr missing.
@@ -177,4 +177,3 @@
.Lnames_abbrev_end0:
.Lnames_entries0:
.Lnames_end0:
-
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-cu-lists.s b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-cu-lists.s
index 809fa81b1e3af..092c3bf79c6fb 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-cu-lists.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-cu-lists.s
@@ -1,5 +1,5 @@
# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj | \
-# RUN: not llvm-dwarfdump -verify - | FileCheck %s
+# RUN: not llvm-dwarfdump -verify --verify-num-threads 1 - | FileCheck %s
# CHECK: error: Name Index @ 0x0 does not index any CU
# CHECK: error: Name Index @ 0x28 references a non-existing CU @ 0x1
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-entries.s b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-entries.s
index d447678e44610..b819ed40a42cf 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-entries.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/debug-names-verify-entries.s
@@ -1,4 +1,4 @@
-# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj -o - | not llvm-dwarfdump -verify - | FileCheck %s
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj -o - | not llvm-dwarfdump -verify --verify-num-threads 1 - | FileCheck %s
# CHECK: error: Name Index @ 0x0: Unable to get string associated with name 1.
# CHECK: error: Name Index @ 0x0: Entry @ 0x73 contains an invalid CU index (47).
diff --git a/llvm/test/tools/llvm-dwarfdump/verify_split_dwarf_debug_names.test b/llvm/test/tools/llvm-dwarfdump/verify_split_dwarf_debug_names.test
index 754ec4e7b87e4..48e7f3e3efe50 100644
--- a/llvm/test/tools/llvm-dwarfdump/verify_split_dwarf_debug_names.test
+++ b/llvm/test/tools/llvm-dwarfdump/verify_split_dwarf_debug_names.test
@@ -5,7 +5,7 @@
# RUN: yaml2obj %p/Inputs/verify_split_dwarf_debug_names_exe.yaml > %t1/a.out
# RUN: yaml2obj %p/Inputs/verify_split_dwarf_debug_names_main_dwo.yaml > %t1/main.dwo
# RUN: cd %t1
-# RUN: llvm-dwarfdump --verify %t1/a.out | FileCheck %s
+# RUN: llvm-dwarfdump --verify --verify-num-threads 1 %t1/a.out | FileCheck %s
# CHECK: Verifying unit: 1 / 1
# CHECK: Verifying dwo Units...
@@ -17,7 +17,7 @@
# Now verify if we remove the "main.dwo" file that we get an error letting us
# know that the .dwo file was not able to be found.
# RUN: rm %t1/main.dwo
-# RUN: not llvm-dwarfdump --verify %t1/a.out | FileCheck --check-prefix=NODWO %s
+# RUN: not llvm-dwarfdump --verify --verify-num-threads 1 %t1/a.out | FileCheck --check-prefix=NODWO %s
# NODWO: Verifying unit: 1 / 1
# NODWO: Verifying dwo Units...
diff --git a/llvm/test/tools/llvm-dwarfdump/verify_split_dwarf_debug_names_ftus.test b/llvm/test/tools/llvm-dwarfdump/verify_split_dwarf_debug_names_ftus.test
index 03ba63bd9b0d7..16d076a7a575d 100644
--- a/llvm/test/tools/llvm-dwarfdump/verify_split_dwarf_debug_names_ftus.test
+++ b/llvm/test/tools/llvm-dwarfdump/verify_split_dwarf_debug_names_ftus.test
@@ -5,7 +5,7 @@
# RUN: yaml2obj %p/Inputs/verify_split_dwarf_debug_names_ftus_exe.yaml > %t1/a.out
# RUN: yaml2obj %p/Inputs/verify_split_dwarf_debug_names_ftus_dwo.yaml > %t1/main.dwo
# RUN: cd %t1
-# RUN: llvm-dwarfdump --verify %t1/a.out | FileCheck %s
+# RUN: llvm-dwarfdump --verify --verify-num-threads 1 %t1/a.out | FileCheck %s
# CHECK: Verifying unit: 1 / 1
# CHECK: Verifying dwo Units...
@@ -17,7 +17,7 @@
# Now verify if we remove the "main.dwo" file that we get an error letting us
# know that the .dwo file was not able to be found.
# RUN: rm %t1/main.dwo
-# RUN: not llvm-dwarfdump --verify %t1/a.out | FileCheck --check-prefix=NODWO %s
+# RUN: not llvm-dwarfdump --verify --verify-num-threads 1 %t1/a.out | FileCheck --check-prefix=NODWO %s
# NODWO: Verifying unit: 1 / 1
# NODWO: Verifying dwo Units...
diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
index d63c51566e80c..b3cf61e313630 100644
--- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
+++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
@@ -30,9 +30,11 @@
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Parallel.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
#include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/Threading.h"
#include "llvm/Support/ToolOutputFile.h"
#include "llvm/Support/WithColor.h"
#include "llvm/Support/raw_ostream.h"
@@ -284,6 +286,9 @@ static cl::opt<bool>
cat(DwarfDumpCategory));
static opt<bool> Verify("verify", desc("Verify the DWARF debug info."),
cat(DwarfDumpCategory));
+static opt<unsigned> VerifyNumThreads(
+ "verify-num-threads", init(hardware_concurrency().compute_thread_count()),
+ desc("Number of threads to use for --verify."), cat(DwarfDumpCategory));
static opt<ErrorDetailLevel> ErrorDetails(
"error-display", init(Unspecified),
desc("Set the level of detail and summary to display when verifying "
@@ -775,7 +780,7 @@ static bool handleBuffer(StringRef Filename, MemoryBufferRef Buffer,
if (filterArch(*Obj)) {
std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(
*Obj, DWARFContext::ProcessDebugRelocations::Process, nullptr, "",
- RecoverableErrorHandler);
+ RecoverableErrorHandler, WithColor::defaultWarningHandler, true);
DICtx->setParseCUTUIndexManually(ManuallyGenerateUnitIndex);
if (!HandleObj(*Obj, *DICtx, Filename, OS))
Result = false;
@@ -903,6 +908,7 @@ int main(int argc, char **argv) {
bool Success = true;
if (Verify) {
+ parallel::strategy = hardware_concurrency(VerifyNumThreads);
for (StringRef Object : Objects)
Success &= handleFile(Object, verifyObjectFile, OutputFile.os());
} else if (Statistics) {
>From 31c5a8b543db61eaa155728b40ea01163238ab12 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Mon, 24 Feb 2025 17:26:24 -0800
Subject: [PATCH 2/6] addressed comments, fixed tsan issues
---
.../llvm/DebugInfo/DWARF/DWARFVerifier.h | 11 ++++++--
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 25 ++++++++++---------
llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp | 3 ++-
3 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index 27a790ac6bec4..b559f3c095023 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -35,7 +35,7 @@ class OutputCategoryAggregator {
private:
std::mutex WriteMutex;
std::map<std::string, unsigned> Aggregation;
- unsigned NumErrors = 0;
+ uint64_t NumErrors = 0;
bool IncludeDetail;
public:
@@ -46,7 +46,7 @@ class OutputCategoryAggregator {
void Report(StringRef s, std::function<void()> detailCallback);
void EnumerateResults(std::function<void(StringRef, unsigned)> handleCounts);
/// Return the number of errors that have been reported.
- unsigned GetNumErrors() const { return NumErrors; }
+ uint64_t GetNumErrors() const { return NumErrors; }
};
/// A class that verifies DWARF debug information given a DWARF Context.
@@ -307,6 +307,13 @@ class DWARFVerifier {
void verifyDebugNames(const DWARFSection &AccelSection,
const DataExtractor &StrData);
+ /// Constructs a full name for a DIE. Potentially it does recursive lookup on
+ /// DIEs. This can lead to extraction of DIEs in a different CU or TU.
+ SmallVector<std::string, 3> getNames(const DWARFDie &DIE,
+ bool IncludeStrippedTemplateNames,
+ bool IncludeObjCNames = true,
+ bool IncludeLinkageName = true);
+
public:
DWARFVerifier(raw_ostream &S, DWARFContext &D,
DIDumpOptions DumpOpts = DIDumpOptions::getForSingleDIE());
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index db7d159f492fc..e4c582ef81214 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1298,7 +1298,7 @@ void DWARFVerifier::verifyNameIndexBuckets(const DWARFDebugNames::NameIndex &NI,
// each Name is reachable from the appropriate bucket.
std::vector<BucketInfo> BucketStarts;
BucketStarts.reserve(NI.getBucketCount() + 1);
- const unsigned OrigNumberOfErrors = ErrorCategory.GetNumErrors();
+ const uint64_t OrigNumberOfErrors = ErrorCategory.GetNumErrors();
for (uint32_t Bucket = 0, End = NI.getBucketCount(); Bucket < End; ++Bucket) {
uint32_t Index = NI.getBucketArrayEntry(Bucket);
if (Index > NI.getNameCount()) {
@@ -1516,12 +1516,16 @@ void DWARFVerifier::verifyNameIndexAbbrevs(
}
}
-static SmallVector<std::string, 3> getNames(const DWARFDie &DIE,
- bool IncludeStrippedTemplateNames,
- bool IncludeObjCNames = true,
- bool IncludeLinkageName = true) {
+SmallVector<std::string, 3>
+DWARFVerifier::getNames(const DWARFDie &DIE, bool IncludeStrippedTemplateNames,
+ bool IncludeObjCNames, bool IncludeLinkageName) {
SmallVector<std::string, 3> Result;
- if (const char *Str = DIE.getShortName()) {
+ const char *Str = nullptr;
+ {
+ std::lock_guard<std::mutex> Lock(AccessMutex);
+ Str = DIE.getShortName();
+ }
+ if (Str) {
StringRef Name(Str);
Result.emplace_back(Name);
if (IncludeStrippedTemplateNames) {
@@ -1695,15 +1699,12 @@ void DWARFVerifier::verifyNameIndexEntries(
// NonSkeletonUnitDie to point to the actual type unit in the .dwo/.dwp.
NonSkeletonUnit =
NonSkeletonDCtx.getTypeUnitForHash(TypeSig, /*IsDWO=*/true);
- DWARFDie NonSkeletonUnitDie = DWARFDie();
- {
- std::lock_guard<std::mutex> Lock(AccessMutex);
- NonSkeletonUnitDie = NonSkeletonUnit->getUnitDIE(true);
- }
// If we have foreign type unit in a DWP file, then we need to ignore
// any entries from type units that don't match the one that made it into
// the .dwp file.
if (NonSkeletonDCtx.isDWP()) {
+ std::lock_guard<std::mutex> Lock(AccessMutex);
+ DWARFDie NonSkeletonUnitDie = NonSkeletonUnit->getUnitDIE(true);
StringRef DUDwoName = dwarf::toStringRef(
UnitDie.find({DW_AT_dwo_name, DW_AT_GNU_dwo_name}));
StringRef TUDwoName = dwarf::toStringRef(
@@ -1958,7 +1959,7 @@ void DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
[&]() { error() << Msg << '\n'; });
return;
}
- const unsigned OriginalNumErrors = ErrorCategory.GetNumErrors();
+ const uint64_t OriginalNumErrors = ErrorCategory.GetNumErrors();
verifyDebugNamesCULists(AccelTable);
for (const auto &NI : AccelTable)
verifyNameIndexBuckets(NI, StrData);
diff --git a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
index b3cf61e313630..f38430f622261 100644
--- a/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
+++ b/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp
@@ -780,7 +780,8 @@ static bool handleBuffer(StringRef Filename, MemoryBufferRef Buffer,
if (filterArch(*Obj)) {
std::unique_ptr<DWARFContext> DICtx = DWARFContext::create(
*Obj, DWARFContext::ProcessDebugRelocations::Process, nullptr, "",
- RecoverableErrorHandler, WithColor::defaultWarningHandler, true);
+ RecoverableErrorHandler, WithColor::defaultWarningHandler,
+ /*ThreadSafe=*/true);
DICtx->setParseCUTUIndexManually(ManuallyGenerateUnitIndex);
if (!HandleObj(*Obj, *DICtx, Filename, OS))
Result = false;
>From b04395b3d4f7565a19cc75aaec0c4732b0309b7b Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Wed, 26 Feb 2025 20:40:17 -0800
Subject: [PATCH 3/6] fixed performance regression, removed locks, refactored
how it checks if die should be in NameIndex. For latter it was hitting
pathological case with bolted binaries. Making it 7x slower.
---
.../llvm/DebugInfo/DWARF/DWARFVerifier.h | 13 +-
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 144 +++++++++++++-----
2 files changed, 106 insertions(+), 51 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
index b559f3c095023..f304e195f2f55 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFVerifier.h
@@ -9,6 +9,7 @@
#ifndef LLVM_DEBUGINFO_DWARF_DWARFVERIFIER_H
#define LLVM_DEBUGINFO_DWARF_DWARFVERIFIER_H
+#include "llvm/ADT/StringMap.h"
#include "llvm/DebugInfo/DIContext.h"
#include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
@@ -284,8 +285,9 @@ class DWARFVerifier {
const DWARFDebugNames::NameIndex &NI,
const DWARFDebugNames::NameTableEntry &NTE,
const DenseMap<uint64_t, DWARFUnit *> &CUOffsetsToDUMap);
- void verifyNameIndexCompleteness(const DWARFDie &Die,
- const DWARFDebugNames::NameIndex &NI);
+ void verifyNameIndexCompleteness(
+ const DWARFDie &Die, const DWARFDebugNames::NameIndex &NI,
+ const StringMap<DenseSet<uint64_t>> &NamesToDieOffsets);
/// Verify that the DWARF v5 accelerator table is valid.
///
@@ -307,13 +309,6 @@ class DWARFVerifier {
void verifyDebugNames(const DWARFSection &AccelSection,
const DataExtractor &StrData);
- /// Constructs a full name for a DIE. Potentially it does recursive lookup on
- /// DIEs. This can lead to extraction of DIEs in a different CU or TU.
- SmallVector<std::string, 3> getNames(const DWARFDie &DIE,
- bool IncludeStrippedTemplateNames,
- bool IncludeObjCNames = true,
- bool IncludeLinkageName = true);
-
public:
DWARFVerifier(raw_ostream &S, DWARFContext &D,
DIDumpOptions DumpOpts = DIDumpOptions::getForSingleDIE());
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index e4c582ef81214..e0ce3ec7da16c 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1516,15 +1516,14 @@ void DWARFVerifier::verifyNameIndexAbbrevs(
}
}
-SmallVector<std::string, 3>
-DWARFVerifier::getNames(const DWARFDie &DIE, bool IncludeStrippedTemplateNames,
- bool IncludeObjCNames, bool IncludeLinkageName) {
+/// Constructs a full name for a DIE. Potentially it does recursive lookup on
+/// DIEs. This can lead to extraction of DIEs in a different CU or TU.
+static SmallVector<std::string, 3> getNames(const DWARFDie &DIE,
+ bool IncludeStrippedTemplateNames,
+ bool IncludeObjCNames = true,
+ bool IncludeLinkageName = true) {
SmallVector<std::string, 3> Result;
- const char *Str = nullptr;
- {
- std::lock_guard<std::mutex> Lock(AccessMutex);
- Str = DIE.getShortName();
- }
+ const char *Str = DIE.getShortName();
if (Str) {
StringRef Name(Str);
Result.emplace_back(Name);
@@ -1703,7 +1702,6 @@ void DWARFVerifier::verifyNameIndexEntries(
// any entries from type units that don't match the one that made it into
// the .dwp file.
if (NonSkeletonDCtx.isDWP()) {
- std::lock_guard<std::mutex> Lock(AccessMutex);
DWARFDie NonSkeletonUnitDie = NonSkeletonUnit->getUnitDIE(true);
StringRef DUDwoName = dwarf::toStringRef(
UnitDie.find({DW_AT_dwo_name, DW_AT_GNU_dwo_name}));
@@ -1727,22 +1725,6 @@ void DWARFVerifier::verifyNameIndexEntries(
});
continue;
}
- {
- // TODO: Right now getDIEForOffset extracts everything into a DIE.
- // Ideally we want another API that extracts only specfic DIE.
- // Alternatively foreach NameIndex we can iterate over
- // NameTableEntries/Entries, and put Entries into buckets based on CUs
- // they access. Then extract all the DIEs for that CUs. For ThinLTO and
- // BOLT there could be multiple CUs in NameIndex.
- // This way we can remove the lock, and reduce memory foot print by
- // releasing DIEs memory in CU after we are done.
- std::lock_guard<std::mutex> Lock(AccessMutex);
- if (Error E = NonSkeletonUnit->tryExtractDIEsIfNeeded(false)) {
- std::string Msg = toString(std::move(E));
- ErrorCategory.Report("Accelerator Table Error",
- [&]() { error() << Msg << '\n'; });
- }
- }
DWARFDie DIE = NonSkeletonUnit->getDIEForOffset(DIEOffset);
if (!DIE) {
ErrorCategory.Report("NameIndex references nonexistent DIE", [&]() {
@@ -1838,7 +1820,8 @@ static bool isVariableIndexable(const DWARFDie &Die, DWARFContext &DCtx) {
}
void DWARFVerifier::verifyNameIndexCompleteness(
- const DWARFDie &Die, const DWARFDebugNames::NameIndex &NI) {
+ const DWARFDie &Die, const DWARFDebugNames::NameIndex &NI,
+ const StringMap<DenseSet<uint64_t>> &NamesToDieOffsets) {
// First check, if the Die should be indexed. The code follows the DWARF v5
// wording as closely as possible.
@@ -1930,9 +1913,8 @@ void DWARFVerifier::verifyNameIndexCompleteness(
// that's the case.
uint64_t DieUnitOffset = Die.getOffset() - Die.getDwarfUnit()->getOffset();
for (StringRef Name : EntryNames) {
- if (none_of(NI.equal_range(Name), [&](const DWARFDebugNames::Entry &E) {
- return E.getDIEUnitOffset() == DieUnitOffset;
- })) {
+ auto iter = NamesToDieOffsets.find(Name);
+ if (iter == NamesToDieOffsets.end() || !iter->second.count(DieUnitOffset)) {
ErrorCategory.Report("Name Index DIE entry missing name", [&]() {
error() << formatv(
"Name Index @ {0:x}: Entry for DIE @ {1:x} ({2}) with "
@@ -1943,6 +1925,45 @@ void DWARFVerifier::verifyNameIndexCompleteness(
}
}
+/// Extracts all the data for CU/TUs so we can access it in parallel without
+/// locks.
+static void extractCUsTus(DWARFContext &DCtx) {
+ // Abbrev DeclSet is shared beween the units.
+ for (auto &CUTU : DCtx.normal_units()) {
+ if (CUTU->getVersion() < 5)
+ continue;
+ CUTU->getUnitDIE();
+ CUTU->getBaseAddress();
+ }
+ parallelForEach(DCtx.normal_units(), [&](const auto &CUTU) {
+ if (CUTU->getVersion() < 5)
+ return;
+ if (Error E = CUTU->tryExtractDIEsIfNeeded(false))
+ DCtx.getRecoverableErrorHandler()(std::move(E));
+ });
+
+ // Invoking getNonSkeletonUnitDIE() sets up all the base pointers for DWO
+ // Units. This is needed for getBaseAddress().
+ for (const auto &CU : DCtx.compile_units()) {
+ if (!(CU->getVersion() >= 5 && CU->getDWOId()))
+ continue;
+ DWARFUnit *NonSkeletonUnit = CU->getNonSkeletonUnitDIE().getDwarfUnit();
+ DWARFContext &NonSkeletonContext = NonSkeletonUnit->getContext();
+ // Iterates over CUs and TUs.
+ for (auto &CUTU : NonSkeletonContext.dwo_units()) {
+ CUTU->getUnitDIE();
+ CUTU->getBaseAddress();
+ }
+ parallelForEach(NonSkeletonContext.dwo_units(), [&](const auto &CUTU) {
+ if (Error E = CUTU->tryExtractDIEsIfNeeded(false))
+ DCtx.getRecoverableErrorHandler()(std::move(E));
+ });
+ // If context is for DWP we only need to extract once.
+ if (NonSkeletonContext.isDWP())
+ break;
+ }
+}
+
void DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
const DataExtractor &StrData) {
DWARFDataExtractor AccelSectionData(DCtx.getDWARFObj(), AccelSection,
@@ -1972,24 +1993,61 @@ void DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
return;
DenseMap<uint64_t, DWARFUnit *> CUOffsetsToDUMap;
for (const auto &CU : DCtx.compile_units()) {
- if (CU->getDWOId()) {
- CUOffsetsToDUMap[CU->getOffset()] =
- CU->getNonSkeletonUnitDIE().getDwarfUnit();
- DWARFContext &Context =
- CU->getNonSkeletonUnitDIE().getDwarfUnit()->getContext();
- Context.getDWARFObj();
- }
+ if (!(CU->getVersion() >= 5 && CU->getDWOId()))
+ continue;
+ CUOffsetsToDUMap[CU->getOffset()] =
+ CU->getNonSkeletonUnitDIE().getDwarfUnit();
}
+ extractCUsTus(DCtx);
for (const DWARFDebugNames::NameIndex &NI : AccelTable) {
parallelForEach(NI, [&](DWARFDebugNames::NameTableEntry NTE) {
verifyNameIndexEntries(NI, NTE, CUOffsetsToDUMap);
});
}
- for (const std::unique_ptr<DWARFUnit> &U : DCtx.info_section_units()) {
- if (const DWARFDebugNames::NameIndex *NI =
- AccelTable.getCUOrTUNameIndex(U->getOffset())) {
- DWARFCompileUnit *CU = dyn_cast<DWARFCompileUnit>(U.get());
+ auto populateNameToOffset =
+ [&](const DWARFDebugNames::NameIndex &NI,
+ StringMap<DenseSet<uint64_t>> &NamesToDieOffsets) {
+ for (DWARFDebugNames::NameTableEntry NTE : NI) {
+ const std::string Name = NTE.getString();
+ uint64_t EntryID = NTE.getEntryOffset();
+ Expected<DWARFDebugNames::Entry> EntryOr = NI.getEntry(&EntryID);
+ auto Iter = NamesToDieOffsets.insert({Name, DenseSet<uint64_t>(3)});
+ for (; EntryOr; EntryOr = NI.getEntry(&EntryID)) {
+ if (std::optional<uint64_t> DieOffset = EntryOr->getDIEUnitOffset())
+ Iter.first->second.insert(*DieOffset);
+ }
+ handleAllErrors(
+ EntryOr.takeError(),
+ [&](const DWARFDebugNames::SentinelError &) {
+ if (!NamesToDieOffsets.empty())
+ return;
+ 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(), Name);
+ });
+ },
+ [&](const ErrorInfoBase &Info) {
+ ErrorCategory.Report("Uncategorized NameIndex error", [&]() {
+ error() << formatv(
+ "Name Index @ {0:x}: Name {1} ({2}): {3}\n",
+ NI.getUnitOffset(), NTE.getIndex(), Name, Info.message());
+ });
+ });
+ }
+ };
+ // NameIndex can have multiple CUs. For example if it was created by BOLT.
+ // So better to iterate over NI, and then over CUs in it.
+ for (const DWARFDebugNames::NameIndex &NI : AccelTable) {
+ StringMap<DenseSet<uint64_t>> NamesToDieOffsets(NI.getNameCount());
+ populateNameToOffset(NI, NamesToDieOffsets);
+ for (uint32_t i = 0, iEnd = NI.getCUCount(); i < iEnd; ++i) {
+ const uint64_t CUOffset = NI.getCUOffset(i);
+ DWARFUnit *U = DCtx.getUnitForOffset(CUOffset);
+ DWARFCompileUnit *CU = dyn_cast<DWARFCompileUnit>(U);
if (CU) {
if (CU->getDWOId()) {
DWARFDie CUDie = CU->getUnitDIE(true);
@@ -2000,12 +2058,14 @@ void DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
NonSkeletonUnitDie.getDwarfUnit()->dies(),
[&](const DWARFDebugInfoEntry &Die) {
verifyNameIndexCompleteness(
- DWARFDie(NonSkeletonUnitDie.getDwarfUnit(), &Die), *NI);
+ DWARFDie(NonSkeletonUnitDie.getDwarfUnit(), &Die), NI,
+ NamesToDieOffsets);
});
}
} else {
parallelForEach(CU->dies(), [&](const DWARFDebugInfoEntry &Die) {
- verifyNameIndexCompleteness(DWARFDie(CU, &Die), *NI);
+ verifyNameIndexCompleteness(DWARFDie(CU, &Die), NI,
+ NamesToDieOffsets);
});
}
}
>From 5ceb8df48a8f13e7569f1828304e0b77fa62d4a4 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Thu, 27 Feb 2025 14:29:45 -0800
Subject: [PATCH 4/6] fixed handling of corrupt name table, and handling errors
in verifyDebugNamesCULists. Latter had a small bug that used return. This
meant depending on which NI was processed first, it might not get to all the
errors.
---
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index e0ce3ec7da16c..8fa662af6b822 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1247,7 +1247,7 @@ void DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
"Name Index @ {0:x} references a non-existing CU @ {1:x}\n",
NI.getUnitOffset(), Offset);
});
- return;
+ continue;
}
uint64_t DuplicateCUOffset = 0;
{
@@ -1265,7 +1265,7 @@ void DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
"this CU is already indexed by Name Index @ {2:x}\n",
NI.getUnitOffset(), Offset, DuplicateCUOffset);
});
- return;
+ continue;
}
}
});
@@ -2008,8 +2008,9 @@ void DWARFVerifier::verifyDebugNames(const DWARFSection &AccelSection,
auto populateNameToOffset =
[&](const DWARFDebugNames::NameIndex &NI,
StringMap<DenseSet<uint64_t>> &NamesToDieOffsets) {
- for (DWARFDebugNames::NameTableEntry NTE : NI) {
- const std::string Name = NTE.getString();
+ for (const DWARFDebugNames::NameTableEntry &NTE : NI) {
+ const char *tName = NTE.getString();
+ const std::string Name = tName ? std::string(tName) : "";
uint64_t EntryID = NTE.getEntryOffset();
Expected<DWARFDebugNames::Entry> EntryOr = NI.getEntry(&EntryID);
auto Iter = NamesToDieOffsets.insert({Name, DenseSet<uint64_t>(3)});
>From 386cee74d7dd501f581987411e53c765ed877865 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Thu, 27 Feb 2025 15:38:59 -0800
Subject: [PATCH 5/6] minor nits, made a test single threaded. I think
technically speaking it's not legal
---
.../llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h | 2 +-
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 10 ++++------
.../tools/llvm-dwarfdump/X86/verify_no_linkage_name.s | 4 +++-
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index ccd8ac147a287..2687d133aeb40 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -775,7 +775,7 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
using value_type = NameTableEntry;
using difference_type = uint32_t;
using pointer = NameTableEntry *;
- using reference = NameTableEntry;
+ using reference = NameTableEntry; // We return entries by value.
/// Creates an iterator whose initial position is name CurrentName in
/// CurrentIndex.
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index 8fa662af6b822..c7d8e97c9f015 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -38,7 +38,6 @@
#include "llvm/Support/raw_ostream.h"
#include <map>
#include <set>
-#include <unordered_set>
#include <vector>
using namespace llvm;
@@ -1227,7 +1226,7 @@ void DWARFVerifier::verifyDebugNamesCULists(const DWARFDebugNames &AccelTable) {
DenseMap<uint64_t, uint64_t> CUMap;
CUMap.reserve(DCtx.getNumCompileUnits());
- std::unordered_set<uint64_t> CUOffsets;
+ DenseSet<uint64_t> CUOffsets;
for (const auto &CU : DCtx.compile_units())
CUOffsets.insert(CU->getOffset());
@@ -1523,8 +1522,7 @@ static SmallVector<std::string, 3> getNames(const DWARFDie &DIE,
bool IncludeObjCNames = true,
bool IncludeLinkageName = true) {
SmallVector<std::string, 3> Result;
- const char *Str = DIE.getShortName();
- if (Str) {
+ if (const char *Str = DIE.getShortName()) {
StringRef Name(Str);
Result.emplace_back(Name);
if (IncludeStrippedTemplateNames) {
@@ -1947,8 +1945,8 @@ static void extractCUsTus(DWARFContext &DCtx) {
for (const auto &CU : DCtx.compile_units()) {
if (!(CU->getVersion() >= 5 && CU->getDWOId()))
continue;
- DWARFUnit *NonSkeletonUnit = CU->getNonSkeletonUnitDIE().getDwarfUnit();
- DWARFContext &NonSkeletonContext = NonSkeletonUnit->getContext();
+ DWARFContext &NonSkeletonContext =
+ CU->getNonSkeletonUnitDIE().getDwarfUnit()->getContext();
// Iterates over CUs and TUs.
for (auto &CUTU : NonSkeletonContext.dwo_units()) {
CUTU->getUnitDIE();
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s b/llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s
index a7a0ae7727ed1..ebc14eb305749 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s
@@ -1,6 +1,8 @@
# This test generates a DW_TAG_structure_type with a linkage name. This linkage
# name will not be part of the accelerator table and the verifier should not
# complain about this.
+# Technically speaking this test is invalid because .debug_names is a DWARF 5 feature.
+# In this test the accelerator table is referencing DWARF4.
#
# DW_TAG_structure_type
# DW_AT_name ("C")
@@ -9,7 +11,7 @@
# RUN: llvm-mc %s -filetype obj -triple x86_64-unknown-linux-gnu -o %t.o
# RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s
# RUN: llvm-dwarfdump -debug-names %t.o | FileCheck %s --check-prefix ACCEL
-# RUN: llvm-dwarfdump -verify -debug-names %t.o
+# RUN: llvm-dwarfdump -verify --verify-num-threads 1 -debug-names %t.o
# CHECK: DW_AT_name ("Bool")
# CHECK-NEXT: DW_AT_linkage_name ("$SSbD")
>From 880510ea05b49f54164ced3e9c66f9cd9d508b70 Mon Sep 17 00:00:00 2001
From: Alexander Yermolovich <ayermolo at meta.com>
Date: Thu, 27 Feb 2025 15:51:56 -0800
Subject: [PATCH 6/6] removed check for DWARF5, it will pre-load DWARF4 also.
---
llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp | 6 +-----
llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s | 4 +---
2 files changed, 2 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
index c7d8e97c9f015..40658bc5178cd 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
@@ -1928,14 +1928,10 @@ void DWARFVerifier::verifyNameIndexCompleteness(
static void extractCUsTus(DWARFContext &DCtx) {
// Abbrev DeclSet is shared beween the units.
for (auto &CUTU : DCtx.normal_units()) {
- if (CUTU->getVersion() < 5)
- continue;
CUTU->getUnitDIE();
CUTU->getBaseAddress();
}
parallelForEach(DCtx.normal_units(), [&](const auto &CUTU) {
- if (CUTU->getVersion() < 5)
- return;
if (Error E = CUTU->tryExtractDIEsIfNeeded(false))
DCtx.getRecoverableErrorHandler()(std::move(E));
});
@@ -1943,7 +1939,7 @@ static void extractCUsTus(DWARFContext &DCtx) {
// Invoking getNonSkeletonUnitDIE() sets up all the base pointers for DWO
// Units. This is needed for getBaseAddress().
for (const auto &CU : DCtx.compile_units()) {
- if (!(CU->getVersion() >= 5 && CU->getDWOId()))
+ if (!CU->getDWOId())
continue;
DWARFContext &NonSkeletonContext =
CU->getNonSkeletonUnitDIE().getDwarfUnit()->getContext();
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s b/llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s
index ebc14eb305749..a7a0ae7727ed1 100644
--- a/llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s
+++ b/llvm/test/tools/llvm-dwarfdump/X86/verify_no_linkage_name.s
@@ -1,8 +1,6 @@
# This test generates a DW_TAG_structure_type with a linkage name. This linkage
# name will not be part of the accelerator table and the verifier should not
# complain about this.
-# Technically speaking this test is invalid because .debug_names is a DWARF 5 feature.
-# In this test the accelerator table is referencing DWARF4.
#
# DW_TAG_structure_type
# DW_AT_name ("C")
@@ -11,7 +9,7 @@
# RUN: llvm-mc %s -filetype obj -triple x86_64-unknown-linux-gnu -o %t.o
# RUN: llvm-dwarfdump -debug-info %t.o | FileCheck %s
# RUN: llvm-dwarfdump -debug-names %t.o | FileCheck %s --check-prefix ACCEL
-# RUN: llvm-dwarfdump -verify --verify-num-threads 1 -debug-names %t.o
+# RUN: llvm-dwarfdump -verify -debug-names %t.o
# CHECK: DW_AT_name ("Bool")
# CHECK-NEXT: DW_AT_linkage_name ("$SSbD")
More information about the llvm-commits
mailing list