Changeset 243839 in webkit

Timestamp:

Apr 3, 2019, 4:51:12 PM (7 years ago)

Author:

msaboff@apple.com

Message:

REGRESSION (r243642): com.apple.JavaScriptCore crash in JSC::RegExpObject::execInline
​https://bugs.webkit.org/show_bug.cgi?id=196477

Reviewed by Keith Miller.

Source/JavaScriptCore:

The problem here is that when we advance the index by 2 for a character class that only
has non-BMP characters, we might go past the end of the string. This can happen for
greedy counted character classes that are part of a alternative where there is one
character to match after the greedy non-BMP character class.

The "do we have string left to match" check at the top of the JIT loop for the counted
character class checks to see if index is not equal to the string length. For non-BMP
character classes, we need to check to see if there are at least 2 characters left.
Therefore we now temporarily add 1 to the current index before comparing. This checks
to see if there are iat least 2 characters left to match, instead of 1.

• yarr/YarrJIT.cpp:

(JSC::Yarr::YarrGenerator::generateCharacterClassGreedy):
(JSC::Yarr::YarrGenerator::backtrackCharacterClassNonGreedy):

LayoutTests:

Updated the test with a couple more test cases to test a few variants of this bug.
Also added a couple of non-greedy counted non-BMP character class tests that don't have
the bug just to be sure.

• js/regexp-unicode-expected.txt:
• js/script-tests/regexp-unicode.js:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/js/regexp-unicode-expected.txt (modified) (1 diff)
• LayoutTests/js/script-tests/regexp-unicode.js (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/yarr/YarrJIT.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-04-03  Michael Saboff  <msaboff@apple.com>
+
+        REGRESSION (r243642): com.apple.JavaScriptCore crash in JSC::RegExpObject::execInline
+        https://bugs.webkit.org/show_bug.cgi?id=196477
+
+        Reviewed by Keith Miller.
+
+        Updated the test with a couple more test cases to test a few variants of this bug.
+        Also added a couple of non-greedy counted non-BMP character class tests that don't have
+        the bug just to be sure.
+
+        * js/regexp-unicode-expected.txt:
+        * js/script-tests/regexp-unicode.js:
+
 2019-04-03  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/LayoutTests/js/regexp-unicode-expected.txt

@@ -149 +149 @@
 PASS "𐌑𐌐𐌑".match(/[𐌁𐌑]+a|[𐌐𐌑]+./iu)[0] is "𐌑𐌐𐌑"
 PASS "𐌑𐌐𐌑".match(/[𐌁𐌑]+?a|[𐌐𐌑]+?./iu)[0] is "𐌑𐌐"
+PASS "𐌑𐌐𐌑".match(/[𐌁𐌑]+?a$|[𐌐𐌑]+?.$/iu)[0] is "𐌑𐌐𐌑"
+PASS "𐌑𐌐𐌑".match(/[𐌁𐌑x]+a|[𐌐𐌑x]+./iu)[0] is "𐌑𐌐𐌑"
+PASS "𐌑𐌐𐌑".match(/[𐌁𐌑x]+?a|[𐌐𐌑x]+?./iu)[0] is "𐌑𐌐"
 PASS "C83|НАЧАТЬ".match(re8)[0] is "C83|НАЧАТЬ"
 PASS "This.Is.16.Chars|НАЧАТЬ".match(re8)[0] is "This.Is.16.Chars|НАЧАТЬ"

trunk/LayoutTests/js/script-tests/regexp-unicode.js

@@ -206 +206 @@
 shouldBe('"\u{10311}\u{10310}\u{10311}".match(/[\u{10301}\u{10311}]+a|[\u{10310}\u{10311}]+./iu)[0]', '"\u{10311}\u{10310}\u{10311}"');
 shouldBe('"\u{10311}\u{10310}\u{10311}".match(/[\u{10301}\u{10311}]+?a|[\u{10310}\u{10311}]+?./iu)[0]', '"\u{10311}\u{10310}"');
+shouldBe('"\u{10311}\u{10310}\u{10311}".match(/[\u{10301}\u{10311}]+?a$|[\u{10310}\u{10311}]+?.$/iu)[0]', '"\u{10311}\u{10310}\u{10311}"');
+shouldBe('"\u{10311}\u{10310}\u{10311}".match(/[\u{10301}\u{10311}x]+a|[\u{10310}\u{10311}x]+./iu)[0]', '"\u{10311}\u{10310}\u{10311}"');
+shouldBe('"\u{10311}\u{10310}\u{10311}".match(/[\u{10301}\u{10311}x]+?a|[\u{10310}\u{10311}x]+?./iu)[0]', '"\u{10311}\u{10310}"');
 
 var re8 = new  RegExp("^([0-9a-z\.]{3,16})\\|\u{041d}\u{0410}\u{0427}\u{0410}\u{0422}\u{042c}", "ui");

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-04-03  Michael Saboff  <msaboff@apple.com>
+
+        REGRESSION (r243642): com.apple.JavaScriptCore crash in JSC::RegExpObject::execInline
+        https://bugs.webkit.org/show_bug.cgi?id=196477
+
+        Reviewed by Keith Miller.
+
+        The problem here is that when we advance the index by 2 for a character class that only
+        has non-BMP characters, we might go past the end of the string.  This can happen for
+        greedy counted character classes that are part of a alternative where there is one
+        character to match after the greedy non-BMP character class.
+
+        The "do we have string left to match" check at the top of the JIT loop for the counted
+        character class checks to see if index is not equal to the string length.  For non-BMP
+        character classes, we need to check to see if there are at least 2 characters left.
+        Therefore we now temporarily add 1 to the current index before comparing.  This checks
+        to see if there are iat least 2 characters left to match, instead of 1.
+
+        * yarr/YarrJIT.cpp:
+        (JSC::Yarr::YarrGenerator::generateCharacterClassGreedy):
+        (JSC::Yarr::YarrGenerator::backtrackCharacterClassNonGreedy):
+
 2019-04-03  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/Source/JavaScriptCore/yarr/YarrJIT.cpp

@@ -1850 +1850 @@
         JumpList failures;
         Label loop(this);
-        failures.append(atEndOfInput());
+#ifdef JIT_UNICODE_EXPRESSIONS
+        if (term->characterClass->hasOneCharacterSize() && !term->invert() && term->characterClass->hasNonBMPCharacters()) {
+            move(TrustedImm32(1), character);
+            failures.append(checkNotEnoughInput(character));
+        } else
+#endif
+            failures.append(atEndOfInput());
 
         if (term->invert()) {
@@ -1957 +1963 @@
         m_backtrackingState.link(this);
 
+#ifdef JIT_UNICODE_EXPRESSIONS
         if (m_decodeSurrogatePairs) {
             if (!term->characterClass->hasOneCharacterSize() || term->invert())
@@ -1962 +1969 @@
             loadFromFrame(term->frameLocation + BackTrackInfoCharacterClass::matchAmountIndex(), countRegister);
         }
+#endif
 
         nonGreedyFailures.append(atEndOfInput());