Bug 67455 - Different regular expression result
Summary: Different regular expression result
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-01 17:01 PDT by Dave Mandelin
Modified: 2011-10-02 17:26 PDT (History)
4 users (show)

See Also:


Attachments
Patch, needs change log & layout test (2.08 KB, patch)
2011-10-02 12:16 PDT, Gavin Barraclough
no flags Details | Formatted Diff | Diff
Fix (5.53 KB, patch)
2011-10-02 15:36 PDT, Gavin Barraclough
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Mandelin 2011-09-01 17:01:58 PDT
This is https://bugzilla.mozilla.org/show_bug.cgi?id=683838

In JSC:

var rg = /(X(?:.(?!X))*?Y)|(Y(?:.(?!Y))*?Z)/g;
var str = "Y aaa X Match1 Y aaa Y Match2 Z";
print(str.match(rg));

==> Y Match2 Z

should be 

==> X Match1 Y,Y Match2 Z
Comment 1 Julien Chaffraix 2011-09-02 09:22:05 PDT
Confirmed on ToT for JSC. V8 is not impacted nor is Opera or IE.
Comment 2 Gavin Barraclough 2011-09-29 22:51:10 PDT
Reduction:

print(/a|b(?:[^b])*?c/.exec("badbc"));

should print a, prints bc.
Comment 3 Gavin Barraclough 2011-10-02 11:37:19 PDT
Looks like the problem is in YARR interpreter, in backtrackParentheses with QuantifierNonGreedy, and where the expression is passed input to match against that will match the parentheses more than once but then fail.

For the above reduction I believe the interpreter will have torn off a couple of disjunction contexts (recording the matches of 'a' and 'd'), but only one of these contexts gets backtracked back into.  Since the minimum size of these matches is one, failing to backtrack means the input position has not been reverted correctly.  When the match has failed starting at input position 0, we try again at input +1, but instead of starting at position 1 the second pass starts at position 2, hence the first alternative doesn't get a chance to match.
Comment 4 Gavin Barraclough 2011-10-02 12:01:29 PDT
The bug was introduced by r72140.  It added a return to the backtracking loop for backtrackParentheses/QuantifierNonGreedy, so it always returns after one iteration.  We've already returned if the match succeeded, the additional return should only be triggered if an error is detected.
Comment 5 Gavin Barraclough 2011-10-02 12:16:40 PDT
Created attachment 109427 [details]
Patch, needs change log & layout test
Comment 6 Gavin Barraclough 2011-10-02 15:36:23 PDT
Created attachment 109429 [details]
Fix
Comment 7 WebKit Review Bot 2011-10-02 15:38:48 PDT
Attachment 109429 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1

LayoutTests/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Source/JavaScriptCore/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Gavin Barraclough 2011-10-02 17:26:35 PDT
Fixed in r96479