[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198

oliver at apple.com oliver at apple.com
Sun Feb 20 23:00:47 UTC 2011


The following commit has been merged in the webkit-1.3 branch:
commit 1a6a9f7b51db62c4e9ff80fe7f9a3adcbb537426
Author: oliver at apple.com <oliver at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Sat Jan 15 01:22:58 2011 +0000

    2011-01-14  Oliver Hunt  <oliver at apple.com>
    
            Reviewed by Gavin Barraclough.
    
            [jsfunfuzz] parser doesn't enforce continue restrictions correctly.
            https://bugs.webkit.org/show_bug.cgi?id=52493
    
            Add a few tests for continue to cover the cases where continue
            isn't syntactically valid.
    
            * fast/js/js-continue-break-restrictions-expected.txt: Added.
            * fast/js/js-continue-break-restrictions.html: Added.
            * fast/js/script-tests/js-continue-break-restrictions.js: Added.
    2011-01-14  Oliver Hunt  <oliver at apple.com>
    
            Reviewed by Gavin Barraclough.
    
            [jsfunfuzz] parser doesn't enforce continue restrictions correctly.
            https://bugs.webkit.org/show_bug.cgi?id=52493
    
            This patch reworks handling of break, continue and label statements
            to correctly handle all the valid and invalid cases.  Previously certain
            errors would be missed by the parser in strict mode, but the bytecode
            generator needed to handle those cases for non-strict code so nothing
            failed, it simply became non-standard behaviour.
    
            Now that we treat break and continue errors as early faults in non-strict
            mode as well that safety net has been removed so the parser bugs result in
            crashes at codegen time.
    
            * parser/JSParser.cpp:
            (JSC::JSParser::ScopeLabelInfo::ScopeLabelInfo):
            (JSC::JSParser::next):
            (JSC::JSParser::nextTokenIsColon):
            (JSC::JSParser::continueIsValid):
                Continue is only valid in loops so we can't use breakIsValid()
            (JSC::JSParser::pushLabel):
                We now track whether the label is for a loop (and is therefore a
                valid target for continue.
            (JSC::JSParser::popLabel):
            (JSC::JSParser::getLabel):
                Replace hasLabel with getLabel so that we can validate the target
                when parsing continue statements.
            (JSC::JSParser::Scope::continueIsValid):
            (JSC::JSParser::Scope::pushLabel):
            (JSC::JSParser::Scope::getLabel):
            (JSC::JSParser::JSParser):
            (JSC::JSParser::parseBreakStatement):
            (JSC::JSParser::parseContinueStatement):
            (JSC::LabelInfo::LabelInfo):
            (JSC::JSParser::parseExpressionOrLabelStatement):
                Consecutive labels now get handled iteratively so that we can determine
                whether they're valid targets for continue.
            * parser/Lexer.cpp:
            (JSC::Lexer::nextTokenIsColon):
            * parser/Lexer.h:
            (JSC::Lexer::setOffset):
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@75852 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index ccf6b28..a168380 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2011-01-14  Oliver Hunt  <oliver at apple.com>
+
+        Reviewed by Gavin Barraclough.
+
+        [jsfunfuzz] parser doesn't enforce continue restrictions correctly.
+        https://bugs.webkit.org/show_bug.cgi?id=52493
+
+        Add a few tests for continue to cover the cases where continue
+        isn't syntactically valid.
+
+        * fast/js/js-continue-break-restrictions-expected.txt: Added.
+        * fast/js/js-continue-break-restrictions.html: Added.
+        * fast/js/script-tests/js-continue-break-restrictions.js: Added.
+
 2011-01-14  Maciej Stachowiak  <mjs at apple.com>
 
         Reviewed by Anders Carlsson.
diff --git a/LayoutTests/fast/js/js-continue-break-restrictions-expected.txt b/LayoutTests/fast/js/js-continue-break-restrictions-expected.txt
new file mode 100644
index 0000000..75b073c
--- /dev/null
+++ b/LayoutTests/fast/js/js-continue-break-restrictions-expected.txt
@@ -0,0 +1,37 @@
+Verify that invalid continue and break statements are handled correctly
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS L:{true;break L;false} is true
+PASS if (0) { L:{ break; } } threw exception SyntaxError: Parse error.
+PASS if (0) { L:{ continue L; } } threw exception SyntaxError: Parse error.
+PASS if (0) { L:{ continue; } } threw exception SyntaxError: Parse error.
+PASS if (0) { switch (1) { case 1: continue; } } threw exception SyntaxError: Parse error.
+PASS A:L:{true;break L;false} is true
+PASS if (0) { A:L:{ break; } } threw exception SyntaxError: Parse error.
+PASS if (0) { A:L:{ continue L; } } threw exception SyntaxError: Parse error.
+PASS if (0) { A:L:{ continue; } } threw exception SyntaxError: Parse error.
+PASS L:A:{true;break L;false} is true
+PASS if (0) { L:A:{ break; } } threw exception SyntaxError: Parse error.
+PASS if (0) { L:A:{ continue L; } } threw exception SyntaxError: Parse error.
+PASS if (0) { L:A:{ continue; } } threw exception SyntaxError: Parse error.
+PASS if(0){ L:for(;;) continue L; } is undefined.
+PASS if(0){ L:A:for(;;) continue L; } is undefined.
+PASS if(0){ A:L:for(;;) continue L; } is undefined.
+PASS if(0){ A:for(;;) L:continue L; } threw exception SyntaxError: Parse error.
+PASS if(0){ L:for(;;) A:continue L; } is undefined.
+PASS if(0){ L:do continue L; while(0); } is undefined.
+PASS if(0){ L:A:do continue L; while(0); } is undefined.
+PASS if(0){ A:L:do continue L; while(0);} is undefined.
+PASS if(0){ A:do L:continue L; while(0); } threw exception SyntaxError: Parse error.
+PASS if(0){ L:do A:continue L; while(0); } is undefined.
+PASS if(0){ L:while(0) continue L; } is undefined.
+PASS if(0){ L:A:while(0) continue L; } is undefined.
+PASS if(0){ A:L:while(0) continue L; } is undefined.
+PASS if(0){ A:while(0) L:continue L; } threw exception SyntaxError: Parse error.
+PASS if(0){ L:while(0) A:continue L; } is undefined.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/js-continue-break-restrictions.html b/LayoutTests/fast/js/js-continue-break-restrictions.html
new file mode 100644
index 0000000..6aa9e88
--- /dev/null
+++ b/LayoutTests/fast/js/js-continue-break-restrictions.html
@@ -0,0 +1,13 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/js-continue-break-restrictions.js"></script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/js/script-tests/js-continue-break-restrictions.js b/LayoutTests/fast/js/script-tests/js-continue-break-restrictions.js
new file mode 100644
index 0000000..f50485d
--- /dev/null
+++ b/LayoutTests/fast/js/script-tests/js-continue-break-restrictions.js
@@ -0,0 +1,38 @@
+description("Verify that invalid continue and break statements are handled correctly");
+
+shouldBeTrue("L:{true;break L;false}");
+shouldThrow("if (0) { L:{ break; } }");
+shouldThrow("if (0) { L:{ continue L; } }");
+shouldThrow("if (0) { L:{ continue; } }");
+shouldThrow("if (0) { switch (1) { case 1: continue; } }");
+shouldBeTrue("A:L:{true;break L;false}");
+shouldThrow("if (0) { A:L:{ break; } }");
+shouldThrow("if (0) { A:L:{ continue L; } }");
+shouldThrow("if (0) { A:L:{ continue; } }");
+
+shouldBeTrue("L:A:{true;break L;false}");
+shouldThrow("if (0) { L:A:{ break; } }");
+shouldThrow("if (0) { L:A:{ continue L; } }");
+shouldThrow("if (0) { L:A:{ continue; } }");
+
+shouldBeUndefined("if(0){ L:for(;;) continue L; }")
+shouldBeUndefined("if(0){ L:A:for(;;) continue L; }")
+shouldBeUndefined("if(0){ A:L:for(;;) continue L; }")
+shouldThrow("if(0){ A:for(;;) L:continue L; }")
+shouldBeUndefined("if(0){ L:for(;;) A:continue L; }")
+
+shouldBeUndefined("if(0){ L:do continue L; while(0); }")
+shouldBeUndefined("if(0){ L:A:do continue L; while(0); }")
+shouldBeUndefined("if(0){ A:L:do continue L; while(0);}")
+shouldThrow("if(0){ A:do L:continue L; while(0); }")
+shouldBeUndefined("if(0){ L:do A:continue L; while(0); }")
+
+
+shouldBeUndefined("if(0){ L:while(0) continue L; }")
+shouldBeUndefined("if(0){ L:A:while(0) continue L; }")
+shouldBeUndefined("if(0){ A:L:while(0) continue L; }")
+shouldThrow("if(0){ A:while(0) L:continue L; }")
+shouldBeUndefined("if(0){ L:while(0) A:continue L; }")
+
+
+var successfullyParsed = true;
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 4f01a8b..a3043fb 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,48 @@
+2011-01-14  Oliver Hunt  <oliver at apple.com>
+
+        Reviewed by Gavin Barraclough.
+
+        [jsfunfuzz] parser doesn't enforce continue restrictions correctly.
+        https://bugs.webkit.org/show_bug.cgi?id=52493
+
+        This patch reworks handling of break, continue and label statements
+        to correctly handle all the valid and invalid cases.  Previously certain
+        errors would be missed by the parser in strict mode, but the bytecode 
+        generator needed to handle those cases for non-strict code so nothing
+        failed, it simply became non-standard behaviour.
+
+        Now that we treat break and continue errors as early faults in non-strict
+        mode as well that safety net has been removed so the parser bugs result in
+        crashes at codegen time.
+
+        * parser/JSParser.cpp:
+        (JSC::JSParser::ScopeLabelInfo::ScopeLabelInfo):
+        (JSC::JSParser::next):
+        (JSC::JSParser::nextTokenIsColon):
+        (JSC::JSParser::continueIsValid):
+            Continue is only valid in loops so we can't use breakIsValid()
+        (JSC::JSParser::pushLabel):
+            We now track whether the label is for a loop (and is therefore a
+            valid target for continue.
+        (JSC::JSParser::popLabel):
+        (JSC::JSParser::getLabel):
+            Replace hasLabel with getLabel so that we can validate the target
+            when parsing continue statements.
+        (JSC::JSParser::Scope::continueIsValid):
+        (JSC::JSParser::Scope::pushLabel):
+        (JSC::JSParser::Scope::getLabel):
+        (JSC::JSParser::JSParser):
+        (JSC::JSParser::parseBreakStatement):
+        (JSC::JSParser::parseContinueStatement):
+        (JSC::LabelInfo::LabelInfo):
+        (JSC::JSParser::parseExpressionOrLabelStatement):
+            Consecutive labels now get handled iteratively so that we can determine
+            whether they're valid targets for continue.
+        * parser/Lexer.cpp:
+        (JSC::Lexer::nextTokenIsColon):
+        * parser/Lexer.h:
+        (JSC::Lexer::setOffset):
+
 2011-01-14  Patrick Gansterer  <paroga at webkit.org>
 
         Reviewed by Adam Roben.
diff --git a/Source/JavaScriptCore/parser/JSParser.cpp b/Source/JavaScriptCore/parser/JSParser.cpp
index 142f403..792d19b 100644
--- a/Source/JavaScriptCore/parser/JSParser.cpp
+++ b/Source/JavaScriptCore/parser/JSParser.cpp
@@ -84,6 +84,16 @@ private:
         JSParser* m_parser;
         bool m_oldAllowsIn;
     };
+    
+    struct ScopeLabelInfo {
+        ScopeLabelInfo(StringImpl* ident, bool isLoop)
+        : m_ident(ident)
+        , m_isLoop(isLoop)
+        {
+        }
+        StringImpl* m_ident;
+        bool m_isLoop;
+    };
 
     void next(Lexer::LexType lexType = Lexer::IdentifyReservedWords)
     {
@@ -91,7 +101,11 @@ private:
         m_lastTokenEnd = m_token.m_info.endOffset;
         m_lexer->setLastLineNumber(m_lastLine);
         m_token.m_type = m_lexer->lex(&m_token.m_data, &m_token.m_info, lexType, strictMode());
-        m_tokenCount++;
+    }
+    
+    bool nextTokenIsColon()
+    {
+        return m_lexer->nextTokenIsColon();
     }
 
     bool consume(JSTokenType expected)
@@ -140,18 +154,29 @@ private:
         }
         return true;
     }
