Attachment 274817 Details for Bug 155829 – Patch

[patch] Patch

155829.patch (text/plain), 7.25 KB, created by Michael Saboff on 2016-03-23 22:15:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Saboff

Created: 2016-03-23 22:15:17 PDT

Size: 7.25 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 198615)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,25 @@
>+2016-03-23  Michael Saboff  <msaboff@apple.com>
>+
>+        [ES6] Greedy unicode RegExp's don't properly backtrack past non BMP characters
>+        https://bugs.webkit.org/show_bug.cgi?id=155829
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When we backup when matching part of a unicode pattern, we can't just backup one character.
>+        Instead we need to save our start position before trying to match a character and
>+        restore the position if the match fails.  This was done in other places, but wasn't
>+        done for all greedy types.
>+
>+        Fixed matchGlobal() to properly handle advancing past non BMP characters.
>+
>+        * runtime/RegExpObject.cpp:
>+        (JSC::RegExpObject::matchGlobal):
>+        * runtime/RegExpObjectInlines.h:
>+        (JSC::RegExpObject::advanceStringUnicode):
>+        * yarr/YarrInterpreter.cpp:
>+        (JSC::Yarr::Interpreter::matchCharacterClass):
>+        (JSC::Yarr::Interpreter::matchDisjunction):
>+
> 2016-03-23  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r198582.
>Index: Source/JavaScriptCore/runtime/RegExpObject.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/RegExpObject.cpp	(revision 198609)
>+++ Source/JavaScriptCore/runtime/RegExpObject.cpp	(working copy)
>@@ -191,6 +191,7 @@ JSValue RegExpObject::matchGlobal(ExecSt
>     const size_t maximumReasonableMatchSize = 1000000000;
> 
>     if (regExp->unicode()) {
>+        size_t stringLength = s.length();
>         while (result) {
>             if (list.size() > maximumReasonableMatchSize) {
>                 throwOutOfMemoryError(exec);
>@@ -201,7 +202,7 @@ JSValue RegExpObject::matchGlobal(ExecSt
>             size_t length = end - result.start;
>             list.append(jsSubstring(exec, s, result.start, length));
>             if (!length)
>-                end = advanceStringUnicode(s, length, end);
>+                end = advanceStringUnicode(s, stringLength, end);
>             result = regExpConstructor->performMatch(*vm, regExp, string, s, end);
>         }
>     } else {
>Index: Source/JavaScriptCore/runtime/RegExpObjectInlines.h
>===================================================================
>--- Source/JavaScriptCore/runtime/RegExpObjectInlines.h	(revision 198609)
>+++ Source/JavaScriptCore/runtime/RegExpObjectInlines.h	(working copy)
>@@ -117,7 +117,7 @@ unsigned RegExpObject::advanceStringUnic
>     if (first < 0xD800 || first > 0xDBFF)
>         return currentIndex + 1;
> 
>-    UChar second = s[currentIndex];
>+    UChar second = s[currentIndex + 1];
>     if (second < 0xDC00 || second > 0xDFFF)
>         return currentIndex + 1;
> 
>Index: Source/JavaScriptCore/yarr/YarrInterpreter.cpp
>===================================================================
>--- Source/JavaScriptCore/yarr/YarrInterpreter.cpp	(revision 198609)
>+++ Source/JavaScriptCore/yarr/YarrInterpreter.cpp	(working copy)
>@@ -507,14 +507,16 @@ public:
>         }
> 
>         case QuantifierGreedy: {
>-            backTrack->begin = input.getPos();
>+            unsigned position = input.getPos();
>+            backTrack->begin = position;
>             unsigned matchAmount = 0;
>             while ((matchAmount < term.atom.quantityCount) && input.checkInput(1)) {
>                 if (!checkCharacterClass(term.atom.characterClass, term.invert(), term.inputPosition + 1)) {
>-                    input.uncheckInput(1);
>+                    input.setPos(position);
>                     break;
>                 }
>                 ++matchAmount;
>+                position = input.getPos();
>             }
>             backTrack->matchAmount = matchAmount;
> 
>@@ -1242,12 +1244,14 @@ public:
>         case ByteTerm::TypePatternCharacterGreedy: {
>             BackTrackInfoPatternCharacter* backTrack = reinterpret_cast<BackTrackInfoPatternCharacter*>(context->frame + currentTerm().frameLocation);
>             unsigned matchAmount = 0;
>+            unsigned position = input.getPos(); // May need to back out reading a surrogate pair.
>             while ((matchAmount < currentTerm().atom.quantityCount) && input.checkInput(1)) {
>                 if (!checkCharacter(currentTerm().atom.patternCharacter, currentTerm().inputPosition + 1)) {
>-                    input.uncheckInput(1);
>+                    input.setPos(position);
>                     break;
>                 }
>                 ++matchAmount;
>+                position = input.getPos();
>             }
>             backTrack->matchAmount = matchAmount;
> 
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 198609)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,15 @@
>+2016-03-23  Michael Saboff  <msaboff@apple.com>
>+
>+        [ES6] Greedy unicode RegExp's don't properly backtrack past non BMP characters
>+        https://bugs.webkit.org/show_bug.cgi?id=155829
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added new test cases.
>+
>+        * js/regexp-unicode-expected.txt:
>+        * js/script-tests/regexp-unicode.js:
>+
> 2016-03-23  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r198538.
>Index: LayoutTests/js/regexp-unicode-expected.txt
>===================================================================
>--- LayoutTests/js/regexp-unicode-expected.txt	(revision 198609)
>+++ LayoutTests/js/regexp-unicode-expected.txt	(working copy)
>@@ -77,6 +77,11 @@ PASS "að¨ð¨ð¨c".match(/ac|að*cd
> PASS "abð¨ð¨ð¨cð¨".match(/abc|abð*cd|abð+cð¨d|abð+cð¨/iu)[0] is "abð¨ð¨ð¨cð¨"
> PASS "abð¨ð¨ð¨".match(/abc|abð¨*./u)[0] is "abð¨ð¨ð¨"
> PASS "abð¨ð¨ð¨".match(/abc|abð*./iu)[0] is "abð¨ð¨ð¨"
>+PASS "ð".match(/a*/u)[0].length is 0
>+PASS "ð".match(/a*/ui)[0].length is 0
>+PASS "ð".match(/\d*/u)[0].length is 0
>+PASS "123ð".match(/\d*/u)[0] is "123"
>+PASS "12X3ð4".match(/\d{0,1}/ug) is ["1", "2", "", "3", "", "4", ""]
> PASS match3[0] is "aððb"
> PASS match3[1] is undefined.
> PASS match3[2] is "aððb"
>Index: LayoutTests/js/script-tests/regexp-unicode.js
>===================================================================
>--- LayoutTests/js/script-tests/regexp-unicode.js	(revision 198609)
>+++ LayoutTests/js/script-tests/regexp-unicode.js	(working copy)
>@@ -113,6 +113,11 @@ shouldBe('"a\u{10428}\u{10428}\u{10428}c
> shouldBe('"ab\u{10428}\u{10428}\u{10428}c\u{10a01}".match(/abc|ab\u{10400}*cd|ab\u{10400}+c\u{10a01}d|ab\u{10400}+c\u{10a01}/iu)[0]', '"ab\u{10428}\u{10428}\u{10428}c\u{10a01}"');
> shouldBe('"ab\u{10428}\u{10428}\u{10428}".match(/abc|ab\u{10428}*./u)[0]', '"ab\u{10428}\u{10428}\u{10428}"');
> shouldBe('"ab\u{10428}\u{10428}\u{10428}".match(/abc|ab\u{10400}*./iu)[0]', '"ab\u{10428}\u{10428}\u{10428}"');
>+shouldBe('"\u{10400}".match(/a*/u)[0].length', '0');
>+shouldBe('"\u{10400}".match(/a*/ui)[0].length', '0');
>+shouldBe('"\u{10400}".match(/\\d*/u)[0].length', '0');
>+shouldBe('"123\u{10400}".match(/\\d*/u)[0]', '"123"');
>+shouldBe('"12X3\u{10400}4".match(/\\d{0,1}/ug)', '["1", "2", "", "3", "", "4", ""]');
> 
> var re3 = new RegExp("(a\u{10410}*bc)|(a\u{10410}*b)", "u");
> var match3 = "a\u{10410}\u{10410}b".match(re3);

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 155829: 274817