REGRESSION (r28165): Yuku.com navigation prints "jsRegExpExecute failed with result -2"

[jenny]

A problem with Yuku.com,  when I click on Navigation nothing happens.

[Matt Lilek]

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]

Bisect builds says this regressed between r28128 and r28233 which included a bunch of PCRE changes.

[Matt Lilek]

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]

(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)]

<rdar://problem/5665249>

[David Kilzer (:ddkilzer)]

Internal autospade script reports:
Works: r28163  Fails: r28165

[David Kilzer (:ddkilzer)]

Created attachment 18174 [details]
Reduced test case

[Eric Seidel (no email)]

Created attachment 18175 [details]
More reduced test case

[David Kilzer (:ddkilzer)]

(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]

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]

I don't see how to fix this without restoring the stack that patch removed!

[Darin Adler]

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]

Created attachment 18218 [details]
patch

[Geoffrey Garen]

Comment on attachment 18218 [details]
patch

r=me

[Darin Adler]

r29066

[David Kilzer (:ddkilzer)]

*** Bug 16460 has been marked as a duplicate of this bug. ***