Merge pull request #1375 from ethereum/inline-assembly-stack-height

Fix inline assembly stack warnings when using variables

3 files changed, 23 insertions, 12 deletions

diff --git a/Changelog.md b/Changelog.md
index 98dababd..4d68fc69 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -7,6 +7,7 @@ Features:
 
 Bugfixes:
 
+ * Inline assembly: calculate stack height warning correctly even when local variables are used.
  * Parser: disallow empty enum definitions.
  * Type checker: disallow conversion between different enum types.
  * Interface JSON: do not include trailing new line.
diff --git a/libsolidity/inlineasm/AsmCodeGen.cpp b/libsolidity/inlineasm/AsmCodeGen.cpp
index 5d920cb7..76c71048 100644
--- a/libsolidity/inlineasm/AsmCodeGen.cpp
+++ b/libsolidity/inlineasm/AsmCodeGen.cpp
@@ -216,10 +216,18 @@ public:
 		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));
-		deposit = m_state.assembly.deposit() - deposit;
+
+		// pop variables
+		while (m_state.variables.size() > numVariables)
+		{
+			m_state.assembly.append(solidity::Instruction::POP);
+			m_state.variables.pop_back();
+		}
 
 		m_state.assembly.setSourceLocation(_block.location);
 
+		deposit = m_state.assembly.deposit() - deposit;
+
 		// issue warnings for stack height discrepancies
 		if (deposit < 0)
 		{
@@ -238,12 +246,6 @@ public:
 			);
 		}
 
-		// pop variables
-		while (m_state.variables.size() > numVariables)
-		{
-			m_state.assembly.append(solidity::Instruction::POP);
-			m_state.variables.pop_back();
-		}
 	}
 
 private:
diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp
index 6c04367f..a80a44a2 100644
--- a/test/libsolidity/InlineAssembly.cpp
+++ b/test/libsolidity/InlineAssembly.cpp
@@ -41,7 +41,7 @@ namespace test
 namespace
 {
 
-bool successParse(std::string const& _source, bool _assemble = false)
+bool successParse(std::string const& _source, bool _assemble = false, bool _allowWarnings = true)
 {
 	assembly::InlineAssemblyStack stack;
 	try
@@ -51,8 +51,9 @@ bool successParse(std::string const& _source, bool _assemble = false)
 		if (_assemble)
 		{
 			stack.assemble();
-			if (!stack.errors().empty() && !Error::containsOnlyWarnings(stack.errors()))
-				return false;
+			if (!stack.errors().empty())
+				if (!_allowWarnings || !Error::containsOnlyWarnings(stack.errors()))
+					return false;
 		}
 	}
 	catch (FatalError const&)
@@ -67,9 +68,9 @@ bool successParse(std::string const& _source, bool _assemble = false)
 	return true;
 }
 
-bool successAssemble(string const& _source)
+bool successAssemble(string const& _source, bool _allowWarnings = true)
 {
-	return successParse(_source, true);
+	return successParse(_source, true, _allowWarnings);
 }
 
 }
@@ -169,6 +170,13 @@ BOOST_AUTO_TEST_CASE(magic_variables)
 	BOOST_CHECK(successAssemble("{ let ecrecover := 1 ecrecover }"));
 }
 
+BOOST_AUTO_TEST_CASE(imbalanced_stack)
+{
+	BOOST_CHECK(successAssemble("{ 1 2 mul pop }", false));
+	BOOST_CHECK(!successAssemble("{ 1 }", false));
+	BOOST_CHECK(successAssemble("{ let x := 4 7 add }", false));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }