| Summary: | Wrong result in case of non-iterative matching of subpatterns in non-greedy cases in YARR Interpreter | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Peter Varga <pvarga> | ||||||
| Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abecsi, barraclough, darin, ggaren, zherczeg, zoltan | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
Created attachment 56366 [details]
proposed patch
Comment on attachment 56366 [details]
proposed patch
Looks like a good code change. But needs a test case to demonstrate at least one regular expression that malfunctions without this and works with it. We don't take patches without test cases unless there's a reason a test case can't be created.
I see the test case here in the bug report. Now it needs to be added to existing tests in LayoutTests. I suggest putting it into one of the existing tests in the fast/js directory that tests other regular expression cases. And add other similar test cases that are not affected. Created attachment 56489 [details]
proposed patch v2
I fixed the previous change and added a layout test. My previous change caused problems in some test cases and these cases have been added to the layout test.
Comment on attachment 56489 [details] proposed patch v2 > - output[(subpatternId << 1) + 1] = input.getPos() + term.inputPosition; > + output[(subpatternId << 1)] = input.getPos() + term.inputPosition; I suggest omitting the parentheses here. output[subpatternId << 1] would be easier to read. > + The backtrackParentesesOnceEnd function should store the start position Typo here in the name of the function. Comment on attachment 56489 [details]
proposed patch v2
cq+ removed to fix the mentioned issues.
(In reply to comment #6) > (From update of attachment 56489 [details]) > cq+ removed to fix the mentioned issues. Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/yarr/RegexInterpreter.cpp Sending LayoutTests/ChangeLog Adding LayoutTests/fast/js/regexp-non-greedy-parentheses-expected.txt Adding LayoutTests/fast/js/regexp-non-greedy-parentheses.html Adding LayoutTests/fast/js/script-tests/regexp-non-greedy-parentheses.js Transmitting file data ...... Committed revision 59766. Patch landed. Closing bug. |
The start of the non-iterative matching of subpatterns isn't stored in case of non-greedy matching. Eg.: print("ab".match(/(a)??b/)); Results: YARR Interpreter: ab, Expected: ab,a