Implicit creation of a regular expression should eagerly check for syntax errors

https://bugs.webkit.org/show_bug.cgi?id=76642

Source/JavaScriptCore:

Reviewed by Oliver Hunt.

This is a correctness fix and a slight optimization.

* runtime/StringPrototype.cpp:
(JSC::stringProtoFuncMatch):
(JSC::stringProtoFuncSearch): Check for syntax errors because that's the
correct behavior.

* runtime/RegExp.cpp:
(JSC::RegExp::match): ASSERT that we aren't a syntax error. (One line
of code change, many lines of indentation change.)

Since we have no clients that try to match a RegExp that is a syntax error,
let's optimize out the check.

LayoutTests:

Reviewed by Oliver Hunt.

* fast/js/code-serialize-paren-expected.txt:
* fast/js/script-tests/code-serialize-paren.js: This test was secretly
broken due to a regexp syntax error. Now fixed.

* fast/regex/syntax-errors-expected.txt: Added.
* fast/regex/syntax-errors.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@105444 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 8ff4b55..db6471d 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2012-01-19  Geoffrey Garen  <ggaren@apple.com>
+
+        Implicit creation of a regular expression should eagerly check for syntax errors
+        https://bugs.webkit.org/show_bug.cgi?id=76642
+
+        Reviewed by Oliver Hunt.
+
+        * fast/js/code-serialize-paren-expected.txt:
+        * fast/js/script-tests/code-serialize-paren.js: This test was secretly
+        broken due to a regexp syntax error. Now fixed.
+
+        * fast/regex/syntax-errors-expected.txt: Added.
+        * fast/regex/syntax-errors.html: Added.
+
 2012-01-13  Ryosuke Niwa  <rniwa@webkit.org>
 
         Crash in CompositeEditCommand::ensureComposition

diff --git a/LayoutTests/fast/js/code-serialize-paren-expected.txt b/LayoutTests/fast/js/code-serialize-paren-expected.txt
index 4821030..ca373b2 100644 (file)
--- a/LayoutTests/fast/js/code-serialize-paren-expected.txt
+++ b/LayoutTests/fast/js/code-serialize-paren-expected.txt
@@ -3,7 +3,7 @@ This test checks whether converting function code to a string preserves semantic
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS (function () { return (x + y) * z; }).toString().search('return.*\(') < 0 is true
+PASS (function () { return (x + y) * z; }).toString().search('return.*[(]') != -1 is true
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/LayoutTests/fast/js/script-tests/code-serialize-paren.js b/LayoutTests/fast/js/script-tests/code-serialize-paren.js
index 7135f30..2a3377b 100644 (file)
--- a/LayoutTests/fast/js/script-tests/code-serialize-paren.js
+++ b/LayoutTests/fast/js/script-tests/code-serialize-paren.js
@@ -2,4 +2,4 @@ description(
 "This test checks whether converting function code to a string preserves semantically significant parentheses."
 )
 
-shouldBeTrue("(function () { return (x + y) * z; }).toString().search('return.*\\(') < 0");
+shouldBeTrue("(function () { return (x + y) * z; }).toString().search('return.*[(]') != -1");

diff --git a/LayoutTests/fast/regex/syntax-errors-expected.txt b/LayoutTests/fast/regex/syntax-errors-expected.txt
new file mode 100644 (file)
index 0000000..32dfb80
--- /dev/null
+++ b/LayoutTests/fast/regex/syntax-errors-expected.txt
@@ -0,0 +1,7 @@
+This test verifies that implicit creation of a regular expression eagerly checks for syntax errors.
+
+If the test passes, you'll see pass messages below.
+
+PASS: "abc".search("[") should throw an exception and did: SyntaxError: Invalid regular expression: missing terminating ] for character class.
+PASS: "abc".match("[") should throw an exception and did: SyntaxError: Invalid regular expression: missing terminating ] for character class.
+

