Fix segfault with roaring64 intersect_with_range and empty bitmap (#636)

* Add unit tests for lower bound of an empty bitmap64 * art_iterator_lower_bound works on empty bitmap64 When we reset the iterator to the root, check if the root is null
RoaringBitmap · Jul 3, 2024 · 9526ecf · 9526ecf
1 parent 5da0593
commit 9526ecf
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 3 deletions.
diff --git a/src/art/art.c b/src/art/art.c
@@ -1693,8 +1693,11 @@ bool art_iterator_lower_bound(art_iterator_t *iterator,
         // a valid key. Start from the root.
         iterator->frame = 0;
         iterator->depth = 0;
-        return art_node_iterator_lower_bound(art_iterator_node(iterator),
-                                             iterator, key);
+        art_node_t *root = art_iterator_node(iterator);
+        if (root == NULL) {
+            return false;
+        }
+        return art_node_iterator_lower_bound(root, iterator, key);
     }
     int compare_result =
         art_compare_prefix(iterator->key, 0, key, 0, ART_KEY_BYTES);

diff --git a/tests/art_unit.cpp b/tests/art_unit.cpp
@@ -319,6 +319,16 @@ DEFINE_TEST(test_art_iterator_prev) {
 }
 
 DEFINE_TEST(test_art_iterator_lower_bound) {
+    {
+        art_t art{NULL};
+        art_iterator_t iterator = art_init_iterator(&art, true);
+        assert_null(iterator.value);
+        assert_false(
+            art_iterator_lower_bound(&iterator, (art_key_chunk_t*)"000000"));
+        assert_false(
+            art_iterator_lower_bound(&iterator, (art_key_chunk_t*)"000001"));
+        art_free(&art);
+    }
     {
         std::vector<const char*> keys = {
             "000001", "000002", "000003", "000004", "001005",

diff --git a/tests/roaring64_unit.cpp b/tests/roaring64_unit.cpp
@@ -1053,6 +1053,11 @@ DEFINE_TEST(test_intersect) {
 DEFINE_TEST(test_intersect_with_range) {
     roaring64_bitmap_t* r = roaring64_bitmap_create();
 
+    // Empty bitmap never intersects
+    assert_false(roaring64_bitmap_intersect_with_range(r, 0, 0));
+    assert_false(roaring64_bitmap_intersect_with_range(r, 0, 50000));
+    assert_false(roaring64_bitmap_intersect_with_range(r, 0, UINT64_MAX));
+
     roaring64_bitmap_add(r, 50000);
     roaring64_bitmap_add(r, 100000);
     roaring64_bitmap_add(r, 100001);
@@ -1065,6 +1070,13 @@ DEFINE_TEST(test_intersect_with_range) {
     assert_true(roaring64_bitmap_intersect_with_range(r, 50001, 100001));
     assert_false(roaring64_bitmap_intersect_with_range(r, 300001, UINT64_MAX));
 
+    // Empty ranges never intersect
+    assert_false(roaring64_bitmap_intersect_with_range(r, 0, 0));
+    assert_false(
+        roaring64_bitmap_intersect_with_range(r, UINT64_MAX, UINT64_MAX));
+    assert_false(roaring64_bitmap_intersect_with_range(r, UINT64_MAX, 0));
+    assert_false(roaring64_bitmap_intersect_with_range(r, 50000, 50000));
+
     roaring64_bitmap_free(r);
 }
 
@@ -1674,13 +1686,17 @@ DEFINE_TEST(test_iterator_previous) {
 DEFINE_TEST(test_iterator_move_equalorlarger) {
     roaring64_bitmap_t* r = roaring64_bitmap_create();
 
+    roaring64_iterator_t* it = roaring64_iterator_create(r);
+    assert_false(roaring64_iterator_move_equalorlarger(it, 0));
+    assert_false(roaring64_iterator_move_equalorlarger(it, UINT64_MAX));
+
     roaring64_bitmap_add(r, 0);
     roaring64_bitmap_add(r, 1ULL << 35);
     roaring64_bitmap_add(r, (1ULL << 35) + 1);
     roaring64_bitmap_add(r, (1ULL << 35) + 2);
     roaring64_bitmap_add(r, (1ULL << 36));
 
-    roaring64_iterator_t* it = roaring64_iterator_create(r);
+    roaring64_iterator_reinit(r, it);
     assert_true(roaring64_iterator_move_equalorlarger(it, 0));
     assert_true(roaring64_iterator_has_value(it));
     assert_int_equal(roaring64_iterator_value(it), 0);