From fd1662d1c49776232b491f1871391ad1cd90309a Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 15 Feb 2018 15:18:09 +0100 Subject: Warn about using loose inline assembly features as experimental 0.5.0 feature. --- libsolidity/inlineasm/AsmAnalysis.cpp | 45 +++++++++++++++++++++++++++++------ libsolidity/inlineasm/AsmAnalysis.h | 17 ++++++++++++- 2 files changed, 54 insertions(+), 8 deletions(-) (limited to 'libsolidity/inlineasm') diff --git a/libsolidity/inlineasm/AsmAnalysis.cpp b/libsolidity/inlineasm/AsmAnalysis.cpp index a7f764a5..abf7ddf2 100644 --- a/libsolidity/inlineasm/AsmAnalysis.cpp +++ b/libsolidity/inlineasm/AsmAnalysis.cpp @@ -54,7 +54,10 @@ bool AsmAnalyzer::analyze(Block const& _block) bool AsmAnalyzer::operator()(Label const& _label) { - solAssert(m_flavour == AsmFlavour::Loose, ""); + checkLooseFeature( + _label.location, + "The use of labels is deprecated. Please use \"if\", \"switch\", \"for\" or function calls instead." + ); m_info.stackHeightInfo[&_label] = m_stackHeight; warnOnInstructions(solidity::Instruction::JUMPDEST, _label.location); return true; @@ -62,7 +65,10 @@ bool AsmAnalyzer::operator()(Label const& _label) bool AsmAnalyzer::operator()(assembly::Instruction const& _instruction) { - solAssert(m_flavour == AsmFlavour::Loose, ""); + checkLooseFeature( + _instruction.location, + "The use of non-functional instructions is deprecated. Please use functional notation instead." + ); auto const& info = instructionInfo(_instruction.instruction); m_stackHeight += info.ret - info.args; m_info.stackHeightInfo[&_instruction] = m_stackHeight; @@ -170,18 +176,31 @@ bool AsmAnalyzer::operator()(FunctionalInstruction const& _instr) bool AsmAnalyzer::operator()(assembly::ExpressionStatement const& _statement) { - size_t initialStackHeight = m_stackHeight; + int initialStackHeight = m_stackHeight; bool success = boost::apply_visitor(*this, _statement.expression); - if (m_flavour != AsmFlavour::Loose) - if (!expectDeposit(0, initialStackHeight, _statement.location)) + if (m_stackHeight != initialStackHeight && (m_flavour != AsmFlavour::Loose || m_errorTypeForLoose)) + { + Error::Type errorType = m_flavour == AsmFlavour::Loose ? *m_errorTypeForLoose : Error::Type::TypeError; + string msg = + "Top-level expressions are not supposed to return values (this expression returns " + + boost::lexical_cast(m_stackHeight - initialStackHeight) + + " value" + + (m_stackHeight - initialStackHeight == 1 ? "" : "s") + + "). Use ``pop()`` or assign them."; + m_errorReporter.error(errorType, _statement.location, msg); + if (errorType != Error::Type::Warning) success = false; + } m_info.stackHeightInfo[&_statement] = m_stackHeight; return success; } bool AsmAnalyzer::operator()(assembly::StackAssignment const& _assignment) { - solAssert(m_flavour == AsmFlavour::Loose, ""); + checkLooseFeature( + _assignment.location, + "The use of stack assignment is deprecated. Please use assignment in functional notation instead." + ); bool success = checkAssignment(_assignment.variableName, size_t(-1)); m_info.stackHeightInfo[&_assignment] = m_stackHeight; return success; @@ -577,10 +596,22 @@ void AsmAnalyzer::warnOnInstructions(solidity::Instruction _instr, SourceLocatio ); if (_instr == solidity::Instruction::JUMP || _instr == solidity::Instruction::JUMPI || _instr == solidity::Instruction::JUMPDEST) - m_errorReporter.warning( + { + solAssert(m_flavour == AsmFlavour::Loose, ""); + m_errorReporter.error( + m_errorTypeForLoose ? *m_errorTypeForLoose : Error::Type::Warning, _location, "Jump instructions and labels are low-level EVM features that can lead to " "incorrect stack access. Because of that they are discouraged. " "Please consider using \"switch\", \"if\" or \"for\" statements instead." ); + } +} + +void AsmAnalyzer::checkLooseFeature(SourceLocation const& _location, string const& _description) +{ + if (m_flavour != AsmFlavour::Loose) + solAssert(false, _description); + else if (m_errorTypeForLoose) + m_errorReporter.error(*m_errorTypeForLoose, _location, _description); } diff --git a/libsolidity/inlineasm/AsmAnalysis.h b/libsolidity/inlineasm/AsmAnalysis.h index 867711c7..8d2a71f0 100644 --- a/libsolidity/inlineasm/AsmAnalysis.h +++ b/libsolidity/inlineasm/AsmAnalysis.h @@ -30,6 +30,7 @@ #include #include +#include #include #include @@ -56,9 +57,17 @@ public: AsmAnalysisInfo& _analysisInfo, ErrorReporter& _errorReporter, EVMVersion _evmVersion, + boost::optional _errorTypeForLoose, AsmFlavour _flavour = AsmFlavour::Loose, julia::ExternalIdentifierAccess::Resolver const& _resolver = julia::ExternalIdentifierAccess::Resolver() - ): m_resolver(_resolver), m_info(_analysisInfo), m_errorReporter(_errorReporter), m_evmVersion(_evmVersion), m_flavour(_flavour) {} + ): + m_resolver(_resolver), + m_info(_analysisInfo), + m_errorReporter(_errorReporter), + m_evmVersion(_evmVersion), + m_flavour(_flavour), + m_errorTypeForLoose(_errorTypeForLoose) + {} bool analyze(assembly::Block const& _block); @@ -91,6 +100,11 @@ private: void expectValidType(std::string const& type, SourceLocation const& _location); void warnOnInstructions(solidity::Instruction _instr, SourceLocation const& _location); + /// Depending on @a m_flavour and @a m_errorTypeForLoose, throws an internal compiler + /// exception (if the flavour is not Loose), reports an error/warning + /// (if m_errorTypeForLoose is set) or does nothing. + void checkLooseFeature(SourceLocation const& _location, std::string const& _description); + int m_stackHeight = 0; julia::ExternalIdentifierAccess::Resolver m_resolver; Scope* m_currentScope = nullptr; @@ -101,6 +115,7 @@ private: ErrorReporter& m_errorReporter; EVMVersion m_evmVersion; AsmFlavour m_flavour = AsmFlavour::Loose; + boost::optional m_errorTypeForLoose; }; } -- cgit