Bug 50570

Summary: REGRESSION (r73065?): A regex no longer works
Product: WebKit Reporter: Francisco Tolmasky <tolmasky>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: antoine.mercadal, barraclough, ggaren, klesper, msaboff
Priority: P1 Keywords: InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.6   
Attachments:
Description Flags
URL regex that matches in FF, IE, and Safari but not nightlies.
none
Patch to fix linking of parentheses tail code.
none
New Patch with redundant {increment/decrement}ParenNestingLevel calls
none
Patch addressing reviewers comments barraclough: review+

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>