-    void pushLabel(const Identifier* label) { currentScope()->pushLabel(label); }
-    void popLabel() { currentScope()->popLabel(); }
-    bool hasLabel(const Identifier* label)
+    bool continueIsValid()
     {
         ScopeRef current = currentScope();
-        while (!current->hasLabel(label)) {
+        while (!current->continueIsValid()) {
             if (!current.hasContainingScope())
                 return false;
             current = current.containingScope();
         }
         return true;
     }
+    void pushLabel(const Identifier* label, bool isLoop) { currentScope()->pushLabel(label, isLoop); }
+    void popLabel() { currentScope()->popLabel(); }
+    ScopeLabelInfo* getLabel(const Identifier* label)
+    {
+        ScopeRef current = currentScope();
+        ScopeLabelInfo* result = 0;
+        while (!(result = current->getLabel(label))) {
+            if (!current.hasContainingScope())
+                return 0;
+            current = current.containingScope();
+        }
+        return result;
+    }
 
     enum SourceElementsMode { CheckForStrictMode, DontCheckForStrictMode };
     template <SourceElementsMode mode, class TreeBuilder> TreeSourceElements parseSourceElements(TreeBuilder&);
@@ -224,7 +249,6 @@ private:
     JSGlobalData* m_globalData;
     JSToken m_token;
     bool m_allowsIn;
-    int m_tokenCount;
     int m_lastLine;
     int m_lastTokenEnd;
     int m_assignmentCount;
@@ -250,7 +274,7 @@ private:
         int m_originalDepth;
         int* m_depth;
     };
