Skip to content

shufti-double: fix regressions from #325#368

Merged
markos merged 4 commits intoVectorCamp:developfrom
AhnLab-OSSG:shufti-double-last-byte-v3
Apr 7, 2026
Merged

shufti-double: fix regressions from #325#368
markos merged 4 commits intoVectorCamp:developfrom
AhnLab-OSSG:shufti-double-last-byte-v3

Conversation

@byeonguk-jeong
Copy link
Copy Markdown

@byeonguk-jeong byeonguk-jeong commented Mar 4, 2026

  1. As reported in BUG: scanning returns no matches with multi-pattern database; but minimal set still matches #358, Fix double shufti's vector end false positive #325 introduced a false negative. The caller side told the buffer length as c_end - 1, so that shuftiDoubleExec() unexpectedly tried to match the byte before the last byte with single char patterns.

  2. check_last_byte() reduces the mask to make a wildcard mask, which matches any character that could match as the second byte of a double-byte pattern. However, the loop condition i > 2 skipped the final vshr(1) step, causing only even-indexed bytes to be folded into the reduce result.

  3. After peel-off stage for alignment, the carry-over state was discarded.

  4. Add some unit tests for this fixes.

@AhnLab-OSS @AhnLab-OSSG

@byeonguk-jeong
Copy link
Copy Markdown
Author

byeonguk-jeong commented Mar 5, 2026

Test failures on AVX512 are fixed at #373. It is not related to these commits.

Copy link
Copy Markdown

@ypicchi-arm ypicchi-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside the assumption of vector size in the test, LGTM. Thanks for the fix.

Comment thread unit/internal/shufti.cpp Outdated
@markos
Copy link
Copy Markdown

markos commented Apr 3, 2026

Could you please rebase this one against current develop as well?

@byeonguk-jeong byeonguk-jeong force-pushed the shufti-double-last-byte-v3 branch from ee06af2 to 80a762a Compare April 6, 2026 01:04
@byeonguk-jeong
Copy link
Copy Markdown
Author

Could you please rebase this one against current develop as well?

Done. :)

@markos
Copy link
Copy Markdown

markos commented Apr 6, 2026

There seems to be a cppcheck related error, could you please check and while at it rebase against current develop again, as there have been some PRs merged? Thanks again.

run_accel() passed c_end - 1 to shuftiDoubleExec().
shuftiDoubleExecReal already handles the last-byte boundary internally
via check_last_byte(), so shortening the buffer caused it to miss
valid matches near the end and apply the wildcard check to the wrong
byte. Changed to pass c_end.

Fixes: ca70a3d ("Fix double shufti's
vector end false positive (VectorCamp#325)")

Signed-off-by: Byeonguk Jeong <jungbu2855@gmail.com>
first_char_mask was reset to Ones() after the peel-off block,
discarding carry-over state for cross-boundary pattern detection.
Remove the reset.

Fixes: ca70a3d ("Fix double shufti's
vector end false positive (VectorCamp#325)")

Signed-off-by: Byeonguk Jeong <jungbu2855@gmail.com>
loop condition i > 2 missed the final vshr(1), leaving odd-indexed
bytes out of the reduce. Use i >= 2.

Fixes: ca70a3d ("Fix double shufti's
vector end false positive (VectorCamp#325)")

Signed-off-by: Byeonguk Jeong <jungbu2855@gmail.com>
Add three tests exercising the double shufti edge cases.

- ExecMatchVectorEdge: Two-byte pair ("ab") spanning the peel-off to
  aligned block boundary is correctly detected. Validates that
  first_char_mask state carries over and is not reset after peel-off.

- ExecNoMatchLastByte: First character of a double-byte pair ('x' from
  "xy") at the last buffer byte does not cause a false positive when
  the second character is absent.

- ExecMatchLastByte: Single-byte pattern ('a') at the last buffer byte
  is detected via check_last_byte's reduce.

Signed-off-by: Byeonguk Jeong <jungbu2855@gmail.com>
@byeonguk-jeong byeonguk-jeong force-pushed the shufti-double-last-byte-v3 branch from 80a762a to 5279232 Compare April 6, 2026 23:15
@byeonguk-jeong
Copy link
Copy Markdown
Author

There seems to be a cppcheck related error, could you please check and while at it rebase against current develop again, as there have been some PRs merged? Thanks again.

The error may not from the source code itself. cppcheck has been crashed during the test. Anyway, I just have rebased. :)

Copy link
Copy Markdown

@markos markos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, again thank you for another high quality patch!

@markos markos merged commit de31729 into VectorCamp:develop Apr 7, 2026
109 of 111 checks passed
@byeonguk-jeong byeonguk-jeong deleted the shufti-double-last-byte-v3 branch April 7, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants