Skip to content

Fix interpreter callout start_match after \K - #1043

Merged
NWilson merged 2 commits into
PCRE2Project:mainfrom
CAOShurong:codex/fix-callout-start-after-reset
Oct 23, 2026
Merged

NWilson merged 2 commits into
PCRE2Project:mainfrom
CAOShurong:codex/fix-callout-start-after-reset

Conversation

@CAOShurong

Copy link
Copy Markdown
Contributor

Fixes #1040.

Set the interpreter callout's start_match from the current match frame, so \K and backtracking report the same start offset as JIT.

Add regressions for numeric, string, automatic and conditional callouts, repeated resets, subroutines and retries. Full CTest passes on Windows/MSVC with 8-, 16- and 32-bit libraries and JIT; an installed CMake consumer also passes the API checks.

@bsloane1650

Copy link
Copy Markdown

I can confirm that this fixes #1040

@NWilson

NWilson commented Oct 17, 2026

Copy link
Copy Markdown
Member

Great, I have a few review comments.

  1. There's a dead assignment you could now remove, please:
    /* OK, we can now run the match. If "hitend" is set afterwards, remember the
    first starting point for which a partial match was found. */

    cb.start_match = (PCRE2_SIZE)(start_match - subject);
    cb.callout_flags |= PCRE2_CALLOUT_STARTMATCH;

The callout_flags do need to be updated, but the write to start_match now has no effect.

No thanks to GPT - I found that one myself.

  1. GPT suggests the following additional test case. I think I agree it improves path coverage:
/(?C1)A\K(?C2)B/no_start_optimize
    _AC_AB_

Everything else looks good, to me.

I can see that the documentation already agrees with this change. I can see that the DFA engine already has the correct behaviour (it doesn't have \K, but it's using this pattern already to set up the callout struct).

@CAOShurong

Copy link
Copy Markdown
Contributor Author

Removed the redundant assignment and added the no_start_optimize bumpalong case. I also merged current main and resolved the changelog conflict. The native test suite and the new case pass for 8-, 16-, and 32-bit builds with and without JIT on Windows.

@NWilson NWilson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you! This looks good, I will merge after CI completes

@NWilson
NWilson merged commit 4ffe2a4 into PCRE2Project:main Oct 23, 2026
45 checks passed
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.

\K produces inconsistent behaviour with pcre2_callout_block->start_match when not using JIT

3 participants