⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 243948 in webkit


Ignore:
Timestamp:
Apr 5, 2019, 2:58:32 PM (7 years ago)
Author:
ysuzuki@apple.com
Message:

SIGSEGV in JSC::BytecodeGenerator::addStringConstant
​https://bugs.webkit.org/show_bug.cgi?id=196486

Reviewed by Saam Barati.

JSTests:

  • stress/arrow-function-and-use-strict-directive.js: Added.
  • stress/arrow-function-syntax.js: Added. Checking EOF token handling.

(checkSyntax):
(checkSyntaxError): Currently not using it. But it is useful for testing more things related to arrow function syntax.

Source/JavaScriptCore:

When parsing a FunctionExpression / FunctionDeclaration etc., we use SyntaxChecker for the body of the function because we do not have any interest on the nodes of the body at that time.
The nodes will be parsed with the ASTBuilder when the function itself is parsed for code generation. This works well previously because all the function ends with "}" previously.
SyntaxChecker lexes this "}" token, and parser restores the context back to ASTBuilder and continues parsing.

But now, we have ArrowFunctionExpression without braces arrow => expr. Let's consider the following code.

arrow => expr
"string!"

We parse arrow function's body with SyntaxChecker. At that time, we lex "string!" token under the SyntaxChecker context. But this means that we may not build string content for this token
since SyntaxChecker may not have interest on string content itself in certain case. After the parser is back to ASTBuilder, we parse "string!" as ExpressionStatement with string constant,
generate StringNode with non-built identifier (nullptr), and we accidentally create StringNode with nullptr.

This patch fixes this problem. The root cause of this problem is that the last token lexed in the previous context is used. We add lexCurrentTokenAgainUnderCurrentContext which will re-lex
the current token under the current context (may be ASTBuilder). This should be done only when the caller's context is different from SyntaxChecker, which avoids unnecessary lexing.
We leverage existing SavePoint mechanism to implement lexCurrentTokenAgainUnderCurrentContext cleanly.

And we also fix the bug in the existing SavePoint mechanism, which is shown in the attached test script. When we save LexerState, we do not save line terminator status. This patch also introduces
lexWithoutClearingLineTerminator, which lex the token without clearing line terminator status.

  • parser/ASTBuilder.h:

(JSC::ASTBuilder::createString):

  • parser/Lexer.cpp:

(JSC::Lexer<T>::parseMultilineComment):
(JSC::Lexer<T>::lexWithoutClearingLineTerminator): EOF token also should record offset information. This offset information is correctly handled in Lexer::setOffset too.
(JSC::Lexer<T>::lex): Deleted.

  • parser/Lexer.h:

(JSC::Lexer::hasLineTerminatorBeforeToken const):
(JSC::Lexer::setHasLineTerminatorBeforeToken):
(JSC::Lexer<T>::lex):
(JSC::Lexer::prevTerminator const): Deleted.
(JSC::Lexer::setTerminator): Deleted.

  • parser/Parser.cpp:

(JSC::Parser<LexerType>::allowAutomaticSemicolon):
(JSC::Parser<LexerType>::parseSingleFunction):
(JSC::Parser<LexerType>::parseStatementListItem):
(JSC::Parser<LexerType>::maybeParseAsyncFunctionDeclarationStatement):
(JSC::Parser<LexerType>::parseFunctionInfo):
(JSC::Parser<LexerType>::parseClass):
(JSC::Parser<LexerType>::parseExportDeclaration):
(JSC::Parser<LexerType>::parseAssignmentExpression):
(JSC::Parser<LexerType>::parseYieldExpression):
(JSC::Parser<LexerType>::parseProperty):
(JSC::Parser<LexerType>::parsePrimaryExpression):
(JSC::Parser<LexerType>::parseMemberExpression):

  • parser/Parser.h:

