Fix double-shufti accelerator missing matches across stream chunk boundaries (#402)

* Fix double-shufti accelerator dropping cross-chunk matches in streaming mode

shuftiDoubleExec finds the first position i where byte[i] is in class1 and
byte[i+1] is in class2, so it must look one byte past the position it reports.
shuftiDoubleExecReal excludes the final buffer byte from its main loop and
defers it to check_last_byte, which zero-pads the absent byte[i+1].

check_last_byte only stopped on the last byte when it completed a single-char
(wildcard second char) pattern. When the last byte is merely the first char of
a double it was skipped. That is correct at true end-of-data, but at an
intermediate stream chunk boundary the second char is the first byte of the
next chunk, so the straddling match was silently dropped - a streaming
split-invariance violation.

Also stop on the last byte when it is in the first-char class (last_elem !=
0xff), returning buf_end-1. That is a resume point, not a reported match: the
engine re-examines the byte, carries the pending-first-char state into the next
chunk (fixing the false negative), and at true EOD finds nothing (no false
positive). The change is off the SIMD hot loop.

Add DoubleShuftiStreaming.SplitInvariant (block match set vs every streaming
split) and update the internal shufti tests to the new last-byte contract.

* double shufti accelerator fix for sve too

diff --git a/src/nfa/shufti_simd.hpp b/src/nfa/shufti_simd.hpp
index 9b3d5a29a382459aaab2bccd13db6202dc3c4029..d4d7b94752c60d2f94aa5b8245ea73ff29405cf7 100644 (file)
--- a/src/nfa/shufti_simd.hpp
+++ b/src/nfa/shufti_simd.hpp
@@ -213,8 +213,10 @@ const u8 *check_last_byte(SuperVector<S> mask2_lo, SuperVector<S> mask2_hi,
     }
     uint8_t match_inverted = reduce.u.u8[0] | last_elem;
 
     }
     uint8_t match_inverted = reduce.u.u8[0] | last_elem;
 

-    // if 0xff, then no match
-    int match = match_inverted != 0xff;
+    // Also stop when the last byte is the first char of a double (last_elem): its
+    // second char may be in the next stream chunk. This returns a resume point, not
+    // a match, so it adds no false positives but fixes cross-chunk false negatives.
+    int match = (match_inverted != 0xff) || (last_elem != 0xff);

     if(match) {
         return buf_end - 1;
     }
     if(match) {
         return buf_end - 1;
     }

diff --git a/src/nfa/shufti_sve.hpp b/src/nfa/shufti_sve.hpp
index 2348c0913d87d511c7a3ff335aa3e55328d74706..73cf381d78b456b9633950ab25342cce4b5c15b1 100644 (file)
--- a/src/nfa/shufti_sve.hpp
+++ b/src/nfa/shufti_sve.hpp
@@ -182,7 +182,10 @@ const u8 *check_last_byte(svuint8_t mask2_lo, svuint8_t mask2_hi,
     uint8_t wild_lo = svorv(svptrue_b8(), mask2_lo);
     uint8_t wild_hi = svorv(svptrue_b8(), mask2_hi);
     uint8_t match_inverted = wild_lo | wild_hi | last_elem;
     uint8_t wild_lo = svorv(svptrue_b8(), mask2_lo);
     uint8_t wild_hi = svorv(svptrue_b8(), mask2_hi);
     uint8_t match_inverted = wild_lo | wild_hi | last_elem;

-    int match = match_inverted != 0xff;
+    // Also stop when the last byte is the first char of a double (last_elem): its
+    // second char may be in the next stream chunk. This returns a resume point, not
+    // a match, so it adds no false positives but fixes cross-chunk false negatives.
+    int match = (match_inverted != 0xff) || (last_elem != 0xff);

     if(match) {
         return buf_end - 1;
     }
     if(match) {
         return buf_end - 1;
     }

diff --git a/unit/hyperscan/identical.cpp b/unit/hyperscan/identical.cpp
index cbeb02422e941b0bbd0d9513c76265691208d505..2cfbfca157e66b84661e56272e26d2a9d03a30d0 100644 (file)
--- a/unit/hyperscan/identical.cpp
+++ b/unit/hyperscan/identical.cpp
@@ -194,4 +194,73 @@ void PrintTo(const PatternInfo &p, ::std::ostream *os) {
     *os << p.expr << ":" << p.flags << ", " << p.corpus;
 }
 
     *os << p.expr << ":" << p.flags << ", " << p.corpus;
 }
 