-
+    
     struct Scope {
         Scope(JSGlobalData* globalData, bool isFunction, bool strictMode)
             : m_globalData(globalData)
@@ -274,12 +298,13 @@ private:
         void endLoop() { ASSERT(m_loopDepth); m_loopDepth--; }
         bool inLoop() { return !!m_loopDepth; }
         bool breakIsValid() { return m_loopDepth || m_switchDepth; }
+        bool continueIsValid() { return m_loopDepth; }
 
-        void pushLabel(const Identifier* label)
+        void pushLabel(const Identifier* label, bool isLoop)
         {
             if (!m_labels)
                 m_labels = new LabelStack;
-            m_labels->append(label->impl());
+            m_labels->append(ScopeLabelInfo(label->impl(), isLoop));
         }
 
         void popLabel()
@@ -289,15 +314,15 @@ private:
             m_labels->removeLast();
         }
 
-        bool hasLabel(const Identifier* label)
+        ScopeLabelInfo* getLabel(const Identifier* label)
         {
             if (!m_labels)
-                return false;
+                return 0;
             for (int i = m_labels->size(); i > 0; i--) {
-                if (m_labels->at(i - 1) == label->impl())
-                    return true;
+                if (m_labels->at(i - 1).m_ident == label->impl())
+                    return &m_labels->at(i - 1);
             }
-            return false;
+            return 0;
         }
 
         void setIsFunction()
