mruby-regexp: undo what a lookaround captured when the match backtracks past it - #7273

Stacked on #7276, which numbers atomic groups instead of giving them a nesting depth: the first commit here is that PR's, and the second is this one. The review of the first version found that a possessive repeat wrapped around a lookaround shared the lookaround's depth, and #7276 is what tells them apart, for a lookaround here as for an atomic group on master. The second revision adds one thing to the fix: a repeat around a positive lookaround re-enters its sub-pattern while the frames of the run before are still up, and a loop inside the sub-pattern could take the run before's record for its own (/(?=(b|)+)+/ on "b" answered ["", "b"]); the entry records now name the pass that wrote them (the last bullet of the fix). The tests and the differential section cover it, the latter with #7275 merged as well, since the shape mostly sits behind master's refusal of a quantified empty group. The rest is as reviewed.

bt_match() runs a lookaround's sub-pattern in a call of its own. The
sub-pattern ends in RE_MATCH and answers BT_MATCH, so every RE_SAVE
frame on that path returns with its write kept (a write is undone for any
answer other than BT_MATCH), and the frame that ran the opener goes on with
pc = inst.offset in place. From then on the frames that could undo the
sub-pattern's writes are gone: when the text after the lookaround fails and
the engine backtracks past it, nothing undoes what the sub-pattern captured.
A plain group undoes, because its RE_SAVE frames are still on the stack
while the text after it runs:

/(?:(a)b|)/.match("a")[1]         # nil in both
/(?:(?=(a))b|)/.match("a")[1]     # CRuby: nil,   mruby: "a"
/(?=(a))b|/.match("a")[1]         # CRuby: nil,   mruby: "a"
/(?:(?=(a))b)?/.match("a")[1]     # CRuby: nil,   mruby: "a"
/(?:(?=(a))b)*/.match("a")[1]     # CRuby: nil,   mruby: "a"

A negative lookaround leaks the same way one step earlier: RE_NEG_LOOKAHEAD
and RE_NEG_LOOKBEHIND turn the sub-pattern's BT_MATCH into BT_FAIL and
return, and the writes stay:

/(?!(a))|/.match("a")[1]          # CRuby: nil,   mruby: "a"
/(?!(a))*/.match("a")[1]          # CRuby: nil,   mruby: "a"
/(?!(a))?/.match("a")[1]          # CRuby: nil,   mruby: "a"

A backreference to the leaked group then consumes text, and the match itself
changes:

/(?:(?=(a))b|)\1/ =~ "aa"                # CRuby: nil,   mruby: 0
/(?:(?!(a))|a)\1/ =~ "aa"                # CRuby: nil,   mruby: 0
/(?:(?!(a))|a)\1?b/.match("aab")[0]      # CRuby: "ab",  mruby: "aab"

The sub-pattern failing was handled: /(?:(?=(a)b)|)\1/ =~ "ac" is nil in
both, since there the RE_SAVE frames undid on the way up. The leak surfaced
in the differential run for #7269: on master a repetition of such a
lookaround ran to the recursion limit and failed outright, and with the
empty-iteration stop the loop stops, the match goes on, and the leaked
capture is what gets read. The engine was already wrong on master for the
shapes above; #7269 reaches more of them.

The fix

