[ybai08]

Fixes #1045.

Problem

When a group called with a capture list (for example (?&r(2))) ends with (*ACCEPT), the interpreter's ACCEPT-in-recursion path goes back to the caller's frame and moves past OP_RECURSE, but it does not handle the OP_CREF list that follows. Matching then resumes at OP_CREF, which reaches the unreachable-opcode default: in match():

/(?&r(2))b(?(DEFINE)(?<r>(a)(*ACCEPT)x))/
    ab

• Debug build: Assertion failed: ... "Execution reached unexpected point" ... pcre2_match.c, line 7639
• PCRE2_DEBUG=OFF build: Failed: error -44: internal error - pattern overwritten?
• JIT: 0: ab, 1: <unset>, 2: a

The callee's captures were also discarded on this path, so skipping past the list would not have been enough.

Fix

The ACCEPT path now does what the normal end-of-recursion path (OP_KET for the called group) already does. If the OP_RECURSE is followed by a capture list, recurse_update_offsets() builds the caller's capture vector in the current (callee) frame, keeping the listed groups from the callee, and moves past the OP_CREF list. The result and its offset_top are copied into the caller's frame before matching continues there. When there is no capture list, behaviour is the same as before: the caller's frame is resumed just after OP_RECURSE.

Tests

New cases sit at the end of the "recurse with returned captures" block in testinput2/testoutput2:

• the reproducer from the issue;
• a caller capture that is kept while a different group is returned;
• a returned group that is still unset when (*ACCEPT) fires;
• two calls returning different groups;
• whole-pattern recursion (?R(1));
• a no-match case showing that (*ACCEPT) still does not backtrack into the called group.

The expected output was generated by the patched interpreter and agrees with JIT. RunTest checks test 2 against both engines.

Without the fix, ../RunTest 2 fails in the interpreter at all three widths with the assertion above, and passes OK with JIT. With the fix, it passes in both modes at 8, 16 and 32 bits.

What was run

• CMake, Clang, Debug, JIT on, 8/16/32-bit, warnings as errors: ctest, 4/4 passed.
• The same with the devcontainer-asan-ubsan preset's flags: ctest, 4/4 passed.
• Release, PCRE2_DEBUG=OFF, JIT off: ctest, 3/3 passed.
• About 24,000 generated patterns mixing (*ACCEPT), subroutine calls and capture lists were compared between the interpreter and JIT (part of them under ASan/UBSan). No crashes, and no differences apart from the existing ones mentioned below.

Not run: Valgrind, GCC, Autotools, maint/ManyConfigTests, EBCDIC, Windows, and maint/ApplySourceFormatters.py (clang-format-20 is not installed; the new code follows the surrounding formatting by hand).

Two existing interpreter/JIT differences in this area turned up while testing and are not changed here: trailing <unset> groups after a return with a capture list (same on main), and JIT keeping a returned capture from an abandoned attempt in some ACCEPT-in-subroutine cases. Happy to file separate issues with reproducers if that is useful.

This touches the same OP_ACCEPT recursion path that #1011 routes OP_ASSERT_ACCEPT through, so whichever lands second may need a small rebase.

[NWilson]

Two existing interpreter/JIT differences in this area turned up while testing and are not changed here: trailing <unset> groups after a return with a capture list (same on main), and JIT keeping a returned capture from an abandoned attempt in some ACCEPT-in-subroutine cases. Happy to file separate issues with reproducers if that is useful.

Yes, that would be useful, thanks!

I think one of them has the following reproducer:

/^(?:(?&r(2))X|a)(?(2)Y|b)$(?(DEFINE)(?<r>(a)(*ACCEPT)x))/
    ab
    aY

# interpreter, with fix in this PR: matches ab, rejects aY -- correct.
# JIT: rejects ab, matches aY -- incorrectly retains capture 2

[NWilson]

I think there's one more thing that's needed before (*ACCEPT) works fully with the 'return captures' feature.

Here's an example:

/^(?&r(2))b\2$(?(DEFINE)(?<r>(a(*ACCEPT)x)))/
    aba

The documentation for (*ACCEPT) was written before the 'return captures' feature, but it does say that any in-progress capture groups are automatically closed when the ACCEPT is processed.

Unfortunately, the bytecode before ACCEPT which does this is skipped inside recursions; instead in should be changed to auto-close the groups that we are returning.

[ybai08]

Thanks, filed both: #1047 (JIT keeping the capture from the abandoned attempt, with your reproducer) and #1048 (different capture counts when the recursion returns the called group itself).

[ybai08]

Thanks, pushed a follow-up commit for this. OP_CLOSE is now processed inside recursions too. The search for the group's frame stops at the recursion's frame, so only groups opened within the recursion are closed: the called group itself and groups still open in the caller are left alone, which keeps the called group consistent with the normal end of a recursion. Your example now gives 0: aba / 1: <unset> / 2: a in the interpreter.

New tests in testinput2 cover your example, two nested open groups returned together, an open group that is not listed (it stays unset), whole-pattern recursion (?R(1)), and a group that is open in the caller. JIT still returns "No match" for the first four, so those are marked (*NO_JIT); I can file a separate JIT issue for that if you'd like.

One side effect to check: this also applies to (*ACCEPT) in an assertion inside a recursion without a capture list. For example /^(?1)(?(DEFINE)((?=(a(*ACCEPT)b))\2a))$/ on aa now matches in the interpreter, the same as the non-recursive /^(?=(a(*ACCEPT)b))\1a$/; JIT doesn't match it. If you'd rather limit the closing to recursions with a capture list, I can do that.

Run: CMake Debug build with -DPCRE2_SUPPORT_JIT=ON -DPCRE2_BUILD_PCRE2_16=ON -DPCRE2_BUILD_PCRE2_32=ON -DPCRE2_DEBUG=ON, then ctest (4/4 pass; Test 2 OK with and without JIT at 8/16/32 bits), and the same with -fsanitize=address,undefined. With the pcre2_match.c change reverted, the new tests fail in Test 2. clang-format-20 was not run.

[NWilson]

Thank you!

Completely by chance, there's another open PR right now (#1011) which also deals with ACCEPT, and adds a new Fcurrent_recurse value for assertions.

That could be very useful here.

I'm going to be quite cautious about altering the behaviour of ACCEPT in general. The case of ACCEPT inside assertions and recursions (and assertions within recursions, and recursions within assertions) is something that we can compare against Perl (current and old versions). In each case, knowing whether an in-progress group is closed by the ACCEPT should have an observable effect that can be tested with a Perl script and pcre2test case.

I'll take a few days to think about this. I would be willing to alter the behaviour for cases other than recurse-with-returned-groups, but only if it's a correctness improvement (not just accidental or convenient), and only if there's no compatibility issue with other engines.

[NWilson]

The best route here is probably to merge the PR #1011 as a pre-requisite, and then rebase this one.

The behaviour of ACCEPT should ultimately depend on whether the heapframe-stack shows the nearest enclosing block to be an assertion, or a subroutine, or neither. That depends on how the ACCEPT was reached, dynamically.

[ybai08]

Sounds good. I'll wait for #1011 to land, then rebase this on top. I'll redo the closing of in-progress groups so that it depends on whether the nearest enclosing frame is an assertion, a subroutine or neither, and I'll check the cases against Perl as you describe.