@@ -405,7 +430,8 @@ private:
         bool m_isValidStrictMode : 1;
         int m_loopDepth;
         int m_switchDepth;
-        typedef Vector<StringImpl*, 2> LabelStack;
+
+        typedef Vector<ScopeLabelInfo, 2> LabelStack;
         LabelStack* m_labels;
         IdentifierSet m_declaredVariables;
         IdentifierSet m_usedVariables;
@@ -532,7 +558,6 @@ JSParser::JSParser(Lexer* lexer, JSGlobalData* globalData, FunctionParameters* p
     , m_errorMessage("Parse error")
     , m_globalData(globalData)
     , m_allowsIn(true)
-    , m_tokenCount(0)
     , m_lastLine(0)
     , m_lastTokenEnd(0)
     , m_assignmentCount(0)
@@ -862,7 +887,7 @@ template <class TreeBuilder> TreeStatement JSParser::parseBreakStatement(TreeBui
     }
     matchOrFail(IDENT);
     const Identifier* ident = m_token.m_data.ident;
-    failIfFalse(hasLabel(ident));
+    failIfFalse(getLabel(ident));
     endCol = tokenEnd();
     endLine = tokenLine();
     next();
@@ -880,12 +905,14 @@ template <class TreeBuilder> TreeStatement JSParser::parseContinueStatement(Tree
     next();
 
     if (autoSemiColon()) {
-        failIfFalse(breakIsValid());
+        failIfFalse(continueIsValid());
         return context.createContinueStatement(startCol, endCol, startLine, endLine);
     }
     matchOrFail(IDENT);
     const Identifier* ident = m_token.m_data.ident;
-    failIfFalse(hasLabel(ident));
+    ScopeLabelInfo* label = getLabel(ident);
+    failIfFalse(label);
+    failIfFalse(label->m_isLoop);
     endCol = tokenEnd();
     endLine = tokenLine();
     next();
@@ -1240,35 +1267,79 @@ template <class TreeBuilder> TreeStatement JSParser::parseFunctionDeclaration(Tr
     return context.createFuncDeclStatement(name, body, parameters, openBracePos, closeBracePos, bodyStartLine, m_lastLine);
 }
 
+struct LabelInfo {
+    LabelInfo(const Identifier* ident, int start, int end)
+        : m_ident(ident)
+        , m_start(start)
+        , m_end(end)
+    {
+    }
+
+    const Identifier* m_ident;
+    int m_start;
+    int m_end;
+};
+
 template <class TreeBuilder> TreeStatement JSParser::parseExpressionOrLabelStatement(TreeBuilder& context)
 {
 
-    /* Expression and Label statements are ambiguous at LL(1), to avoid
-     * the cost of having a token buffer to support LL(2) we simply assume
-     * we have an expression statement, and then only look for a label if that
-     * parse fails.
+    /* Expression and Label statements are ambiguous at LL(1), so we have a
+     * special case that looks for a colon as the next character in the input.
      */
-    int start = tokenStart();
-    int startLine = tokenLine();
-    const Identifier* ident = m_token.m_data.ident;
-    int currentToken = m_tokenCount;
-    TreeExpression expression = parseExpression(context);
-    failIfFalse(expression);
-    if (autoSemiColon())
-        return context.createExprStatement(expression, startLine, m_lastLine);
-    failIfFalse(currentToken + 1 == m_tokenCount);
-    int end = tokenEnd();
-    consumeOrFail(COLON);
+    Vector<LabelInfo> labels;
+
+    do {
+        int start = tokenStart();
+        int startLine = tokenLine();
+        if (!nextTokenIsColon()) {
+            // If we hit this path we're making a expression statement, which
+            // by definition can't make use of continue/break so we can just
+            // ignore any labels we might have accumulated.
+            TreeExpression expression = parseExpression(context);
+            failIfFalse(expression);
+            failIfFalse(autoSemiColon());
+            return context.createExprStatement(expression, startLine, m_lastLine);
+        }
+        const Identifier* ident = m_token.m_data.ident;
+        int end = tokenEnd();
+        next();
+        consumeOrFail(COLON);
+        if (!m_syntaxAlreadyValidated) {
+            // This is O(N^2) over the current list of consecutive labels, but I
+            // have never seen more than one label in a row in the real world.
+            for (size_t i = 0; i < labels.size(); i++)
+                failIfTrue(ident == labels[i].m_ident);
+            failIfTrue(getLabel(ident));
+            labels.append(LabelInfo(ident, start, end));
+        }
+    } while (match(IDENT));
+    bool isLoop = false;
+    switch (m_token.m_type) {
+    case FOR:
+    case WHILE:
+    case DO:
+        isLoop = true;
+        break;
+
+    default:
+        break;
+    }
     const Identifier* unused = 0;
     if (!m_syntaxAlreadyValidated) {
-        failIfTrue(hasLabel(ident));
-        pushLabel(ident);
+        for (size_t i = 0; i < labels.size(); i++)
+            pushLabel(labels[i].m_ident, isLoop);
     }
     TreeStatement statement = parseStatement(context, unused);
-    if (!m_syntaxAlreadyValidated)
-        popLabel();
+    if (!m_syntaxAlreadyValidated) {
+        for (size_t i = 0; i < labels.size(); i++)
+            popLabel();
+    }
     failIfFalse(statement);
-    return context.createLabelStatement(ident, statement, start, end);
+    for (size_t i = 0; i < labels.size(); i++) {
+        const LabelInfo& info = labels[labels.size() - i - 1];
+        statement = context.createLabelStatement(info.m_ident, statement, info.m_start, info.m_end);
+    }
+    return statement;
 }
 
 template <class TreeBuilder> TreeStatement JSParser::parseExpressionStatement(TreeBuilder& context)
diff --git a/Source/JavaScriptCore/parser/Lexer.cpp b/Source/JavaScriptCore/parser/Lexer.cpp
index ff7079f..35d0906 100644
--- a/Source/JavaScriptCore/parser/Lexer.cpp
+++ b/Source/JavaScriptCore/parser/Lexer.cpp
@@ -703,6 +703,15 @@ ALWAYS_INLINE bool Lexer::parseMultilineComment()
     }
 }
 
