Extend ContainedRangeMap and StaticContainedRangeMap

This adds a new mode in ContainedRangeMap which allows existance of
equal ranges. Among those equal ranges, the most recently added range is
the innermost range.

This also adds a function to ContainedRangeMap and
StaticContainedRangeMap to allow users get a vector of entries that
contains given address from innermost to outermost ranges.

Change-Id: I84c1f2e49ffcaf8238df60e41498730103d1ead6
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3291137
Reviewed-by: Joshua Peraza <jperaza@chromium.org>

src/processor/contained_range_map-inl.h

@@ -84,10 +84,12 @@ bool ContainedRangeMap<AddressType, EntryType>::StoreRange(
     // range's, it violates the containment rules, and an attempt to store
     // it must fail.  iterator_base->first contains the key, which was the
     // containing child's high address.
-    if (iterator_base->second->base_ == base && iterator_base->first == high) {
+    if (!allow_equal_range_ && iterator_base->second->base_ == base &&
+        iterator_base->first == high) {
       // TODO(nealsid): See the TODO above on why this is commented out.
-//       BPLOG(INFO) << "StoreRange failed, identical range is already "
+      //       BPLOG(INFO) << "StoreRange failed, identical range is already "
-//                      "present: " << HexString(base) << "+" << HexString(size);
+      //                      "present: " << HexString(base) << "+" <<
+      //                      HexString(size);
       return false;
     }
 
@@ -141,8 +143,10 @@ bool ContainedRangeMap<AddressType, EntryType>::StoreRange(
   // the new child range contains were formerly children of this range but
   // are now this range's grandchildren.  Ownership of these is transferred
   // to the new child range.
-  map_->insert(MapValue(high,
+  ContainedRangeMap* new_child =
-                        new ContainedRangeMap(base, entry, child_map)));
+      new ContainedRangeMap(base, entry, child_map, allow_equal_range_);
+
+  map_->insert(MapValue(high, new_child));
   return true;
 }
 
@@ -177,6 +181,20 @@ bool ContainedRangeMap<AddressType, EntryType>::RetrieveRange(
   return true;
 }
 
+template <typename AddressType, typename EntryType>
+bool ContainedRangeMap<AddressType, EntryType>::RetrieveRanges(
+    const AddressType& address,
+    std::vector<const EntryType*>& entries) const {
+  // If nothing was ever stored, then there's nothing to retrieve.
+  if (!map_)
+    return false;
+  MapIterator iterator = map_->lower_bound(address);
+  if (iterator == map_->end() || address < iterator->second->base_)
+    return false;
+  iterator->second->RetrieveRanges(address, entries);
+  entries.push_back(&iterator->second->entry_);
+  return true;
+}
 
 template<typename AddressType, typename EntryType>
 void ContainedRangeMap<AddressType, EntryType>::Clear() {

src/processor/contained_range_map.h

@@ -62,6 +62,7 @@
 
 
 #include <map>
+#include <vector>
 
 
 namespace google_breakpad {
@@ -75,7 +76,8 @@ class ContainedRangeMap {
   // The default constructor creates a ContainedRangeMap with no geometry
   // and no entry, and as such is only suitable for the root node of a
   // ContainedRangeMap tree.
-  ContainedRangeMap() : base_(), entry_(), map_(NULL) {}
+  explicit ContainedRangeMap(bool allow_equal_range = false)
+      : base_(), entry_(), map_(NULL), allow_equal_range_(allow_equal_range) {}
 
   ~ContainedRangeMap();
 
@@ -95,7 +97,12 @@ class ContainedRangeMap {
   // child ranges, and not the entry contained by |this|.  This is necessary
   // to support a sparsely-populated root range.  If no descendant range
   // encompasses the address, returns false.
-  bool RetrieveRange(const AddressType& address, EntryType* entry) const;
+  bool RetrieveRange(const AddressType& address, EntryType* entries) const;
+
+  // Retrieves the vector of entries encompassing the specified address from the
+  // innermost entry to the outermost entry.
+  bool RetrieveRanges(const AddressType& address,
+                      std::vector<const EntryType*>& entries) const;
 
   // Removes all children.  Note that Clear only removes descendants,
   // leaving the node on which it is called intact.  Because the only
@@ -118,9 +125,14 @@ class ContainedRangeMap {
   // Creates a new ContainedRangeMap with the specified base address, entry,
   // and initial child map, which may be NULL.  This is only used internally
   // by ContainedRangeMap when it creates a new child.
-  ContainedRangeMap(const AddressType& base, const EntryType& entry,
+  ContainedRangeMap(const AddressType& base,
-                    AddressToRangeMap* map)
+                    const EntryType& entry,
-      : base_(base), entry_(entry), map_(map) {}
+                    AddressToRangeMap* map,
+                    bool allow_equal_range)
+      : base_(base),
+        entry_(entry),
+        map_(map),
+        allow_equal_range_(allow_equal_range) {}
 
   // The base address of this range.  The high address does not need to
   // be stored, because it is used as the key to an object in its parent's
@@ -141,6 +153,12 @@ class ContainedRangeMap {
   // address.  This is a pointer to avoid allocating map structures for
   // leaf nodes, where they are not needed.
   AddressToRangeMap* map_;
+
+  // Whether or not we allow storing an entry into a range that equals to
+  // existing range in the map. Default is false.
+  // If this is true, the newly added range will become a child of existing
+  // innermost range which has same base and size.
+  bool allow_equal_range_;
 };
 
 

src/processor/contained_range_map_unittest.cc

@@ -51,9 +51,82 @@ namespace {
 
 
 using google_breakpad::ContainedRangeMap;
+// The first is the querying address, the second is the entries vector result.
+using EntriesTestPair = std::pair<unsigned, std::vector<int>>;
+using EntriesTestPairVec = std::vector<EntriesTestPair>;
 
+static bool RunTestsWithRetrieveRange(
+    const ContainedRangeMap<unsigned int, int>& crm,
+    const int* test_data,
+    unsigned int test_length) {
+  // Now, do the RetrieveRange tests.  This further validates that the
+  // objects were stored properly and that retrieval returns the correct
+  // object.
+  // If GENERATE_TEST_DATA is defined, instead of the retrieval tests, a
+  // new test_data array will be printed.  Exercise caution when doing this.
+  // Be sure to verify the results manually!
+#ifdef GENERATE_TEST_DATA
+  printf("  const int test_data[] = {\n");
+#endif  // GENERATE_TEST_DATA
 
-static bool RunTests() {
+  for (unsigned int address = 0; address < test_length; ++address) {
+    int value;
+    if (!crm.RetrieveRange(address, &value))
+      value = 0;
+
+#ifndef GENERATE_TEST_DATA
+    // Don't use ASSERT inside the loop because it won't show the failed
+    // |address|, and the line number will always be the same.  That makes
+    // it difficult to figure out which test failed.
+    if (value != test_data[address]) {
+      fprintf(stderr, "FAIL: retrieve %d expected %d observed %d @ %s:%d\n",
+              address, test_data[address], value, __FILE__, __LINE__);
+      return false;
+    }
+#else   // !GENERATE_TEST_DATA
+    printf("    %d%c%s  // %d\n", value, address == test_high - 1 ? ' ' : ',',
+           value < 10 ? " " : "", address);
+#endif  // !GENERATE_TEST_DATA
+  }
+
+#ifdef GENERATE_TEST_DATA
+  printf("  };\n");
+#endif  // GENERATE_TEST_DATA
+
+  return true;
+}
+
+static bool RunTestsWithRetrieveRangeVector(
+    const ContainedRangeMap<unsigned int, int>& crm,
+    const EntriesTestPairVec& entries_tests) {
+  for (const EntriesTestPair& entries_test : entries_tests) {
+    std::vector<const int*> entries;
+    crm.RetrieveRanges(entries_test.first, entries);
+    if (entries.size() != entries_test.second.size()) {
+      fprintf(stderr,
+              "FAIL: retrieving entries at address %u has size %zu "
+              "expected to have size %zu "
+              "@ %s: %d\n",
+              entries_test.first, entries.size(), entries_test.second.size(),
+              __FILE__, __LINE__);
+      return false;
+    }
+    for (size_t i = 0; i < entries.size(); ++i) {
+      if (*entries[i] != entries_test.second[i]) {
+        fprintf(stderr,
+                "FAIL: retrieving entries at address %u entries[%zu] is %d "
+                "expected %d"
+                "@ %s: %d\n",
+                entries_test.first, i, *entries[i], entries_test.second[i],
+                __FILE__, __LINE__);
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+static bool RunTestsWithNoEqualRange() {
   ContainedRangeMap<unsigned int, int> crm;
 
   // First, do the StoreRange tests.  This validates the containment
@@ -211,45 +284,89 @@ static bool RunTests() {
     0,   // 98
     0    // 99
   };
-  unsigned int test_high = sizeof(test_data) / sizeof(int);
+  unsigned int test_length = sizeof(test_data) / sizeof(int);
+  return RunTestsWithRetrieveRange(crm, test_data, test_length);
+}
 
-  // Now, do the RetrieveRange tests.  This further validates that the
+static bool RunTestsWithEqualRange() {
-  // objects were stored properly and that retrieval returns the correct
+  ContainedRangeMap<unsigned int, int> crm(true);
-  // object.
-  // If GENERATE_TEST_DATA is defined, instead of the retrieval tests, a
-  // new test_data array will be printed.  Exercise caution when doing this.
-  // Be sure to verify the results manually!
-#ifdef GENERATE_TEST_DATA
-  printf("  const int test_data[] = {\n");
-#endif  // GENERATE_TEST_DATA
 
-  for (unsigned int address = 0; address < test_high; ++address) {
+  // First, do the StoreRange tests.  This validates the containment
-    int value;
+  // rules.
-    if (!crm.RetrieveRange(address, &value))
+  ASSERT_TRUE (crm.StoreRange(1,  3,  1));
-      value = 0;
+  ASSERT_TRUE (crm.StoreRange(1,  3,  2));  // exactly equal to 1
+  ASSERT_TRUE (crm.StoreRange(1,  3,  3));  // exactly equal to 1, 2
+  ASSERT_TRUE (crm.StoreRange(1,  3,  4));  // exactly equal to 1, 2, 3
+  ASSERT_FALSE(crm.StoreRange(0,  3,  5));  // partial overlap.
+  ASSERT_FALSE(crm.StoreRange(2,  3,  6));  // partial overlap.
 
-#ifndef GENERATE_TEST_DATA
+  ASSERT_TRUE (crm.StoreRange(5,  3,  7));
-    // Don't use ASSERT inside the loop because it won't show the failed
+  ASSERT_TRUE (crm.StoreRange(5,  3,  8));  // exactly equal to 7
-    // |address|, and the line number will always be the same.  That makes
+  ASSERT_TRUE (crm.StoreRange(5,  3,  9));  // exactly equal to 7, 8
-    // it difficult to figure out which test failed.
+  ASSERT_TRUE (crm.StoreRange(5,  4, 10));  // encompasses 7, 8, 9
-    if (value != test_data[address]) {
+  ASSERT_TRUE (crm.StoreRange(5,  5, 11));  // encompasses 7, 8, 9, 10
-      fprintf(stderr, "FAIL: retrieve %d expected %d observed %d @ %s:%d\n",
-              address, test_data[address], value, __FILE__, __LINE__);
-      return false;
-    }
-#else  // !GENERATE_TEST_DATA
-    printf("    %d%c%s  // %d\n", value,
-                                  address == test_high - 1 ? ' ' : ',',
-                                  value < 10 ? " " : "",
-                                  address);
-#endif  // !GENERATE_TEST_DATA
-  }
 
-#ifdef GENERATE_TEST_DATA
+  ASSERT_TRUE (crm.StoreRange(10, 3, 12));
-  printf("  };\n");
+  ASSERT_TRUE (crm.StoreRange(10, 3, 13));  // exactly equal to 12
-#endif  // GENERATE_TEST_DATA
+  ASSERT_TRUE (crm.StoreRange(11, 2, 14));  // encompasses by 12
+  ASSERT_TRUE (crm.StoreRange(11, 1, 15));  // encompasses by 12, 13
 
-  return true;
+  ASSERT_TRUE (crm.StoreRange(14, 3, 16));
+  ASSERT_TRUE (crm.StoreRange(14, 3, 17));  // exactly equal to 14
+  ASSERT_TRUE (crm.StoreRange(14, 1, 18));  // encompasses by 14, 15
+  ASSERT_TRUE (crm.StoreRange(14, 2, 19));  // encompasses by 14, 15 and encompasses 16
+  ASSERT_TRUE (crm.StoreRange(14, 1, 20));  // exactly equal to 18
+  ASSERT_TRUE (crm.StoreRange(14, 2, 21));  // exactly equal to 19
+
+  // Each element in test_data contains the expected result when calling
+  // RetrieveRange on an address.
+  const int test_data[] = {
+      0,   // 0
+      4,   // 1
+      4,   // 2
+      4,   // 3
+      0,   // 4
+      9,   // 5
+      9,   // 6
+      9,   // 7
+      10,  // 8
+      11,   // 9
+      13,  // 10
+      15,  // 11
+      14,  // 12
+      0,   // 13
+      20,  // 14
+      21,  // 15
+      17,  // 16
+      0,   // 17
+  };
+  unsigned int test_length = sizeof(test_data) / sizeof(int);
+  EntriesTestPairVec entries_tests = {
+      {0,  {}},
+      {1,  {4, 3, 2, 1}},
+      {2,  {4, 3, 2, 1}},
+      {3,  {4, 3, 2, 1}},
+      {4,  {}},
+      {5,  {9, 8, 7, 10, 11}},
+      {6,  {9, 8, 7, 10, 11}},
+      {7,  {9, 8, 7, 10, 11}},
+      {8,  {10, 11}},
+      {9,  {11}},
+      {10, {13, 12}},
+      {11, {15, 14, 13, 12}},
+      {12, {14, 13, 12}},
+      {13, {}},
+      {14, {20, 18, 21, 19, 17, 16}},
+      {15, {21, 19, 17, 16}},
+      {16, {17, 16}},
+      {17, {}},
+  };
+  return RunTestsWithRetrieveRange(crm, test_data, test_length) &&
+         RunTestsWithRetrieveRangeVector(crm, entries_tests);
+}
+
+static bool RunTests() {
+  return RunTestsWithNoEqualRange() && RunTestsWithEqualRange();
 }
 
 

src/processor/static_contained_range_map-inl.h

@@ -87,6 +87,23 @@ bool StaticContainedRangeMap<AddressType, EntryType>::RetrieveRange(
   return true;
 }
 
+template <typename AddressType, typename EntryType>
+bool StaticContainedRangeMap<AddressType, EntryType>::RetrieveRanges(
+    const AddressType& address,
+    std::vector<const EntryType*>& entries) const {
+  MapConstIterator iterator = map_.lower_bound(address);
+  if (iterator == map_.end())
+    return false;
+  const char* memory_child =
+      reinterpret_cast<const char*>(iterator.GetValuePtr());
+  StaticContainedRangeMap child_map(memory_child);
+  if (address < child_map.base_)
+    return false;
+  child_map.RetrieveRanges(address, entries);
+  entries.push_back(child_map.entry_ptr_);
+  return true;
+}
+
 }  // namespace google_breakpad
 
 #endif  // PROCESSOR_STATIC_CONTAINED_RANGE_MAP_INL_H__

src/processor/static_contained_range_map.h

@@ -42,6 +42,7 @@
 #ifndef PROCESSOR_STATIC_CONTAINED_RANGE_MAP_H__
 #define PROCESSOR_STATIC_CONTAINED_RANGE_MAP_H__
 
+#include <vector>
 #include "processor/static_map-inl.h"
 
 namespace google_breakpad {
@@ -59,6 +60,11 @@ class StaticContainedRangeMap {
   // encompasses the address, returns false.
   bool RetrieveRange(const AddressType& address, const EntryType*& entry) const;
 
+  // Retrieves the vector of entries encompassing the specified address from the
+  // innermost entry to the outermost entry.
+  bool RetrieveRanges(const AddressType& address,
+                      std::vector<const EntryType*>& entry) const;
+
  private:
   friend class ModuleComparer;
   // AddressToRangeMap stores pointers.  This makes reparenting simpler in

src/processor/static_contained_range_map_unittest.cc

@@ -273,6 +273,25 @@ TEST_F(TestStaticCRMMap, TestSingleElementMap) {
   ASSERT_EQ(*entry_test, entry);
 }
 
+TEST_F(TestStaticCRMMap, TestRetrieveRangeEntries) {
+  CRMMap crm_map;
+
+  crm_map.StoreRange(2, 5, 0);
+  crm_map.StoreRange(2, 6, 1);
+  crm_map.StoreRange(2, 7, 2);
+
+  unsigned int size;
+  scoped_array<char> serialized_data;
+  serialized_data.reset(serializer_.Serialize(&crm_map, &size));
+  scoped_ptr<TestMap> test_map(new TestMap(serialized_data.get()));
+
+  std::vector<const int*> entry_tests;
+  ASSERT_TRUE(test_map->RetrieveRanges(3, entry_tests));
+  ASSERT_EQ(*entry_tests[0], 0);
+  ASSERT_EQ(*entry_tests[1], 1);
+  ASSERT_EQ(*entry_tests[2], 2);
+}
+
 TEST_F(TestStaticCRMMap, RunTestData) {
   unsigned int test_high = sizeof(test_data) / sizeof(test_data[0]);