Commit 1a6a9f7b authored by oliver@apple.com's avatar oliver@apple.com
Browse files

2011-01-14 Oliver Hunt <oliver@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@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
parent 660edfe5
2011-01-14 Oliver Hunt <oliver@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@apple.com>
 
Reviewed by Anders Carlsson.
......
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
<!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>
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;
2011-01-14 Oliver Hunt <oliver@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@webkit.org>
 
Reviewed by Adam Roben.
......
......@@ -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)
......
......@@ -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);
......
......@@ -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;
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment