Arithmetic overflow in code, generated by `GeneratedRegexAttribute`

[MaceWindu]

Description

Note that we use checked builds (which probably not used by runtime tests): <CheckForOverflowUnderflow>True</CheckForOverflowUnderflow>

Reproduction Steps

using System.Text.RegularExpressions;

internal partial class Program
{
    [GeneratedRegex(@"^\s*GO\s*$\n*", RegexOptions.IgnoreCase | RegexOptions.Multiline | RegexOptions.Compiled)]
    private static partial Regex BuildRegex();

    private static void Main(string[] args)
    {
        BuildRegex().Split("X\r\n XX");
    }
}

Expected behavior

No exceptions

Actual behavior

System.OverflowException
  HResult=0x80131516
  Message=Arithmetic operation resulted in an overflow.
  Source=ConsoleApp41
  StackTrace:
   at System.Text.RegularExpressions.Generated.<RegexGenerator_g>F74B1AE921BCEFE4BA601AA541C7A23B1CA9711EA81E8FE504B5B6446748E035A__BuildRegex_0.RunnerFactory.Runner.TryFindNextPossibleStartingPosition(ReadOnlySpan`1 inputSpan) in RegexGenerator.g.cs:line 95
   at System.Text.RegularExpressions.Generated.<RegexGenerator_g>F74B1AE921BCEFE4BA601AA541C7A23B1CA9711EA81E8FE504B5B6446748E035A__BuildRegex_0.RunnerFactory.Runner.Scan(ReadOnlySpan`1 inputSpan) in RegexGenerator.g.cs:line 68
   at System.Text.RegularExpressions.Regex.ScanInternal(RegexRunnerMode mode, Boolean reuseMatchObject, String input, Int32 beginning, RegexRunner runner, ReadOnlySpan`1 span, Boolean returnNullIfReuseMatchObject)
   at System.Text.RegularExpressions.Regex.RunAllMatchesWithCallback[TState](String inputString, ReadOnlySpan`1 inputSpan, Int32 startat, TState& state, MatchCallback`1 callback, RegexRunnerMode mode, Boolean reuseMatchObject)
   at System.Text.RegularExpressions.Regex.RunAllMatchesWithCallback[TState](String input, Int32 startat, TState& state, MatchCallback`1 callback, RegexRunnerMode mode, Boolean reuseMatchObject)
   at System.Text.RegularExpressions.Regex.Split(Regex regex, String input, Int32 count, Int32 startat)
   at Program.Main(String[] args) in 

Regression?

No response

Known Workarounds

No response

Configuration

tfm: net7.0
extra compilation options: <CheckForOverflowUnderflow>True</CheckForOverflowUnderflow>

Other information

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

Note that we use checked builds (which probably not used by runtime tests): <CheckForOverflowUnderflow>True</CheckForOverflowUnderflow>

Reproduction Steps

using System.Text.RegularExpressions;

internal partial class Program
{
    [GeneratedRegex(@"^\s*GO\s*$\n*", RegexOptions.IgnoreCase | RegexOptions.Multiline | RegexOptions.Compiled)]
    private static partial Regex BuildRegex();

    private static void Main(string[] args)
    {
        BuildRegex().Split("X\r\n XX");
    }
}

Expected behavior

No exceptions

Actual behavior

System.OverflowException
  HResult=0x80131516
  Message=Arithmetic operation resulted in an overflow.
  Source=ConsoleApp41
  StackTrace:
   at System.Text.RegularExpressions.Generated.<RegexGenerator_g>F74B1AE921BCEFE4BA601AA541C7A23B1CA9711EA81E8FE504B5B6446748E035A__BuildRegex_0.RunnerFactory.Runner.TryFindNextPossibleStartingPosition(ReadOnlySpan`1 inputSpan) in RegexGenerator.g.cs:line 95
   at System.Text.RegularExpressions.Generated.<RegexGenerator_g>F74B1AE921BCEFE4BA601AA541C7A23B1CA9711EA81E8FE504B5B6446748E035A__BuildRegex_0.RunnerFactory.Runner.Scan(ReadOnlySpan`1 inputSpan) in RegexGenerator.g.cs:line 68
   at System.Text.RegularExpressions.Regex.ScanInternal(RegexRunnerMode mode, Boolean reuseMatchObject, String input, Int32 beginning, RegexRunner runner, ReadOnlySpan`1 span, Boolean returnNullIfReuseMatchObject)
   at System.Text.RegularExpressions.Regex.RunAllMatchesWithCallback[TState](String inputString, ReadOnlySpan`1 inputSpan, Int32 startat, TState& state, MatchCallback`1 callback, RegexRunnerMode mode, Boolean reuseMatchObject)
   at System.Text.RegularExpressions.Regex.RunAllMatchesWithCallback[TState](String input, Int32 startat, TState& state, MatchCallback`1 callback, RegexRunnerMode mode, Boolean reuseMatchObject)
   at System.Text.RegularExpressions.Regex.Split(Regex regex, String input, Int32 count, Int32 startat)
   at Program.Main(String[] args) in 

Regression?

No response

Known Workarounds

No response

Configuration

tfm: net7.0
extra compilation options: <CheckForOverflowUnderflow>True</CheckForOverflowUnderflow>

Other information

Author:	MaceWindu
Assignees:	-
Labels:	area-System.Text.RegularExpressions
Milestone:	-

[MaceWindu]

I'm currently deciding how to mitigate it in our code - should we rollback to runtime Regex for all regular expressions or this issue exists only for specific patterns, so any information on issue impact will be useful.

[stephentoub]

Thanks. From a correctness perspective, this highlights one of the downsides to CheckForOverflowUnderflow; we use casts like this in many places for efficient bounds checks, and the failure is a false positive. Still, I agree the generated code should work in projects that set this flag, and we'll want to wrap the generated code in unchecked blocks if the project has set this option.

[EgorBo]

I wonder if eventually we want to get rid of that pattern, e.g.: https://sharplab.io/#v2:C4LghgzgtgPgAgJgIwFgBQcDMACR2DC2A3utmdqeZWVrgCzYCySAFAMoAOYAdgDwCW3YAD5sELtwA02QcBkBKasSXkZAM2wsWAV1nz+2Xpt1D54ngDoAMgFNuAc2AALRWlXvy57gG1+AXWwAXmwABgBuJQBfJSVaOAZGBHYJASFRL2lZBSUSNw91TQNhYJDsADIymUMxCWs7RxcVfK9fAJKIvLJotEigA===

Here both don't emit bound checks, the 2nd variation should be improved with the work being done around branchless optimizations (if-conversion, etc).

[stephentoub]

the 2nd variation should be improved with the work being done around branchless optimizations (if-conversion, etc)

Won't that still be more expensive? Or are you saying the JIT will end up emitting identical codegen rather than conditionals?

[EgorBo]

the 2nd variation should be improved with the work being done around branchless optimizations (if-conversion, etc)

Won't that still be more expensive? Or are you saying the JIT will end up emitting identical codegen rather than conditionals?

Yes, we already landed a few branchless optimizations so I don't see why we can't handle this one as well in .NET 8 🙂

[danmoseley]

Do our other source generators need a similar change?

[stephentoub]

Do our other source generators need a similar change?

Not to my knowledge.