+// Regression: a double-class shufti accelerator must report identical matches in
+// block and streaming mode, including when a two-char anchor straddles a chunk
+// boundary (the trailing first-char byte must not be skipped). For each pattern
+// the block match set is the oracle; streaming must reproduce it at every split.
+static const PatternInfo accelStreamSplitPatterns[] = {
+    { "[0-9]{1,2}c[a-f]*", 0,
+      ".q3q..y1y.cz13czx0c31bc2 1z2bz1_qq2 12yebxz.yd.f.cy__ .ybezd112cz", 0 },
+    { "[b-d]{1,3}[cd]+\\d*", 0,
+      "xf2a2b .3a21xbzz0 eq1qdxayyy20dadaa_ a22qbe..cxycx1 301.bd3", 0 },
+    { "[a-c]{1,3}[ac][ab]*", 0,
+      "f1fzxq.qedb31dxa0d .2xc0 .3yy3_f01yxa01a3c ya1e. az.qdaa", 0 },
+    { "[ac]{1,3}[cd]+", 0, "f2z0c_xxzd23cabe_bdczd0xqeb0fcc", 0 },
+};
+
+TEST(DoubleShuftiStreaming, SplitInvariant) {
+    const size_t num = sizeof(accelStreamSplitPatterns) /
+                       sizeof(accelStreamSplitPatterns[0]);
+    for (size_t p = 0; p < num; p++) {
+        const PatternInfo &info = accelStreamSplitPatterns[p];
+        SCOPED_TRACE(info.expr);
+
+        hs_database_t *bdb =
+            buildDB(pattern(info.expr, info.flags, 0), HS_MODE_BLOCK);
+        ASSERT_NE(nullptr, bdb);
+        hs_database_t *sdb =
+            buildDB(pattern(info.expr, info.flags, 0), HS_MODE_STREAM);
+        ASSERT_NE(nullptr, sdb);
+
+        hs_scratch_t *scratch = nullptr;
+        ASSERT_EQ(HS_SUCCESS, hs_alloc_scratch(bdb, &scratch));
+        ASSERT_EQ(HS_SUCCESS, hs_alloc_scratch(sdb, &scratch));
+
+        // Block-mode oracle: the set of match end-offsets.
+        CallBackContext block_cb;
+        ASSERT_EQ(HS_SUCCESS,
+                  hs_scan(bdb, info.corpus.c_str(), info.corpus.size(), 0,
+                          scratch, record_cb, &block_cb));
+        std::set<unsigned long long> oracle;
+        for (size_t i = 0; i < block_cb.matches.size(); i++) {
+            oracle.insert(block_cb.matches[i].to);
+        }
+
+        const size_t len = info.corpus.size();
+        for (size_t k = 1; k < len; k++) {
+            CallBackContext cb;
+            hs_stream_t *stream = nullptr;
+            ASSERT_EQ(HS_SUCCESS, hs_open_stream(sdb, 0, &stream));
+            ASSERT_EQ(HS_SUCCESS,
+                      hs_scan_stream(stream, info.corpus.c_str(), k, 0, scratch,
+                                     record_cb, &cb));
+            ASSERT_EQ(HS_SUCCESS,
+                      hs_scan_stream(stream, info.corpus.c_str() + k, len - k, 0,
+                                     scratch, record_cb, &cb));
+            ASSERT_EQ(HS_SUCCESS,
+                      hs_close_stream(stream, scratch, record_cb, &cb));
+
+            std::set<unsigned long long> got;
+            for (size_t i = 0; i < cb.matches.size(); i++) {
+                got.insert(cb.matches[i].to);
+            }
+            ASSERT_EQ(oracle, got) << "stream split at offset " << k;
+        }
+
+        ASSERT_EQ(HS_SUCCESS, hs_free_scratch(scratch));
+        hs_free_database(bdb);
+        hs_free_database(sdb);
+    }
+}
+

 } // namespace
 } // namespace

