storeInitialRingTopPatch: fix large delta bug

Check for staleness up front, so that it is safe to use u32 values to
handle adding more tops.

Adds LargeGap unit tests.

diff --git a/src/nfa/repeat.c b/src/nfa/repeat.c
index 0344423b73ae60b86044c237f7a76c7e7241c7a0..59205ae41cecccb3fe7efa3d7c764314b9db8b10 100644 (file)
--- a/src/nfa/repeat.c
+++ b/src/nfa/repeat.c
@@ -1165,7 +1165,7 @@ static
 void storeInitialRingTopPatch(const struct RepeatInfo *info,
                               struct RepeatRingControl *xs,
                               u8 *state, u64a offset) {
-    DEBUG_PRINTF("set the first patch\n");
+    DEBUG_PRINTF("set the first patch, offset=%llu\n", offset);
     xs->offset = offset;
 
     u8 *active = state;
@@ -1340,21 +1340,33 @@ void repeatStoreSparseOptimalP(const struct RepeatInfo *info,
                                union RepeatControl *ctrl, void *state,
                                u64a offset, char is_alive) {
     struct RepeatRingControl *xs = &ctrl->ring;
+    u8 *active = (u8 *)state;
 
-    u64a delta = offset - xs->offset;
-    u32 patch_size = info->patchSize;
-    u32 patch_count = info->patchCount;
-    u32 encoding_size = info->encodingSize;
-    u32 patch = delta / patch_size;
-    DEBUG_PRINTF("offset: %llu encoding_size: %u\n", offset, encoding_size);
+    DEBUG_PRINTF("offset: %llu encoding_size: %u\n", offset,
+                 info->encodingSize);
 
-    u8 *active = (u8 *)state;
-    if (!is_alive) {
+    // If (a) this is the first top, or (b) the ring is stale, initialize the
+    // ring and write this offset in as the first top.
+    if (!is_alive ||
+        offset >
+            repeatLastTopSparseOptimalP(info, ctrl, state) + info->repeatMax) {
         storeInitialRingTopPatch(info, xs, active, offset);
         return;
     }
 
-    assert(offset >= xs->offset);
+    // Tops should arrive in order, with no duplicates.
+    assert(offset > repeatLastTopSparseOptimalP(info, ctrl, state));
+
+    // As the ring is not stale, our delta should fit within a u32.
+    assert(offset - xs->offset <= UINT32_MAX);
+    u32 delta = (u32)(offset - xs->offset);
+    u32 patch_size = info->patchSize;
+    u32 patch_count = info->patchCount;
+    u32 encoding_size = info->encodingSize;
+    u32 patch = delta / patch_size;
+
+    DEBUG_PRINTF("delta=%u, patch_size=%u, patch=%u\n", delta, patch_size,
+                 patch);
 
     u8 *ring = active + info->patchesOffset;
     u32 occ = ringOccupancy(xs, patch_count);
@@ -1365,10 +1377,6 @@ void repeatStoreSparseOptimalP(const struct RepeatInfo *info,
                  patch, patch_count, occ);
     if (patch >= patch_count) {
         u32 patch_shift_count = patch - patch_count + 1;
-        if (patch_shift_count >= patch_count) {
-            storeInitialRingTopPatch(info, xs, active, offset);
-            return;
-        }
         assert(patch >= patch_shift_count);
         DEBUG_PRINTF("shifting by %u\n", patch_shift_count);
         xs->offset += patch_size * patch_shift_count;

diff --git a/unit/internal/repeat.cpp b/unit/internal/repeat.cpp
index db797d2410067c96d92252c0b0c2d033d0b803fb..94f1bdc10f1858ed875719541d76bf0a4f9ef41c 100644 (file)
--- a/unit/internal/repeat.cpp
+++ b/unit/internal/repeat.cpp
@@ -448,6 +448,25 @@ TEST_P(RepeatTest, Pack) {
     }
 }
 
+TEST_P(RepeatTest, LargeGap) {
+    SCOPED_TRACE(testing::Message() << "Repeat: " << info);
+
+    if (info.repeatMax == REPEAT_INF) {
+        return; // Test not valid for FIRST-type repeats.
+    }
+
+    for (int i = 0; i < 64; i++) {
+        u64a top1 = 1000;
+        repeatStore(&info, ctrl, state, top1, 0); // first top
+        ASSERT_EQ(top1, repeatLastTop(&info, ctrl, state));
+
+        // Add a second top after a gap of 2^i bytes.
+        u64a top2 = top1 + (1ULL << i);
+        repeatStore(&info, ctrl, state, top2, 1); // second top
+        ASSERT_EQ(top2, repeatLastTop(&info, ctrl, state));
+    }
+}
+
 static
 const u32 sparsePeriods[] = {
     2,
@@ -895,6 +914,24 @@ TEST_P(SparseOptimalTest, Simple3e) {
     test_sparse3entryExpire(info, ctrl, state, 2 * info->minPeriod - 1);
 }
 
+TEST_P(SparseOptimalTest, LargeGap) {
+    SCOPED_TRACE(testing::Message() << "Repeat: " << *info);
+
+    for (int i = 0; i < 64; i++) {
+        u64a top1 = 1000;
+        repeatStore(info, ctrl, state, top1, 0); // first top
+        ASSERT_EQ(top1, repeatLastTop(info, ctrl, state));
+
+        // Add a second top after a gap of 2^i bytes.
+        u64a top2 = top1 + (1ULL << i);
+        if (top2 - top1 < info->minPeriod) {
+            continue; // not a valid top
+        }
+        repeatStore(info, ctrl, state, top2, 1); // second top
+        ASSERT_EQ(top2, repeatLastTop(info, ctrl, state));
+    }
+}
+
 TEST_P(SparseOptimalTest, ThreeTops) {
     SCOPED_TRACE(testing::Message() << "Repeat: " << *info);