Make partial matching consistent between JIT and interpreter (3/3, stacked on #1005) - #1006
Open
shivneelmistry wants to merge 3 commits into
Open
shivneelmistry wants to merge 3 commits into
shivneelmistry wants to merge 3 commits into
Conversation
Four fixes found by differential testing of the JIT against the interpreter: - JIT: a variable-length lookbehind no longer moves STR_END to the lookbehind point. Assertions inside the lookbehind (\b, \B, $, lookaheads) now see the real end of the subject, and each branch is checked to end exactly at the lookbehind point instead. This makes the STR_END restore added for OP_ASSERTBACK_NA backtracking in PCRE2Project#912 unnecessary; its test still passes. Example: /y(?<!.{1,2}\b)b/ on "eyb" matched with the interpreter only. - JIT: (*THEN) in a branch of a variable-length lookbehind retried the next start position instead of moving to the next alternative. Example: /(?<=a?(*THEN)(?<=e)|x)B/ on "eBx" matched with JIT only. - JIT: (*THEN) in a standalone positive assertion nested inside a negative assertion escaped to the outer assertion, contrary to pcre2pattern ("The effect of (*THEN) is not allowed to escape beyond an assertion"). Example: /(?!(?=(*THEN)(*F))?)/ on "x" matched with JIT only. - Compile: a conditional group without a "no" branch inside a lookbehind was given the length of its "yes" branch only, so the lookbehind was treated as fixed length. Both matchers could miss matches, and the JIT could return a match that ends before it starts. Example: /(*naplb:(?(?=x)a))/ at offset 2 of "ae" returned (2,1) from the JIT. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two JIT bugs where a match attempt started at the wrong place:
- With PCRE2_USE_OFFSET_LIMIT, the bumpalong loop checked the limit
before advancing, so a multi-unit UTF character or a skipped CRLF
could move the next attempt past the limit.
Example: /\b/utf,use_offset_limit on "\x{300}\x{4e2d}\x{1f600}1" with
offset_limit=6 matched at offset 9 with JIT only.
- scan_prefix() kept the repeat count from an OP_TYPEEXACT whose type it
could not handle and applied it to the next alternative. The computed
prefix was then wrong and fast-forwarding skipped real matches (and it
tripped an SLJIT_ASSERT in debug builds).
Example: /(?:xyzw\R{4}|abc)/ did not match "abc" with JIT.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Five places where pcre2_match() and the JIT disagreed about reporting a partial match. Two are JIT fixes that follow the documented behaviour; three change the interpreter and are judgement calls - happy to move any of them to the other matcher instead. JIT: - \R at a CR that ends the subject did not record a possible partial match in soft mode (the comment said it was not needed), so a later failure lost it. Example: /\R(*F)/ on "\r" with partial_soft. - check_str_end() ignored allow_empty_partial, unlike check_partial() and detect_partial_match(), so \b at the end of an empty subject gave no partial match in hard mode although pcre2partial says \b "always" gives one. Example: /\b/ on "" with partial_hard. Interpreter: - A back reference to an unset group at the end of the subject reported a partial match, although no extra characters can make it match. The maximizing repeat path already reported partial only when the subject ran out, as the JIT does. Example: /(?<!(x))\1/ on "ab". - Repeated \R (\R?, \R+, ...) did not check for a CR at the end of the subject as a single \R does. Example: /x\R?/ on "x\r" with partial_hard gave a complete match. - The anchored first code unit check rejected an empty remainder in partial mode; the unanchored path deliberately lets that attempt run. Example: /^\bw/ on "" with partial_hard. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Sep 24, 2026
Member
|
Nice! I will review just the pcre2_match changes in this one. |
Member
|
@shivneelmistry Would you be able to verify whether you fixed this existing case: #974 |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1005, which is stacked on #1004. Only the last commit is new; review f7c5fce. I'll rebase as the earlier PRs merge.
This is the last of three stacked PRs from JIT vs. interpreter differential fuzzing:
(*THEN)in assertionsThese are five cases where
pcre2_match()and the JIT disagreed about reporting a partial match. None of them affects complete (non-partial) matching.The three interpreter changes are judgement calls. For each one I picked the behaviour that matches pcre2partial and the matchers' other code paths, but the opposite fix (changing the JIT instead) would also make them consistent. I'm happy to switch any of them, or split them out, if you prefer.
JIT fixes
\Rat a CR that ends the subject now records a possible partial match in soft mode, ascheck_str_end()does; the old comment said it was not needed/\R(*F)/\r\=pscheck_str_end()now honoursallow_empty_partiallikecheck_partial()anddetect_partial_match(); pcre2partial says\bat the end "always" gives a partial match withPARTIAL_HARD/\b/\=ph(empty)Interpreter changes (judgement calls)
rrc > 0)/(?<!(x))\1/ab\=ps\R(\R?,\R+,\R{n,m}, min and max loops) now checks for a CR at the end of the subject like a single\R/x\R?/x\r\=phx\r(?<=abc)defcomment there)/^\bw/\=ph(empty)Tests
testinput2cases. Unpatched, 7 give different output with JIT and 5 without JIT.pcre2_jit_test.c.The whole stack was validated as described in #1004.
🤖 Generated with Claude Code