Bug 16648

Summary: REGRESSION (r28165): Yuku.com navigation prints "jsRegExpExecute failed with result -2"
Product: WebKit Reporter: jenny <jennytisme>
Component: JavaScriptCoreAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, ddkilzer, eric
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.yuku.com/
Attachments:
Description Flags
Reduced test case
none
More reduced test case
none
patch ggaren: review+

jenny
Reported 2007-12-28 15:04:30 PST
A problem with Yuku.com, when I click on Navigation nothing happens.
Attachments
Reduced test case (284 bytes, text/html)
2007-12-29 20:53 PST, David Kilzer (:ddkilzer)
no flags
More reduced test case (136 bytes, text/html)
2007-12-29 21:41 PST, Eric Seidel (no email)
no flags
patch (24.23 KB, patch)
2007-12-31 15:47 PST, Darin Adler
ggaren: review+
Matt Lilek
Comment 1 2007-12-28 15:19:38 PST
Confirmed with r29011. This is a regression from Safari 3.0.4. To reproduce: 1. Go to http://www.yuku.com and enter the username and password of webkit/w3kb1t. 2. In the upper right hand corner of the page, click the light blue "NAVIGATION" button. 3. A large profile-ish section should appear and push the bar down, in ToT the following is printed to the console and nothing appears: jsRegExpExecute failed with result -2 jsRegExpExecute failed with result -2
Matt Lilek
Comment 2 2007-12-28 16:12:56 PST
Bisect builds says this regressed between r28128 and r28233 which included a bunch of PCRE changes.
Matt Lilek
Comment 3 2007-12-28 17:03:14 PST
I think this regressed in <http://trac.webkit.org/projects/webkit/changeset/28165>, but I'm not entirely convinced that commit was the long culprit as I receive a different message that's printed to the console: KJS: pcre_exec() failed with result -5 after Safari beachballs for a few seconds.
Matt Lilek
Comment 4 2007-12-28 17:05:47 PST
(In reply to comment #3) > I think this regressed in > <http://trac.webkit.org/projects/webkit/changeset/28165>, but I'm not entirely > convinced that commit was the long culprit as I receive a different message > that's printed to the console: KJS: pcre_exec() failed with result -5 after > Safari beachballs for a few seconds. > s/long/lone
David Kilzer (:ddkilzer)
Comment 5 2007-12-29 19:21:50 PST
David Kilzer (:ddkilzer)
Comment 6 2007-12-29 20:07:30 PST
Internal autospade script reports: Works: r28163 Fails: r28165
David Kilzer (:ddkilzer)
Comment 7 2007-12-29 20:53:29 PST
Created attachment 18174 [details] Reduced test case
Eric Seidel (no email)
Comment 8 2007-12-29 21:41:38 PST
Created attachment 18175 [details] More reduced test case
David Kilzer (:ddkilzer)
Comment 9 2007-12-30 07:20:51 PST
(In reply to comment #1) > 1. Go to http://www.yuku.com and enter the username and password of > webkit/w3kb1t. Note that the password is actually: w3bk1t
Darin Adler
Comment 10 2007-12-31 15:02:09 PST
The changeset that broke this was definitely <http://trac.webkit.org/projects/webkit/changeset/28165>. The code that's broken has this comment: /* For a non-repeating ket, just continue at this level. This also happens for a repeating ket if no characters were matched in the group. This is the forcible breaking of infinite loops as implemented in Perl 5.005. If there is an options reset, it will get obeyed in the normal course of events. */ The code is checking the local that used to be called saved_eptr that is now called subjectPtrAtStartOfInstruction, and that local is uninitialized since changeset 28165 removed the code to initialize it.
Darin Adler
Comment 11 2007-12-31 15:12:49 PST
I don't see how to fix this without restoring the stack that patch removed!
Darin Adler
Comment 12 2007-12-31 15:20:11 PST
The comment deleted in this changeset makes it even more clear what went wrong: /* Structure for building a chain of data that actually lives on the stack, for holding the values of the subject pointer at the start of each subpattern, so as to detect when an empty string has been matched by a subpattern - to break infinite loops. */ That's what's failing to happen here. We're not breaking the infinite loop any more! I think we need to bring the stack back (basically roll out this change).
Darin Adler
Comment 13 2007-12-31 15:47:20 PST
Geoffrey Garen
Comment 14 2008-01-01 10:22:22 PST
Comment on attachment 18218 [details] patch r=me
Darin Adler
Comment 15 2008-01-01 12:03:48 PST
David Kilzer (:ddkilzer)
Comment 16 2008-01-02 13:49:04 PST
*** Bug 16460 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.