(JSC::Parser::nextWithoutClearingLineTerminator):
(JSC::Parser::lexCurrentTokenAgainUnderCurrentContext):
(JSC::Parser::internalSaveLexerState):
(JSC::Parser::restoreLexerState):

Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r243943 r243948  
     12019-04-05  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        SIGSEGV in JSC::BytecodeGenerator::addStringConstant
     4        https://bugs.webkit.org/show_bug.cgi?id=196486
     5
     6        Reviewed by Saam Barati.
     7
     8        * stress/arrow-function-and-use-strict-directive.js: Added.
     9        * stress/arrow-function-syntax.js: Added. Checking EOF token handling.
     10        (checkSyntax):
     11        (checkSyntaxError): Currently not using it. But it is useful for testing more things related to arrow function syntax.
     12
    1132019-04-05  Caitlin Potter  <caitp@igalia.com>
    214

  • trunk/Source/JavaScriptCore/ChangeLog

    r243943 r243948  
     12019-04-05  Yusuke Suzuki  <ysuzuki@apple.com>
     2
     3        SIGSEGV in JSC::BytecodeGenerator::addStringConstant
     4        https://bugs.webkit.org/show_bug.cgi?id=196486
     5
     6        Reviewed by Saam Barati.
     7
     8        When parsing a FunctionExpression / FunctionDeclaration etc., we use SyntaxChecker for the body of the function because we do not have any interest on the nodes of the body at that time.
     9        The nodes will be parsed with the ASTBuilder when the function itself is parsed for code generation. This works well previously because all the function ends with "}" previously.
     10        SyntaxChecker lexes this "}" token, and parser restores the context back to ASTBuilder and continues parsing.
     11
     12        But now, we have ArrowFunctionExpression without braces `arrow => expr`. Let's consider the following code.
     13
     14                arrow => expr
     15                "string!"
     16
     17        We parse arrow function's body with SyntaxChecker. At that time, we lex "string!" token under the SyntaxChecker context. But this means that we may not build string content for this token
     18        since SyntaxChecker may not have interest on string content itself in certain case. After the parser is back to ASTBuilder, we parse "string!" as ExpressionStatement with string constant,
     19        generate StringNode with non-built identifier (nullptr), and we accidentally create StringNode with nullptr.
     20
     21        This patch fixes this problem. The root cause of this problem is that the last token lexed in the previous context is used. We add lexCurrentTokenAgainUnderCurrentContext which will re-lex
     22        the current token under the current context (may be ASTBuilder). This should be done only when the caller's context is different from SyntaxChecker, which avoids unnecessary lexing.
     23        We leverage existing SavePoint mechanism to implement lexCurrentTokenAgainUnderCurrentContext cleanly.
     24
     25        And we also fix the bug in the existing SavePoint mechanism, which is shown in the attached test script. When we save LexerState, we do not save line terminator status. This patch also introduces
     26        lexWithoutClearingLineTerminator, which lex the token without clearing line terminator status.
     27
     28        * parser/ASTBuilder.h:
     29        (JSC::ASTBuilder::createString):
     30        * parser/Lexer.cpp:
     31        (JSC::Lexer<T>::parseMultilineComment):
     32        (JSC::Lexer<T>::lexWithoutClearingLineTerminator): EOF token also should record offset information. This offset information is correctly handled in Lexer::setOffset too.
     33        (JSC::Lexer<T>::lex): Deleted.
     34        * parser/Lexer.h:
     35        (JSC::Lexer::hasLineTerminatorBeforeToken const):
     36        (JSC::Lexer::setHasLineTerminatorBeforeToken):
     37        (JSC::Lexer<T>::lex):
     38        (JSC::Lexer::prevTerminator const): Deleted.
     39        (JSC::Lexer::setTerminator): Deleted.
     40        * parser/Parser.cpp:
     41        (JSC::Parser<LexerType>::allowAutomaticSemicolon):
     42        (JSC::Parser<LexerType>::parseSingleFunction):
     43        (JSC::Parser<LexerType>::parseStatementListItem):
     44        (JSC::Parser<LexerType>::maybeParseAsyncFunctionDeclarationStatement):
     45        (JSC::Parser<LexerType>::parseFunctionInfo):
     46        (JSC::Parser<LexerType>::parseClass):
     47        (JSC::Parser<LexerType>::parseExportDeclaration):
     48        (JSC::Parser<LexerType>::parseAssignmentExpression):
     49        (JSC::Parser<LexerType>::parseYieldExpression):
     50        (JSC::Parser<LexerType>::parseProperty):
     51        (JSC::Parser<LexerType>::parsePrimaryExpression):
     52        (JSC::Parser<LexerType>::parseMemberExpression):
     53        * parser/Parser.h:
     54        (JSC::Parser::nextWithoutClearingLineTerminator):
     55        (JSC::Parser::lexCurrentTokenAgainUnderCurrentContext):
     56        (JSC::Parser::internalSaveLexerState):
     57        (JSC::Parser::restoreLexerState):
     58
    1592019-04-05  Caitlin Potter  <caitp@igalia.com>
    260

  • trunk/Source/JavaScriptCore/parser/ASTBuilder.h

    r237241 r243948  
    242242    ExpressionNode* createString(const JSTokenLocation& location, const Identifier* string)
    243243    {
     244        ASSERT(string);
    244245        incConstants();
    245246        return new (m_parserArena) StringNode(location, *string);

  • trunk/Source/JavaScriptCore/parser/Lexer.cpp

    r241751 r243948  
    16921692        if (isLineTerminator(m_current)) {
    16931693            shiftLineTerminator();
    1694             m_terminator = true;
     1694            m_hasLineTerminatorBeforeToken = true;
    16951695        } else
    16961696            shift();
    … …  
    17711771
    17721772template <typename T>
    1773 JSTokenType Lexer<T>::lex(JSToken* tokenRecord, unsigned lexerFlags, bool strictMode)
     1773JSTokenType Lexer<T>::lexWithoutClearingLineTerminator(JSToken* tokenRecord, unsigned lexerFlags, bool strictMode)
    17741774{
    17751775    JSTokenData* tokenData = &tokenRecord->m_data;
    … …  
    17821782
    17831783    JSTokenType token = ERRORTOK;
    1784     m_terminator = false;
    17851784
    17861785start:
    17871786    skipWhitespace();
    17881787
    1789     if (atEnd())
    1790         return EOFTOK;
    1791    
    17921788    tokenLocation->startOffset = currentOffset();
    17931789    ASSERT(currentOffset() >= currentLineStartOffset());
    17941790    tokenRecord->m_startPosition = currentPosition();
     1791
     1792    if (atEnd()) {
     1793        token = EOFTOK;
     1794        goto returnToken;
     1795    }
    17951796
    17961797    CharacterType type;
    … …  
    19031904        if (m_current == '+') {
    19041905            shift();
    1905             token = (!m_terminator) ? PLUSPLUS : AUTOPLUSPLUS;
     1906            token = (!m_hasLineTerminatorBeforeToken) ? PLUSPLUS : AUTOPLUSPLUS;
    19061907            break;
    19071908        }
    … …  
    19171918        if (m_current == '-') {
    19181919            shift();
    1919             if ((m_atLineStart || m_terminator) && m_current == '>') {
     1920            if ((m_atLineStart || m_hasLineTerminatorBeforeToken) && m_current == '>') {
    19201921                if (m_scriptMode == JSParserScriptMode::Classic) {
    19211922                    shift();
    … …  
    19231924                }
    19241925            }
    1925             token = (!m_terminator) ? MINUSMINUS : AUTOMINUSMINUS;
     1926            token = (!m_hasLineTerminatorBeforeToken) ? MINUSMINUS : AUTOMINUSMINUS;
    19261927            break;
    19271928        }
    … …  
    22942295        shiftLineTerminator();
    22952296        m_atLineStart = true;
    2296         m_terminator = true;
     2297        m_hasLineTerminatorBeforeToken = true;
    22972298        m_lineStart = m_code;
    22982299        goto start;
    … …  
    23342335
    23352336        while (!isLineTerminator(m_current)) {
    2336             if (atEnd())
    2337                 return EOFTOK;
     2337            if (atEnd()) {
     2338                token = EOFTOK;
     2339                fillTokenInfo(tokenRecord, token, lineNumber, endOffset, lineStartOffset, endPosition);
     2340                return token;
     2341            }
    23382342            shift();
    23392343        }
    23402344        shiftLineTerminator();
    23412345        m_atLineStart = true;
    2342         m_terminator = true;
     2346        m_hasLineTerminatorBeforeToken = true;
    23432347        m_lineStart = m_code;
    23442348        if (!lastTokenWasRestrKeyword())

  • trunk/Source/JavaScriptCore/parser/Lexer.h

    r241645 r243948  
    6666
    6767    JSTokenType lex(JSToken*, unsigned, bool strictMode);
     68    JSTokenType lexWithoutClearingLineTerminator(JSToken*, unsigned, bool strictMode);
    6869    bool nextTokenIsColon();
    6970    int lineNumber() const { return m_lineNumber; }
    … …  
    7879    void setLastLineNumber(int lastLineNumber) { m_lastLineNumber = lastLineNumber; }
    7980    int lastLineNumber() const { return m_lastLineNumber; }
    80     bool prevTerminator() const { return m_terminator; }
     81    bool hasLineTerminatorBeforeToken() const { return m_hasLineTerminatorBeforeToken; }
    8182    JSTokenType scanRegExp(JSToken*, UChar patternPrefix = 0);
    8283    enum class RawStringsBuildMode { BuildRawStrings, DontBuildRawStrings };
    … …  
    111112        m_lineNumber = line;
    112113    }
    113     void setTerminator(bool terminator)
     114    void setHasLineTerminatorBeforeToken(bool terminator)
    114115    {
    115         m_terminator = terminator;
     116        m_hasLineTerminatorBeforeToken = terminator;
    116117    }
    117118
    … …  
    203204    Vector<UChar> m_buffer16;
    204205    Vector<UChar> m_bufferForRawTemplateString16;
    205     bool m_terminator;
     206    bool m_hasLineTerminatorBeforeToken;
    206207    int m_lastToken;
    207208
    … …  
    404405}
    405406
     407template <typename T>
     408ALWAYS_INLINE JSTokenType Lexer<T>::lex(JSToken* tokenRecord, unsigned lexerFlags, bool strictMode)
     409{
     410    m_hasLineTerminatorBeforeToken = false;
     411    return lexWithoutClearingLineTerminator(tokenRecord, lexerFlags, strictMode);
     412}
     413
    406414} // namespace JSC

  • trunk/Source/JavaScriptCore/parser/Parser.cpp

    r242193 r243948  
    346346bool Parser<LexerType>::allowAutomaticSemicolon()
    347347{
    348     return match(CLOSEBRACE) || match(EOFTOK) || m_lexer->prevTerminator();
     348    return match(CLOSEBRACE) || match(EOFTOK) || m_lexer->hasLineTerminatorBeforeToken();
    349349}
    350350
    … …  
    626626        if (*m_token.m_data.ident == m_vm->propertyNames->async && !m_token.m_data.escaped) {
    627627            next();
    628             failIfFalse(match(FUNCTION) && !m_lexer->prevTerminator(), "Cannot parse the async function");
     628            failIfFalse(match(FUNCTION) && !m_lexer->hasLineTerminatorBeforeToken(), "Cannot parse the async function");
    629629            statement = parseAsyncFunctionDeclaration(context, ExportType::NotExported, DeclarationDefaultContext::Standard, functionConstructorParametersEndPosition);
    630630            break;
    … …  
    697697            SavePoint savePoint = createSavePoint();
    698698            next();
    699             if (UNLIKELY(match(FUNCTION) && !m_lexer->prevTerminator())) {
     699            if (UNLIKELY(match(FUNCTION) && !m_lexer->hasLineTerminatorBeforeToken())) {
    700700                result = parseAsyncFunctionDeclaration(context);
    701701                break;
    … …  
    20272027    SavePoint savePoint = createSavePoint();
    20282028    next();
    2029     if (match(FUNCTION) && !m_lexer->prevTerminator()) {
     2029    if (match(FUNCTION) && !m_lexer->hasLineTerminatorBeforeToken()) {
    20302030        const bool isAsync = true;
    20312031        result = parseFunctionDeclarationStatement(context, isAsync, parentAllowsFunctionDeclarationAsStatement);
    … …  
    24222422        matchOrFail(ARROWFUNCTION, "Expected a '=>' after arrow function parameter declaration");
    24232423
    2424         if (m_lexer->prevTerminator())
     2424        if (m_lexer->hasLineTerminatorBeforeToken())
    24252425            failDueToUnexpectedToken();
    24262426
    … …  
    26132613        newInfo = SourceProviderCacheItem::create(parameters);
    26142614    }
     2615
     2616    bool functionScopeWasStrictMode = functionScope->strictMode();
    26152617   
    26162618    popScope(functionScope, TreeBuilder::NeedsFreeVariableInfo);
    … …  
    26192621        matchOrFail(CLOSEBRACE, "Expected a closing '}' after a ", stringForFunctionMode(mode), " body");
    26202622        next();
     2623    } else {
     2624        // We need to lex the last token again because the last token is lexed under the different context because of the following possibilities.
     2625        // 1. which may have different strict mode.
     2626        // 2. which may not build strings for tokens.
     2627        // But (1) is not possible because we do not recognize the string literal in ArrowFunctionBodyExpression as directive and this is correct in terms of the spec (`value => "use strict"`).
     2628        // So we only check TreeBuilder's type here.
     2629        ASSERT_UNUSED(functionScopeWasStrictMode, functionScopeWasStrictMode == currentScope()->strictMode());
     2630        if (!std::is_same<TreeBuilder, SyntaxChecker>::value)
     2631            lexCurrentTokenAgainUnderCurrentContext();
    26212632    }
    26222633
    … …  
    28772888                    ident = m_token.m_data.ident;
    28782889                    next();
    2879                     if (match(OPENPAREN) || match(COLON) || match(EQUAL) || m_lexer->prevTerminator())
     2890                    if (match(OPENPAREN) || match(COLON) || match(EQUAL) || m_lexer->hasLineTerminatorBeforeToken())
    28802891                        break;
    28812892                    if (UNLIKELY(consume(TIMES)))
    … …  
    33973408            SavePoint savePoint = createSavePoint();
    33983409            next();
    3399             if (match(FUNCTION) && !m_lexer->prevTerminator()) {
     3410            if (match(FUNCTION) && !m_lexer->hasLineTerminatorBeforeToken()) {
    34003411                next();
    34013412                if (match(IDENT))
    … …  
    35423553            if (*m_token.m_data.ident == m_vm->propertyNames->async && !m_token.m_data.escaped) {
    35433554                next();
    3544                 semanticFailIfFalse(match(FUNCTION) && !m_lexer->prevTerminator(), "Expected 'function' keyword following 'async' keyword with no preceding line terminator");
     3555                semanticFailIfFalse(match(FUNCTION) && !m_lexer->hasLineTerminatorBeforeToken(), "Expected 'function' keyword following 'async' keyword with no preceding line terminator");
    35453556                DepthManager statementDepth(&m_statementDepth);
    35463557                m_statementDepth = 1;
    … …  
    36603671                if (matchContextualKeyword(m_vm->propertyNames->async)) {
    36613672                    next();
    3662                     isAsyncArrow = !m_lexer->prevTerminator();
     3673                    isAsyncArrow = !m_lexer->hasLineTerminatorBeforeToken();
    36633674                }
    36643675            }
    … …  
    37773788    SavePoint savePoint = createSavePoint();
    37783789    next();
    3779     if (m_lexer->prevTerminator())
     3790    if (m_lexer->hasLineTerminatorBeforeToken())
    37803791        return context.createYield(location);
    37813792
    … …  
    39373948                }
    39383949
    3939                 failIfTrue(m_lexer->prevTerminator(), "Expected a property name following keyword 'async'");
     3950                failIfTrue(m_lexer->hasLineTerminatorBeforeToken(), "Expected a property name following keyword 'async'");
    39403951                if (UNLIKELY(consume(TIMES)))
    39413952                    parseMode = SourceParseMode::AsyncGeneratorWrapperMethodMode;
    … …  
    44864497            JSTokenLocation location(tokenLocation());
    44874498            next();
    4488             if (match(FUNCTION) && !m_lexer->prevTerminator())
     4499            if (match(FUNCTION) && !m_lexer->hasLineTerminatorBeforeToken())
    44894500                return parseAsyncFunctionExpression(context);
    44904501
    … …  
    47524763        base = parsePrimaryExpression(context);
    47534764        failIfFalse(base, "Cannot parse base expression");
    4754         if (UNLIKELY(isAsync && context.isResolve(base) && !m_lexer->prevTerminator())) {
     4765        if (UNLIKELY(isAsync && context.isResolve(base) && !m_lexer->hasLineTerminatorBeforeToken())) {
    47554766            if (matchSpecIdentifier()) {
    47564767                // AsyncArrowFunction

  • trunk/Source/JavaScriptCore/parser/Parser.h

    r241645 r243948  
    13641364    }
    13651365
     1366    ALWAYS_INLINE void nextWithoutClearingLineTerminator(unsigned lexerFlags = 0)
     1367    {
     1368        int lastLine = m_token.m_location.line;
     1369        int lastTokenEnd = m_token.m_location.endOffset;
     1370        int lastTokenLineStart = m_token.m_location.lineStartOffset;
     1371        m_lastTokenEndPosition = JSTextPosition(lastLine, lastTokenEnd, lastTokenLineStart);
     1372        m_lexer->setLastLineNumber(lastLine);
     1373        m_token.m_type = m_lexer->lexWithoutClearingLineTerminator(&m_token, lexerFlags, strictMode());
     1374    }
     1375
    13661376    ALWAYS_INLINE void nextExpectIdentifier(unsigned lexerFlags = 0)
    13671377    {
    … …  
    13721382        m_lexer->setLastLineNumber(lastLine);
    13731383        m_token.m_type = m_lexer->lexExpectIdentifier(&m_token, lexerFlags, strictMode());
     1384    }
     1385
     1386    ALWAYS_INLINE void lexCurrentTokenAgainUnderCurrentContext()
     1387    {
     1388        auto savePoint = createSavePoint();
     1389        restoreSavePoint(savePoint);
    13741390    }
    13751391
    … …  
    17631779        unsigned oldLastLineNumber;
    17641780        unsigned oldLineNumber;
     1781        bool hasLineTerminatorBeforeToken;
    17651782    };
    17661783
    … …  
    17761793        result.oldLastLineNumber = m_lexer->lastLineNumber();
    17771794        result.oldLineNumber = m_lexer->lineNumber();
     1795        result.hasLineTerminatorBeforeToken = m_lexer->hasLineTerminatorBeforeToken();
    17781796        ASSERT(static_cast<unsigned>(result.startOffset) >= result.oldLineStartOffset);
    17791797        return result;
    … …  
    17851803        m_lexer->setOffset(lexerState.startOffset, lexerState.oldLineStartOffset);
    17861804        m_lexer->setLineNumber(lexerState.oldLineNumber);
    1787         next();
     1805        m_lexer->setHasLineTerminatorBeforeToken(lexerState.hasLineTerminatorBeforeToken);
     1806        nextWithoutClearingLineTerminator();
    17881807        m_lexer->setLastLineNumber(lexerState.oldLastLineNumber);
    17891808    }
Note: See TracChangeset for help on using the changeset viewer.