diff --git a/Changelog.md b/Changelog.md index 343d8aa73..39e030146 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,6 +4,7 @@ Features: * Inline assembly: support both ``suicide`` and ``selfdestruct`` opcodes (note: ``suicide`` is deprecated). + * Inline assembly: issue warning if stack is not balanced after block. * Include ``keccak256()`` as an alias to ``sha3()``. * Support shifting constant numbers. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 332ce2c30..b5955c8f0 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -718,11 +718,10 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement) { if (ref->dataStoredIn(DataLocation::Storage)) { - auto err = make_shared(Error::Type::Warning); - *err << - errinfo_sourceLocation(varDecl.location()) << - errinfo_comment("Uninitialized storage pointer. Did you mean ' memory " + varDecl.name() + "'?"); - m_errors.push_back(err); + warning( + varDecl.location(), + "Uninitialized storage pointer. Did you mean ' memory " + varDecl.name() + "'?" + ); } } varDecl.accept(*this); diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 18b42fce4..ebb84784e 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -575,7 +575,7 @@ bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly) return true; } ); - solAssert(errors.empty(), "Code generation for inline assembly with errors requested."); + solAssert(Error::containsOnlyWarnings(errors), "Code generation for inline assembly with errors requested."); m_context.setStackOffset(startStackHeight); return false; } diff --git a/libsolidity/inlineasm/AsmCodeGen.cpp b/libsolidity/inlineasm/AsmCodeGen.cpp index 53d19b0af..5d920cb75 100644 --- a/libsolidity/inlineasm/AsmCodeGen.cpp +++ b/libsolidity/inlineasm/AsmCodeGen.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -213,10 +214,31 @@ public: void operator()(assembly::Block const& _block) { size_t numVariables = m_state.variables.size(); + int deposit = m_state.assembly.deposit(); std::for_each(_block.statements.begin(), _block.statements.end(), boost::apply_visitor(*this)); - // pop variables - // we deliberately do not check stack height + deposit = m_state.assembly.deposit() - deposit; + m_state.assembly.setSourceLocation(_block.location); + + // issue warnings for stack height discrepancies + if (deposit < 0) + { + m_state.addError( + Error::Type::Warning, + "Inline assembly block is not balanced. It takes " + toString(-deposit) + " item(s) from the stack.", + _block.location + ); + } + else if (deposit > 0) + { + m_state.addError( + Error::Type::Warning, + "Inline assembly block is not balanced. It leaves " + toString(deposit) + " item(s) on the stack.", + _block.location + ); + } + + // pop variables while (m_state.variables.size() > numVariables) { m_state.assembly.append(solidity::Instruction::POP); diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp index f8655c0c9..6c04367f7 100644 --- a/test/libsolidity/InlineAssembly.cpp +++ b/test/libsolidity/InlineAssembly.cpp @@ -51,7 +51,7 @@ bool successParse(std::string const& _source, bool _assemble = false) if (_assemble) { stack.assemble(); - if (!stack.errors().empty()) + if (!stack.errors().empty() && !Error::containsOnlyWarnings(stack.errors())) return false; } } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 7eedbefae..834723695 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -4079,6 +4079,34 @@ BOOST_AUTO_TEST_CASE(shift_constant_right_excessive_rvalue) BOOST_CHECK(expectError(text, false) == Error::Type::TypeError); } +BOOST_AUTO_TEST_CASE(inline_assembly_unbalanced_positive_stack) +{ + char const* text = R"( + contract test { + function f() { + assembly { + 1 + } + } + } + )"; + BOOST_CHECK(expectError(text, true) == Error::Type::Warning); +} + +BOOST_AUTO_TEST_CASE(inline_assembly_unbalanced_negative_stack) +{ + char const* text = R"( + contract test { + function f() { + assembly { + pop + } + } + } + )"; + BOOST_CHECK(expectError(text, true) == Error::Type::Warning); +} + BOOST_AUTO_TEST_SUITE_END() }