diff --git a/unit/internal/shufti.cpp b/unit/internal/shufti.cpp
index 9b196714ce759d2f8eecd3eb64472249996f283d..bc4e8553d9819fc54ca6ed4b135ea1d18e7d4253 100644 (file)
--- a/unit/internal/shufti.cpp
+++ b/unit/internal/shufti.cpp
@@ -900,12 +900,9 @@ TEST(DoubleShufti, ExecMatchMixed3) {
         const u8 *rv = shuftiDoubleExec(lo1, hi1, lo2, hi2,
                                         reinterpret_cast<u8 *>(t2), reinterpret_cast<u8 *>(t2) + len);
 
         const u8 *rv = shuftiDoubleExec(lo1, hi1, lo2, hi2,
                                         reinterpret_cast<u8 *>(t2), reinterpret_cast<u8 *>(t2) + len);
 

-        if(i < 2) {
-            // i=0 is "xy" out of buffer. i=1 is "x" in buffer but not "y"
-            ASSERT_EQ(reinterpret_cast<const u8 *>(t2 + len), rv);
-        }else {
-            ASSERT_EQ(reinterpret_cast<const u8 *>(&t2[len - i]), rv);
-        }
+        // Same form as the single-char loop above: i==0 is buf_end, i==1 a resume
+        // point (first char on the last byte), i>=2 the real match.
+        ASSERT_EQ(reinterpret_cast<const u8 *>(&t2[len - i]), rv);

     }
 }
 
     }
 }
 


@@ -933,7 +930,8 @@ TEST(DoubleShufti, ExecNoMatchVectorEdge) {
         const u8 *rv = shuftiDoubleExec(lo1, hi1, lo2, hi2,
                                         reinterpret_cast<u8 *>(t1), reinterpret_cast<u8 *>(t1) + len);
 
         const u8 *rv = shuftiDoubleExec(lo1, hi1, lo2, hi2,
                                         reinterpret_cast<u8 *>(t1), reinterpret_cast<u8 *>(t1) + len);
 

-        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len), rv);
+        // i==1: first char 'a' is the last byte, so accel stops at buf_end-1
+        ASSERT_EQ(i == 1 ? reinterpret_cast<const u8 *>(t1 + len - 1) : reinterpret_cast<const u8 *>(t1 + len), rv);

     }
 }
 
     }
 }
 


@@ -1046,7 +1044,7 @@ TEST(DoubleShufti, ExecNoMatchLastByte) {
                                         reinterpret_cast<u8 *>(t1),
                                         reinterpret_cast<u8 *>(t1) + len);
 
                                         reinterpret_cast<u8 *>(t1),
                                         reinterpret_cast<u8 *>(t1) + len);
 

-        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len), rv)
+        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len - 1), rv)

             << "Failed for len=" << len;
     }
 }
             << "Failed for len=" << len;
     }
 }


@@ -1330,8 +1328,8 @@ TEST(DoubleShufti, ExecNoMatchLastByteShortBufNullPair) {
                                         reinterpret_cast<u8 *>(t1),
                                         reinterpret_cast<u8 *>(t1) + len);
 
                                         reinterpret_cast<u8 *>(t1),
                                         reinterpret_cast<u8 *>(t1) + len);
 

-        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len), rv)
-            << "False match for len=" << len;
+        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len - 1), rv)
+            << "Failed for len=" << len;

     }
 }
 
     }
 }
 


@@ -1361,8 +1359,8 @@ TEST(DoubleShufti, ExecNoMatchLastByteNullPairVaryLen) {
                                         reinterpret_cast<u8 *>(t1),
                                         reinterpret_cast<u8 *>(t1) + len);
 
                                         reinterpret_cast<u8 *>(t1),
                                         reinterpret_cast<u8 *>(t1) + len);
 

-        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len), rv)
-            << "False match for len=" << len;
+        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len - 1), rv)
+            << "Failed for len=" << len;

     }
 }
 
     }
 }
 


@@ -1424,8 +1422,8 @@ TEST(DoubleShufti, ExecNoMatchLastByteShortBuf) {
                                         reinterpret_cast<u8 *>(t1),
                                         reinterpret_cast<u8 *>(t1) + len);
 
                                         reinterpret_cast<u8 *>(t1),
                                         reinterpret_cast<u8 *>(t1) + len);
 

-        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len), rv)
-            << "False match for len=" << len;
+        ASSERT_EQ(reinterpret_cast<const u8 *>(t1 + len - 1), rv)
+            << "Failed for len=" << len;

     }
 }
 
     }
 }