diff --git a/LayoutTests/fast/regex/syntax-errors.html b/LayoutTests/fast/regex/syntax-errors.html
new file mode 100644 (file)
index 0000000..993ddc6
--- /dev/null
+++ b/LayoutTests/fast/regex/syntax-errors.html
@@ -0,0 +1,26 @@
+<p>This test verifies that implicit creation of a regular expression eagerly checks for syntax errors.</p>
+<p>If the test passes, you'll see pass messages below.</p>
+<pre id="console"></pre>
+
+<script>
+function log(s)
+{
+    document.getElementById("console").appendChild(document.createTextNode(s + "\n"));
+}
+
+function shouldThrow(program)
+{
+    try {
+        eval(program);
+        log("FAIL: " + program + " should throw an exception but didn't");
+    } catch (e) {
+        log("PASS: " + program + " should throw an exception and did: " + e + ".");
+    }
+}
+
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+shouldThrow('"abc".search("[")');
+shouldThrow('"abc".match("[")');
+</script>

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index a854363..f3e1270 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,24 @@
+2012-01-19  Geoffrey Garen  <ggaren@apple.com>
+
+        Implicit creation of a regular expression should eagerly check for syntax errors
+        https://bugs.webkit.org/show_bug.cgi?id=76642
+
+        Reviewed by Oliver Hunt.
+        
+        This is a correctness fix and a slight optimization.
+
+        * runtime/StringPrototype.cpp:
+        (JSC::stringProtoFuncMatch):
+        (JSC::stringProtoFuncSearch): Check for syntax errors because that's the
+        correct behavior.
+
+        * runtime/RegExp.cpp:
+        (JSC::RegExp::match): ASSERT that we aren't a syntax error. (One line
+        of code change, many lines of indentation change.)
+
+        Since we have no clients that try to match a RegExp that is a syntax error,
+        let's optimize out the check.
+
 2012-01-19  Mark Hahnenberg  <mhahnenberg@apple.com>
 
         Implement a new allocator for backing stores

diff --git a/Source/JavaScriptCore/runtime/RegExp.cpp b/Source/JavaScriptCore/runtime/RegExp.cpp
index 0d513d2..fb5e479 100644 (file)
--- a/Source/JavaScriptCore/runtime/RegExp.cpp
+++ b/Source/JavaScriptCore/runtime/RegExp.cpp
@@ -330,7 +330,6 @@ void RegExp::compileIfNecessary(JSGlobalData& globalData, Yarr::YarrCharSize cha
     compile(&globalData, charSize);
 }
 
