Bug 50570 - REGRESSION (r73065?): A regex no longer works
Summary: REGRESSION (r73065?): A regex no longer works
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac (Intel) OS X 10.6
: P1 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar, Regression
: 50673 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-12-06 09:20 PST by Francisco Tolmasky
Modified: 2010-12-09 13:06 PST (History)
5 users (show)

See Also:


Attachments
URL regex that matches in FF, IE, and Safari but not nightlies. (762 bytes, text/plain)
2010-12-06 09:20 PST, Francisco Tolmasky
no flags Details
Patch to fix linking of parentheses tail code. (11.80 KB, patch)
2010-12-09 09:54 PST, Michael Saboff
no flags Details | Formatted Diff | Diff
New Patch with redundant {increment/decrement}ParenNestingLevel calls (11.66 KB, patch)
2010-12-09 10:38 PST, Michael Saboff
no flags Details | Formatted Diff | Diff
Patch addressing reviewers comments (40.94 KB, patch)
2010-12-09 12:32 PST, Michael Saboff
barraclough: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Francisco Tolmasky 2010-12-06 09:20:05 PST
Created attachment 75703 [details]
URL regex that matches in FF, IE, and Safari but not nightlies.

After this change I believe: http://trac.webkit.org/changeset/73065, regexes that were succeeding before (and succeed in other browsers) now no longer match. I have included a reduction.

In the reduction I've attached, Safari logs the following:

["file:///Users/tolmasky/Desktop/HelloWorld/index.html", "file", "//", "", undefined, undefined, undefined, "", undefined, "/Users/tolmasky/Desktop/HelloWorld/index.html", undefined, undefined]

But the nightly logs "null".
Comment 1 Geoffrey Garen 2010-12-07 12:35:58 PST
<rdar://problem/8739603>
Comment 2 Michael Saboff 2010-12-08 16:21:32 PST
*** Bug 50673 has been marked as a duplicate of this bug. ***
Comment 3 Michael Saboff 2010-12-09 09:54:24 PST
Created attachment 76077 [details]
Patch to fix linking of parentheses tail code.
Comment 4 Alexey Proskuryakov 2010-12-09 10:30:23 PST
+        Changed the handling of adjacent parentheses backtracks in two ways.
+        First, only outer most paren backtracks default to back tracking
+        to the "next character" looping code.  Second, added a jump around 
+        backtracks that fall through to the next backtrack where the
+        second backtrack has some greedy processing before the backtracking
+        from outside the parentheses code.
+        https://bugs.webkit.org/show_bug.cgi?id=50570

A somewhat more readable and common format would be:

+        REGRESSION (r73065): A regex no longer works
+        https://bugs.webkit.org/show_bug.cgi?id=50570
+
+        Changed the handling of adjacent parentheses backtracks in two ways.
+        First, only outer most paren backtracks default to back tracking
+        to the "next character" looping code.  Second, added a jump around 
+        backtracks that fall through to the next backtrack where the
+        second backtrack has some greedy processing before the backtracking
+        from outside the parentheses code.
Comment 5 Michael Saboff 2010-12-09 10:38:59 PST
Created attachment 76089 [details]
New Patch with redundant {increment/decrement}ParenNestingLevel calls
Comment 6 Gavin Barraclough 2010-12-09 11:27:01 PST
Hi Michael,

(1) please fix the change log comment per alexey's request.
(2) please remove the superfluous indent.
(3) please switch JumpList -> Jump.
(4) please comment the new check, with a simple example.

cheers,
G.
Comment 7 Michael Saboff 2010-12-09 12:32:52 PST
Created attachment 76111 [details]
Patch addressing reviewers comments

Note that I also cleaned up ALL of the extra whitespace in the file JavaScriptCore/yarr/RegexJIT.cpp.  Therefore the resulting patch is large.
Comment 8 Michael Saboff 2010-12-09 13:06:39 PST
Committed r73640: <http://trac.webkit.org/changeset/73640>