From e130bc7e7cc647b15c448133f725a060910da587 Mon Sep 17 00:00:00 2001 From: Lu Guanqun Date: Thu, 14 Jan 2016 01:58:09 +0000 Subject: [PATCH 1/4] check whether break/continue is in the loop --- libsolidity/analysis/SyntaxChecker.cpp | 87 +++++++++++++++++++ libsolidity/analysis/SyntaxChecker.h | 58 +++++++++++++ libsolidity/ast/ASTAnnotations.h | 1 + libsolidity/interface/CompilerStack.cpp | 6 ++ libsolidity/interface/Exceptions.cpp | 3 + libsolidity/interface/Exceptions.h | 1 + test/libsolidity/SolidityEndToEndTest.cpp | 12 --- .../SolidityNameAndTypeResolution.cpp | 31 +++++++ 8 files changed, 187 insertions(+), 12 deletions(-) create mode 100644 libsolidity/analysis/SyntaxChecker.cpp create mode 100644 libsolidity/analysis/SyntaxChecker.h diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp new file mode 100644 index 000000000..927434618 --- /dev/null +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -0,0 +1,87 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum 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. + + cpp-ethereum 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 cpp-ethereum. If not, see . +*/ + +#include +#include +#include + +using namespace std; +using namespace dev; +using namespace dev::solidity; + + +bool SyntaxChecker::checkSyntax(SourceUnit const& _sourceUnit) +{ + _sourceUnit.accept(*this); + return m_errors.empty(); +} + +void SyntaxChecker::syntaxError(SourceLocation const& _location, std::string const& _description) +{ + auto err = make_shared(Error::Type::SyntaxError); + *err << + errinfo_sourceLocation(_location) << + errinfo_comment(_description); + + m_errors.push_back(err); +} + +bool SyntaxChecker::visit(WhileStatement const& _whileStatement) +{ + _whileStatement.body().annotation().isInLoop = true; + return true; +} + +bool SyntaxChecker::visit(ForStatement const& _forStatement) +{ + _forStatement.body().annotation().isInLoop = true; + return true; +} + +bool SyntaxChecker::visit(Block const& _blockStatement) +{ + bool inLoop = _blockStatement.annotation().isInLoop; + for (auto& statement : _blockStatement.statements()) + statement->annotation().isInLoop = inLoop; + return true; +} + +bool SyntaxChecker::visit(IfStatement const& _ifStatement) +{ + bool inLoop = _ifStatement.annotation().isInLoop; + _ifStatement.trueStatement().annotation().isInLoop = inLoop; + if (_ifStatement.falseStatement()) + _ifStatement.falseStatement()->annotation().isInLoop = inLoop; + return true; +} + +bool SyntaxChecker::visit(Continue const& _continueStatement) +{ + if (!_continueStatement.annotation().isInLoop) + // we're not in a for/while loop, report syntax error + syntaxError(_continueStatement.location(), "\"continue\" has to be in a \"for\" or \"while\" loop."); + return true; +} + +bool SyntaxChecker::visit(Break const& _breakStatement) +{ + if (!_breakStatement.annotation().isInLoop) + // we're not in a for/while loop, report syntax error + syntaxError(_breakStatement.location(), "\"break\" has to be in a \"for\" or \"while\" loop."); + return true; +} + diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h new file mode 100644 index 000000000..66f6d94d1 --- /dev/null +++ b/libsolidity/analysis/SyntaxChecker.h @@ -0,0 +1,58 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum 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. + + cpp-ethereum 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 cpp-ethereum. If not, see . +*/ + +#pragma once + +#include +#include +#include +#include +#include + +namespace dev +{ +namespace solidity +{ + +/** + * The module that performs syntax analysis on the AST: + * - whether continue/break is in a for/while loop. + */ +class SyntaxChecker : private ASTConstVisitor +{ +public: + /// @param _errors the reference to the list of errors and warnings to add them found during type checking. + SyntaxChecker(ErrorList& _errors): m_errors(_errors) {} + + bool checkSyntax(SourceUnit const& _sourceUnit); + +private: + /// Adds a new error to the list of errors. + void syntaxError(SourceLocation const& _location, std::string const& _description); + + virtual bool visit(WhileStatement const& _whileStatement) override; + virtual bool visit(ForStatement const& _forStatement) override; + virtual bool visit(Block const& _blockStatement) override; + virtual bool visit(IfStatement const& _ifStatement) override; + virtual bool visit(Continue const& _continueStatement) override; + virtual bool visit(Break const& _breakStatement) override; + + ErrorList& m_errors; +}; + +} +} diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index 235338bb8..cae606cac 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -108,6 +108,7 @@ struct VariableDeclarationAnnotation: ASTAnnotation struct StatementAnnotation: ASTAnnotation, DocumentedAnnotation { + bool isInLoop = false; }; struct ReturnAnnotation: StatementAnnotation diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index a6f6f224b..8a0d2f5ec 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -122,6 +123,11 @@ bool CompilerStack::parse() resolveImports(); bool noErrors = true; + SyntaxChecker syntaxChecker(m_errors); + for (Source const* source: m_sourceOrder) + if (!syntaxChecker.checkSyntax(*source->ast)) + return false; + DocStringAnalyser docStringAnalyser(m_errors); for (Source const* source: m_sourceOrder) if (!docStringAnalyser.analyseDocStrings(*source->ast)) diff --git a/libsolidity/interface/Exceptions.cpp b/libsolidity/interface/Exceptions.cpp index 465c3d2f9..6d72520bf 100644 --- a/libsolidity/interface/Exceptions.cpp +++ b/libsolidity/interface/Exceptions.cpp @@ -39,6 +39,9 @@ Error::Error(Type _type): m_type(_type) case Type::ParserError: m_typeName = "Parser Error"; break; + case Type::SyntaxError: + m_typeName = "Syntax Error"; + break; case Type::TypeError: m_typeName = "Type Error"; break; diff --git a/libsolidity/interface/Exceptions.h b/libsolidity/interface/Exceptions.h index 14be3c3dc..078353200 100644 --- a/libsolidity/interface/Exceptions.h +++ b/libsolidity/interface/Exceptions.h @@ -47,6 +47,7 @@ public: DocstringParsingError, ParserError, TypeError, + SyntaxError, Why3TranslatorError, Warning }; diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 34c5dffc4..3ef5ebbee 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -166,18 +166,6 @@ BOOST_AUTO_TEST_CASE(while_loop) testSolidityAgainstCppOnRange("f(uint256)", while_loop_cpp, 0, 5); } -BOOST_AUTO_TEST_CASE(break_outside_loop) -{ - // break and continue outside loops should be simply ignored - char const* sourceCode = "contract test {\n" - " function f(uint x) returns(uint y) {\n" - " break; continue; return 2;\n" - " }\n" - "}\n"; - compileAndRun(sourceCode); - testSolidityAgainstCpp("f(uint256)", [](u256 const&) -> u256 { return 2; }, u256(0)); -} - BOOST_AUTO_TEST_CASE(nested_loops) { // tests that break and continue statements in nested loops jump to the correct place diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 4697e756f..11e78d946 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -57,6 +58,10 @@ parseAnalyseAndReturnError(string const& _source, bool _reportWarnings = false) if(!sourceUnit) return make_pair(sourceUnit, nullptr); + SyntaxChecker syntaxChecker(errors); + if (!syntaxChecker.checkSyntax(*sourceUnit)) + return make_pair(sourceUnit, std::make_shared(errors[0]->type())); + std::shared_ptr globalContext = make_shared(); NameAndTypeResolver resolver(globalContext->declarations(), errors); solAssert(Error::containsOnlyWarnings(errors), ""); @@ -2903,6 +2908,32 @@ BOOST_AUTO_TEST_CASE(lvalues_as_inline_array) BOOST_CHECK(expectError(text) == Error::Type::TypeError); } +BOOST_AUTO_TEST_CASE(break_not_in_loop) +{ + char const* text = R"( + contract C { + function f() { + if (true) + break; + } + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::SyntaxError); +} + +BOOST_AUTO_TEST_CASE(continue_not_in_loop) +{ + char const* text = R"( + contract C { + function f() { + if (true) + continue; + } + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::SyntaxError); +} + BOOST_AUTO_TEST_SUITE_END() } From c8886ed5cfc4f0b7bf4e8d52eb94da7137c9b70e Mon Sep 17 00:00:00 2001 From: Lu Guanqun Date: Tue, 19 Jan 2016 02:16:13 +0000 Subject: [PATCH 2/4] code changes according to Chris's comments --- libsolidity/analysis/SyntaxChecker.cpp | 29 ++++++++++--------------- libsolidity/analysis/SyntaxChecker.h | 9 +++++--- libsolidity/interface/CompilerStack.cpp | 2 +- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index 927434618..f1cdaf0f8 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -42,36 +42,29 @@ void SyntaxChecker::syntaxError(SourceLocation const& _location, std::string con bool SyntaxChecker::visit(WhileStatement const& _whileStatement) { - _whileStatement.body().annotation().isInLoop = true; + m_inLoopDepth++; return true; } +void SyntaxChecker::endVisit(WhileStatement const& _whileStatement) +{ + m_inLoopDepth--; +} + bool SyntaxChecker::visit(ForStatement const& _forStatement) { - _forStatement.body().annotation().isInLoop = true; + m_inLoopDepth++; return true; } -bool SyntaxChecker::visit(Block const& _blockStatement) +void SyntaxChecker::endVisit(ForStatement const& _forStatement) { - bool inLoop = _blockStatement.annotation().isInLoop; - for (auto& statement : _blockStatement.statements()) - statement->annotation().isInLoop = inLoop; - return true; -} - -bool SyntaxChecker::visit(IfStatement const& _ifStatement) -{ - bool inLoop = _ifStatement.annotation().isInLoop; - _ifStatement.trueStatement().annotation().isInLoop = inLoop; - if (_ifStatement.falseStatement()) - _ifStatement.falseStatement()->annotation().isInLoop = inLoop; - return true; + m_inLoopDepth--; } bool SyntaxChecker::visit(Continue const& _continueStatement) { - if (!_continueStatement.annotation().isInLoop) + if (m_inLoopDepth <= 0) // we're not in a for/while loop, report syntax error syntaxError(_continueStatement.location(), "\"continue\" has to be in a \"for\" or \"while\" loop."); return true; @@ -79,7 +72,7 @@ bool SyntaxChecker::visit(Continue const& _continueStatement) bool SyntaxChecker::visit(Break const& _breakStatement) { - if (!_breakStatement.annotation().isInLoop) + if (m_inLoopDepth <= 0) // we're not in a for/while loop, report syntax error syntaxError(_breakStatement.location(), "\"break\" has to be in a \"for\" or \"while\" loop."); return true; diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h index 66f6d94d1..c836d49f8 100644 --- a/libsolidity/analysis/SyntaxChecker.h +++ b/libsolidity/analysis/SyntaxChecker.h @@ -32,7 +32,7 @@ namespace solidity * The module that performs syntax analysis on the AST: * - whether continue/break is in a for/while loop. */ -class SyntaxChecker : private ASTConstVisitor +class SyntaxChecker: private ASTConstVisitor { public: /// @param _errors the reference to the list of errors and warnings to add them found during type checking. @@ -45,13 +45,16 @@ private: void syntaxError(SourceLocation const& _location, std::string const& _description); virtual bool visit(WhileStatement const& _whileStatement) override; + virtual void endVisit(WhileStatement const& _whileStatement) override; virtual bool visit(ForStatement const& _forStatement) override; - virtual bool visit(Block const& _blockStatement) override; - virtual bool visit(IfStatement const& _ifStatement) override; + virtual void endVisit(ForStatement const& _forStatement) override; + virtual bool visit(Continue const& _continueStatement) override; virtual bool visit(Break const& _breakStatement) override; ErrorList& m_errors; + + int m_inLoopDepth = 0; }; } diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 8a0d2f5ec..8ff630564 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -126,7 +126,7 @@ bool CompilerStack::parse() SyntaxChecker syntaxChecker(m_errors); for (Source const* source: m_sourceOrder) if (!syntaxChecker.checkSyntax(*source->ast)) - return false; + noErrors = false; DocStringAnalyser docStringAnalyser(m_errors); for (Source const* source: m_sourceOrder) From df728581ce1bb05a338519efc6b834adfed68d90 Mon Sep 17 00:00:00 2001 From: Lu Guanqun Date: Tue, 19 Jan 2016 02:18:01 +0000 Subject: [PATCH 3/4] add another test case for continue not in loop --- libsolidity/analysis/SyntaxChecker.cpp | 8 ++++---- .../libsolidity/SolidityNameAndTypeResolution.cpp | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index f1cdaf0f8..e94ce9fe7 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -40,24 +40,24 @@ void SyntaxChecker::syntaxError(SourceLocation const& _location, std::string con m_errors.push_back(err); } -bool SyntaxChecker::visit(WhileStatement const& _whileStatement) +bool SyntaxChecker::visit(WhileStatement const&) { m_inLoopDepth++; return true; } -void SyntaxChecker::endVisit(WhileStatement const& _whileStatement) +void SyntaxChecker::endVisit(WhileStatement const&) { m_inLoopDepth--; } -bool SyntaxChecker::visit(ForStatement const& _forStatement) +bool SyntaxChecker::visit(ForStatement const&) { m_inLoopDepth++; return true; } -void SyntaxChecker::endVisit(ForStatement const& _forStatement) +void SyntaxChecker::endVisit(ForStatement const&) { m_inLoopDepth--; } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 11e78d946..a95021ea3 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -2934,6 +2934,21 @@ BOOST_AUTO_TEST_CASE(continue_not_in_loop) BOOST_CHECK(expectError(text) == Error::Type::SyntaxError); } +BOOST_AUTO_TEST_CASE(continue_not_in_loop_2) +{ + char const* text = R"( + contract C { + function f() { + while (true) + { + } + continue; + } + } + )"; + BOOST_CHECK(expectError(text) == Error::Type::SyntaxError); +} + BOOST_AUTO_TEST_SUITE_END() } From cb3e07acfcbd9a77b27bfbac600a40cd2725594b Mon Sep 17 00:00:00 2001 From: Lu Guanqun Date: Tue, 19 Jan 2016 15:00:19 +0000 Subject: [PATCH 4/4] remove the unused line --- libsolidity/ast/ASTAnnotations.h | 1 - 1 file changed, 1 deletion(-) diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index cae606cac..235338bb8 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -108,7 +108,6 @@ struct VariableDeclarationAnnotation: ASTAnnotation struct StatementAnnotation: ASTAnnotation, DocumentedAnnotation { - bool isInLoop = false; }; struct ReturnAnnotation: StatementAnnotation