[lldb] Store *signed* ranges in lldb_private::Block

[labath]

This is to support functions whose entry points aren't their lowest address
(https://discourse.llvm.org/t/rfcish-support-for-discontinuous-functions/83244). The alternative is to keep blocks relative to the lowest address, but then introduce a separate concept for the function entry point, which I think would be more confusing.

This patch just changes the type signedness, it doesn't create any negative offsets yet. Since combining values with different signs can sometimes produce unexpected results, and since this is the first use of RangeVector with a signed type, I'm adding a test to verify that at least the core functionality works correctly.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

This is to support functions whose entry points aren't their lowest address
(https://discourse.llvm.org/t/rfcish-support-for-discontinuous-functions/83244). The alternative is to keep blocks relative to the lowest address, but then introduce a separate concept for the function entry point, which I think would be more confusing.

This patch just changes the type signedness, it doesn't create any negative offsets yet. Since combining values with different signs can sometimes produce unexpected results, and since this is the first use of RangeVector with a signed type, I'm adding a test to verify that at least the core functionality works correctly.

---

Full diff: https://github.com/llvm/llvm-project/pull/120224.diff

2 Files Affected:

• (modified) lldb/include/lldb/Symbol/Block.h (+1-1)
• (modified) lldb/unittests/Utility/RangeMapTest.cpp (+23)

diff --git a/lldb/include/lldb/Symbol/Block.h b/lldb/include/lldb/Symbol/Block.h
index 7c7a66de831998..d0063f132cc0ff 100644
--- a/lldb/include/lldb/Symbol/Block.h
+++ b/lldb/include/lldb/Symbol/Block.h
@@ -40,7 +40,7 @@ namespace lldb_private {
 /// blocks.
 class Block : public UserID, public SymbolContextScope {
 public:
-  typedef RangeVector<uint32_t, uint32_t, 1> RangeList;
+  typedef RangeVector<int32_t, uint32_t, 1> RangeList;
   typedef RangeList::Entry Range;
 
   // Creates a block representing the whole function. Only meant to be used from
diff --git a/lldb/unittests/Utility/RangeMapTest.cpp b/lldb/unittests/Utility/RangeMapTest.cpp
index 0b4c236062f20b..981fa2a7d1c34e 100644
--- a/lldb/unittests/Utility/RangeMapTest.cpp
+++ b/lldb/unittests/Utility/RangeMapTest.cpp
@@ -12,6 +12,29 @@
 
 using namespace lldb_private;
 
+TEST(RangeVector, SignedBaseType) {
+  using RangeVector = RangeVector<int32_t, uint32_t>;
+  using Entry = RangeVector::Entry;
+
+  RangeVector V;
+  V.Append(10, 5);
+  V.Append(-3, 6);
+  V.Append(-10, 3);
+  V.Sort();
+  EXPECT_THAT(V,
+              testing::ElementsAre(Entry(-10, 3), Entry(-3, 6), Entry(10, 5)));
+  Entry e = *V.begin();
+  EXPECT_EQ(e.GetRangeBase(), -10);
+  EXPECT_EQ(e.GetByteSize(), 3u);
+  EXPECT_EQ(e.GetRangeEnd(), -7);
+  EXPECT_TRUE(e.Contains(-10));
+  EXPECT_TRUE(e.Contains(-8));
+  EXPECT_FALSE(e.Contains(-7));
+  EXPECT_TRUE(e.Union(Entry(-8, 2)));
+  EXPECT_EQ(e, Entry(-10, 4));
+  EXPECT_EQ(e.Intersect(Entry(-7, 3)), Entry(-7, 1));
+}
+
 TEST(RangeVector, CombineConsecutiveRanges) {
   using RangeVector = RangeVector<uint32_t, uint32_t>;
   using Entry = RangeVector::Entry;

[DavidSpickett]

Fine assuming no one puts part of a function > 2GB before its entry point. Seems unlikely given that no one tried to put anything > 4GB after the entry point either.

LGTM.