+bool Lexer::nextTokenIsColon()
+{
+    const UChar* code = m_code;
+    while (code < m_codeEnd && (isWhiteSpace(*code) || isLineTerminator(*code)))
+        code++;
+        
+    return code < m_codeEnd && *code == ':';
+}
+
 JSTokenType Lexer::lex(JSTokenData* lvalp, JSTokenInfo* llocp, LexType lexType, bool strictMode)
 {
     ASSERT(!m_error);
diff --git a/Source/JavaScriptCore/parser/Lexer.h b/Source/JavaScriptCore/parser/Lexer.h
index 4d2513d..e72888f 100644
--- a/Source/JavaScriptCore/parser/Lexer.h
+++ b/Source/JavaScriptCore/parser/Lexer.h
@@ -53,6 +53,7 @@ namespace JSC {
         // Functions for the parser itself.
         enum LexType { IdentifyReservedWords, IgnoreReservedWords };
         JSTokenType lex(JSTokenData* lvalp, JSTokenInfo* llocp, LexType, bool strictMode);
+        bool nextTokenIsColon();
         int lineNumber() const { return m_lineNumber; }
         void setLastLineNumber(int lastLineNumber) { m_lastLineNumber = lastLineNumber; }
         int lastLineNumber() const { return m_lastLineNumber; }
@@ -67,6 +68,7 @@ namespace JSC {
         int currentOffset() { return m_code - m_codeStart; }
         void setOffset(int offset)
         {
+            m_error = 0;
             m_code = m_codeStart + offset;
             m_current = *m_code;
         }

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list