Fix longstanding regex interpreter bug around lazy loops with empty matches (#120872)

This has been an issue in the interpreter forever, as far as I can tell.
We've had multiple issues over the years all flagging problems with
different symptoms that stem from the same core problem.

Fixes #43314
Fixes #58786
Fixes #63385
Fixes #111051
Fixes #114626

The problem came down to how the regex interpreter handled lazy
quantifiers over expressions that can match the empty string. When the
interpreter reaches one of these lazy loops, it uses an internal
instruction called `Lazybranchmark` to manage entering and potentially
looping the subexpression. To keep track of loop state and capture
boundaries, the interpreter uses two internal stacks, a grouping stack,
that tracks positions relevant to capturing groups (e.g. where a group
started), and a backtracking stack, that tracks states that are needed
if the engine has to go back and try a different match. The bug occurred
in the case when the subpattern inside the lazy loop matches nothing. In
this case, the interpreter unconditionally pushed a placeholder onto the
grouping stack. If the rest of the pattern then succeeded without
backtracking through this loop, that extra placeholder remained on the
grouping stack. This polluted the capture bookkeeping: later parts of
the pattern popped that placeholder, treating it as a real start
position, and shifted captures to the wrong place.

The fix is to stop pushing onto the grouping stack when the loop matches
empty. Instead, the interpreter records two things on the backtracking
stack: the old group boundary and a flag indicating whether the grouping
stack needs to be popped later. If the interpreter ends up backtracking
through this lazy loop, it checks the flag: if a grouping stack entry
was added earlier, it pops it; if not, it leaves the grouping stack
untouched. This keeps the grouping stack and backtracking stack in sync
in both forward and backtracking paths. As a result, empty lazy loops no
longer leave stray entries on the grouping stack. This also prevents the
unbounded stack growth that previously caused overflows or hangs on some
patterns involving nested lazy quantifiers.

Author: stephentoub

src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexInterpreter.cs

@@ -521,12 +521,10 @@ private bool TryMatchAtCurrentPosition(ReadOnlySpan<char> inputSpan)
                             }
                             else
                             {
-                                // The inner expression found an empty match, so we'll go directly to 'back2' if we
-                                // backtrack.  In this case, we need to push something on the stack, since back2 pops.
-                                // However, in the case of ()+? or similar, this empty match may be legitimate, so push the text
-                                // position associated with that empty match.
-                                StackPush(oldMarkPos);
-                                TrackPush2(StackPeek()); // Save old mark
+                                // The inner expression found an empty match. We'll go directly to BacktrackingSecond
+                                // if we backtrack. We do not touch the grouping stack here... instead, we record the
+                                // old mark and a "no-stack-pop" flag (0) on the backtracking stack.
+                                TrackPush2(oldMarkPos, 0);
                             }
                         }
                         advance = 1;
@@ -541,7 +539,11 @@ private bool TryMatchAtCurrentPosition(ReadOnlySpan<char> inputSpan)
                             // fails, we go to Lazybranchmark | RegexOpcode.Back2
                             TrackPop(2);
                             int pos = TrackPeek(1);
-                            TrackPush2(TrackPeek()); // Save old mark
+
+                            // Store the previous mark. The second value (1) flags that a grouping stack frame
+                            // must be popped when backtracking, because a new frame will be pushed next.
+                            TrackPush2(TrackPeek(), 1);
+
                             StackPush(pos);          // Make new mark
                             runtextpos = pos;        // Recall position
                             Goto(Operand(0));        // Loop
@@ -551,9 +553,17 @@ private bool TryMatchAtCurrentPosition(ReadOnlySpan<char> inputSpan)
                     case RegexOpcode.Lazybranchmark | RegexOpcode.BacktrackingSecond:
                         // The lazy loop has failed.  We'll do a true backtrack and
                         // start over before the lazy loop.
