Skip to content

Commit 4618230

Browse files
tzaeschketzaeschke
authored
multimap relocate() fails asan (#116)
1 parent 998f7c0 commit 4618230

6 files changed

Lines changed: 173 additions & 14 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
2020
[#98](https://github.com/tzaeschke/phtree-cpp/pull/98),
2121
[#99](https://github.com/tzaeschke/phtree-cpp/pull/99),
2222
[#101](https://github.com/tzaeschke/phtree-cpp/pull/101),
23-
[#104](https://github.com/tzaeschke/phtree-cpp/pull/104)
23+
[#104](https://github.com/tzaeschke/phtree-cpp/pull/104),
24+
[#115](https://github.com/tzaeschke/phtree-cpp/issues/115)
2425
- Cleaned up HandleCollision() and key comparison functions. [#97](https://github.com/tzaeschke/phtree-cpp/pull/97)
2526
- Improved performance by eliminating memory indirection for DIM > 3.
2627
This was enabled by referencing "Node" directly in "Entry" which was enabled by

‎fuzzer/phtree_mm_relocate_fuzzer.cc‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*
2+
* Copyright 2023 Tilmann Zäschke
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include <assert.h>
18+
#include <cstddef>
19+
#include <cstdint>
20+
#include <iostream>
21+
#include <ostream>
22+
23+
#include <map>
24+
25+
#include "include/phtree/phtree_multimap.h"
26+
27+
// clang++ -g -std=c++17 -fsanitize=address,fuzzer fuzzer/phtree_mm_relocate_fuzzer.cc -I. -I./include
28+
29+
30+
using namespace improbable::phtree;
31+
32+
33+
constexpr bool PRINT = !true;
34+
35+
void print() {}
36+
37+
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* Data, size_t Size) {
38+
assert(Data);
39+
40+
const dimension_t DIM = 1;
41+
42+
if (PRINT) {
43+
std::cout << "TEST(PhTreeMMTest, FuzzTest1) {" << std::endl;
44+
std::cout << " const dimension_t DIM = 1;" << std::endl;
45+
std::cout << " using Key = PhPoint<DIM>;" << std::endl;
46+
std::cout << " using Value = std::uint8_t;" << std::endl;
47+
std::cout << " PhTreeMultiMap<DIM, Value, ConverterNoOp<DIM, std::int64_t>> tree{};" << std::endl;
48+
}
49+
50+
using Instruction = std::uint8_t;
51+
using Key = PhPoint<1>;
52+
using Value = std::uint8_t;
53+
54+
PhTreeMultiMap<DIM, Value, ConverterNoOp<DIM, std::int64_t>> tree;
55+
std::multimap<Key, Value> map;
56+
57+
size_t pos = 0;
58+
59+
while (pos + 4 < Size) {
60+
Instruction inst = Data[pos++] % 2;
61+
Key key{Data[pos++]};
62+
Key key2{Data[pos++]};
63+
Value value = Data[pos++];
64+
switch (inst) {
65+
case 0: {
66+
if (PRINT)
67+
std::cout << " tree.emplace({" << key[0] << "}, " << (int)value << ");"
68+
<< std::endl;
69+
tree.emplace({key[0]}, value);
70+
// map.emplace(key, value);
71+
break;
72+
}
73+
case 1: {
74+
if (PRINT)
75+
std::cout << " tree.relocate({" << key[0] << "}, {" << key2[0] << "}, " << (int)value << ");" << std::endl;
76+
// tree.erase(key);
77+
// map.erase(key);
78+
tree.relocate({key[0]}, {key2[0]}, value);
79+
break;
80+
}
81+
// case 2: {
82+
// if (PRINT)
83+
// std::cout << " auto it = tree.find(" << (int)key << ");" << std::endl;
84+
// auto it = tree.find(key);
85+
// if (it != tree.end()) {
86+
// if (PRINT)
87+
// std::cout << " tree.erase(it);" << std::endl;
88+
// tree.erase(it);
89+
// }
90+
// auto it2 = map.find(key);
91+
// if (it2 != map.end()) {
92+
// map.erase(it2);
93+
// }
94+
// break;
95+
// }
96+
// case 3: {
97+
// if (PRINT)
98+
// std::cout << " auto it = tree.lower_bound(" << (int)key << ");" << std::endl;
99+
// auto it = tree.lower_bound(key);
100+
// if (PRINT)
101+
// std::cout << " tree.emplace_hint(it, " << (int)key << ", " << (int)value << ");"
102+
// << std::endl;
103+
// tree.emplace_hint(it, key, value);
104+
// auto it2 = map.lower_bound(key);
105+
// map.emplace_hint(it2, key, value);
106+
// break;
107+
// }
108+
default:
109+
std::cout << "Unexpected instruction: " << inst << std::endl;
110+
}
111+
}
112+
113+
//tree._check();
114+
115+
// for (auto& entry : map) {
116+
// const Key& vRef = entry.first;
117+
// Key vMap = tree.find(vRef)->first;
118+
// assert(vMap == vRef);
119+
// }
120+
// for (auto& entry : tree) {
121+
// Key v = entry.first;
122+
// const Key& vRef = map.find(v)->first;
123+
// Key vMap = tree.find(v)->first;
124+
// assert(vMap == vRef);
125+
// }
126+
// assert(tree.size() == map.size());
127+
128+
return 0;
129+
}

‎include/phtree/v16/phtree_v16.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -533,8 +533,9 @@ class PhTreeV16 {
533533

534534
if (result == 0) {
535535
clean_up(new_key, new_entry, new_node_entry);
536+
} else {
537+
clean_up(old_key, old_entry, old_node_entry);
536538
}
537-
clean_up(old_key, old_entry, old_node_entry);
538539
return result;
539540
}
540541

‎test/phtree_f_test.cc‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -947,22 +947,30 @@ TEST(PhTreeFTest, SmokeTestPointInfinity) {
947947
// Note that the tree returns result in z-order, however, since the z-order is based on
948948
// the (unsigned) bit representation, negative values come _after_ positive values.
949949
auto q_window = tree.begin_query({p_neg, p_pos});
950-
ASSERT_EQ(1, q_window->_i);
950+
std::set<int> result;
951+
result.emplace(q_window->_i);
951952
++q_window;
952-
ASSERT_EQ(10, q_window->_i);
953+
result.emplace(q_window->_i);
953954
++q_window;
954-
ASSERT_EQ(-10, q_window->_i);
955+
result.emplace(q_window->_i);
955956
++q_window;
956957
ASSERT_EQ(q_window, tree.end());
958+
ASSERT_EQ(1, result.count(1));
959+
ASSERT_EQ(1, result.count(10));
960+
ASSERT_EQ(1, result.count(-10));
957961

958962
auto q_extent = tree.begin();
959-
ASSERT_EQ(1, q_extent->_i);
963+
result.clear();
964+
result.emplace(q_extent->_i);
960965
++q_extent;
961-
ASSERT_EQ(10, q_extent->_i);
966+
result.emplace(q_extent->_i);
962967
++q_extent;
963-
ASSERT_EQ(-10, q_extent->_i);
968+
result.emplace(q_extent->_i);
964969
++q_extent;
965970
ASSERT_EQ(q_extent, tree.end());
971+
ASSERT_EQ(1, result.count(1));
972+
ASSERT_EQ(1, result.count(10));
973+
ASSERT_EQ(1, result.count(-10));
966974

967975
auto q_knn = tree.begin_knn_query(10, p, DistanceEuclidean<3>());
968976
ASSERT_EQ(1, q_knn->_i);

‎test/phtree_multimap_d_test.cc‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,22 +1131,30 @@ TEST(PhTreeMMDTest, SmokeTestPointInfinity) {
11311131
// Note that the tree returns result in z-order, however, since the z-order is based on
11321132
// the (unsigned) bit representation, negative values come _after_ positive values.
11331133
auto q_window = tree.begin_query({p_neg, p_pos});
1134-
ASSERT_EQ(1, q_window->_i);
1134+
std::set<int> result;
1135+
result.emplace(q_window->_i);
11351136
++q_window;
1136-
ASSERT_EQ(10, q_window->_i);
1137+
result.emplace(q_window->_i);
11371138
++q_window;
1138-
ASSERT_EQ(-10, q_window->_i);
1139+
result.emplace(q_window->_i);
11391140
++q_window;
11401141
ASSERT_EQ(q_window, tree.end());
1142+
ASSERT_EQ(1, result.count(1));
1143+
ASSERT_EQ(1, result.count(10));
1144+
ASSERT_EQ(1, result.count(-10));
11411145

11421146
auto q_extent = tree.begin();
1143-
ASSERT_EQ(1, q_extent->_i);
1147+
result.clear();
1148+
result.emplace(q_extent->_i);
11441149
++q_extent;
1145-
ASSERT_EQ(10, q_extent->_i);
1150+
result.emplace(q_extent->_i);
11461151
++q_extent;
1147-
ASSERT_EQ(-10, q_extent->_i);
1152+
result.emplace(q_extent->_i);
11481153
++q_extent;
11491154
ASSERT_EQ(q_extent, tree.end());
1155+
ASSERT_EQ(1, result.count(1));
1156+
ASSERT_EQ(1, result.count(10));
1157+
ASSERT_EQ(1, result.count(-10));
11501158

11511159
auto q_knn = tree.begin_knn_query(10, p, DistanceEuclidean<3>());
11521160
ASSERT_EQ(1, q_knn->_i);
@@ -1285,4 +1293,15 @@ TEST(PhTreeMMDTest, TestMovableIterators) {
12851293
// 3, {2, 3, 4}, DistanceEuclidean<3>()))>);
12861294
}
12871295

1296+
TEST(PhTreeMMTest, FuzzTest1) {
1297+
// See issue #115
1298+
const dimension_t DIM = 1;
1299+
// using Key = PhPoint<DIM>;
1300+
using Value = std::uint8_t;
1301+
PhTreeMultiMap<DIM, Value, ConverterNoOp<DIM, std::int64_t>> tree{};
1302+
tree.emplace({0}, 63);
1303+
tree.emplace({0}, 214);
1304+
tree.relocate({0}, {17}, 0);
1305+
}
1306+
12881307
} // namespace phtree_multimap_d_test

‎test/phtree_test.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@ TEST(PhTreeTest, SmokeTestBasicOps) {
266266
SmokeTestBasicOps<6>(10000);
267267
SmokeTestBasicOps<10>(10000);
268268
SmokeTestBasicOps<20>(10000);
269+
SmokeTestBasicOps<32>(1000);
269270
SmokeTestBasicOps<63>(100);
270271
}
271272

0 commit comments

Comments
 (0)