From ca3afea1d74b06908ca810b76d0ad3225228c1d9 Mon Sep 17 00:00:00 2001 From: rocky Date: Mon, 27 May 2019 10:13:27 -0400 Subject: [PATCH 1/2] Add Steve Johnson-style parser recovery rules: SourceUnit = Error $ Block = '{' Error '}' ContractDefinition = '{' Error '}' Statement = Error ';' Co-Authored-By: chriseth --- Changelog.md | 2 +- liblangutil/CharStream.cpp | 9 +- liblangutil/CharStream.h | 6 + liblangutil/ErrorReporter.cpp | 5 + liblangutil/ErrorReporter.h | 4 +- liblangutil/ParserBase.cpp | 106 ++++++-- liblangutil/ParserBase.h | 28 +- liblangutil/Scanner.cpp | 7 + liblangutil/Scanner.h | 3 + libsolidity/interface/CompilerStack.cpp | 2 +- libsolidity/interface/CompilerStack.h | 9 + libsolidity/parsing/Parser.cpp | 254 +++++++++++------- libsolidity/parsing/Parser.h | 5 +- solc/CommandLineInterface.cpp | 5 + test/InteractiveTests.h | 2 +- test/liblangutil/CharStream.cpp | 53 ++++ test/libsolidity/AnalysisFramework.cpp | 5 +- test/libsolidity/AnalysisFramework.h | 3 +- test/libsolidity/SMTChecker.cpp | 6 +- test/libsolidity/SyntaxTest.cpp | 4 +- test/libsolidity/SyntaxTest.h | 11 +- .../constructor_recovers.sol | 20 ++ .../errorRecoveryTests/contract_recovery.sol | 7 + .../do_not_delete_at_error.sol | 13 + .../errorRecoveryTests/error_to_eos.sol | 23 ++ .../errorRecoveryTests/missing_rhs.sol | 11 + .../errorRecoveryTests/multiple_errors.sol | 29 ++ 27 files changed, 492 insertions(+), 140 deletions(-) create mode 100644 test/liblangutil/CharStream.cpp create mode 100644 test/libsolidity/errorRecoveryTests/constructor_recovers.sol create mode 100644 test/libsolidity/errorRecoveryTests/contract_recovery.sol create mode 100644 test/libsolidity/errorRecoveryTests/do_not_delete_at_error.sol create mode 100644 test/libsolidity/errorRecoveryTests/error_to_eos.sol create mode 100644 test/libsolidity/errorRecoveryTests/missing_rhs.sol create mode 100644 test/libsolidity/errorRecoveryTests/multiple_errors.sol diff --git a/Changelog.md b/Changelog.md index fa920413b..994cff927 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,7 @@ Language Features: Compiler Features: * Optimizer: Add rule to simplify SUB(~0, X) to NOT(X). + * Commandline Interface: Experimental parser error recovery via the ``--error-recovery`` commandline switch. @@ -45,7 +46,6 @@ Compiler Features: * Yul Optimizer: Do not inline recursive functions. * Yul Optimizer: Do not remove instructions that affect ``msize()`` if ``msize()`` is used. - Bugfixes: * Code Generator: Explicitly turn uninitialized internal function pointers into invalid functions when loaded from storage. * Code Generator: Fix assertion failure when assigning structs containing array of mapping. diff --git a/liblangutil/CharStream.cpp b/liblangutil/CharStream.cpp index 2c10bd608..58edced3b 100644 --- a/liblangutil/CharStream.cpp +++ b/liblangutil/CharStream.cpp @@ -73,6 +73,13 @@ char CharStream::rollback(size_t _amount) return get(); } +char CharStream::setPosition(size_t _location) +{ + solAssert(_location <= m_source.size(), "Attempting to set position past end of source."); + m_position = _location; + return get(); +} + string CharStream::lineAtPosition(int _position) const { // if _position points to \n, it returns the line before the \n @@ -106,5 +113,3 @@ tuple CharStream::translatePositionToLineColumn(int _position) const } return tuple(lineNumber, searchPosition - lineStart); } - - diff --git a/liblangutil/CharStream.h b/liblangutil/CharStream.h index c11340b21..504c39da5 100644 --- a/liblangutil/CharStream.h +++ b/liblangutil/CharStream.h @@ -76,7 +76,13 @@ public: char get(size_t _charsForward = 0) const { return m_source[m_position + _charsForward]; } char advanceAndGet(size_t _chars = 1); + /// Sets scanner position to @ _amount characters backwards in source text. + /// @returns The character of the current location after update is returned. char rollback(size_t _amount); + /// Sets scanner position to @ _location if it refers a valid offset in m_source. + /// If not, nothing is done. + /// @returns The character of the current location after update is returned. + char setPosition(size_t _location); void reset() { m_position = 0; } diff --git a/liblangutil/ErrorReporter.cpp b/liblangutil/ErrorReporter.cpp index 0bf3a4167..7f8a77f1a 100644 --- a/liblangutil/ErrorReporter.cpp +++ b/liblangutil/ErrorReporter.cpp @@ -86,6 +86,11 @@ void ErrorReporter::error(Error::Type _type, SourceLocation const& _location, Se m_errorList.push_back(err); } +bool ErrorReporter::hasExcessiveErrors() const +{ + return m_errorCount > c_maxErrorsAllowed; +} + bool ErrorReporter::checkForExcessiveErrors(Error::Type _type) { if (_type == Error::Type::Warning) diff --git a/liblangutil/ErrorReporter.h b/liblangutil/ErrorReporter.h index 30b494a0f..6d6b4ef41 100644 --- a/liblangutil/ErrorReporter.h +++ b/liblangutil/ErrorReporter.h @@ -118,6 +118,9 @@ public: return m_errorCount > 0; } + // @returns true if the maximum error count has been reached. + bool hasExcessiveErrors() const; + private: void error( Error::Type _type, @@ -149,4 +152,3 @@ private: }; } - diff --git a/liblangutil/ParserBase.cpp b/liblangutil/ParserBase.cpp index edd23fbaf..2432ac711 100644 --- a/liblangutil/ParserBase.cpp +++ b/liblangutil/ParserBase.cpp @@ -47,7 +47,7 @@ Token ParserBase::peekNextToken() const return m_scanner->peekNextToken(); } -std::string ParserBase::currentLiteral() const +string ParserBase::currentLiteral() const { return m_scanner->currentLiteral(); } @@ -57,34 +57,83 @@ Token ParserBase::advance() return m_scanner->next(); } +string ParserBase::tokenName(Token _token) +{ + if (_token == Token::Identifier) + return "identifier"; + else if (_token == Token::EOS) + return "end of source"; + else if (TokenTraits::isReservedKeyword(_token)) + return "reserved keyword '" + TokenTraits::friendlyName(_token) + "'"; + else if (TokenTraits::isElementaryTypeName(_token)) //for the sake of accuracy in reporting + { + ElementaryTypeNameToken elemTypeName = m_scanner->currentElementaryTypeNameToken(); + return "'" + elemTypeName.toString() + "'"; + } + else + return "'" + TokenTraits::friendlyName(_token) + "'"; +} + void ParserBase::expectToken(Token _value, bool _advance) { Token tok = m_scanner->currentToken(); if (tok != _value) { - auto tokenName = [this](Token _token) - { - if (_token == Token::Identifier) - return string("identifier"); - else if (_token == Token::EOS) - return string("end of source"); - else if (TokenTraits::isReservedKeyword(_token)) - return string("reserved keyword '") + TokenTraits::friendlyName(_token) + "'"; - else if (TokenTraits::isElementaryTypeName(_token)) //for the sake of accuracy in reporting - { - ElementaryTypeNameToken elemTypeName = m_scanner->currentElementaryTypeNameToken(); - return string("'") + elemTypeName.toString() + "'"; - } - else - return string("'") + TokenTraits::friendlyName(_token) + "'"; - }; - - fatalParserError(string("Expected ") + tokenName(_value) + string(" but got ") + tokenName(tok)); + string const expectedToken = ParserBase::tokenName(_value); + if (m_parserErrorRecovery) + parserError("Expected " + expectedToken + " but got " + tokenName(tok)); + else + fatalParserError("Expected " + expectedToken + " but got " + tokenName(tok)); + // Do not advance so that recovery can sync or make use of the current token. + // This is especially useful if the expected token + // is the only one that is missing and is at the end of a construct. + // "{ ... ; }" is such an example. + // ^ + _advance = false; } if (_advance) m_scanner->next(); } +void ParserBase::expectTokenOrConsumeUntil(Token _value, string const& _currentNodeName, bool _advance) +{ + Token tok = m_scanner->currentToken(); + if (tok != _value) + { + int startPosition = position(); + SourceLocation errorLoc = SourceLocation{startPosition, endPosition(), source()}; + while (m_scanner->currentToken() != _value && m_scanner->currentToken() != Token::EOS) + m_scanner->next(); + + string const expectedToken = ParserBase::tokenName(_value); + string const msg = "In " + _currentNodeName + ", " + expectedToken + "is expected; got " + ParserBase::tokenName(tok) + "instead."; + if (m_scanner->currentToken() == Token::EOS) + { + // rollback to where the token started, and raise exception to be caught at a higher level. + m_scanner->setPosition(startPosition); + m_inParserRecovery = true; + fatalParserError(errorLoc, msg); + } + else + { + if (m_inParserRecovery) + parserWarning("Recovered in " + _currentNodeName + " at " + expectedToken + "."); + else + parserError(errorLoc, msg + "Recovered at next " + expectedToken); + m_inParserRecovery = false; + } + } + else if (m_inParserRecovery) + { + string expectedToken = ParserBase::tokenName(_value); + parserWarning("Recovered in " + _currentNodeName + " at " + expectedToken + "."); + m_inParserRecovery = false; + } + + if (_advance) + m_scanner->next(); +} + void ParserBase::increaseRecursionDepth() { m_recursionDepth++; @@ -98,12 +147,27 @@ void ParserBase::decreaseRecursionDepth() m_recursionDepth--; } +void ParserBase::parserWarning(string const& _description) +{ + m_errorReporter.warning(SourceLocation{position(), endPosition(), source()}, _description); +} + +void ParserBase::parserError(SourceLocation const& _location, string const& _description) +{ + m_errorReporter.parserError(_location, _description); +} + void ParserBase::parserError(string const& _description) { - m_errorReporter.parserError(SourceLocation{position(), endPosition(), source()}, _description); + parserError(SourceLocation{position(), endPosition(), source()}, _description); } void ParserBase::fatalParserError(string const& _description) { - m_errorReporter.fatalParserError(SourceLocation{position(), endPosition(), source()}, _description); + fatalParserError(SourceLocation{position(), endPosition(), source()}, _description); +} + +void ParserBase::fatalParserError(SourceLocation const& _location, string const& _description) +{ + m_errorReporter.fatalParserError(_location, _description); } diff --git a/liblangutil/ParserBase.h b/liblangutil/ParserBase.h index 1c6f298c1..98123b813 100644 --- a/liblangutil/ParserBase.h +++ b/liblangutil/ParserBase.h @@ -36,7 +36,14 @@ class Scanner; class ParserBase { public: - explicit ParserBase(ErrorReporter& errorReporter): m_errorReporter(errorReporter) {} + /// Set @a _parserErrorRecovery to true for additional error + /// recovery. This is experimental and intended for use + /// by front-end tools that need partial AST information even + /// when errors occur. + explicit ParserBase(ErrorReporter& errorReporter, bool _parserErrorRecovery = false): m_errorReporter(errorReporter) + { + m_parserErrorRecovery = _parserErrorRecovery; + } std::shared_ptr source() const { return m_scanner->charStream(); } @@ -62,10 +69,17 @@ protected: ///@{ ///@name Helper functions - /// If current token value is not _value, throw exception otherwise advance token. + /// If current token value is not @a _value, throw exception otherwise advance token + // @a if _advance is true and error recovery is in effect. void expectToken(Token _value, bool _advance = true); + + /// Like expectToken but if there is an error ignores tokens until + /// the expected token or EOS is seen. If EOS is encountered, back up to the error point, + /// and throw an exception so that a higher grammar rule has an opportunity to recover. + void expectTokenOrConsumeUntil(Token _value, std::string const& _currentNodeName, bool _advance = true); Token currentToken() const; Token peekNextToken() const; + std::string tokenName(Token _token); std::string currentLiteral() const; Token advance(); ///@} @@ -77,16 +91,26 @@ protected: /// Creates a @ref ParserError and annotates it with the current position and the /// given @a _description. void parserError(std::string const& _description); + void parserError(SourceLocation const& _location, std::string const& _description); + + /// Creates a @ref ParserWarning and annotates it with the current position and the + /// given @a _description. + void parserWarning(std::string const& _description); /// Creates a @ref ParserError and annotates it with the current position and the /// given @a _description. Throws the FatalError. void fatalParserError(std::string const& _description); + void fatalParserError(SourceLocation const& _location, std::string const& _description); std::shared_ptr m_scanner; /// The reference to the list of errors and warning to add errors/warnings during parsing ErrorReporter& m_errorReporter; /// Current recursion depth during parsing. size_t m_recursionDepth = 0; + /// True if we are in parser error recovery. Usually this means we are scanning for + /// a synchronization token like ';', or '}'. We use this to reduce cascaded error messages. + bool m_inParserRecovery = false; + bool m_parserErrorRecovery = false; }; } diff --git a/liblangutil/Scanner.cpp b/liblangutil/Scanner.cpp index 852a92e9b..a317c79b6 100644 --- a/liblangutil/Scanner.cpp +++ b/liblangutil/Scanner.cpp @@ -156,6 +156,13 @@ void Scanner::reset() next(); } +void Scanner::setPosition(size_t _offset) +{ + m_char = m_source->setPosition(_offset); + scanToken(); + next(); +} + void Scanner::supportPeriodInIdentifier(bool _value) { m_supportPeriodInIdentifier = _value; diff --git a/liblangutil/Scanner.h b/liblangutil/Scanner.h index 9a4a170a6..555575129 100644 --- a/liblangutil/Scanner.h +++ b/liblangutil/Scanner.h @@ -110,6 +110,9 @@ public: /// @returns the next token and advances input Token next(); + /// Set scanner to a specific offset. This is used in error recovery. + void setPosition(size_t _offset); + ///@{ ///@name Information about the current token diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 2910a9d95..c7846c8e2 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -217,7 +217,7 @@ bool CompilerStack::parse() string const& path = sourcesToParse[i]; Source& source = m_sources[path]; source.scanner->reset(); - source.ast = Parser(m_errorReporter, m_evmVersion).parse(source.scanner); + source.ast = Parser(m_errorReporter, m_evmVersion, m_parserErrorRecovery).parse(source.scanner); if (!source.ast) solAssert(!Error::containsOnlyWarnings(m_errorReporter.errors()), "Parser returned null but did not report error."); else diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h index de90553cd..25718e698 100644 --- a/libsolidity/interface/CompilerStack.h +++ b/libsolidity/interface/CompilerStack.h @@ -132,6 +132,14 @@ public: /// Must be set before parsing. void setOptimiserSettings(OptimiserSettings _settings); + /// Set whether or not parser error is desired. + /// When called without an argument it will revert to the default. + /// Must be set before parsing. + void setParserErrorRecovery(bool _wantErrorRecovery = false) + { + m_parserErrorRecovery = _wantErrorRecovery; + } + /// Set the EVM version used before running compile. /// When called without an argument it will revert to the default version. /// Must be set before parsing. @@ -386,6 +394,7 @@ private: langutil::ErrorList m_errorList; langutil::ErrorReporter m_errorReporter; bool m_metadataLiteralSources = false; + bool m_parserErrorRecovery = false; State m_stackState = Empty; bool m_release = VersionIsRelease; }; diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 5b5c56523..d8e0fa4bc 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -258,57 +258,75 @@ ASTPointer Parser::parseContractDefinition() { RecursionGuard recursionGuard(*this); ASTNodeFactory nodeFactory(*this); + ASTPointer name = nullptr; ASTPointer docString; - if (m_scanner->currentCommentLiteral() != "") - docString = make_shared(m_scanner->currentCommentLiteral()); - ContractDefinition::ContractKind contractKind = parseContractKind(); - ASTPointer name = expectIdentifierToken(); vector> baseContracts; - if (m_scanner->currentToken() == Token::Is) - do - { - m_scanner->next(); - baseContracts.push_back(parseInheritanceSpecifier()); - } - while (m_scanner->currentToken() == Token::Comma); vector> subNodes; - expectToken(Token::LBrace); - while (true) + ContractDefinition::ContractKind contractKind = ContractDefinition::ContractKind::Contract; + try { - Token currentTokenValue = m_scanner->currentToken(); - if (currentTokenValue == Token::RBrace) - break; - else if (currentTokenValue == Token::Function || currentTokenValue == Token::Constructor) - // This can be a function or a state variable of function type (especially - // complicated to distinguish fallback function from function type state variable) - subNodes.push_back(parseFunctionDefinitionOrFunctionTypeStateVariable()); - else if (currentTokenValue == Token::Struct) - subNodes.push_back(parseStructDefinition()); - else if (currentTokenValue == Token::Enum) - subNodes.push_back(parseEnumDefinition()); - else if ( - currentTokenValue == Token::Identifier || - currentTokenValue == Token::Mapping || - TokenTraits::isElementaryTypeName(currentTokenValue) - ) + if (m_scanner->currentCommentLiteral() != "") + docString = make_shared(m_scanner->currentCommentLiteral()); + contractKind = parseContractKind(); + name = expectIdentifierToken(); + if (m_scanner->currentToken() == Token::Is) + do + { + m_scanner->next(); + baseContracts.push_back(parseInheritanceSpecifier()); + } + while (m_scanner->currentToken() == Token::Comma); + expectToken(Token::LBrace); + while (true) { - VarDeclParserOptions options; - options.isStateVariable = true; - options.allowInitialValue = true; - subNodes.push_back(parseVariableDeclaration(options)); - expectToken(Token::Semicolon); + Token currentTokenValue = m_scanner->currentToken(); + if (currentTokenValue == Token::RBrace) + break; + else if (currentTokenValue == Token::Function || currentTokenValue == Token::Constructor) + // This can be a function or a state variable of function type (especially + // complicated to distinguish fallback function from function type state variable) + subNodes.push_back(parseFunctionDefinitionOrFunctionTypeStateVariable()); + else if (currentTokenValue == Token::Struct) + subNodes.push_back(parseStructDefinition()); + else if (currentTokenValue == Token::Enum) + subNodes.push_back(parseEnumDefinition()); + else if ( + currentTokenValue == Token::Identifier || + currentTokenValue == Token::Mapping || + TokenTraits::isElementaryTypeName(currentTokenValue) + ) + { + VarDeclParserOptions options; + options.isStateVariable = true; + options.allowInitialValue = true; + subNodes.push_back(parseVariableDeclaration(options)); + expectToken(Token::Semicolon); + } + else if (currentTokenValue == Token::Modifier) + subNodes.push_back(parseModifierDefinition()); + else if (currentTokenValue == Token::Event) + subNodes.push_back(parseEventDefinition()); + else if (currentTokenValue == Token::Using) + subNodes.push_back(parseUsingDirective()); + else + fatalParserError(string("Function, variable, struct or modifier declaration expected.")); } - else if (currentTokenValue == Token::Modifier) - subNodes.push_back(parseModifierDefinition()); - else if (currentTokenValue == Token::Event) - subNodes.push_back(parseEventDefinition()); - else if (currentTokenValue == Token::Using) - subNodes.push_back(parseUsingDirective()); - else - fatalParserError(string("Function, variable, struct or modifier declaration expected.")); + } + catch (FatalError const&) + { + if ( + !m_errorReporter.hasErrors() || + !m_parserErrorRecovery || + m_errorReporter.hasExcessiveErrors() + ) + BOOST_THROW_EXCEPTION(FatalError()); /* Don't try to recover here. */ + m_inParserRecovery = true; } nodeFactory.markEndPosition(); - expectToken(Token::RBrace); + if (m_inParserRecovery) + expectTokenOrConsumeUntil(Token::RBrace, "ContractDefinition"); + else + expectToken(Token::RBrace); return nodeFactory.createNode( name, docString, @@ -959,10 +977,26 @@ ASTPointer Parser::parseBlock(ASTPointer const& _docString) ASTNodeFactory nodeFactory(*this); expectToken(Token::LBrace); vector> statements; - while (m_scanner->currentToken() != Token::RBrace) - statements.push_back(parseStatement()); - nodeFactory.markEndPosition(); - expectToken(Token::RBrace); + try + { + while (m_scanner->currentToken() != Token::RBrace) + statements.push_back(parseStatement()); + nodeFactory.markEndPosition(); + } + catch (FatalError const&) + { + if ( + !m_errorReporter.hasErrors() || + !m_parserErrorRecovery || + m_errorReporter.hasExcessiveErrors() + ) + BOOST_THROW_EXCEPTION(FatalError()); /* Don't try to recover here. */ + m_inParserRecovery = true; + } + if (m_parserErrorRecovery) + expectTokenOrConsumeUntil(Token::RBrace, "Block"); + else + expectToken(Token::RBrace); return nodeFactory.createNode(_docString, statements); } @@ -970,67 +1004,83 @@ ASTPointer Parser::parseStatement() { RecursionGuard recursionGuard(*this); ASTPointer docString; - if (m_scanner->currentCommentLiteral() != "") - docString = make_shared(m_scanner->currentCommentLiteral()); ASTPointer statement; - switch (m_scanner->currentToken()) + try { - case Token::If: - return parseIfStatement(docString); - case Token::While: - return parseWhileStatement(docString); - case Token::Do: - return parseDoWhileStatement(docString); - case Token::For: - return parseForStatement(docString); - case Token::LBrace: - return parseBlock(docString); - // starting from here, all statements must be terminated by a semicolon - case Token::Continue: - statement = ASTNodeFactory(*this).createNode(docString); - m_scanner->next(); - break; - case Token::Break: - statement = ASTNodeFactory(*this).createNode(docString); - m_scanner->next(); - break; - case Token::Return: - { - ASTNodeFactory nodeFactory(*this); - ASTPointer expression; - if (m_scanner->next() != Token::Semicolon) + if (m_scanner->currentCommentLiteral() != "") + docString = make_shared(m_scanner->currentCommentLiteral()); + switch (m_scanner->currentToken()) { - expression = parseExpression(); - nodeFactory.setEndPositionFromNode(expression); - } - statement = nodeFactory.createNode(docString, expression); - break; - } - case Token::Throw: - { - statement = ASTNodeFactory(*this).createNode(docString); - m_scanner->next(); - break; - } - case Token::Assembly: - return parseInlineAssembly(docString); - case Token::Emit: - statement = parseEmitStatement(docString); - break; - case Token::Identifier: - if (m_insideModifier && m_scanner->currentLiteral() == "_") - { - statement = ASTNodeFactory(*this).createNode(docString); + case Token::If: + return parseIfStatement(docString); + case Token::While: + return parseWhileStatement(docString); + case Token::Do: + return parseDoWhileStatement(docString); + case Token::For: + return parseForStatement(docString); + case Token::LBrace: + return parseBlock(docString); + // starting from here, all statements must be terminated by a semicolon + case Token::Continue: + statement = ASTNodeFactory(*this).createNode(docString); m_scanner->next(); - } - else + break; + case Token::Break: + statement = ASTNodeFactory(*this).createNode(docString); + m_scanner->next(); + break; + case Token::Return: + { + ASTNodeFactory nodeFactory(*this); + ASTPointer expression; + if (m_scanner->next() != Token::Semicolon) + { + expression = parseExpression(); + nodeFactory.setEndPositionFromNode(expression); + } + statement = nodeFactory.createNode(docString, expression); + break; + } + case Token::Throw: + { + statement = ASTNodeFactory(*this).createNode(docString); + m_scanner->next(); + break; + } + case Token::Assembly: + return parseInlineAssembly(docString); + case Token::Emit: + statement = parseEmitStatement(docString); + break; + case Token::Identifier: + if (m_insideModifier && m_scanner->currentLiteral() == "_") + { + statement = ASTNodeFactory(*this).createNode(docString); + m_scanner->next(); + } + else + statement = parseSimpleStatement(docString); + break; + default: statement = parseSimpleStatement(docString); - break; - default: - statement = parseSimpleStatement(docString); - break; + break; + } } - expectToken(Token::Semicolon); + catch (FatalError const&) + { + if ( + !m_errorReporter.hasErrors() || + !m_parserErrorRecovery || + m_errorReporter.hasExcessiveErrors() + ) + BOOST_THROW_EXCEPTION(FatalError()); /* Don't try to recover here. */ + m_inParserRecovery = true; + } + if (m_inParserRecovery) + expectTokenOrConsumeUntil(Token::Semicolon, "Statement"); + else + expectToken(Token::Semicolon); return statement; } diff --git a/libsolidity/parsing/Parser.h b/libsolidity/parsing/Parser.h index 51b3b5be4..cb3d75f51 100644 --- a/libsolidity/parsing/Parser.h +++ b/libsolidity/parsing/Parser.h @@ -41,9 +41,10 @@ class Parser: public langutil::ParserBase public: explicit Parser( langutil::ErrorReporter& _errorReporter, - langutil::EVMVersion _evmVersion + langutil::EVMVersion _evmVersion, + bool _errorRecovery = false ): - ParserBase(_errorReporter), + ParserBase(_errorReporter, _errorRecovery), m_evmVersion(_evmVersion) {} diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index 764376236..a6e2f6025 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -112,6 +112,7 @@ static string const g_strBinaryRuntime = "bin-runtime"; static string const g_strCombinedJson = "combined-json"; static string const g_strCompactJSON = "compact-format"; static string const g_strContracts = "contracts"; +static string const g_strErrorRecovery = "error-recovery"; static string const g_strEVM = "evm"; static string const g_strEVM15 = "evm15"; static string const g_strEVMVersion = "evm-version"; @@ -163,6 +164,7 @@ static string const g_argBinary = g_strBinary; static string const g_argBinaryRuntime = g_strBinaryRuntime; static string const g_argCombinedJson = g_strCombinedJson; static string const g_argCompactJSON = g_strCompactJSON; +static string const g_argErrorRecovery = g_strErrorRecovery; static string const g_argGas = g_strGas; static string const g_argHelp = g_strHelp; static string const g_argInputFile = g_strInputFile; @@ -689,6 +691,7 @@ Allowed options)", (g_argColor.c_str(), "Force colored output.") (g_argNoColor.c_str(), "Explicitly disable colored output, disabling terminal auto-detection.") (g_argNewReporter.c_str(), "Enables new diagnostics reporter.") + (g_argErrorRecovery.c_str(), "Enables additional parser error recovery.") (g_argIgnoreMissingFiles.c_str(), "Ignore missing files."); po::options_description outputComponents("Output Components"); outputComponents.add_options() @@ -923,6 +926,8 @@ bool CommandLineInterface::processInput() m_compiler->setSources(m_sourceCodes); if (m_args.count(g_argLibraries)) m_compiler->setLibraries(m_libraries); + if (m_args.count(g_argErrorRecovery)) + m_compiler->setParserErrorRecovery(true); m_compiler->setEVMVersion(m_evmVersion); // TODO: Perhaps we should not compile unless requested diff --git a/test/InteractiveTests.h b/test/InteractiveTests.h index 24a5ad69e..09cc08a29 100644 --- a/test/InteractiveTests.h +++ b/test/InteractiveTests.h @@ -56,6 +56,7 @@ Testsuite const g_interactiveTestsuites[] = { {"Yul Interpreter", "libyul", "yulInterpreterTests", false, false, &yul::test::YulInterpreterTest::create}, {"Yul Object Compiler", "libyul", "objectCompiler", false, false, &yul::test::ObjectCompilerTest::create}, {"Syntax", "libsolidity", "syntaxTests", false, false, &SyntaxTest::create}, + {"ErrorRecovery", "libsolidity", "errorRecoveryTests", false, false, &SyntaxTest::createErrorRecovery}, {"Semantic", "libsolidity", "semanticTests", false, true, &SemanticTest::create}, {"JSON AST", "libsolidity", "ASTJSON", false, false, &ASTJSONTest::create}, {"SMT Checker", "libsolidity", "smtCheckerTests", true, false, &SyntaxTest::create}, @@ -66,4 +67,3 @@ Testsuite const g_interactiveTestsuites[] = { } } } - diff --git a/test/liblangutil/CharStream.cpp b/test/liblangutil/CharStream.cpp new file mode 100644 index 000000000..abd3d02db --- /dev/null +++ b/test/liblangutil/CharStream.cpp @@ -0,0 +1,53 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see . +*/ +/** + * @author Rocky Bernstein + * @date 2019 + * Unit tests for the CharStream class. + */ + +#include +#include + +#include + +namespace langutil +{ +namespace test +{ + +BOOST_AUTO_TEST_SUITE(CharStreamtest) + +BOOST_AUTO_TEST_CASE(test_fail) +{ + auto const source = std::make_shared("now is the time for testing", "source"); + + BOOST_CHECK('n' == source->get()); + BOOST_CHECK('n' == source->get()); + BOOST_CHECK('o' == source->advanceAndGet()); + BOOST_CHECK('n' == source->rollback(1)); + BOOST_CHECK('w' == source->setPosition(2)); + BOOST_REQUIRE_THROW( + source->setPosition(200), + ::langutil::InternalCompilerError + ); +} + +BOOST_AUTO_TEST_SUITE_END() + +} +} // end namespaces diff --git a/test/libsolidity/AnalysisFramework.cpp b/test/libsolidity/AnalysisFramework.cpp index faebfc3f5..c94e6c745 100644 --- a/test/libsolidity/AnalysisFramework.cpp +++ b/test/libsolidity/AnalysisFramework.cpp @@ -44,12 +44,15 @@ AnalysisFramework::parseAnalyseAndReturnError( string const& _source, bool _reportWarnings, bool _insertVersionPragma, - bool _allowMultipleErrors + bool _allowMultipleErrors, + bool _allowRecoveryErrors ) { compiler().reset(); compiler().setSources({{"", _insertVersionPragma ? "pragma solidity >=0.0;\n" + _source : _source}}); compiler().setEVMVersion(dev::test::Options::get().evmVersion()); + compiler().setParserErrorRecovery(_allowRecoveryErrors); + _allowMultipleErrors = _allowMultipleErrors || _allowRecoveryErrors; if (!compiler().parse()) { BOOST_FAIL("Parsing contract failed in analysis test suite:" + formatErrors()); diff --git a/test/libsolidity/AnalysisFramework.h b/test/libsolidity/AnalysisFramework.h index cb590674f..3fcb9e74a 100644 --- a/test/libsolidity/AnalysisFramework.h +++ b/test/libsolidity/AnalysisFramework.h @@ -50,7 +50,8 @@ protected: std::string const& _source, bool _reportWarnings = false, bool _insertVersionPragma = true, - bool _allowMultipleErrors = false + bool _allowMultipleErrors = false, + bool _allowRecoveryErrors = false ); virtual ~AnalysisFramework() = default; diff --git a/test/libsolidity/SMTChecker.cpp b/test/libsolidity/SMTChecker.cpp index a08721f17..4cb7b6667 100644 --- a/test/libsolidity/SMTChecker.cpp +++ b/test/libsolidity/SMTChecker.cpp @@ -42,14 +42,16 @@ protected: std::string const& _source, bool _reportWarnings = false, bool _insertVersionPragma = true, - bool _allowMultipleErrors = false + bool _allowMultipleErrors = false, + bool _allowRecoveryErrors = false ) { return AnalysisFramework::parseAnalyseAndReturnError( "pragma experimental SMTChecker;\n" + _source, _reportWarnings, _insertVersionPragma, - _allowMultipleErrors + _allowMultipleErrors, + _allowRecoveryErrors ); } }; diff --git a/test/libsolidity/SyntaxTest.cpp b/test/libsolidity/SyntaxTest.cpp index 61d1d20e6..8845cd7e6 100644 --- a/test/libsolidity/SyntaxTest.cpp +++ b/test/libsolidity/SyntaxTest.cpp @@ -52,7 +52,7 @@ int parseUnsignedInteger(string::iterator& _it, string::iterator _end) } -SyntaxTest::SyntaxTest(string const& _filename, langutil::EVMVersion _evmVersion): m_evmVersion(_evmVersion) +SyntaxTest::SyntaxTest(string const& _filename, langutil::EVMVersion _evmVersion, bool _errorRecovery): m_evmVersion(_evmVersion) { ifstream file(_filename); if (!file) @@ -67,6 +67,7 @@ SyntaxTest::SyntaxTest(string const& _filename, langutil::EVMVersion _evmVersion m_settings.erase("optimize-yul"); } m_expectations = parseExpectations(file); + m_errorRecovery = _errorRecovery; } TestCase::TestResult SyntaxTest::run(ostream& _stream, string const& _linePrefix, bool _formatted) @@ -75,6 +76,7 @@ TestCase::TestResult SyntaxTest::run(ostream& _stream, string const& _linePrefix compiler().reset(); compiler().setSources({{"", versionPragma + m_source}}); compiler().setEVMVersion(m_evmVersion); + compiler().setParserErrorRecovery(m_errorRecovery); compiler().setOptimiserSettings( m_optimiseYul ? OptimiserSettings::full() : diff --git a/test/libsolidity/SyntaxTest.h b/test/libsolidity/SyntaxTest.h index 37a078107..dcf922de7 100644 --- a/test/libsolidity/SyntaxTest.h +++ b/test/libsolidity/SyntaxTest.h @@ -54,8 +54,14 @@ class SyntaxTest: AnalysisFramework, public EVMVersionRestrictedTestCase { public: static std::unique_ptr create(Config const& _config) - { return std::make_unique(_config.filename, _config.evmVersion); } - SyntaxTest(std::string const& _filename, langutil::EVMVersion _evmVersion); + { + return std::make_unique(_config.filename, _config.evmVersion, false); + } + static std::unique_ptr createErrorRecovery(Config const& _config) + { + return std::make_unique(_config.filename, _config.evmVersion, true); + } + SyntaxTest(std::string const& _filename, langutil::EVMVersion _evmVersion, bool _errorRecovery = false); TestResult run(std::ostream& _stream, std::string const& _linePrefix = "", bool _formatted = false) override; @@ -84,6 +90,7 @@ protected: std::vector m_errorList; bool m_optimiseYul = false; langutil::EVMVersion const m_evmVersion; + bool m_errorRecovery = false; }; } diff --git a/test/libsolidity/errorRecoveryTests/constructor_recovers.sol b/test/libsolidity/errorRecoveryTests/constructor_recovers.sol new file mode 100644 index 000000000..804bb7283 --- /dev/null +++ b/test/libsolidity/errorRecoveryTests/constructor_recovers.sol @@ -0,0 +1,20 @@ +pragma solidity >=0.0.0; + +contract Error1 { + constructor() public { + balances[tx.origin] = ; // missing RHS. + } + + // Without error recovery we stop due to the above error. + // Error recovery however recovers at the above ';' + // There should be an AST for the above, albeit with error + // nodes. + + // This function parses properly and should give AST info. + function five() public view returns(uint) { + return 5; + } +} +// ---- +// ParserError: (95-96): Expected primary expression. +// Warning: (95-96): Recovered in Statement at ';'. diff --git a/test/libsolidity/errorRecoveryTests/contract_recovery.sol b/test/libsolidity/errorRecoveryTests/contract_recovery.sol new file mode 100644 index 000000000..4047adb31 --- /dev/null +++ b/test/libsolidity/errorRecoveryTests/contract_recovery.sol @@ -0,0 +1,7 @@ +contract Errort6 { + using foo for ; // missing type name +} + +// ---- +// ParserError: (36-37): Expected type name +// Warning: (59-60): Recovered in ContractDefinition at '}'. diff --git a/test/libsolidity/errorRecoveryTests/do_not_delete_at_error.sol b/test/libsolidity/errorRecoveryTests/do_not_delete_at_error.sol new file mode 100644 index 000000000..a7e4254b0 --- /dev/null +++ b/test/libsolidity/errorRecoveryTests/do_not_delete_at_error.sol @@ -0,0 +1,13 @@ +pragma solidity >=0.0.0; + +// Example to show why deleting the token at the +// is bad when error recovery is in effect. Here, ")" is missing +// and there is a ";" instead. That causes us to +// not be able to synchronize to ';'. Advance again and +// '}' is deleted and then we can't synchronize the contract. +// There should be an an AST created this contract (with errors). +contract Error2 { + mapping (address => uint balances; // missing ) before "balances" +} +// ---- +// ParserError: (417-425): Expected ')' but got identifier diff --git a/test/libsolidity/errorRecoveryTests/error_to_eos.sol b/test/libsolidity/errorRecoveryTests/error_to_eos.sol new file mode 100644 index 000000000..d396207b0 --- /dev/null +++ b/test/libsolidity/errorRecoveryTests/error_to_eos.sol @@ -0,0 +1,23 @@ +// Example which where scanning hits EOS, so we reset. +// Here we recover in the contractDefinition. +// There should be an an AST created this contract (with errors). +contract Error2 { + mapping (address => uint balances) // missing ; +} + +// There is no error in this contract +contract SendCoin { + function sendCoin(address receiver, uint amount) public returns(bool sufficient) { + if (balances[msg.sender] < amount) return false; + balances[msg.sender] -= amount; + balances[receiver] += amount; + emit Transfer(msg.sender, receiver, amount); + return true; + } +} + +// ---- +// ParserError: (212-220): Expected ')' but got identifier +// ParserError: (220-221): Expected ';' but got ')' +// ParserError: (220-221): Function, variable, struct or modifier declaration expected. +// Warning: (235-236): Recovered in ContractDefinition at '}'. diff --git a/test/libsolidity/errorRecoveryTests/missing_rhs.sol b/test/libsolidity/errorRecoveryTests/missing_rhs.sol new file mode 100644 index 000000000..991525b46 --- /dev/null +++ b/test/libsolidity/errorRecoveryTests/missing_rhs.sol @@ -0,0 +1,11 @@ +pragma solidity >=0.0.0; + +contract Error3 { + constructor() public { + balances[tx.origin] = ; // missing RHS. + } + +} +// ---- +// ParserError: (95-96): Expected primary expression. +// Warning: (95-96): Recovered in Statement at ';'. diff --git a/test/libsolidity/errorRecoveryTests/multiple_errors.sol b/test/libsolidity/errorRecoveryTests/multiple_errors.sol new file mode 100644 index 000000000..9127ef387 --- /dev/null +++ b/test/libsolidity/errorRecoveryTests/multiple_errors.sol @@ -0,0 +1,29 @@ +// An example with multiple errors. +// Most are caught by inserting an expected token. +// However some us S C Johnson recovery to +// skip over tokens. + +pragma solidity >=0.0.0; + +contract Error4 { + constructor() public { + balances[tx.origin] = 1 2; // missing operator + } + + function sendCoin(address receiver, uint amount) public returns(bool sufficient) { + if (balances[msg.sender] < amount) return false; + balances[msg.sender] -= amount // Missing ";" + balances[receiver] += amount // Another missing ";" + emit Transfer(msg.sender // truncated line + return true; + } + + +} +// ---- +// ParserError: (249-250): Expected ';' but got 'Number' +// ParserError: (471-479): Expected ';' but got identifier +// ParserError: (529-533): Expected ';' but got 'emit' +// ParserError: (577-583): Expected ',' but got 'return' +// ParserError: (577-583): Expected primary expression. +// Warning: (588-589): Recovered in Statement at ';'. From 0b65cf8af5af7ee3aa9ddad331f0733ec8361d2e Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Mon, 17 Jun 2019 12:01:31 +0200 Subject: [PATCH 2/2] Fixes stack-too-deep errors (soltest) on Windows by reducing recursion depth accordingly. (Caused by introducing try/catch blocks increased stack frame size) --- liblangutil/ParserBase.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/liblangutil/ParserBase.cpp b/liblangutil/ParserBase.cpp index 2432ac711..7d00fedc4 100644 --- a/liblangutil/ParserBase.cpp +++ b/liblangutil/ParserBase.cpp @@ -137,7 +137,7 @@ void ParserBase::expectTokenOrConsumeUntil(Token _value, string const& _currentN void ParserBase::increaseRecursionDepth() { m_recursionDepth++; - if (m_recursionDepth >= 2560) + if (m_recursionDepth >= 1200) fatalParserError("Maximum recursion depth reached during parsing."); }