-                        StackPop();
-                        TrackPop();
-                        StackPush(TrackPeek()); // Recall old mark
+                        int needsPop = runtrack![runtrackpos]; // flag: 0 or 1
+                        int oldMark = runtrack[runtrackpos + 1]; // saved old mark
+                        runtrackpos += 2; // consume both payload ints
+                        if (needsPop != 0)
+                        {
+                            // We pushed on the grouping stack in the Backtracking arm; balance it now.
+                            StackPop();
+                        }
+
+                        // Restore the old mark and backtrack
+                        StackPush(oldMark);
                         break;
 
                     case RegexOpcode.Setcount:

src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.Match.Tests.cs

@@ -329,6 +329,26 @@ public static IEnumerable<object[]> Match_MemberData()
             yield return (@"(b|a|aa)((?:aa)+?)+?$", "aaaaaaaa", RegexOptions.None, 0, 8, true, "aaaaaaaa");
             yield return (@"(|a|aa)(((?:aa)+?)+?|aaaaab)\w$", "aaaaaabc", RegexOptions.None, 0, 8, true, "aaaaaabc");
 
+            // Lazy loops with empty matches
+            if (!PlatformDetection.IsNetFramework)
+            {
+                // Fails on .NET Framework: https://github.com/dotnet/runtime/issues/111051
+                yield return (@"(.)+()+?b", "xyzb", RegexOptions.None, 0, 4, true, "xyzb");
+                yield return (@"^(.)+()+?b", "xyzb", RegexOptions.None, 0, 4, true, "xyzb");
+
+                if (!RegexHelpers.IsNonBacktracking(engine))
+                {
+                    // Fails on .NET Framework: https://github.com/dotnet/runtime/issues/58786
+                    yield return (@"(?<!a*(?:a?)+?)", @"x", RegexOptions.None, 0, 1, false, "");
+
+                    // Fails on .NET Framework: https://github.com/dotnet/runtime/issues/114626
+                    yield return (@"(?>(-*)+?-*)$", "abc", RegexOptions.None, 0, 3, true, "");
+
+                    // Fails on .NET Framework: https://github.com/dotnet/runtime/issues/63385
+                    yield return (@"(^+?)?()", "1", RegexOptions.None, 0, 1, true, "");
+                }
+            }
+
             // Nested loops
             yield return (@"(abcd*)+e", "abcde", RegexOptions.None, 0, 5, true, "abcde");
             yield return (@"(abcd*?)+e", "abcde", RegexOptions.None, 0, 5, true, "abcde");

src/libraries/System.Text.RegularExpressions/tests/FunctionalTests/Regex.MultipleMatches.Tests.cs

@@ -452,9 +452,18 @@ public static IEnumerable<object[]> Matches_TestData()
                             new CaptureData("x", 3, 1),
                         }
                     };
+
+                    // Fails on .NET Framework: https://github.com/dotnet/runtime/issues/111051
+                    yield return new object[]
+                    {
+                        engine, @"anyexpress1(?<=(.(any express|(any express)*)+?)anyexpress1)", "anystring anyexpress1", RegexOptions.None, new[]
+                        {
+                            new CaptureData("anyexpress1", 10, 11),
+                        }
+                    };
                 }
 
-                // Fails on .NET Framework: [ActiveIssue("https://github.com/dotnet/runtime/issues/62094")]
+                // Fails on .NET Framework: https://github.com/dotnet/runtime/issues/62094
                 yield return new object[]
                 {
                     engine, @"(?:){93}", "x", RegexOptions.None, new[]
@@ -463,6 +472,16 @@ public static IEnumerable<object[]> Matches_TestData()
                         new CaptureData("", 1, 0)
                     }
                 };
+
+                // Fails on .NET Framework: https://github.com/dotnet/runtime/issues/43314
+                yield return new object[]
+                {
+                    engine, @"(?:(?:0?)+?(?:a?)+?)?", "0a", RegexOptions.None, new[]
+                    {
+                        new CaptureData("0a", 0, 2),
+                        new CaptureData("", 2, 0),
+                    }
+                };
 #endif
             }
         }