[llvm] Fix __apple_XXX iterator that iterates over all entries. (PR #157538)
Greg Clayton via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 9 10:47:58 PDT 2025
https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/157538
>From 6e775b1908261a06cf590b19c14e8915e87be278 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Mon, 8 Sep 2025 13:01:12 -0700
Subject: [PATCH 1/2] Fix __apple_XXX iterator that iterates over all entries.
The previous iterator for __apple_XXX sections was assuming that all entries in the table would be contiguous and it wasn't using the offsets table to access each chain of entries for a given name. This patch fixes it so the iterator does the right thing.
This issue became apparent after a modification to strip template names from DW_AT_name entries to allow adding both the template class base name as an entry and also include the name with template names. The commit hash is 2e7ee4dc21430b0fe4c9ee306dc1d8c7986a6646. The problem is if the name starts with a "<" it will try and split the name. So if the name is "<get-size>" it will return an empty string as the function name, and this empty string gets added to the __apple_names table and causes large delays when using the iterators.
---
.../DebugInfo/DWARF/DWARFAcceleratorTable.h | 36 +++++++++++++------
.../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 24 ++++++++-----
2 files changed, 41 insertions(+), 19 deletions(-)
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 87586eda90682..1fda6b8664283 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -50,7 +50,6 @@ class LLVM_ABI DWARFAcceleratorTable {
Entry &operator=(Entry &&) = default;
~Entry() = default;
-
public:
/// Returns the Offset of the Compilation Unit associated with this
/// Accelerator Entry or std::nullopt if the Compilation Unit offset is not
@@ -153,7 +152,11 @@ class LLVM_ABI AppleAcceleratorTable : public DWARFAcceleratorTable {
uint64_t getHashBase() const { return getBucketBase() + getNumBuckets() * 4; }
/// Return the offset into the section where the I-th hash is.
- uint64_t getIthHashBase(uint32_t I) const { return getHashBase() + I * 4; }
+ std::optional<uint64_t> getIthHashBase(uint32_t I) const {
+ if (I < Hdr.HashCount)
+ return getHashBase() + I * 4;
+ return std::nullopt;
+ }
/// Return the offset into the section where the offset list begins.
uint64_t getOffsetBase() const { return getHashBase() + getNumHashes() * 4; }
@@ -164,8 +167,10 @@ class LLVM_ABI AppleAcceleratorTable : public DWARFAcceleratorTable {
}
/// Return the offset into the section where the I-th offset is.
- uint64_t getIthOffsetBase(uint32_t I) const {
- return getOffsetBase() + I * 4;
+ std::optional<uint64_t> getIthOffsetBase(uint32_t I) const {
+ if (I < Hdr.HashCount)
+ return getOffsetBase() + I * 4;
+ return std::nullopt;
}
/// Returns the index of the bucket where a hypothetical Hash would be.
@@ -188,14 +193,22 @@ class LLVM_ABI AppleAcceleratorTable : public DWARFAcceleratorTable {
/// Reads the I-th hash in the hash list.
std::optional<uint32_t> readIthHash(uint32_t I) const {
- uint64_t Offset = getIthHashBase(I);
- return readU32FromAccel(Offset);
+ std::optional<uint64_t> OptOffset = getIthHashBase(I);
+ if (OptOffset) {
+ uint64_t Offset = *OptOffset;
+ return readU32FromAccel(Offset);
+ }
+ return std::nullopt;
}
/// Reads the I-th offset in the offset list.
std::optional<uint32_t> readIthOffset(uint32_t I) const {
- uint64_t Offset = getIthOffsetBase(I);
- return readU32FromAccel(Offset);
+ std::optional<uint64_t> OptOffset = getIthOffsetBase(I);
+ if (OptOffset) {
+ uint64_t Offset = *OptOffset;
+ return readU32FromAccel(Offset);
+ }
+ return std::nullopt;
}
/// Reads a string offset from the accelerator table at Offset, which is
@@ -282,6 +295,7 @@ class LLVM_ABI AppleAcceleratorTable : public DWARFAcceleratorTable {
constexpr static auto EndMarker = std::numeric_limits<uint64_t>::max();
EntryWithName Current;
+ uint32_t OffsetIdx = 0;
uint64_t Offset = EndMarker;
uint32_t NumEntriesToCome = 0;
@@ -423,7 +437,7 @@ class LLVM_ABI DWARFDebugNames : public DWARFAcceleratorTable {
struct Abbrev {
uint64_t AbbrevOffset; /// < Abbreviation offset in the .debug_names section
uint32_t Code; ///< Abbreviation code
- dwarf::Tag Tag; ///< Dwarf Tag of the described entity.
+ dwarf::Tag Tag; ///< Dwarf Tag of the described entity.
std::vector<AttributeEncoding> Attributes; ///< List of index attributes.
Abbrev(uint32_t Code, dwarf::Tag Tag, uint64_t AbbrevOffset,
@@ -712,8 +726,8 @@ class LLVM_ABI DWARFDebugNames : public DWARFAcceleratorTable {
bool IsLocal;
std::optional<Entry> CurrentEntry;
- uint64_t DataOffset = 0; ///< Offset into the section.
- std::string Key; ///< The Key we are searching for.
+ uint64_t DataOffset = 0; ///< Offset into the section.
+ std::string Key; ///< The Key we are searching for.
std::optional<uint32_t> Hash; ///< Hash of Key, if it has been computed.
bool getEntryAtCurrentOffset();
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index ea336378bebb3..463455b5d344b 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -248,8 +248,8 @@ LLVM_DUMP_METHOD void AppleAcceleratorTable::dump(raw_ostream &OS) const {
}
for (unsigned HashIdx = Index; HashIdx < Hdr.HashCount; ++HashIdx) {
- uint64_t HashOffset = HashesBase + HashIdx*4;
- uint64_t OffsetsOffset = OffsetsBase + HashIdx*4;
+ uint64_t HashOffset = HashesBase + HashIdx * 4;
+ uint64_t OffsetsOffset = OffsetsBase + HashIdx * 4;
uint32_t Hash = AccelSection.getU32(&HashOffset);
if (Hash % Hdr.BucketCount != Bucket)
@@ -321,7 +321,14 @@ void AppleAcceleratorTable::Iterator::prepareNextEntryOrEnd() {
}
void AppleAcceleratorTable::Iterator::prepareNextStringOrEnd() {
- std::optional<uint32_t> StrOffset = getTable().readStringOffsetAt(Offset);
+ const AppleAcceleratorTable &Table = getTable();
+ // Always start looking for strings using a valid offset from the Offsets
+ // table. Entries are not always consecutive.
+ std::optional<uint64_t> OptOffset = Table.readIthOffset(OffsetIdx++);
+ if (!OptOffset)
+ return setToEnd();
+ Offset = *OptOffset;
+ std::optional<uint32_t> StrOffset = Table.readStringOffsetAt(Offset);
if (!StrOffset)
return setToEnd();
@@ -331,7 +338,7 @@ void AppleAcceleratorTable::Iterator::prepareNextStringOrEnd() {
return prepareNextStringOrEnd();
Current.StrOffset = *StrOffset;
- std::optional<uint32_t> MaybeNumEntries = getTable().readU32FromAccel(Offset);
+ std::optional<uint32_t> MaybeNumEntries = Table.readU32FromAccel(Offset);
if (!MaybeNumEntries || *MaybeNumEntries == 0)
return setToEnd();
NumEntriesToCome = *MaybeNumEntries;
@@ -339,7 +346,7 @@ void AppleAcceleratorTable::Iterator::prepareNextStringOrEnd() {
AppleAcceleratorTable::Iterator::Iterator(const AppleAcceleratorTable &Table,
bool SetEnd)
- : Current(Table), Offset(Table.getEntriesBase()), NumEntriesToCome(0) {
+ : Current(Table), Offset(0), NumEntriesToCome(0) {
if (SetEnd)
setToEnd();
else
@@ -443,7 +450,7 @@ void DWARFDebugNames::Header::dump(ScopedPrinter &W) const {
}
Error DWARFDebugNames::Header::extract(const DWARFDataExtractor &AS,
- uint64_t *Offset) {
+ uint64_t *Offset) {
auto HeaderError = [Offset = *Offset](Error E) {
return createStringError(errc::illegal_byte_sequence,
"parsing .debug_names header at 0x%" PRIx64 ": %s",
@@ -830,8 +837,9 @@ bool DWARFDebugNames::NameIndex::dumpEntry(ScopedPrinter &W,
uint64_t EntryId = *Offset;
auto EntryOr = getEntry(Offset);
if (!EntryOr) {
- handleAllErrors(EntryOr.takeError(), [](const SentinelError &) {},
- [&W](const ErrorInfoBase &EI) { EI.log(W.startLine()); });
+ handleAllErrors(
+ EntryOr.takeError(), [](const SentinelError &) {},
+ [&W](const ErrorInfoBase &EI) { EI.log(W.startLine()); });
return false;
}
>From 4d40921c0d243a770c0a6112c416f190d4d73376 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Tue, 9 Sep 2025 10:46:48 -0700
Subject: [PATCH 2/2] Fix a test suite error.
The first version of this PR broke collision iteration. Fixed now.
---
.../DebugInfo/DWARF/DWARFAcceleratorTable.cpp | 22 +++++++++++--------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 463455b5d344b..558a059a65952 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -322,20 +322,24 @@ void AppleAcceleratorTable::Iterator::prepareNextEntryOrEnd() {
void AppleAcceleratorTable::Iterator::prepareNextStringOrEnd() {
const AppleAcceleratorTable &Table = getTable();
- // Always start looking for strings using a valid offset from the Offsets
- // table. Entries are not always consecutive.
- std::optional<uint64_t> OptOffset = Table.readIthOffset(OffsetIdx++);
- if (!OptOffset)
- return setToEnd();
- Offset = *OptOffset;
+ if (Offset == 0) {
+ // Always start looking for strings using a valid offset from the Offsets
+ // table. Entries are not always consecutive.
+ std::optional<uint64_t> OptOffset = Table.readIthOffset(OffsetIdx++);
+ if (!OptOffset)
+ return setToEnd();
+ Offset = *OptOffset;
+ }
std::optional<uint32_t> StrOffset = Table.readStringOffsetAt(Offset);
if (!StrOffset)
return setToEnd();
- // A zero denotes the end of the collision list. Read the next string
- // again.
- if (*StrOffset == 0)
+ // A zero denotes the end of the collision list. Skip to the next offset
+ // in the offsets table.
+ if (*StrOffset == 0) {
+ Offset = 0;
return prepareNextStringOrEnd();
+ }
Current.StrOffset = *StrOffset;
std::optional<uint32_t> MaybeNumEntries = Table.readU32FromAccel(Offset);
More information about the llvm-commits
mailing list