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/analysis/ReferencesResolver.cpp | 3 +- libsolidity/analysis/TypeChecker.cpp | 5 ++++ libsolidity/codegen/CompilerContext.cpp | 1 + libsolidity/inlineasm/AsmAnalysis.cpp | 45 ++++++++++++++++++++++++----- libsolidity/inlineasm/AsmAnalysis.h | 17 ++++++++++- libsolidity/interface/AssemblyStack.cpp | 2 +- 6 files changed, 63 insertions(+), 10 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index 296a39c2..f91eaf6e 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -280,7 +280,8 @@ bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly) // Will be re-generated later with correct information // We use the latest EVM version because we will re-run it anyway. assembly::AsmAnalysisInfo analysisInfo; - assembly::AsmAnalyzer(analysisInfo, errorsIgnored, EVMVersion(), assembly::AsmFlavour::Loose, resolver).analyze(_inlineAssembly.operations()); + boost::optional errorTypeForLoose = m_experimental050Mode ? Error::Type::SyntaxError : Error::Type::Warning; + assembly::AsmAnalyzer(analysisInfo, errorsIgnored, EVMVersion(), errorTypeForLoose, assembly::AsmFlavour::Loose, resolver).analyze(_inlineAssembly.operations()); return false; } diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 1748b518..5d28e776 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -894,10 +894,15 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) }; solAssert(!_inlineAssembly.annotation().analysisInfo, ""); _inlineAssembly.annotation().analysisInfo = make_shared(); + boost::optional errorTypeForLoose = + m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) ? + Error::Type::SyntaxError : + Error::Type::Warning; assembly::AsmAnalyzer analyzer( *_inlineAssembly.annotation().analysisInfo, m_errorReporter, m_evmVersion, + errorTypeForLoose, assembly::AsmFlavour::Loose, identifierAccess ); diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index ebf0213a..0bf88267 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -330,6 +330,7 @@ void CompilerContext::appendInlineAssembly( analysisInfo, errorReporter, m_evmVersion, + boost::none, assembly::AsmFlavour::Strict, identifierAccess.resolve ).analyze(*parserResult); 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; }; } diff --git a/libsolidity/interface/AssemblyStack.cpp b/libsolidity/interface/AssemblyStack.cpp index 7a9fffbf..7f97336b 100644 --- a/libsolidity/interface/AssemblyStack.cpp +++ b/libsolidity/interface/AssemblyStack.cpp @@ -91,7 +91,7 @@ bool AssemblyStack::analyze(assembly::Block const& _block, Scanner const* _scann bool AssemblyStack::analyzeParsed() { m_analysisInfo = make_shared(); - assembly::AsmAnalyzer analyzer(*m_analysisInfo, m_errorReporter, m_evmVersion, languageToAsmFlavour(m_language)); + assembly::AsmAnalyzer analyzer(*m_analysisInfo, m_errorReporter, m_evmVersion, boost::none, languageToAsmFlavour(m_language)); m_analysisSuccessful = analyzer.analyze(*m_parserResult); return m_analysisSuccessful; } -- cgit v1.2.3