[Lldb-commits] [lldb] [lldb] Fix RangeDataVector::CombineConsecutiveEntriesWithEqualData (PR #127059)
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Wed Feb 19 03:37:42 PST 2025
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/127059
>From 450371c5fd3ecce3ec54237379fef751fba45751 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Thu, 13 Feb 2025 14:08:25 +0100
Subject: [PATCH 1/2] [lldb] Fix
RangeDataVector::CombineConsecutiveEntriesWithEqualData
Function was merging equal data even if they weren't adjecant. This
caused a problem in command-disassemble.s test because the two ranges
describing the function would be merged and "swallow" the function
between them.
This PR copies/adapts the algorithm from
RangeVector::CombineConsecutiveEntries (which does not have the same
problem) and also adds a call to ComputeUpperBounds as moving entries
around invalidates the binary tree. (The lack of this call wasn't
noticed until now either because we were not calling methods which rely
on upper bounds (right now, it's only the ill-named FindEntryIndexes
method), or because we weren't merging anything.
---
lldb/include/lldb/Utility/RangeMap.h | 47 ++++++++-----------
.../test/Shell/Commands/command-disassemble.s | 3 +-
lldb/unittests/Utility/RangeMapTest.cpp | 21 +++++++++
3 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/lldb/include/lldb/Utility/RangeMap.h b/lldb/include/lldb/Utility/RangeMap.h
index 433466eebced8..2bebe74cc4cfe 100644
--- a/lldb/include/lldb/Utility/RangeMap.h
+++ b/lldb/include/lldb/Utility/RangeMap.h
@@ -493,36 +493,27 @@ class RangeDataVector {
#ifdef ASSERT_RANGEMAP_ARE_SORTED
assert(IsSorted());
#endif
- typename Collection::iterator pos;
- typename Collection::iterator end;
- typename Collection::iterator prev;
- bool can_combine = false;
- // First we determine if we can combine any of the Entry objects so we
- // don't end up allocating and making a new collection for no reason
- for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos != end;
- prev = pos++) {
- if (prev != end && prev->data == pos->data) {
- can_combine = true;
- break;
- }
- }
+ auto first_intersect = std::adjacent_find(
+ m_entries.begin(), m_entries.end(), [](const Entry &a, const Entry &b) {
+ return a.DoesAdjoinOrIntersect(b) && a.data == b.data;
+ });
+ if (first_intersect == m_entries.end())
+ return;
- // We can combine at least one entry, then we make a new collection and
- // populate it accordingly, and then swap it into place.
- if (can_combine) {
- Collection minimal_ranges;
- for (pos = m_entries.begin(), end = m_entries.end(), prev = end;
- pos != end; prev = pos++) {
- if (prev != end && prev->data == pos->data)
- minimal_ranges.back().SetRangeEnd(pos->GetRangeEnd());
- else
- minimal_ranges.push_back(*pos);
- }
- // Use the swap technique in case our new vector is much smaller. We must
- // swap when using the STL because std::vector objects never release or
- // reduce the memory once it has been allocated/reserved.
- m_entries.swap(minimal_ranges);
+ // We can combine at least one entry. Make a new collection and populate it
+ // accordingly, and then swap it into place.
+ auto pos = std::next(first_intersect);
+ Collection minimal_ranges(m_entries.begin(), pos);
+ for (; pos != m_entries.end(); ++pos) {
+ Entry &back = minimal_ranges.back();
+ if (back.DoesAdjoinOrIntersect(*pos) && back.data == pos->data)
+ back.SetRangeEnd(std::max(back.GetRangeEnd(), pos->GetRangeEnd()));
+ else
+ minimal_ranges.push_back(*pos);
}
+ m_entries.swap(minimal_ranges);
+ if (!m_entries.empty())
+ ComputeUpperBounds(0, m_entries.size());
}
void Clear() { m_entries.clear(); }
diff --git a/lldb/test/Shell/Commands/command-disassemble.s b/lldb/test/Shell/Commands/command-disassemble.s
index eb84a9ce39d4a..14f416d221231 100644
--- a/lldb/test/Shell/Commands/command-disassemble.s
+++ b/lldb/test/Shell/Commands/command-disassemble.s
@@ -94,8 +94,7 @@
# CHECK-EMPTY:
# CHECK-NEXT: command-disassemble.s.tmp`n2::case3:
# CHECK-NEXT: command-disassemble.s.tmp[0x9046] <+0>: jmp 0x6046 ; <-12288>
-## FIXME: This should resolve to `middle_of_case3`
-# CHECK-NEXT: command-disassemble.s.tmp[0x904b] <+5>: jmp 0x7046 ; n2::case3 - 8192
+# CHECK-NEXT: command-disassemble.s.tmp[0x904b] <+5>: jmp 0x7046 ; middle_of_case3
# CHECK-NEXT: command-disassemble.s.tmp[0x9050] <+10>: int $0x2a
# CHECK-EMPTY:
# CHECK-NEXT: command-disassemble.s.tmp`n1::case3:
diff --git a/lldb/unittests/Utility/RangeMapTest.cpp b/lldb/unittests/Utility/RangeMapTest.cpp
index 981fa2a7d1c34..2022a2374fb8d 100644
--- a/lldb/unittests/Utility/RangeMapTest.cpp
+++ b/lldb/unittests/Utility/RangeMapTest.cpp
@@ -238,3 +238,24 @@ TEST(RangeDataVector, FindEntryIndexesThatContain_Overlap) {
EXPECT_THAT(FindEntryIndexes(39, Map), testing::ElementsAre(10));
EXPECT_THAT(FindEntryIndexes(40, Map), testing::ElementsAre());
}
+
+TEST(RangeDataVector, CombineConsecutiveEntriesWithEqualData) {
+ RangeDataVectorT Map;
+ Map.Append(EntryT(0, 10, 47));
+ Map.Append(EntryT(10, 10, 47));
+ Map.Sort();
+ Map.CombineConsecutiveEntriesWithEqualData();
+ EXPECT_THAT(FindEntryIndexes(5, Map), testing::ElementsAre(47));
+ EXPECT_THAT(FindEntryIndexes(15, Map), testing::ElementsAre(47));
+ EXPECT_THAT(FindEntryIndexes(25, Map), testing::ElementsAre());
+
+ Map.Clear();
+ Map.Append(EntryT(0, 10, 47));
+ Map.Append(EntryT(20, 10, 47));
+ Map.Sort();
+ Map.CombineConsecutiveEntriesWithEqualData();
+ EXPECT_THAT(FindEntryIndexes(5, Map), testing::ElementsAre(47));
+ EXPECT_THAT(FindEntryIndexes(15, Map), testing::ElementsAre());
+ EXPECT_THAT(FindEntryIndexes(25, Map), testing::ElementsAre(47));
+ EXPECT_THAT(FindEntryIndexes(35, Map), testing::ElementsAre());
+}
>From a698b191276021722bdfeb8900ecb0a6c9d46ee8 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Wed, 19 Feb 2025 12:37:16 +0100
Subject: [PATCH 2/2] add empty line, remove check
---
lldb/include/lldb/Utility/RangeMap.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/Utility/RangeMap.h b/lldb/include/lldb/Utility/RangeMap.h
index 2bebe74cc4cfe..8af690e813c4a 100644
--- a/lldb/include/lldb/Utility/RangeMap.h
+++ b/lldb/include/lldb/Utility/RangeMap.h
@@ -497,6 +497,7 @@ class RangeDataVector {
m_entries.begin(), m_entries.end(), [](const Entry &a, const Entry &b) {
return a.DoesAdjoinOrIntersect(b) && a.data == b.data;
});
+
if (first_intersect == m_entries.end())
return;
@@ -512,8 +513,7 @@ class RangeDataVector {
minimal_ranges.push_back(*pos);
}
m_entries.swap(minimal_ranges);
- if (!m_entries.empty())
- ComputeUpperBounds(0, m_entries.size());
+ ComputeUpperBounds(0, m_entries.size());
}
void Clear() { m_entries.clear(); }
More information about the lldb-commits
mailing list