From d089a1ef2b4d9dbcdeb311edc3efd5df74282ba3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 3 Mar 2017 14:33:54 +0100 Subject: [PATCH 1/3] Tests for cyclic dependencies between constants. --- .../SolidityNameAndTypeResolution.cpp | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 3b1375721..ec80b1331 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -5156,6 +5156,34 @@ BOOST_AUTO_TEST_CASE(address_methods) CHECK_SUCCESS(text); } +BOOST_AUTO_TEST_CASE(cyclic_dependency_for_constants) +{ + char const* text = R"( + contract C { + uint constant a = a; + } + )"; + CHECK_ERROR(text, TypeError, "cyclic dependency via a"); + text = R"( + contract C { + uint constant a = b * c; + uint constant b = 7; + uint constant c = b + uint(sha3(d)); + uint constant d = 2 + a; + } + )"; + CHECK_ERROR_ALLOW_MULTI(text, TypeError, "a has a cyclic dependency via c"); + text = R"( + contract C { + uint constant a = b * c; + uint constant b = 7; + uint constant c = 4 + uint(sha3(d)); + uint constant d = 2 + b; + } + )"; + CHECK_SUCCESS(text); +} + BOOST_AUTO_TEST_SUITE_END() } From 5c5d83fd704e18f88d80e346386e97ed600b7281 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 3 Mar 2017 12:51:51 +0100 Subject: [PATCH 2/3] Check for circular references in constant variables. --- libsolidity/analysis/PostTypeChecker.cpp | 108 ++++++++++++++++++ libsolidity/analysis/PostTypeChecker.h | 69 +++++++++++ libsolidity/analysis/StaticAnalyzer.h | 3 + libsolidity/interface/CompilerStack.cpp | 9 ++ .../SolidityNameAndTypeResolution.cpp | 21 ++-- 5 files changed, 202 insertions(+), 8 deletions(-) create mode 100644 libsolidity/analysis/PostTypeChecker.cpp create mode 100644 libsolidity/analysis/PostTypeChecker.h diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp new file mode 100644 index 000000000..cae77c74d --- /dev/null +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -0,0 +1,108 @@ +/* + 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 . +*/ + +#include +#include +#include +#include + +#include + +#include + +using namespace std; +using namespace dev; +using namespace dev::solidity; + + +bool PostTypeChecker::check(ASTNode const& _astRoot) +{ + _astRoot.accept(*this); + return Error::containsOnlyWarnings(m_errors); +} + +void PostTypeChecker::typeError(SourceLocation const& _location, std::string const& _description) +{ + auto err = make_shared(Error::Type::TypeError); + *err << + errinfo_sourceLocation(_location) << + errinfo_comment(_description); + + m_errors.push_back(err); +} + +bool PostTypeChecker::visit(ContractDefinition const&) +{ + solAssert(!m_currentConstVariable, ""); + solAssert(m_constVariableDependencies.empty(), ""); + return true; +} + +void PostTypeChecker::endVisit(ContractDefinition const&) +{ + solAssert(!m_currentConstVariable, ""); + for (auto declaration: m_constVariables) + if (auto identifier = findCycle(declaration)) + typeError(declaration->location(), "The value of the constant " + declaration->name() + " has a cyclic dependency via " + identifier->name() + "."); +} + +bool PostTypeChecker::visit(VariableDeclaration const& _variable) +{ + solAssert(!m_currentConstVariable, ""); + if (_variable.isConstant()) + { + m_currentConstVariable = &_variable; + m_constVariables.push_back(&_variable); + } + return true; +} + +void PostTypeChecker::endVisit(VariableDeclaration const& _variable) +{ + if (_variable.isConstant()) + { + solAssert(m_currentConstVariable == &_variable, ""); + m_currentConstVariable = nullptr; + } +} + +bool PostTypeChecker::visit(Identifier const& _identifier) +{ + if (m_currentConstVariable) + if (auto var = dynamic_cast(_identifier.annotation().referencedDeclaration)) + if (var->isConstant()) + m_constVariableDependencies[m_currentConstVariable].insert(var); + return true; +} + +VariableDeclaration const* PostTypeChecker::findCycle( + VariableDeclaration const* _startingFrom, + set const& _seen +) +{ + if (_seen.count(_startingFrom)) + return _startingFrom; + else if (m_constVariableDependencies.count(_startingFrom)) + { + set seen(_seen); + seen.insert(_startingFrom); + for (auto v: m_constVariableDependencies[_startingFrom]) + if (findCycle(v, seen)) + return v; + } + return nullptr; +} diff --git a/libsolidity/analysis/PostTypeChecker.h b/libsolidity/analysis/PostTypeChecker.h new file mode 100644 index 000000000..8774f4130 --- /dev/null +++ b/libsolidity/analysis/PostTypeChecker.h @@ -0,0 +1,69 @@ +/* + 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 . +*/ + +#pragma once + +#include +#include +#include +#include +#include + +namespace dev +{ +namespace solidity +{ + +/** + * This module performs analyses on the AST that are done after type checking and assignments of types: + * - whether there are circular references in constant state variables + * @TODO factor out each use-case into an individual class (but do the traversal only once) + */ +class PostTypeChecker: private ASTConstVisitor +{ +public: + /// @param _errors the reference to the list of errors and warnings to add them found during type checking. + PostTypeChecker(ErrorList& _errors): m_errors(_errors) {} + + bool check(ASTNode const& _astRoot); + +private: + /// Adds a new error to the list of errors. + void typeError(SourceLocation const& _location, std::string const& _description); + + virtual bool visit(ContractDefinition const& _contract) override; + virtual void endVisit(ContractDefinition const& _contract) override; + + virtual bool visit(VariableDeclaration const& _declaration) override; + virtual void endVisit(VariableDeclaration const& _declaration) override; + + virtual bool visit(Identifier const& _identifier) override; + + VariableDeclaration const* findCycle( + VariableDeclaration const* _startingFrom, + std::set const& _seen = {} + ); + + ErrorList& m_errors; + + VariableDeclaration const* m_currentConstVariable = nullptr; + std::vector m_constVariables; ///< Required for determinism. + std::map> m_constVariableDependencies; +}; + +} +} diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index b6cf783ee..0cb961bd0 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -36,6 +36,9 @@ namespace solidity /** * The module that performs static analysis on the AST. + * In this context, static analysis is anything that can produce warnings which can help + * programmers write cleaner code. For every warning generated eher, it has to be possible to write + * equivalent code that does generate the warning. */ class StaticAnalyzer: private ASTConstVisitor { diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 9d8d872f4..594efb1e5 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -217,6 +218,14 @@ bool CompilerStack::parse() m_contracts[contract->fullyQualifiedName()].contract = contract; } + if (noErrors) + { + PostTypeChecker postTypeChecker(m_errors); + for (Source const* source: m_sourceOrder) + if (!postTypeChecker.check(*source->ast)) + noErrors = false; + } + if (noErrors) { StaticAnalyzer staticAnalyzer(m_errors); diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index ec80b1331..7576ee098 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -20,19 +20,23 @@ * Unit tests for the name and type resolution of the solidity parser. */ -#include +#include + +#include -#include #include #include #include #include +#include #include #include #include #include -#include "../TestHelper.h" -#include "ErrorCheck.h" + +#include + +#include using namespace std; @@ -93,10 +97,11 @@ parseAnalyseAndReturnError(string const& _source, bool _reportWarnings = false, BOOST_CHECK(success || !errors.empty()); } if (success) - { - StaticAnalyzer staticAnalyzer(errors); - staticAnalyzer.analyze(*sourceUnit); - } + if (!PostTypeChecker(errors).check(*sourceUnit)) + success = false; + if (success) + if (!StaticAnalyzer(errors).analyze(*sourceUnit)) + success = false; if (errors.size() > 1 && !_allowMultipleErrors) BOOST_FAIL("Multiple errors found"); for (auto const& currentError: errors) From efdfacaaeca0bf5fd900ed885e05fb15b282d5cf Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 3 Mar 2017 14:34:39 +0100 Subject: [PATCH 3/3] Changelog entry. --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 5eb1e401e..8e11775b8 100644 --- a/Changelog.md +++ b/Changelog.md @@ -11,6 +11,7 @@ Bugfixes: * Commandline interface: Always escape filenames (replace ``/``, ``:`` and ``.`` with ``_``). * Commandline interface: Do not try creating paths ``.`` and ``..``. * Type system: Fix a crash caused by continuing on fatal errors in the code. + * Type system: Detect cyclic dependencies between constants. * Type system: Disallow arrays with negative length. * Type system: Fix a crash related to invalid binary operators. * Type system: Disallow ``var`` declaration with empty tuple type.