-
 int RegExp::match(JSGlobalData& globalData, const UString& s, int startOffset, Vector<int, 32>* ovector)
 {
     if (startOffset < 0)
@@ -343,55 +342,52 @@ int RegExp::match(JSGlobalData& globalData, const UString& s, int startOffset, V
     if (static_cast<unsigned>(startOffset) > s.length() || s.isNull())
         return -1;
 
-    if (m_state != ParseError) {
-        compileIfNecessary(globalData, s.is8Bit() ? Yarr::Char8 : Yarr::Char16);
+    ASSERT(m_state != ParseError);
+    compileIfNecessary(globalData, s.is8Bit() ? Yarr::Char8 : Yarr::Char16);
 
-        int offsetVectorSize = (m_numSubpatterns + 1) * 2;
-        int* offsetVector;
-        Vector<int, 32> nonReturnedOvector;
-        if (ovector) {
-            ovector->resize(offsetVectorSize);
-            offsetVector = ovector->data();
-        } else {
-            nonReturnedOvector.resize(offsetVectorSize);
-            offsetVector = nonReturnedOvector.data();
-        }
+    int offsetVectorSize = (m_numSubpatterns + 1) * 2;
+    int* offsetVector;
+    Vector<int, 32> nonReturnedOvector;
+    if (ovector) {
+        ovector->resize(offsetVectorSize);
+        offsetVector = ovector->data();
+    } else {
+        nonReturnedOvector.resize(offsetVectorSize);
+        offsetVector = nonReturnedOvector.data();
+    }
 
-        ASSERT(offsetVector);
-        // Initialize offsetVector with the return value (index 0) and the 
-        // first subpattern start indicies (even index values) set to -1.
-        // No need to init the subpattern end indicies.
-        for (unsigned j = 0, i = 0; i < m_numSubpatterns + 1; j += 2, i++)            
-            offsetVector[j] = -1;
+    ASSERT(offsetVector);
+    // Initialize offsetVector with the return value (index 0) and the 
+    // first subpattern start indicies (even index values) set to -1.
+    // No need to init the subpattern end indicies.
+    for (unsigned j = 0, i = 0; i < m_numSubpatterns + 1; j += 2, i++)            
+        offsetVector[j] = -1;
 
-        int result;
+    int result;
 #if ENABLE(YARR_JIT)
-        if (m_state == JITCode) {
-            if (s.is8Bit())
-                result = Yarr::execute(m_representation->m_regExpJITCode, s.characters8(), startOffset, s.length(), offsetVector);
-            else
-                result = Yarr::execute(m_representation->m_regExpJITCode, s.characters16(), startOffset, s.length(), offsetVector);
+    if (m_state == JITCode) {
+        if (s.is8Bit())
+            result = Yarr::execute(m_representation->m_regExpJITCode, s.characters8(), startOffset, s.length(), offsetVector);
+        else
+            result = Yarr::execute(m_representation->m_regExpJITCode, s.characters16(), startOffset, s.length(), offsetVector);
 #if ENABLE(YARR_JIT_DEBUG)
-            matchCompareWithInterpreter(s, startOffset, offsetVector, result);
+        matchCompareWithInterpreter(s, startOffset, offsetVector, result);
 #endif
-        } else
+    } else
 #endif
-            result = Yarr::interpret(m_representation->m_regExpBytecode.get(), s, startOffset, s.length(), offsetVector);
-        ASSERT(result >= -1);
+        result = Yarr::interpret(m_representation->m_regExpBytecode.get(), s, startOffset, s.length(), offsetVector);
+    ASSERT(result >= -1);
 
 #if REGEXP_FUNC_TEST_DATA_GEN
-        RegExpFunctionalTestCollector::get()->outputOneTest(this, s, startOffset, offsetVector, result);
+    RegExpFunctionalTestCollector::get()->outputOneTest(this, s, startOffset, offsetVector, result);
 #endif
 
 #if ENABLE(REGEXP_TRACING)
-        if (result != -1)
-            m_rtMatchFoundCount++;
+    if (result != -1)
+        m_rtMatchFoundCount++;
 #endif
 
-        return result;
-    }
-
-    return -1;
+    return result;
 }
 
 void RegExp::invalidateCode()

diff --git a/Source/JavaScriptCore/runtime/StringPrototype.cpp b/Source/JavaScriptCore/runtime/StringPrototype.cpp
index 3736676..9dd4f81 100644 (file)
--- a/Source/JavaScriptCore/runtime/StringPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/StringPrototype.cpp
@@ -826,6 +826,8 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncMatch(ExecState* exec)
          *  Per ECMA 15.10.4.1, if a0 is undefined substitute the empty string.
          */
         reg = RegExp::create(exec->globalData(), a0.isUndefined() ? UString("") : a0.toString(exec), NoFlags);
+        if (!reg->isValid())
+            return throwVMError(exec, createSyntaxError(exec, reg->errorMessage()));
     }
     RegExpConstructor* regExpConstructor = exec->lexicalGlobalObject()->regExpConstructor();
     int pos;
@@ -876,6 +878,8 @@ EncodedJSValue JSC_HOST_CALL stringProtoFuncSearch(ExecState* exec)
          *  Per ECMA 15.10.4.1, if a0 is undefined substitute the empty string.
          */
         reg = RegExp::create(exec->globalData(), a0.isUndefined() ? UString("") : a0.toString(exec), NoFlags);
+        if (!reg->isValid())
+            return throwVMError(exec, createSyntaxError(exec, reg->errorMessage()));
     }
     RegExpConstructor* regExpConstructor = exec->lexicalGlobalObject()->regExpConstructor();
     int pos;