JavaScriptCore:
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Dec 2008 04:59:06 +0000 (04:59 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Dec 2008 04:59:06 +0000 (04:59 +0000)
2008-12-09  Geoffrey Garen  <ggaren@apple.com>

        Reviewed by Cameron Zwarich.

        In preparation for compiling WREC without PCRE:

        Further relaxed WREC's parsing to be more web-compatible. Fixed PCRE to
        match in cases where it didn't already.

        Changed JavaScriptCore to report syntax errors detected by WREC, rather
        than falling back on PCRE any time WREC sees an error.

        * pcre/pcre_compile.cpp:
        (checkEscape): Relaxed parsing of \c and \N escapes to be more
        web-compatible.

        * runtime/RegExp.cpp:
        (JSC::RegExp::RegExp): Only fall back on PCRE if WREC has not reported
        a syntax error.

        * wrec/WREC.cpp:
        (JSC::WREC::Generator::compileRegExp): Fixed some error reporting to
        match PCRE.

        * wrec/WRECParser.cpp: Added error messages that match PCRE.

        (JSC::WREC::Parser::consumeGreedyQuantifier):
        (JSC::WREC::Parser::parseParentheses):
        (JSC::WREC::Parser::parseCharacterClass):
        (JSC::WREC::Parser::parseNonCharacterEscape): Updated the above functions to
        use the new setError API.

        (JSC::WREC::Parser::consumeEscape): Relaxed parsing of \c \N \u \x \B
        to be more web-compatible.

        (JSC::WREC::Parser::parseAlternative): Distinguish between a malformed
        quantifier and a quantifier with no prefix, like PCRE does.

        (JSC::WREC::Parser::consumeParenthesesType): Updated to use the new setError API.

        * wrec/WRECParser.h:
        (JSC::WREC::Parser::error):
        (JSC::WREC::Parser::syntaxError):
        (JSC::WREC::Parser::parsePattern):
        (JSC::WREC::Parser::reset):
        (JSC::WREC::Parser::setError): Store error messages instead of error codes,
        to provide for exception messages. Use a setter for reporting errors, so
        errors detected early are not overwritten by errors detected later.

LayoutTests:

2008-12-09  Geoffrey Garen  <ggaren@apple.com>

        Reviewed by Cameron Zwarich.

        Updated regular expression layout tests to be agnostic between WREC
        and PCRE quirks. Also, updated results to match new, more web-compatible
        regular expression parsing.

        * fast/js/regexp-charclass-crash-expected.txt:
        * fast/js/regexp-charclass-crash.html:
        * fast/js/regexp-no-extensions-expected.txt:
        * fast/js/resources/regexp-no-extensions.js:
        * fast/regex/test1-expected.txt:

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

12 files changed:
JavaScriptCore/ChangeLog
JavaScriptCore/pcre/pcre_compile.cpp
JavaScriptCore/runtime/RegExp.cpp
JavaScriptCore/wrec/WREC.cpp
JavaScriptCore/wrec/WRECParser.cpp
JavaScriptCore/wrec/WRECParser.h
LayoutTests/ChangeLog
LayoutTests/fast/js/regexp-charclass-crash-expected.txt
LayoutTests/fast/js/regexp-charclass-crash.html
LayoutTests/fast/js/regexp-no-extensions-expected.txt
LayoutTests/fast/js/resources/regexp-no-extensions.js
LayoutTests/fast/regex/test1-expected.txt

index 7ce9df0..51427c3 100644 (file)
@@ -1,3 +1,52 @@
+2008-12-09  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Cameron Zwarich.
+
+        In preparation for compiling WREC without PCRE:
+        
+        Further relaxed WREC's parsing to be more web-compatible. Fixed PCRE to
+        match in cases where it didn't already.
+        
+        Changed JavaScriptCore to report syntax errors detected by WREC, rather
+        than falling back on PCRE any time WREC sees an error.
+        
+        * pcre/pcre_compile.cpp:
+        (checkEscape): Relaxed parsing of \c and \N escapes to be more
+        web-compatible.
+        
+        * runtime/RegExp.cpp:
+        (JSC::RegExp::RegExp): Only fall back on PCRE if WREC has not reported
+        a syntax error.
+
+        * wrec/WREC.cpp:
+        (JSC::WREC::Generator::compileRegExp): Fixed some error reporting to
+        match PCRE.
+
+        * wrec/WRECParser.cpp: Added error messages that match PCRE.
+
+        (JSC::WREC::Parser::consumeGreedyQuantifier):
+        (JSC::WREC::Parser::parseParentheses):
+        (JSC::WREC::Parser::parseCharacterClass):
+        (JSC::WREC::Parser::parseNonCharacterEscape): Updated the above functions to
+        use the new setError API.
+
+        (JSC::WREC::Parser::consumeEscape): Relaxed parsing of \c \N \u \x \B
+        to be more web-compatible.
+
+        (JSC::WREC::Parser::parseAlternative): Distinguish between a malformed
+        quantifier and a quantifier with no prefix, like PCRE does.
+
+        (JSC::WREC::Parser::consumeParenthesesType): Updated to use the new setError API.
+
+        * wrec/WRECParser.h:
+        (JSC::WREC::Parser::error):
+        (JSC::WREC::Parser::syntaxError):
+        (JSC::WREC::Parser::parsePattern):
+        (JSC::WREC::Parser::reset):
+        (JSC::WREC::Parser::setError): Store error messages instead of error codes,
+        to provide for exception messages. Use a setter for reporting errors, so
+        errors detected early are not overwritten by errors detected later.
+
 2008-12-09  Gavin Barraclough  <barraclough@apple.com>
 
         Reviewed by Oliver Hunt.
index fdd90af..a2cfb36 100644 (file)
@@ -236,8 +236,11 @@ static int checkEscape(const UChar** ptrPtr, const UChar* patternEnd, ErrorCode*
                 /* Handle an octal number following \. If the first digit is 8 or 9,
                  this is not octal. */
                 
-                if ((c = *ptr) >= '8')
+                if ((c = *ptr) >= '8') {
+                    c = '\\';
+                    ptr -= 1;
                     break;
+                }
 
             /* \0 always starts an octal number, but we may drop through to here with a
              larger first octal digit. */
@@ -298,8 +301,14 @@ static int checkEscape(const UChar** ptrPtr, const UChar* patternEnd, ErrorCode*
                     *errorCodePtr = ERR2;
                     return 0;
                 }
-                c = *ptr;
                 
+                c = *ptr;
+                if (!isASCIIAlpha(c)) {
+                    c = '\\';
+                    ptr -= 2;
+                    break;
+                }
+
                 /* A letter is upper-cased; then the 0x40 bit is flipped. This coding
                  is ASCII-specific, but then the whole concept of \cx is ASCII-specific. */
                 c = toASCIIUpper(c) ^ 0x40;
index 5556ab5..722914d 100644 (file)
@@ -47,7 +47,7 @@ inline RegExp::RegExp(JSGlobalData* globalData, const UString& pattern)
     UNUSED_PARAM(globalData);
 #if ENABLE(WREC)
     m_wrecFunction = Generator::compileRegExp(globalData, pattern, &m_numSubpatterns, &m_constructionError, m_executablePool);
-    if (m_wrecFunction)
+    if (m_wrecFunction || m_constructionError)
         return;
     // Fall through to non-WREC case.
 #endif
@@ -90,7 +90,7 @@ inline RegExp::RegExp(JSGlobalData* globalData, const UString& pattern, const US
 
 #if ENABLE(WREC)
     m_wrecFunction = Generator::compileRegExp(globalData, pattern, &m_numSubpatterns, &m_constructionError, m_executablePool, (m_flagBits & IgnoreCase), (m_flagBits & Multiline));
-    if (m_wrecFunction)
+    if (m_wrecFunction || m_constructionError)
         return;
     // Fall through to non-WREC case.
 #endif
index 6c1aaad..099931b 100644 (file)
@@ -46,7 +46,7 @@ static const int MaxPatternSize = (1 << 13);
 CompiledRegExp Generator::compileRegExp(JSGlobalData* globalData, const UString& pattern, unsigned* numSubpatterns_ptr, const char** error_ptr, RefPtr<ExecutablePool>& pool, bool ignoreCase, bool multiline)
 {
     if (pattern.size() > MaxPatternSize) {
-        *error_ptr = "Regular expression too large.";
+        *error_ptr = "regular expression too large";
         return 0;
     }
 
@@ -75,7 +75,7 @@ CompiledRegExp Generator::compileRegExp(JSGlobalData* globalData, const UString&
     generator.generateReturnFailure();
 
     if (parser.error()) {
-        *error_ptr = "Regular expression malformed.";
+        *error_ptr = parser.syntaxError(); // NULL in the case of patterns that WREC doesn't support yet.
         return 0;
     }
 
index bca6ca9..b664f36 100644 (file)
@@ -35,6 +35,16 @@ using namespace WTF;
 
 namespace JSC { namespace WREC {
 
+// These error messages match the error messages used by PCRE.
+const char* Parser::QuantifierOutOfOrder = "numbers out of order in {} quantifier";
+const char* Parser::QuantifierWithoutAtom = "nothing to repeat";
+const char* Parser::ParenthesesUnmatched = "unmatched parentheses";
+const char* Parser::ParenthesesTypeInvalid = "unrecognized character after (?";
+const char* Parser::ParenthesesNotSupported = ""; // Not a user-visible syntax error -- just signals a syntax that WREC doesn't support yet.
+const char* Parser::CharacterClassUnmatched = "missing terminating ] for character class";
+const char* Parser::CharacterClassOutOfOrder = "range out of order in character class";
+const char* Parser::EscapeUnterminated = "\\ at end of pattern";
+
 class PatternCharacterSequence {
 typedef Generator::JumpList JumpList;
 
@@ -139,7 +149,7 @@ ALWAYS_INLINE Quantifier Parser::consumeGreedyQuantifier()
             consume();
  
             if (min > max) {
-                m_error = MalformedQuantifier;
+                setError(QuantifierOutOfOrder);
                 return Quantifier(Quantifier::Error);
             }
 
@@ -232,12 +242,12 @@ bool Parser::parseParentheses(JumpList& failures)
             break;
 
         default:
-            m_error = UnsupportedParentheses;
+            setError(ParenthesesNotSupported);
             return false;
     }
 
     if (consume() != ')') {
-        m_error = MalformedParentheses;
+        setError(ParenthesesUnmatched);
         return false;
     }
 
@@ -248,11 +258,11 @@ bool Parser::parseParentheses(JumpList& failures)
             return true;
 
         case Quantifier::Greedy:
-            m_error = UnsupportedParentheses;
+            setError(ParenthesesNotSupported);
             return false;
 
         case Quantifier::NonGreedy:
-            m_error = UnsupportedParentheses;
+            setError(ParenthesesNotSupported);
             return false;
 
         case Quantifier::Error:
@@ -273,11 +283,11 @@ bool Parser::parseCharacterClass(JumpList& failures)
 
     CharacterClassConstructor constructor(m_ignoreCase);
 
-    UChar ch;
+    int ch;
     while ((ch = peek()) != ']') {
         switch (ch) {
         case EndOfPattern:
-            m_error = MalformedCharacterClass;
+            setError(CharacterClassUnmatched);
             return false;
 
         case '\\': {
@@ -298,10 +308,8 @@ bool Parser::parseCharacterClass(JumpList& failures)
                     constructor.append(characterClassEscape.characterClass());
                     break;
                 }
-                case Escape::Error: {
-                    m_error = MalformedEscape;
+                case Escape::Error:
                     return false;
-                }
                 case Escape::Backreference:
                 case Escape::WordBoundaryAssertion: {
                     ASSERT_NOT_REACHED();
@@ -320,7 +328,7 @@ bool Parser::parseCharacterClass(JumpList& failures)
 
     // lazily catch reversed ranges ([z-a])in character classes
     if (constructor.isUpsideDown()) {
-        m_error = MalformedCharacterClass;
+        setError(CharacterClassOutOfOrder);
         return false;
     }
 
@@ -347,7 +355,6 @@ bool Parser::parseNonCharacterEscape(JumpList& failures, const Escape& escape)
             return true;
 
         case Escape::Error:
-            m_error = MalformedEscape;
             return false;
     }
 
@@ -359,6 +366,7 @@ Escape Parser::consumeEscape(bool inCharacterClass)
 {
     switch (peek()) {
     case EndOfPattern:
+        setError(EscapeUnterminated);
         return Escape(Escape::Error);
 
     // Assertions
@@ -370,7 +378,7 @@ Escape Parser::consumeEscape(bool inCharacterClass)
     case 'B':
         consume();
         if (inCharacterClass)
-            return Escape(Escape::Error);
+            return PatternCharacterEscape('B');
         return WordBoundaryAssertionEscape(true); // invert
 
     // CharacterClassEscape
@@ -412,7 +420,7 @@ Escape Parser::consumeEscape(bool inCharacterClass)
         if (peekDigit() > m_numSubpatterns || inCharacterClass) {
             // To match Firefox, we parse an invalid backreference in the range [1-7]
             // as an octal escape.
-            return peekDigit() > 7 ? Escape(Escape::Error) : PatternCharacterEscape(consumeOctal());
+            return peekDigit() > 7 ? PatternCharacterEscape('\\') : PatternCharacterEscape(consumeOctal());
         }
 
         int value = 0;
@@ -451,28 +459,40 @@ Escape Parser::consumeEscape(bool inCharacterClass)
 
     // ControlLetter
     case 'c': {
+        SavedState state(*this);
         consume();
+        
         int control = consume();
-        if (!isASCIIAlpha(control))
-            return Escape(Escape::Error);
+        if (!isASCIIAlpha(control)) {
+            state.restore();
+            return PatternCharacterEscape('\\');
+        }
         return PatternCharacterEscape(control & 31);
     }
 
     // HexEscape
     case 'x': {
         consume();
+
+        SavedState state(*this);
         int x = consumeHex(2);
-        if (x == -1)
-            return Escape(Escape::Error);
+        if (x == -1) {
+            state.restore();
+            return PatternCharacterEscape('x');
+        }
         return PatternCharacterEscape(x);
     }
 
     // UnicodeEscape
     case 'u': {
         consume();
+
+        SavedState state(*this);
         int x = consumeHex(4);
-        if (x == -1)
-            return Escape(Escape::Error);
+        if (x == -1) {
+            state.restore();
+            return PatternCharacterEscape('u');
+        }
         return PatternCharacterEscape(x);
     }
 
@@ -505,8 +525,11 @@ void Parser::parseAlternative(JumpList& failures)
                 continue;
             }
 
-            if (q.type == Quantifier::Error || !sequence.size()) {
-                m_error = MalformedQuantifier;
+            if (q.type == Quantifier::Error)
+                return;
+
+            if (!sequence.size()) {
+                setError(QuantifierWithoutAtom);
                 return;
             }
 
@@ -610,7 +633,7 @@ Generator::ParenthesesType Parser::consumeParenthesesType()
         return Generator::InvertedAssertion;
 
     default:
-        m_error = MalformedParentheses;
+        setError(ParenthesesTypeInvalid);
         return Generator::Error;
     }
 }
index 9405f77..a3e151b 100644 (file)
@@ -47,16 +47,6 @@ namespace JSC { namespace WREC {
     friend class SavedState;
 
     public:
-        enum Error {
-            NoError,
-            MalformedCharacterClass,
-            MalformedParentheses,
-            MalformedPattern,
-            MalformedQuantifier,
-            MalformedEscape,
-            UnsupportedParentheses,
-        };
-
         Parser(const UString& pattern, bool ignoreCase, bool multiline)
             : m_generator(*this)
             , m_data(pattern.data())
@@ -75,7 +65,8 @@ namespace JSC { namespace WREC {
         void recordSubpattern() { ++m_numSubpatterns; }
         unsigned numSubpatterns() const { return m_numSubpatterns; }
         
-        Error error() const { return m_error; }
+        const char* error() const { return m_error; }
+        const char* syntaxError() const { return m_error == ParenthesesNotSupported ? 0 : m_error; }
         
         void parsePattern(JumpList& failures)
         {
@@ -84,7 +75,7 @@ namespace JSC { namespace WREC {
             parseDisjunction(failures);
 
             if (peek() != EndOfPattern)
-                m_error = MalformedPattern; // Parsing the pattern should fully consume it.
+                setError(ParenthesesUnmatched); // Parsing the pattern should fully consume it.
         }
 
         void parseDisjunction(JumpList& failures);
@@ -119,7 +110,14 @@ namespace JSC { namespace WREC {
         {
             m_index = 0;
             m_numSubpatterns = 0;
-            m_error = NoError;
+            m_error = 0;
+        }
+
+        void setError(const char* error)
+        {
+            if (m_error)
+                return;
+            m_error = error;
         }
 
         int peek()
@@ -189,6 +187,16 @@ namespace JSC { namespace WREC {
 
         static const int EndOfPattern = -1;
 
+        // Error messages.
+        static const char* QuantifierOutOfOrder;
+        static const char* QuantifierWithoutAtom;
+        static const char* ParenthesesUnmatched;
+        static const char* ParenthesesTypeInvalid;
+        static const char* ParenthesesNotSupported;
+        static const char* CharacterClassUnmatched;
+        static const char* CharacterClassOutOfOrder;
+        static const char* EscapeUnterminated;
+
         Generator m_generator;
         const UChar* m_data;
         unsigned m_size;
@@ -196,7 +204,7 @@ namespace JSC { namespace WREC {
         bool m_ignoreCase;
         bool m_multiline;
         unsigned m_numSubpatterns;
-        Error m_error;
+        const char* m_error;
     };
 
 } } // namespace JSC::WREC
index 1e5ec75..3d4ed97 100644 (file)
@@ -1,3 +1,17 @@
+2008-12-09  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Cameron Zwarich.
+
+        Updated regular expression layout tests to be agnostic between WREC
+        and PCRE quirks. Also, updated results to match new, more web-compatible
+        regular expression parsing.
+
+        * fast/js/regexp-charclass-crash-expected.txt:
+        * fast/js/regexp-charclass-crash.html:
+        * fast/js/regexp-no-extensions-expected.txt:
+        * fast/js/resources/regexp-no-extensions.js:
+        * fast/regex/test1-expected.txt:
+
 2008-12-09  David Levin  <levin@chromium.org>
 
         Reviewed by Eric Seidel.
index df6d454..6ba7e9a 100644 (file)
@@ -1,3 +1,3 @@
 Tests a crash in the regular expression engine. If this stops with a single "regular expression too large" exception, then the test succeeded.
 
-Got up to iteration 1872 and then got this exception: SyntaxError: Invalid regular expression: regular expression too large.
+Got over 1000 iterations and then got this exception: SyntaxError: Invalid regular expression: regular expression too large.
index ce35d72..9bfcc0b 100644 (file)
@@ -11,8 +11,8 @@ for (; i < 10000; ++i) { // >
     try {
         new RegExp(string);
     } catch (exception) {
-        if (/too large/.test(exception)) {
-            document.writeln("<div>Got up to iteration " + i + " and then got this exception: " + exception + ".</div>");
+        if (/too large/.test(exception) && i > 1000) {
+            document.writeln("<div>Got over 1000 iterations and then got this exception: " + exception + ".</div>");
             break;
         }
     }
index 238eb55..6a995dd 100644 (file)
@@ -10,7 +10,7 @@ PASS /[\x1g]/.exec("x").toString() is "x"
 PASS /[\x1g]/.exec("1").toString() is "1"
 PASS /\2147483648/.exec(String.fromCharCode(140) + "7483648").toString() is String.fromCharCode(140) + "7483648"
 PASS /\4294967296/.exec("\"94967296").toString() is "\"94967296"
-PASS /\8589934592/.exec("8589934592").toString() is "8589934592"
+PASS /\8589934592/.exec("\\8589934592").toString() is "\\8589934592"
 PASS "\nAbc\n".replace(/(\n)[^\n]+$/, "$1") is "\nAbc\n"
 PASS /x$/.exec("x\n") is null
 PASS /x++/ threw exception SyntaxError: Invalid regular expression: nothing to repeat.
@@ -33,7 +33,7 @@ PASS /[\10q]/.exec("y" + String.fromCharCode(8) + "q").toString() is String.from
 PASS /\1q/.exec("y" + String.fromCharCode(1) + "q").toString() is String.fromCharCode(1) + "q"
 PASS /[\1q]/.exec("y" + String.fromCharCode(1) + "q").toString() is String.fromCharCode(1)
 PASS /[\1q]/.exec("yq").toString() is "q"
-PASS /\8q/.exec("y8q").toString() is "8q"
+PASS /\8q/.exec("\\8q").toString() is "\\8q"
 PASS /[\8q]/.exec("y8q").toString() is "8"
 PASS /[\8q]/.exec("yq").toString() is "q"
 PASS /(x)\1q/.exec("xxq").toString() is "xxq,x"
index ced4d6f..8b7e661 100644 (file)
@@ -11,7 +11,7 @@ shouldBe('/[\\x1g]/.exec("x").toString()', '"x"');
 shouldBe('/[\\x1g]/.exec("1").toString()', '"1"');
 shouldBe('/\\2147483648/.exec(String.fromCharCode(140) + "7483648").toString()', 'String.fromCharCode(140) + "7483648"');
 shouldBe('/\\4294967296/.exec("\\"94967296").toString()', '"\\"94967296"');
-shouldBe('/\\8589934592/.exec("8589934592").toString()', '"8589934592"');
+shouldBe('/\\8589934592/.exec("\\\\8589934592").toString()', '"\\\\8589934592"');
 shouldBe('"\\nAbc\\n".replace(/(\\n)[^\\n]+$/, "$1")', '"\\nAbc\\n"');
 shouldBe('/x$/.exec("x\\n")', 'null');
 shouldThrow('/x++/');
@@ -36,7 +36,7 @@ shouldBe('/[\\10q]/.exec("y" + String.fromCharCode(8) + "q").toString()', 'Strin
 shouldBe('/\\1q/.exec("y" + String.fromCharCode(1) + "q").toString()', 'String.fromCharCode(1) + "q"');
 shouldBe('/[\\1q]/.exec("y" + String.fromCharCode(1) + "q").toString()', 'String.fromCharCode(1)');
 shouldBe('/[\\1q]/.exec("yq").toString()', '"q"');
-shouldBe('/\\8q/.exec("y8q").toString()', '"8q"');
+shouldBe('/\\8q/.exec("\\\\8q").toString()', '"\\\\8q"');
 shouldBe('/[\\8q]/.exec("y8q").toString()', '"8"');
 shouldBe('/[\\8q]/.exec("yq").toString()', '"q"');
 shouldBe('/(x)\\1q/.exec("xxq").toString()', '"xxq,x"');
index 4e723e5..f6e47ea 100644 (file)
@@ -108,7 +108,7 @@ Test 1: main functionality.
     babababc: PASS
 
 /^\ca\cA\c[\c{\c:/
-    \ 1\ 1\e;z: FAIL. Actual results: "null"
+FAILED TO COMPILE
 
 /^[ab\]cde]/
     athing: PASS