Run the text after the lookaround while the sub-pattern's frames are still on
the stack, which is what RE_ATOMIC and RE_ATOMIC_END already do for
(?>...) (#7256): the body runs on through its end into the text after the
group inside the same call chain, and a failure there comes back as
BT_CUT(number), which every RE_SAVE on the way undoes for, every RE_SPLIT
passes up without trying its other branch, and the RE_ATOMIC of that number
turns into BT_FAIL. A lookaround is that group with two differences, where
the text after it starts from and what the sub-pattern reaching its end
means, so it reuses the machinery rather than adding a second way to undo:

• compile_look_body() ends the sub-pattern with RE_LOOK_END instead of
  RE_MATCH. The end carries the lookaround's number from the count
  (?>...) takes its numbers from, num_cuts, so that a cut is keyed to the
  one construct that absorbs it however groups and lookarounds nest, and
  a = 1 for a negative one. The opener's offset is patched to the instruction
  after the end as before, so the opener finds its end at offset - 1 and
  the end needs no operand for the text after it.
• Every opener runs its sub-pattern through bt_look(), which records the
  position the lookaround was entered at (for a lookbehind that is sp, not
  the rewound start) in the entry record of the RE_LOOK_END, kept for as
  long as the frame runs the way bt_iter() keeps an iteration's start. The
  per-pc array is the one bt_iter() writes, renamed from iter_at to
  entered_at: what a record means is now which pc keys it, a loop edge's
  being where the running iteration began and a lookaround end's where the
  lookaround was entered.
• The end of a positive sub-pattern reads that record and runs the text after
  the lookaround from there in a sub-call. BT_MATCH goes up with the
  captures kept (/(?=(a))a/.match("a")[1] is "a" in CRuby too);
  BT_FAIL becomes BT_CUT(number), so the sub-pattern's RE_SAVE frames
  undo on the way up and no alternative inside it is retried, which is the
  atomic answer the separate call gave before; a limit and another group's
  cut go up as they are.
• The end of a negative sub-pattern answers BT_CUT(number) outright: the
  frames above undo their writes and try no other branch, and bt_look()
  hands the opener the BT_MATCH the sub-pattern's match is, its captures
  unset by then, which is what CRuby reports for a group inside a negative
  lookaround. The sub-pattern running out of alternatives is BT_FAIL, the
  assertion holding, and the opener goes on with pc = inst.offset in place
  as before.
• Each record also names the pass that wrote it, in a second per-pc array,
  entered_in. A pass is one run of a lookaround's sub-pattern, told by the
  depth of the bt_look() frame running it, which no other frame on the
  stack has since every call goes a level deeper, and 0 is the pattern
  outside every lookaround. bt_iter() writes the running pass beside the
  position, and a loop edge reads a record as its iteration's only when the
  pass is its own. What this is for: the text after a positive lookaround
  runs inside the sub-pattern's frames, so a repeat around the lookaround
  re-enters the sub-pattern while the records of the loops inside it from
  the run before are still live, and the first iteration of an e+, which
  reads its record without having written it, would take one of those for
  its own where the positions coincide. /(?=(b|)+)+/ on "b" re-enters at
  0 while the run before left 1 for (b|)+, and its first iteration, ending
  at 1, would stop there with "b" in the group, where CRuby goes round once
  more and leaves "". The RE_LOOK_END runs the text after the lookaround
  in the pass the lookaround was entered from, kept in the end's record too,
  so the loops around a lookaround key their records by one pass throughout.
  Master is not open to this: its opener runs the sub-pattern to RE_MATCH
  and returns, so the records inside are restored before the repeat comes
  round; and a negative lookaround here is not either, its end cutting the
  sub-pattern's frames at once.

The comment above the BT_* codes said a cut never reaches a lookaround from
inside its sub-pattern. With the text after the lookaround running inside the
sub-pattern's frames it does: /(?>(?=a)ab|a)b/ on "ab" sends the atomic
group's cut up through the lookaround's end, its sub-pattern's frames and its
opener, and the opener has to pass it up because the number is not its own,
or the group's other branch would be tried after the cut. That is why the
number is one count with (?>...), and a possessive repeat wrapped around a
lookaround takes a number of its own from it, as it does around an atomic
group (#7276): /(?:(?=a)a)?+a/ =~ "a" is nil, as in CRuby. compute_fixed_len() looked for
RE_MATCH as the end of a lookbehind's sub-pattern and looks for
RE_LOOK_END now; the RE_MATCH ending the whole pattern is untouched, and
the Pike VM runs no pattern with a lookaround in it.

The text after a positive lookaround now runs as many frames deeper as the
sub-pattern took plus two, where before it ran in the opener's frame, so a
repetition of a lookaround reaches MRB_REGEXP_RECURSION_LIMIT in fewer
iterations: /(?:(?=a)a)*/ against 2,000 as stops at 332 iterations, where
it stopped at 998 before and (?:(?>a))* already stopped at 332; CRuby
matches all 2,000. What the engine answers at the limit is the same question
as before, and not this one.

Time

The default gembox at -O3, Time.now over n matches after three
warm-up calls, pinned to one core, mean of three runs each, master and this
PR alternated:

CASES = [
  ["/\\d+(?!%)/ =~ '100%'",           200000, -> { /\d+(?!%)/ =~ "100%" }],
  ["/(?=(a|ab))\\1c/ =~ 'ab' * 50",   50000,  -> { /(?=(a|ab))\1c/ =~ "ab" * 50 }],
  # ...
]
CASES.each do |label, n, blk|
  3.times { blk.call }
  t0 = Time.now
  n.times { blk.call }
  puts "%-42s %10.3f us/iter" % [label, (Time.now - t0) / n * 1e6]
end

A lookaround that holds once and is followed by text that matches costs what
it did. The two rows that slow down are the ones the fix is about: a
lookaround whose sub-pattern matches and whose text after fails, once per
position for /(?=(a|ab))\1c/ and once per iteration for /(?:(?=a)a)*b/
(the loop nests a frame deeper per iteration), where the failure now unwinds
through the sub-pattern's frames instead of returning from the opener's. The
/(?:(?=(a))b|)\1/ row is faster because the backreference no longer finds a
leaked "a" to compare against. /(?=a+b)a+/ pays for the second record
bt_iter() writes per iteration of the loop inside its sub-pattern. /(a|b)*?c\1/
runs no lookaround; its wall clock moves with code layout from build to build
(0.92x in the first revision's measurement, 1.06x here), and it executed 0.5%
fewer instructions under callgrind on the first revision.

Size

.text of bin/mruby, build_config/ci/gcc-clang.rb, each side from a clean
build directory; the #7276 column is the base this stacks on. re_exec.o
and re_compile.o are the objects that change. re_exec.o shrinks by 4,208
in bintest: on master gcc emits bt_match() twice, once as a constprop
clone for the top-level call with pc and depth 0, and here it emits it
once with bt_iter() and bt_look() inlined into it; re_compile.o grows
by 128 over #7276 for compile_look_body().

Verification

The tests go in regexp_syntax.rb beside the lookaround tests: the examples
above for each of the four lookarounds, a backreference reading the leaked
group, the captures a lookaround that holds keeps, the atomic answer of a
positive lookaround (/(?=(a|ab))\1c/ =~ "abc" is nil in both), and the cut
of an atomic group passing through a lookaround on its way to the group,
a possessive repeat wrapped around each kind of lookaround, which cuts as a
group of its own, and a repeat re-entering a positive lookaround whose
sub-pattern holds a loop (/(?=(b|)+)+/ and four more shapes, with text
after the loop, a backreference, a nested lookahead and a possessive repeat).
Twelve of the assertions fail on master, the possessive ones on the first
revision of this PR, and the re-entering ones on the second.

Differential against CRuby 4.0.6, master and this PR against the same
cases. 10,000 random patterns over a, b, a?, b?, [ab]?, \w?,
(?:), (a|), (|b), (?:a|), the four lookarounds, the two lookaheads
also with a capture inside, backreferences, plain, non-capturing and atomic
groups, possessive repeats and alternation, groups nested up to two deep,
every atom quantified with probability 0.5, each run with match against one
subject over a and b and compared as MatchData#to_a. 1,592 of the
patterns both sides refuse to compile (a quantifier on an empty group, which
#7275 lets through) and 2 CRuby cannot finish under a memory cap; the rest
compare:

88 of the 89 have a capture inside a lookaround, and the 89th is
#7276's, a possessive repeat around an atomic group. The 1 line that only this
PR answers differently,
/(?:((?:)(b|)++(?:)|\2??a)$??(?=[a](|a)\1*)??|(?:\2?+)[^a](?<=ab))+(?<=ab)(?<!bb)??/
on "baab", reduces to /(?:((b|)++|a)(?=a(|a)\1*)??|b)+(?<=ab)/: CRuby
reports group 3 as "" at offset 2, written by the lazily optional lookahead
entered at 1, a position where the iteration matches empty; this engine stops
the repetition on that iteration and reports the match it goes on to find,
which never enters the lookahead, so the group is unset. Master agrees with
CRuby by leaking the group from an attempt it backtracked out of, and the
first revision answers as this one does. Of the 23 lines where both differ, 6
have a capture inside a lookaround, each in a pattern with a {n}? (which
this engine reads as a lazy {n}) or a repeat of a group that matches empty,
the standing differences the run for #7269 turned up.

A second run of 10,000 with the features narrowed to lookarounds, captures
inside them, backreferences, atomic groups and empty-matching atoms: 1,621
refused by both, 10 CRuby cannot finish, 8,369 compared, 162 lines master
differs on and this PR agrees, 7 both differ on (1 with a capture inside a
lookaround), 0 only this PR differs on. The first revision differs alone on 1
line of this run, /(?=((?:b|)?(?:b|)+)+)+(?<!bb)/ on "bbab", group 1
"bb" for CRuby's "", which is the re-entry the second revision fixes.

The re-entry mostly hides behind the refusals: a repeat around a positive
lookaround with a loop inside is what the generator draws with a (?:)
quantified, which master refuses, so the narrowed run above shows one line of
it and the full run none. With #7275 merged into both revisions, over the two
runs above and four more of 10,000 (seeds 4 to 7, the full feature set or the
narrowed one, one of them with possessive repeats), the first revision alone
differs from CRuby on 11 lines, /(?=(a|)+)++/ on "abab" the shortest,
answering ["", "a"] for ["", ""], and the second revision on none; the
lines where the merged builds of both revisions differ where master refuses
(3, 1, 3, 2, 3 and 18 per run) are the same on both and are #7275's reading
of a quantified empty group.

rake test, build_config/ci/gcc-clang.rb, no compiler warning:

The default configuration: 2,141 total, 0 KO, 0 crash, plus its 112 bintests.

Environment

# ci/gcc-clang bintest
gcc -MMD -c -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_GC_FIXED_ARENA -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -DMRB_USE_DEBUG_HOOK -I"include" -I"mrbgems/mruby-regexp/include" -I"build/bintest/include" -o "build/bintest/mrbgems/mruby-regexp/src/re_exec.o" "mrbgems/mruby-regexp/src/re_exec.c"

# ci/gcc-clang full-debug
gcc -MMD -c -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -g3 -O0 -DMRB_GC_STRESS -DMRB_USE_DEBUG_HOOK -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_DEBUG -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -I"include" -I"mrbgems/mruby-regexp/include" -I"build/full-debug/include" -o "build/full-debug/mrbgems/mruby-regexp/src/re_exec.o" "mrbgems/mruby-regexp/src/re_exec.c"

# ci/gcc-clang cxx_abi
gcc -MMD -c -g -O3 -Wall -Wundef -Wwrite-strings -x c++ -std=gnu++03 -DMRB_GC_FIXED_ARENA -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_USE_CXX_EXCEPTION -DMRB_USE_CXX_ABI -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -I"include" -I"mrbgems/mruby-regexp/include" -I"build/cxx_abi/include" -o "build/cxx_abi/mrbgems/mruby-regexp/src/re_exec.o" "mrbgems/mruby-regexp/src/re_exec.c"

# ci/gcc-clang byte-string
gcc -MMD -c -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -I"include" -I"mrbgems/mruby-regexp/include" -I"build/byte-string/include" -o "build/byte-string/mrbgems/mruby-regexp/src/re_exec.o" "mrbgems/mruby-regexp/src/re_exec.c"

# ci/gcc-clang ascii-ctype
gcc -MMD -c -std=gnu99 -g -O3 -Wall -Wundef -Werror-implicit-function-declaration -Wwrite-strings -DMRB_USE_ASCII_CTYPE -DMRBGEM_MRUBY_REGEXP_VERSION=0.0.0 -DMRB_USE_BIGINT -DMRB_USE_COMPLEX -DHAVE_MRUBY_ENCODING_GEM -DMRB_UTF8_STRING -DHAVE_MRUBY_IO_GEM -DMRB_USE_RATIONAL -DHAVE_MRUBY_REGEXP_GEM -DMRB_USE_SET -DMRB_USE_TASK_SCHEDULER -I"include" -I"mrbgems/mruby-regexp/include" -I"build/ascii-ctype/include" -o "build/ascii-ctype/mrbgems/mruby-regexp/src/re_exec.o" "mrbgems/mruby-regexp/src/re_exec.c"

Summary by CodeRabbit

• Bug Fixes

  • Improved regular-expression lookahead and lookbehind behavior during backtracking.
  • Fixed capture handling so failed or backtracked assertions no longer leave incorrect captures.
  • Improved interactions between lookarounds, atomic groups, possessive repeats, and backreferences.
  • Ensured nested atomic constructs operate independently and preserve expected matching behavior.
  • Fixed handling of quantifiers applied to empty-matching expressions.

• Tests

  • Added regression coverage for lookarounds, capture rollback, alternate branches, and possessive-repeat atomicity.