Bug 55479 - Begin Characters Optimization Causes YARR Interpreter Errors
Summary: Begin Characters Optimization Causes YARR Interpreter Errors
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All OS X 10.5
: P2 Normal
Assignee: Nobody
URL: http://www.sirensclef.com/webkit/tiny...
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-01 10:22 PST by Michael Saboff
Modified: 2011-03-03 02:23 PST (History)
3 users (show)

See Also:


Attachments
proposed patch (7.61 KB, patch)
2011-03-02 07:24 PST, Peter Varga
no flags Details | Formatted Diff | Diff
proposed patch v1.1 (7.62 KB, patch)
2011-03-02 07:36 PST, Peter Varga
no flags Details | Formatted Diff | Diff
proposed patch v1.2 (8.87 KB, patch)
2011-03-02 09:57 PST, Peter Varga
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2011-03-01 10:22:07 PST
This issue was found while investigating https://bugs.webkit.org/show_bug.cgi?id=54978.

The temporary resolution to that defect was to disable the setting up of the begin characters optimization.

It appears that the begin characters code is not properly setting up the data.  From the original bug, it appears that the regular expression:
    /<((\/([^>]+)>)|(([^>]+)>))/
correctly finds '<p>' from the string '<p>This content should not appear as HTML <strong>source</strong>.</p>' when run via the interpreter.  If one adds (ABC>) as an alternative to the regular expression:
    /<((ABC>)|(\/([^>]+)>)|(([^>]+)>))/
then the interpreter doesn't find '<p>' in the subject string.

Walking through the code, the setupBeginChars() method determines that the first expression (w/o the (ABC>)) is not a candidate for the begin characters optimization.  The setupBeginChars() determines that the second expression is a candidate for the begin characters optimization and it looks for the strings "<A" and "</" which come from the first two alternatives. The last alternative isn't included in the optimization strings.
Comment 1 Peter Varga 2011-03-02 07:24:52 PST
Created attachment 84418 [details]
proposed patch

Thanks for the report. I fixed the bug and upload some additional test cases.
Comment 2 Peter Varga 2011-03-02 07:36:40 PST
Created attachment 84419 [details]
proposed patch v1.1

regexp-alternatives.js layout test's expected file is fixed.
Comment 3 Peter Varga 2011-03-02 09:57:00 PST
Created attachment 84432 [details]
proposed patch v1.2

Extend the layout test to force the testing of the YARR Interpreter.
Comment 4 Michael Saboff 2011-03-02 11:40:05 PST
Comment on attachment 84432 [details]
proposed patch v1.2

Looks good.   Thanks for addressing this quickly.
Comment 5 WebKit Review Bot 2011-03-02 11:40:52 PST
Comment on attachment 84432 [details]
proposed patch v1.2

Rejecting attachment 84432 [details] from review queue.

msaboff@apple.com does not have reviewer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have reviewer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have reviewer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your reviewer rights.
Comment 6 Oliver Hunt 2011-03-02 11:44:29 PST
Comment on attachment 84432 [details]
proposed patch v1.2

r=me
Comment 7 WebKit Commit Bot 2011-03-03 02:23:13 PST
Comment on attachment 84432 [details]
proposed patch v1.2

Clearing flags on attachment: 84432

Committed r80217: <http://trac.webkit.org/changeset/80217>
Comment 8 WebKit Commit Bot 2011-03-03 02:23:18 PST
All reviewed patches have been landed.  Closing bug.