Add unit tests for VMRange

[labath]

The tests seem fine, but as a matter of style I would suggest two changes:

• replace ASSERT_XXX with EXPECT_XXX: EXPECT macros will not terminate the test, so you'll be able to see additional failures, which is helpful in pinpointing the problem. ASSERT is good for cases where the subsequent checks make no sense or will crash if the previous check is not satisfied (but that is not the case for your checks AFAICT).
• replace ASSERT_TRUE(foo == bar) with ASSERT_EQ(foo, bar) (and similar for other relational operators). Right not that does not matter much, but if someone later implements operator<< for this class, you will automatically get a helpful error message describing the objects instead of a useless "false is not true" message.

[teemperor]

• ASSERT_ -> EXPECT_
• Now using gtest's comparison macros where possible.
• Added a PrintTo function as otherwise the gtest comparison macros won't compile.

[clayborg]

unittests/Utility/VMRangeTest.cpp

42 ↗

(On Diff #155899)

Seems like a few more edge cases might be good:

EXPECT_NE(range1, VMRange(0x100, 0x1ff));
EXPECT_NE(range1, VMRange(0x100, 0x201));
EXPECT_NE(range1, VMRange(0x0ff, 0x200));
EXPECT_NE(range1, VMRange(0x101 0x200));

71 ↗

(On Diff #155899)

Probably don't need the 0xFE as 0xFF will test the condition of an address before 0x100

78 ↗

(On Diff #155899)

Should probably test the UINT64_MAX address instead of just 0xfff as the last edge case

88 ↗

(On Diff #155899)

add a zero sized range check where the address is inside the other:

EXPECT_FALSE(range.Contains(VMRange(0x100, 0x100)));

91 ↗

(On Diff #155899)

Add a test to see that an equal range is also contained:

EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200));

92 ↗

(On Diff #155899)

change 0x208 to 0x201 so it is just outside the range

92 ↗

(On Diff #155899)

Add check for MAX address:

EXPECT_FALSE(range.Contains(VMRange(0x105, UINT64_MAX)));

103 ↗

(On Diff #155899)

Test ordering of empty ranges? Not sure if that makes sense

[labath]

In D49415#1165179, @teemperor wrote:

• Added a PrintTo function as otherwise the gtest comparison macros won't compile.

Sorry, I did not anticipate that. It looks like the iterator typedef inside the VMRange is confusing gtest's universal printing logic (normally it does not require a type to be printable and it will just print a hex dump instead). In any case, having the PrintTo function is a good thing, so thanks for adding it.

[teemperor]

• Applied Greg's requested changes (thank you very much).

[teemperor]

Merging this in as Davide suggested.

[lemo]

The new test cases did catch a real bug:

[ RUN ] VMRange.CollectionContains
llvm/src/tools/lldb/unittests/Utility/VMRangeTest.cpp:146: Failure
Value of: VMRange::ContainsRange(collection, VMRange(0x100, 0x104))

Actual: false

Expected: true

class RangeInRangeUnaryPredicate {
public:
RangeInRangeUnaryPredicate(VMRange range) : _range(range) {} // note that
_range binds to a temporary!
bool operator()(const VMRange &range) const {
return range.Contains(_range);
}
const VMRange &_range;
};

I just sent out a review for the fix (https://reviews.llvm.org/D50290)

lldb/trunk/unittests/Utility/CMakeLists.txt

add_lldb_unittest(UtilityTests
   StringExtractorTest.cpp
   StructuredDataTest.cpp
   TildeExpressionResolverTest.cpp
   TimeoutTest.cpp
   TimerTest.cpp
   UriParserTest.cpp
   UUIDTest.cpp
   VASprintfTest.cpp
+  VMRangeTest.cpp
 
   LINK_LIBS
       lldbUtility
       lldbUtilityHelpers
   LINK_COMPONENTS
     Support
   )
 
 add_unittest_inputs(UtilityTests
   StructuredData-basic.json
   )

lldb/trunk/unittests/Utility/VMRangeTest.cpp

+//===-- VMRangeTest.cpp -----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include <limits>
+
+#include "lldb/Utility/VMRange.h"
+
+using namespace lldb_private;
+
+namespace lldb_private {
+void PrintTo(const VMRange &v, std::ostream *os) {
+  (*os) << "VMRange(" << v.GetBaseAddress() << ", " << v.GetEndAddress() << ")";
+}
+} // namespace lldb_private
+
+TEST(VMRange, IsValid) {
+  VMRange range;
+  EXPECT_FALSE(range.IsValid());
+
+  range.Reset(0x1, 0x100);
+  EXPECT_TRUE(range.IsValid());
+
+  range.Reset(0x1, 0x1);
+  EXPECT_FALSE(range.IsValid());
+}
+
+TEST(VMRange, Clear) {
+  VMRange range(0x100, 0x200);
+  EXPECT_NE(VMRange(), range);
+  range.Clear();
+  EXPECT_EQ(VMRange(), range);
+}
+
+TEST(VMRange, Comparison) {
+  VMRange range1(0x100, 0x200);
+  VMRange range2(0x100, 0x200);
+  EXPECT_EQ(range1, range2);
+
+  EXPECT_NE(VMRange(0x100, 0x1ff), range1);
+  EXPECT_NE(VMRange(0x100, 0x201), range1);
+  EXPECT_NE(VMRange(0x0ff, 0x200), range1);
+  EXPECT_NE(VMRange(0x101, 0x200), range1);
+
+  range2.Clear();
+  EXPECT_NE(range1, range2);
+}
+
+TEST(VMRange, Reset) {
+  VMRange range(0x100, 0x200);
+  EXPECT_FALSE(VMRange(0x200, 0x200) == range);
+  range.Reset(0x200, 0x200);
+  EXPECT_TRUE(VMRange(0x200, 0x200) == range);
+}
+
+TEST(VMRange, SetEndAddress) {
+  VMRange range(0x100, 0x200);
+
+  range.SetEndAddress(0xFF);
+  EXPECT_EQ(0U, range.GetByteSize());
+  EXPECT_FALSE(range.IsValid());
+
+  range.SetEndAddress(0x101);
+  EXPECT_EQ(1U, range.GetByteSize());
+  EXPECT_TRUE(range.IsValid());
+}
+
+TEST(VMRange, ContainsAddr) {
+  VMRange range(0x100, 0x200);
+
+  EXPECT_FALSE(range.Contains(0x00));
+  EXPECT_FALSE(range.Contains(0xFF));
+  EXPECT_TRUE(range.Contains(0x100));
+  EXPECT_TRUE(range.Contains(0x101));
+  EXPECT_TRUE(range.Contains(0x1FF));
+  EXPECT_FALSE(range.Contains(0x200));
+  EXPECT_FALSE(range.Contains(0x201));
+  EXPECT_FALSE(range.Contains(0xFFF));
+  EXPECT_FALSE(range.Contains(std::numeric_limits<lldb::addr_t>::max()));
+}
+
+TEST(VMRange, ContainsRange) {
+  VMRange range(0x100, 0x200);
+
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x100)));
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x101)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x105)));
+  EXPECT_TRUE(range.Contains(VMRange(0x101, 0x105)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x1FF)));
+  EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200)));
+  EXPECT_FALSE(range.Contains(VMRange(0x105, 0x201)));
+  EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200)));
+  EXPECT_FALSE(
+      range.Contains(VMRange(0x105, std::numeric_limits<lldb::addr_t>::max())));
+
+  // Empty range.
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x100)));
+
+  range.Clear();
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+}
+
+TEST(VMRange, Ordering) {
+  VMRange range1(0x44, 0x200);
+  VMRange range2(0x100, 0x1FF);
+  VMRange range3(0x100, 0x200);
+
+  EXPECT_LE(range1, range1);
+  EXPECT_GE(range1, range1);
+
+  EXPECT_LT(range1, range2);
+  EXPECT_LT(range2, range3);
+
+  EXPECT_GT(range2, range1);
+  EXPECT_GT(range3, range2);
+
+  // Ensure that < and > are always false when comparing ranges with themselves.
+  EXPECT_FALSE(range1 < range1);
+  EXPECT_FALSE(range2 < range2);
+  EXPECT_FALSE(range3 < range3);
+
+  EXPECT_FALSE(range1 > range1);
+  EXPECT_FALSE(range2 > range2);
+  EXPECT_FALSE(range3 > range3);
+}
+
+TEST(VMRange, CollectionContains) {
+  VMRange::collection collection = {VMRange(0x100, 0x105),
+                                    VMRange(0x108, 0x110)};
+
+  EXPECT_FALSE(VMRange::ContainsValue(collection, 0xFF));
+  EXPECT_TRUE(VMRange::ContainsValue(collection, 0x100));
+  EXPECT_FALSE(VMRange::ContainsValue(collection, 0x105));
+  EXPECT_TRUE(VMRange::ContainsValue(collection, 0x109));
+
+  EXPECT_TRUE(VMRange::ContainsRange(collection, VMRange(0x100, 0x104)));
+  EXPECT_TRUE(VMRange::ContainsRange(collection, VMRange(0x108, 0x100)));
+  EXPECT_FALSE(VMRange::ContainsRange(collection, VMRange(0xFF, 0x100)));
+
+  // TODO: Implement and test ContainsRange with values that span multiple
+  // ranges in the collection.
+}