From 4458a5965a3c534ab43487ff5ddd4916553d4ab0 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Tue, 30 Nov 2021 13:53:53 -0800 Subject: [PATCH] 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 --- src/processor/contained_range_map-inl.h | 28 ++- src/processor/contained_range_map.h | 28 ++- src/processor/contained_range_map_unittest.cc | 187 ++++++++++++++---- .../static_contained_range_map-inl.h | 17 ++ src/processor/static_contained_range_map.h | 6 + .../static_contained_range_map_unittest.cc | 19 ++ 6 files changed, 240 insertions(+), 45 deletions(-) diff --git a/src/processor/contained_range_map-inl.h b/src/processor/contained_range_map-inl.h index edd11d7c..605f60f2 100644 --- a/src/processor/contained_range_map-inl.h +++ b/src/processor/contained_range_map-inl.h @@ -84,10 +84,12 @@ bool ContainedRangeMap::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 " -// "present: " << HexString(base) << "+" << HexString(size); + // BPLOG(INFO) << "StoreRange failed, identical range is already " + // "present: " << HexString(base) << "+" << + // HexString(size); return false; } @@ -141,8 +143,10 @@ bool ContainedRangeMap::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, - new ContainedRangeMap(base, entry, child_map))); + ContainedRangeMap* new_child = + new ContainedRangeMap(base, entry, child_map, allow_equal_range_); + + map_->insert(MapValue(high, new_child)); return true; } @@ -177,6 +181,20 @@ bool ContainedRangeMap::RetrieveRange( return true; } +template +bool ContainedRangeMap::RetrieveRanges( + const AddressType& address, + std::vector& 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 void ContainedRangeMap::Clear() { diff --git a/src/processor/contained_range_map.h b/src/processor/contained_range_map.h index 18d03af7..963548f4 100644 --- a/src/processor/contained_range_map.h +++ b/src/processor/contained_range_map.h @@ -62,6 +62,7 @@ #include +#include 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& 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, - AddressToRangeMap* map) - : base_(base), entry_(entry), map_(map) {} + ContainedRangeMap(const AddressType& base, + const EntryType& entry, + 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_; }; diff --git a/src/processor/contained_range_map_unittest.cc b/src/processor/contained_range_map_unittest.cc index a97c5d0f..6597cac8 100644 --- a/src/processor/contained_range_map_unittest.cc +++ b/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>; +using EntriesTestPairVec = std::vector; +static bool RunTestsWithRetrieveRange( + const ContainedRangeMap& 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& crm, + const EntriesTestPairVec& entries_tests) { + for (const EntriesTestPair& entries_test : entries_tests) { + std::vector 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 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 - // 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 RunTestsWithEqualRange() { + ContainedRangeMap crm(true); - for (unsigned int address = 0; address < test_high; ++address) { - int value; - if (!crm.RetrieveRange(address, &value)) - value = 0; + // First, do the StoreRange tests. This validates the containment + // rules. + ASSERT_TRUE (crm.StoreRange(1, 3, 1)); + 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 - // 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 - } + ASSERT_TRUE (crm.StoreRange(5, 3, 7)); + ASSERT_TRUE (crm.StoreRange(5, 3, 8)); // exactly equal to 7 + ASSERT_TRUE (crm.StoreRange(5, 3, 9)); // exactly equal to 7, 8 + ASSERT_TRUE (crm.StoreRange(5, 4, 10)); // encompasses 7, 8, 9 + ASSERT_TRUE (crm.StoreRange(5, 5, 11)); // encompasses 7, 8, 9, 10 -#ifdef GENERATE_TEST_DATA - printf(" };\n"); -#endif // GENERATE_TEST_DATA + ASSERT_TRUE (crm.StoreRange(10, 3, 12)); + ASSERT_TRUE (crm.StoreRange(10, 3, 13)); // exactly equal to 12 + 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(); } diff --git a/src/processor/static_contained_range_map-inl.h b/src/processor/static_contained_range_map-inl.h index 87ea6c7f..58c83371 100644 --- a/src/processor/static_contained_range_map-inl.h +++ b/src/processor/static_contained_range_map-inl.h @@ -87,6 +87,23 @@ bool StaticContainedRangeMap::RetrieveRange( return true; } +template +bool StaticContainedRangeMap::RetrieveRanges( + const AddressType& address, + std::vector& entries) const { + MapConstIterator iterator = map_.lower_bound(address); + if (iterator == map_.end()) + return false; + const char* memory_child = + reinterpret_cast(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__ diff --git a/src/processor/static_contained_range_map.h b/src/processor/static_contained_range_map.h index 14fa5e95..efdfbeab 100644 --- a/src/processor/static_contained_range_map.h +++ b/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 #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& entry) const; + private: friend class ModuleComparer; // AddressToRangeMap stores pointers. This makes reparenting simpler in diff --git a/src/processor/static_contained_range_map_unittest.cc b/src/processor/static_contained_range_map_unittest.cc index 4ee47578..e2b25a2a 100644 --- a/src/processor/static_contained_range_map_unittest.cc +++ b/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 serialized_data; + serialized_data.reset(serializer_.Serialize(&crm_map, &size)); + scoped_ptr test_map(new TestMap(serialized_data.get())); + + std::vector 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]);