From: yoavwizstein Date: Fri, 3 Jul 2026 10:14:22 +0000 (+0000) Subject: Fix double-shufti accelerator missing matches across stream chunk boundaries (#402) X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;p=thirdparty%2Fvectorscan.git 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 9b3d5a29..d4d7b947 100644 --- a/src/nfa/shufti_simd.hpp +++ b/src/nfa/shufti_simd.hpp @@ -213,8 +213,10 @@ const u8 *check_last_byte(SuperVector mask2_lo, SuperVector mask2_hi, } 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; } diff --git a/src/nfa/shufti_sve.hpp b/src/nfa/shufti_sve.hpp index 2348c091..73cf381d 100644 --- 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; - 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; } diff --git a/unit/hyperscan/identical.cpp b/unit/hyperscan/identical.cpp index cbeb0242..2cfbfca1 100644 --- 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; } +// 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 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 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 diff --git a/unit/internal/shufti.cpp b/unit/internal/shufti.cpp index 9b196714..bc4e8553 100644 --- 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(t2), reinterpret_cast(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(t2 + len), rv); - }else { - ASSERT_EQ(reinterpret_cast(&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(&t2[len - i]), rv); } } @@ -933,7 +930,8 @@ TEST(DoubleShufti, ExecNoMatchVectorEdge) { const u8 *rv = shuftiDoubleExec(lo1, hi1, lo2, hi2, reinterpret_cast(t1), reinterpret_cast(t1) + len); - ASSERT_EQ(reinterpret_cast(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(t1 + len - 1) : reinterpret_cast(t1 + len), rv); } } @@ -1046,7 +1044,7 @@ TEST(DoubleShufti, ExecNoMatchLastByte) { reinterpret_cast(t1), reinterpret_cast(t1) + len); - ASSERT_EQ(reinterpret_cast(t1 + len), rv) + ASSERT_EQ(reinterpret_cast(t1 + len - 1), rv) << "Failed for len=" << len; } } @@ -1330,8 +1328,8 @@ TEST(DoubleShufti, ExecNoMatchLastByteShortBufNullPair) { reinterpret_cast(t1), reinterpret_cast(t1) + len); - ASSERT_EQ(reinterpret_cast(t1 + len), rv) - << "False match for len=" << len; + ASSERT_EQ(reinterpret_cast(t1 + len - 1), rv) + << "Failed for len=" << len; } } @@ -1361,8 +1359,8 @@ TEST(DoubleShufti, ExecNoMatchLastByteNullPairVaryLen) { reinterpret_cast(t1), reinterpret_cast(t1) + len); - ASSERT_EQ(reinterpret_cast(t1 + len), rv) - << "False match for len=" << len; + ASSERT_EQ(reinterpret_cast(t1 + len - 1), rv) + << "Failed for len=" << len; } } @@ -1424,8 +1422,8 @@ TEST(DoubleShufti, ExecNoMatchLastByteShortBuf) { reinterpret_cast(t1), reinterpret_cast(t1) + len); - ASSERT_EQ(reinterpret_cast(t1 + len), rv) - << "False match for len=" << len; + ASSERT_EQ(reinterpret_cast(t1 + len - 1), rv) + << "Failed for len=" << len; } }