Right Shift in rshift64_m128 fallback path (ARM NEON) (#396)

* Added testcase for issue #326, verified to work on Linux

* Fix cppcheck warnings

* Add comment pointing to the github issue

* simd: use unsigned shift intrinsics in ARM NEON rshift fallback paths

The fallback paths (when HAVE__BUILTIN_CONSTANT_P is not defined, i.e.
clang) in rshift_m128 and rshift64_m128 used vshlq_s32/vshlq_s64
(signed shift), which performs arithmetic right shift with sign
extension. This caused incorrect nibble extraction in shufti validation
when input bytes >= 0x80 landed at byte positions 7 or 15 within a
128-bit register.

Change all four shift helpers (lshift_m128, rshift_m128, lshift64_m128,
rshift64_m128) to use unsigned shifts.

Fixes: 4b41c5fe254e311d193b350a827c81885a30157a ("[NEON] simplify/optimize shift/align primitives")
Reported-by: Alexey Pismensky (#326)
Signed-off-by: Byeonguk Jeong <jungbu2855@gmail.com>
---------

Signed-off-by: Byeonguk Jeong <jungbu2855@gmail.com>
Co-authored-by: Konstantinos Margaritis <konstantinos@vectorcamp.gr>

diff --git a/src/util/arch/arm/simd_utils.h b/src/util/arch/arm/simd_utils.h
index 45c00a2c51dfd8a27c81f929809c9e4f430ef29f..f3dd13a9d86f4e46b92fe5182a55d9f5221c4aa0 100644 (file)
--- a/src/util/arch/arm/simd_utils.h
+++ b/src/util/arch/arm/simd_utils.h
@@ -113,7 +113,7 @@ m128 lshift_m128(m128 a, unsigned b) {
     }
 #endif
   int32x4_t shift_indices = vdupq_n_s32(b);
-  return (m128) vshlq_s32(a, shift_indices);
+  return (m128) vshlq_u32((uint32x4_t)a, shift_indices);
 }
 
 static really_really_inline
@@ -124,7 +124,7 @@ m128 rshift_m128(m128 a, unsigned b) {
     }
 #endif
   int32x4_t shift_indices = vdupq_n_s32(-b);
-  return (m128) vshlq_s32(a, shift_indices);
+  return (m128) vshlq_u32((uint32x4_t)a, shift_indices);
 }
 
 static really_really_inline
@@ -135,7 +135,7 @@ m128 lshift64_m128(m128 a, unsigned b) {
     }
 #endif
   int64x2_t shift_indices = vdupq_n_s64(b);
-  return (m128) vshlq_s64((int64x2_t) a, shift_indices);
+  return (m128) vshlq_u64((uint64x2_t) a, shift_indices);
 }
 
 static really_really_inline
@@ -146,7 +146,7 @@ m128 rshift64_m128(m128 a, unsigned b) {
     }
 #endif
   int64x2_t shift_indices = vdupq_n_s64(-b);
-  return (m128) vshlq_s64((int64x2_t) a, shift_indices);
+  return (m128) vshlq_u64((uint64x2_t) a, shift_indices);
 }
 
 static really_inline m128 eq128(m128 a, m128 b) {

diff --git a/unit/hyperscan/regressions.cpp b/unit/hyperscan/regressions.cpp
index eb3bcd90d363f72839273bc2c25f338aa2dca166..5f1b2b0a1abee16264b76ed22069440bffd2c5d0 100644 (file)
--- a/unit/hyperscan/regressions.cpp
+++ b/unit/hyperscan/regressions.cpp
@@ -462,4 +462,40 @@ TEST(ArmRegression, NoDotAll_Long) {
 
     hs_free_scratch(scratch);
     hs_free_database(db);
+}
+
+
+TEST(utf8, charclass_issue_326) {
+    /*
+     * This is a modified test case from https://github.com/VectorCamp/vectorscan/issues/326
+     * It includes both test patterns mentioned in the issue.
+     */
+    vector<pattern> unicode_patterns = {
+    pattern(R"(\x{ff15}\x{ff10}\x{ff17}\x{ff15}\x{ff10}[\x{ff10}-\x{ff19}]{7})",
+        HS_FLAG_DOTALL | HS_FLAG_PREFILTER | HS_FLAG_MULTILINE | HS_FLAG_CASELESS | HS_FLAG_UCP | HS_FLAG_UTF8, 1),
+    pattern(R"(NL[0-9\x{ff10}-\x{ff19}]{2}[A-Z\x{ff21}-\x{ff3a}a-z\x{ff41}-\x{ff5a}]{4}[0-9\x{ff10}-\x{ff19}]{10})",
+        HS_FLAG_PREFILTER | HS_FLAG_SINGLEMATCH| HS_FLAG_UTF8, 2)
+    };
+    const char *data1 = "５０７５０７８３２４０１";
+    const char *data2 = "NL２０INGB０００１２３４567";
+
+    hs_database_t *db = buildDB(unicode_patterns, HS_MODE_NOSTREAM);
+    ASSERT_NE(nullptr, db);
+
+    hs_scratch_t *scratch = nullptr;
+    hs_error_t err = hs_alloc_scratch(db, &scratch);
+    ASSERT_EQ(HS_SUCCESS, err);
+
+    CallBackContext c1, c2;
+    err = hs_scan(db, data1, strlen(data1), 0, scratch, record_cb, reinterpret_cast<void *>(&c1));
+    ASSERT_EQ(HS_SUCCESS, err);
+    
+    err = hs_scan(db, data2, strlen(data2), 0, scratch, record_cb, reinterpret_cast<void *>(&c2));
+    ASSERT_EQ(HS_SUCCESS, err);
+
+    ASSERT_EQ(MatchRecord(36, 1), c1.matches[0]);
+    ASSERT_EQ(MatchRecord(36, 2), c2.matches[0]);
+    err = hs_free_scratch(scratch);
+    ASSERT_EQ(HS_SUCCESS, err);
+    hs_free_database(db);
 }
\ No newline at end of file