Bug 159233 - REGRESSION (r200946): Improper backtracking from last alternative in sticky patterns
Summary: REGRESSION (r200946): Improper backtracking from last alternative in sticky p...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-06-28 16:03 PDT by Michael Saboff
Modified: 2016-06-28 17:39 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.11 KB, patch)
2016-06-28 16:24 PDT, Michael Saboff
mark.lam: review+
Details | Formatted Diff | Diff
With style fixes and updated tests (5.84 KB, patch)
2016-06-28 16:32 PDT, Michael Saboff
mark.lam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Saboff 2016-06-28 16:03:33 PDT
Various sticky patterns fail in unexpected ways when the last alternative doesn't match.  It seems that we retry with an index that isn't correct.  This manifests itself in a couple different ways.

The expression 's = ”\n”.split(/.{2}|(?=$)./ym)' will crash in JIT'ed code, while the expression 'let m = ".a".match(/x(.)|[-\d]/gy")' will ASSERT in debug builds:
    #0 0x106b8ccb8 in WTFCrash (JavaScriptCore+0x2a87cb8)
    #1 0x1066e889a in JSC::JSRopeString::finishCreationSubstringOfResolved(JSC::VM&, JSC::JSString*, unsigned int, unsigned int) (JavaScriptCore+0x25e389a)
    #2 0x1066e8176 in JSC::JSRopeString::createSubstringOfResolved(JSC::VM&, JSC::JSString*, unsigned int, unsigned int) (JavaScriptCore+0x25e3176)
    #3 0x1066fa423 in JSC::JSValue JSC::collectMatches<JSC::RegExpObject::matchGlobal(JSC::ExecState*, JSC::JSGlobalObject*, JSC::JSString*)::$_1>(JSC::VM&, JSC::ExecState*, JSC::JSString*, WTF::String const&, JSC::RegExpConstructor*, JSC::RegExp*, JSC::RegExpObject::matchGlobal(JSC::ExecState*, JSC::JSGlobalObject*, JSC::JSString*)::$_1 const&)::'lambda'()::operator()() const (JavaScriptCore+0x25f5423)
    #4 0x1066f89b3 in JSC::JSValue JSC::collectMatches<JSC::RegExpObject::matchGlobal(JSC::ExecState*, JSC::JSGlobalObject*, JSC::JSString*)::$_1>(JSC::VM&, JSC::ExecState*, JSC::JSString*, WTF::String const&, JSC::RegExpConstructor*, JSC::RegExp*, JSC::RegExpObject::matchGlobal(JSC::ExecState*, JSC::JSGlobalObject*, JSC::JSString*)::$_1 const&) (JavaScriptCore+0x25f39b3)
    #5 0x1066f6c90 in JSC::RegExpObject::matchGlobal(JSC::ExecState*, JSC::JSGlobalObject*, JSC::JSString*) (JavaScriptCore+0x25f1c90)
Comment 1 Michael Saboff 2016-06-28 16:04:12 PDT
<rdar://problem/27008975>
<rdar://problem/27018880>
Comment 2 Michael Saboff 2016-06-28 16:24:44 PDT
Created attachment 282295 [details]
Patch
Comment 3 WebKit Commit Bot 2016-06-28 16:26:27 PDT
Attachment 282295 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/yarr/YarrJIT.cpp:1848:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/yarr/YarrJIT.cpp:1851:  Missing space before {  [whitespace/braces] [5]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Mark Lam 2016-06-28 16:28:09 PDT
Comment on attachment 282295 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=282295&action=review

Got tests?

> Source/JavaScriptCore/yarr/YarrJIT.cpp:1851
> +                    else if (m_pattern.sticky() && m_ops[op.m_nextOp].m_op == OpBodyAlternativeEnd)
> +                        // It is a sticky pattern and the last alternative failed, jump to the end.
> +                        m_backtrackingState.takeBacktracksToJumpList(lastStickyAlternativeFailures, this);
> +                    else{

Please fix style errors.
Comment 5 Michael Saboff 2016-06-28 16:32:40 PDT
Created attachment 282297 [details]
With style fixes and updated tests
Comment 6 Mark Lam 2016-06-28 16:34:13 PDT
Comment on attachment 282297 [details]
With style fixes and updated tests

r=me
Comment 7 Michael Saboff 2016-06-28 17:39:18 PDT
Committed r202597: <http://trac.webkit.org/changeset/202597>