Skip to content

Fix regex lazy loop handling of backtracking state at max iteration limit#97890

Merged
stephentoub merged 3 commits intodotnet:mainfrom
stephentoub:fixlazylooppop
Feb 3, 2024
Merged

Fix regex lazy loop handling of backtracking state at max iteration limit#97890
stephentoub merged 3 commits intodotnet:mainfrom
stephentoub:fixlazylooppop

Conversation

@stephentoub
Copy link
Copy Markdown
Member

Upon entering a lazy loop, state is pushed onto the backtracking stack if the lazy loop might be backtracked into. That state is then dutifully popped off when unwinding the loop due to failure to match. However, if the loop successfully matches its maximum number of times but still fails because of a failure after the lazy loop, the state still needs to be popped off the stack, but isn't (in the compiler and source generator; it's fine in the interpreter). This fixes that.

…imit

Upon entering a lazy loop, state is pushed onto the backtracking stack if the lazy loop might be backtracked into.  That state is then dutifully popped off when unwinding the loop due to failure to match. However, if the loop successfully matches its maximum number of times but still fails because of a failure after the lazy loop, the state still needs to be popped off the stack, but isn't. This fixes that.
@ghost
Copy link
Copy Markdown

ghost commented Feb 2, 2024

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

Issue Details

Upon entering a lazy loop, state is pushed onto the backtracking stack if the lazy loop might be backtracked into. That state is then dutifully popped off when unwinding the loop due to failure to match. However, if the loop successfully matches its maximum number of times but still fails because of a failure after the lazy loop, the state still needs to be popped off the stack, but isn't (in the compiler and source generator; it's fine in the interpreter). This fixes that.

Author: stephentoub
Assignees: stephentoub
Labels:

area-System.Text.RegularExpressions

Milestone: -

Copy link
Copy Markdown
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

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

Change LGTM. IIRC there has been several edge cases where we have to do fixes like this due to the state not being reset appropriately. I suppose there is no good way for us to ensure that each time you enter a construct we "save" the stack depth and info so that when we exit it we validate that we indeed popped the state to the right place.

@stephentoub
Copy link
Copy Markdown
Member Author

IIRC there has been several edge cases where we have to do fixes like this due to the state not being reset appropriately. I suppose there is no good way for us to ensure that each time you enter a construct we "save" the stack depth and info so that when we exit it we validate that we indeed popped the state to the right place.

Yeah, this is maybe the third I can remember from the last couple of years where we hit something like that. I've had a similar idea, where in a debug build the source generator could emit an additional stack in order to verify that the state being popped is the state that's expected, but I've not had time to implement such a thing.

@stephentoub
Copy link
Copy Markdown
Member Author

/backport to release/8.0-staging

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 3, 2024

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/7768960657

@stephentoub stephentoub merged commit 95a56dc into dotnet:main Feb 3, 2024
@stephentoub stephentoub deleted the fixlazylooppop branch February 3, 2024 20:50
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants