From 4c8f8e949143d0c680a8257adbcc768d908fae9a Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Tue, 15 Jan 2019 13:40:10 +0100 Subject: [PATCH] Disallow mismatching types in switch cases and detect duplicates by value for number literals. --- docs/yul.rst | 7 ++- libyul/AsmAnalysis.cpp | 30 ++++++++-- libyul/CMakeLists.txt | 6 +- libyul/Utilities.cpp | 50 ++++++++++++++++ libyul/Utilities.h | 59 +++++++++++++++++++ libyul/optimiser/ExpressionJoiner.cpp | 2 +- libyul/optimiser/FullInliner.cpp | 2 +- libyul/optimiser/FunctionHoister.cpp | 2 +- .../InlinableExpressionFunctionFinder.cpp | 2 +- .../{Utilities.cpp => OptimizerUtilities.cpp} | 13 +--- .../{Utilities.h => OptimizerUtilities.h} | 2 - libyul/optimiser/SimplificationRules.cpp | 2 +- libyul/optimiser/StructuralSimplifier.cpp | 2 +- libyul/optimiser/UnusedPruner.cpp | 2 +- test/libsolidity/InlineAssembly.cpp | 2 +- test/libyul/Parser.cpp | 13 ++++ 16 files changed, 166 insertions(+), 30 deletions(-) create mode 100644 libyul/Utilities.cpp create mode 100644 libyul/Utilities.h rename libyul/optimiser/{Utilities.cpp => OptimizerUtilities.cpp} (74%) rename libyul/optimiser/{Utilities.h => OptimizerUtilities.h} (94%) diff --git a/docs/yul.rst b/docs/yul.rst index 31555742a..627e6e7cd 100644 --- a/docs/yul.rst +++ b/docs/yul.rst @@ -130,9 +130,10 @@ Restrictions on the Grammar --------------------------- Switches must have at least one case (including the default case). -If all possible values of the expression is covered, the default case should -not be allowed (i.e. a switch with a ``bool`` expression and having both a -true and false case should not allow a default case). +If all possible values of the expression are covered, a default case should +not be allowed (i.e. a switch with a ``bool`` expression that has both a +true and a false case should not allow a default case). All case values need to +have the same type. Every expression evaluates to zero or more values. Identifiers and Literals evaluate to exactly diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp index 6a7b2b61d..a5552c512 100644 --- a/libyul/AsmAnalysis.cpp +++ b/libyul/AsmAnalysis.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include @@ -390,7 +391,29 @@ bool AsmAnalyzer::operator()(Switch const& _switch) if (!expectExpression(*_switch.expression)) success = false; - set> cases; + if (m_dialect->flavour == AsmFlavour::Yul) + { + YulString caseType; + bool mismatchingTypes = false; + for (auto const& _case: _switch.cases) + if (_case.value) + { + if (caseType.empty()) + caseType = _case.value->type; + else if (caseType != _case.value->type) + { + mismatchingTypes = true; + break; + } + } + if (mismatchingTypes) + m_errorReporter.typeError( + _switch.location, + "Switch cases have non-matching types." + ); + } + + set> cases; for (auto const& _case: _switch.cases) { if (_case.value) @@ -404,12 +427,11 @@ bool AsmAnalyzer::operator()(Switch const& _switch) m_stackHeight--; /// Note: the parser ensures there is only one default case - auto val = make_tuple(_case.value->kind, _case.value->value); - if (!cases.insert(val).second) + if (!cases.insert(_case.value.get()).second) { m_errorReporter.declarationError( _case.location, - "Duplicate case defined" + "Duplicate case defined." ); success = false; } diff --git a/libyul/CMakeLists.txt b/libyul/CMakeLists.txt index 52c4ac8ed..ad9812bd6 100644 --- a/libyul/CMakeLists.txt +++ b/libyul/CMakeLists.txt @@ -20,6 +20,8 @@ add_library(yul Object.h ObjectParser.cpp ObjectParser.h + Utilities.cpp + Utilities.h YulString.h backends/evm/AbstractAssembly.h backends/evm/EVMAssembly.cpp @@ -68,6 +70,8 @@ add_library(yul optimiser/NameCollector.h optimiser/NameDispenser.cpp optimiser/NameDispenser.h + optimiser/OptimizerUtilities.cpp + optimiser/OptimizerUtilities.h optimiser/RedundantAssignEliminator.cpp optimiser/RedundantAssignEliminator.h optimiser/Rematerialiser.cpp @@ -90,8 +94,6 @@ add_library(yul optimiser/SyntacticalEquality.h optimiser/UnusedPruner.cpp optimiser/UnusedPruner.h - optimiser/Utilities.cpp - optimiser/Utilities.h optimiser/VarDeclInitializer.cpp optimiser/VarDeclInitializer.h ) diff --git a/libyul/Utilities.cpp b/libyul/Utilities.cpp new file mode 100644 index 000000000..e5f4e517c --- /dev/null +++ b/libyul/Utilities.cpp @@ -0,0 +1,50 @@ +/* + 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 . +*/ +/** + * Some useful snippets for the optimiser. + */ + +#include + +#include +#include + +#include + +using namespace std; +using namespace dev; +using namespace yul; + +u256 yul::valueOfNumberLiteral(Literal const& _literal) +{ + assertThrow(_literal.kind == LiteralKind::Number, OptimizerException, ""); + std::string const& literalString = _literal.value.str(); + assertThrow(isValidDecimal(literalString) || isValidHex(literalString), OptimizerException, ""); + return u256(literalString); +} + +template<> +bool Less::operator()(Literal const& _lhs, Literal const& _rhs) const +{ + if (std::make_tuple(_lhs.kind, _lhs.type) != std::make_tuple(_rhs.kind, _rhs.type)) + return std::make_tuple(_lhs.kind, _lhs.type) < std::make_tuple(_rhs.kind, _rhs.type); + + if (_lhs.kind == LiteralKind::Number) + return valueOfNumberLiteral(_lhs) < valueOfNumberLiteral(_rhs); + else + return _lhs.value < _rhs.value; +} diff --git a/libyul/Utilities.h b/libyul/Utilities.h new file mode 100644 index 000000000..288bc6ee2 --- /dev/null +++ b/libyul/Utilities.h @@ -0,0 +1,59 @@ +/* + 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 . +*/ +/** + * Small useful snippets for the optimiser. + */ + +#pragma once + +#include +#include + +namespace yul +{ + +dev::u256 valueOfNumberLiteral(Literal const& _literal); + +/** + * Linear order on Yul AST nodes. + * + * Defines a linear order on Yul AST nodes to be used in maps and sets. + * Note: the order is total and deterministic, but independent of the semantics, e.g. + * it is not guaranteed that the false Literal is "less" than the true Literal. + */ +template +struct Less +{ + bool operator()(T const& _lhs, T const& _rhs) const; +}; + +template +struct Less +{ + bool operator()(T const* _lhs, T const* _rhs) const + { + if (_lhs && _rhs) + return Less{}(*_lhs, *_rhs); + else + return _lhs < _rhs; + } +}; + +template<> bool Less::operator()(Literal const& _lhs, Literal const& _rhs) const; +extern template struct Less; + +} diff --git a/libyul/optimiser/ExpressionJoiner.cpp b/libyul/optimiser/ExpressionJoiner.cpp index de2b5d53f..02ac4e45e 100644 --- a/libyul/optimiser/ExpressionJoiner.cpp +++ b/libyul/optimiser/ExpressionJoiner.cpp @@ -22,7 +22,7 @@ #include #include -#include +#include #include #include diff --git a/libyul/optimiser/FullInliner.cpp b/libyul/optimiser/FullInliner.cpp index 1f267f960..dd969fafc 100644 --- a/libyul/optimiser/FullInliner.cpp +++ b/libyul/optimiser/FullInliner.cpp @@ -23,7 +23,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/libyul/optimiser/FunctionHoister.cpp b/libyul/optimiser/FunctionHoister.cpp index bd1c781b0..4863b94d9 100644 --- a/libyul/optimiser/FunctionHoister.cpp +++ b/libyul/optimiser/FunctionHoister.cpp @@ -21,7 +21,7 @@ */ #include -#include +#include #include #include diff --git a/libyul/optimiser/InlinableExpressionFunctionFinder.cpp b/libyul/optimiser/InlinableExpressionFunctionFinder.cpp index 662cdf250..f57faa7c2 100644 --- a/libyul/optimiser/InlinableExpressionFunctionFinder.cpp +++ b/libyul/optimiser/InlinableExpressionFunctionFinder.cpp @@ -20,7 +20,7 @@ #include -#include +#include #include using namespace std; diff --git a/libyul/optimiser/Utilities.cpp b/libyul/optimiser/OptimizerUtilities.cpp similarity index 74% rename from libyul/optimiser/Utilities.cpp rename to libyul/optimiser/OptimizerUtilities.cpp index b3b580d5e..f9571a4c2 100644 --- a/libyul/optimiser/Utilities.cpp +++ b/libyul/optimiser/OptimizerUtilities.cpp @@ -1,4 +1,4 @@ -/*( +/* This file is part of solidity. solidity is free software: you can redistribute it and/or modify @@ -18,10 +18,9 @@ * Some useful snippets for the optimiser. */ -#include +#include #include -#include #include @@ -38,11 +37,3 @@ void yul::removeEmptyBlocks(Block& _block) }; boost::range::remove_erase_if(_block.statements, isEmptyBlock); } - -u256 yul::valueOfNumberLiteral(Literal const& _literal) -{ - assertThrow(_literal.kind == LiteralKind::Number, OptimizerException, ""); - std::string const& literalString = _literal.value.str(); - assertThrow(isValidDecimal(literalString) || isValidHex(literalString), OptimizerException, ""); - return u256(literalString); -} diff --git a/libyul/optimiser/Utilities.h b/libyul/optimiser/OptimizerUtilities.h similarity index 94% rename from libyul/optimiser/Utilities.h rename to libyul/optimiser/OptimizerUtilities.h index 1cfff62b1..449a1bc02 100644 --- a/libyul/optimiser/Utilities.h +++ b/libyul/optimiser/OptimizerUtilities.h @@ -29,6 +29,4 @@ namespace yul /// Removes statements that are just empty blocks (non-recursive). void removeEmptyBlocks(Block& _block); -dev::u256 valueOfNumberLiteral(Literal const& _literal); - } diff --git a/libyul/optimiser/SimplificationRules.cpp b/libyul/optimiser/SimplificationRules.cpp index da9c7d9d0..037dad972 100644 --- a/libyul/optimiser/SimplificationRules.cpp +++ b/libyul/optimiser/SimplificationRules.cpp @@ -20,11 +20,11 @@ #include -#include #include #include #include #include +#include #include diff --git a/libyul/optimiser/StructuralSimplifier.cpp b/libyul/optimiser/StructuralSimplifier.cpp index bdf4cb2ab..8d7dcd57c 100644 --- a/libyul/optimiser/StructuralSimplifier.cpp +++ b/libyul/optimiser/StructuralSimplifier.cpp @@ -16,8 +16,8 @@ */ #include #include -#include #include +#include #include #include diff --git a/libyul/optimiser/UnusedPruner.cpp b/libyul/optimiser/UnusedPruner.cpp index 53c412e34..365b255c5 100644 --- a/libyul/optimiser/UnusedPruner.cpp +++ b/libyul/optimiser/UnusedPruner.cpp @@ -22,7 +22,7 @@ #include #include -#include +#include #include #include diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index b69860417..92e4dcf6d 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -316,7 +316,7 @@ BOOST_AUTO_TEST_CASE(switch_no_cases) BOOST_AUTO_TEST_CASE(switch_duplicate_case) { - CHECK_PARSE_ERROR("{ switch 42 case 1 {} case 1 {} default {} }", DeclarationError, "Duplicate case defined"); + CHECK_PARSE_ERROR("{ switch 42 case 1 {} case 1 {} default {} }", DeclarationError, "Duplicate case defined."); } BOOST_AUTO_TEST_CASE(switch_invalid_expression) diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp index 897f18aeb..84f5c14b8 100644 --- a/test/libyul/Parser.cpp +++ b/test/libyul/Parser.cpp @@ -304,6 +304,19 @@ BOOST_AUTO_TEST_CASE(if_statement_invalid) BOOST_CHECK(successParse("{ if 42:u256 { } }")); } +BOOST_AUTO_TEST_CASE(switch_case_types) +{ + CHECK_ERROR("{ switch 0:u256 case 0:u256 {} case 1:u32 {} }", TypeError, "Switch cases have non-matching types."); + // The following should be an error in the future, but this is not yet detected. + BOOST_CHECK(successParse("{ switch 0:u256 case 0:u32 {} case 1:u32 {} }")); +} + +BOOST_AUTO_TEST_CASE(switch_duplicate_case) +{ + CHECK_ERROR("{ switch 0:u256 case 0:u256 {} case 0x0:u256 {} }", DeclarationError, "Duplicate case defined."); + BOOST_CHECK(successParse("{ switch 0:u256 case 42:u256 {} case 0x42:u256 {} }")); +} + BOOST_AUTO_TEST_CASE(builtins_parser) { struct SimpleDialect: public Dialect