Merge pull request #2457 from ethereum/fixNegativeStackHeight

Fix negative stack height

5 files changed, 41 insertions, 1 deletions

diff --git a/Changelog.md b/Changelog.md
index c12afcd2..8b44934d 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -28,6 +28,7 @@ Bugfixes:
  * Fixed crash concerning non-callable types.
  * Unused variable warnings no longer issued for variables used inside inline assembly.
  * Code Generator: Fix ABI encoding of empty literal string.
+ * Code Generator: Fix negative stack size checks.
  * Inline Assembly: Enforce function arguments when parsing functional instructions.
  * Fixed segfault with constant function parameters
 
diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp
index 27199b7b..597fdae1 100644
--- a/libevmasm/Assembly.cpp
+++ b/libevmasm/Assembly.cpp
@@ -328,6 +328,7 @@ Json::Value Assembly::stream(ostream& _out, string const& _prefix, StringMap con
 
 AssemblyItem const& Assembly::append(AssemblyItem const& _i)
 {
+	assertThrow(m_deposit >= 0, AssemblyException, "");
 	m_deposit += _i.deposit();
 	m_items.push_back(_i);
 	if (m_items.back().location().isEmpty() && !m_currentSourceLocation.isEmpty())
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index 977a2c7c..74b07d4d 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -267,13 +267,19 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 	m_context << notFound;
 	if (fallback)
 	{
+		m_context.setStackOffset(0);
 		if (!fallback->isPayable())
 			appendCallValueCheck();
 
+		// Return tag is used to jump out of the function.
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
 		fallback->accept(*this);
 		m_context << returnTag;
-		appendReturnValuePacker(FunctionType(*fallback).returnParameterTypes(), _contract.isLibrary());
+		solAssert(FunctionType(*fallback).parameterTypes().empty(), "");
+		solAssert(FunctionType(*fallback).returnParameterTypes().empty(), "");
+		// Return tag gets consumed.
+		m_context.adjustStackOffset(-1);
+		m_context << Instruction::STOP;
 	}
 	else
 		m_context.appendRevert();
@@ -285,16 +291,26 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 		CompilerContext::LocationSetter locationSetter(m_context, functionType->declaration());
 
 		m_context << callDataUnpackerEntryPoints.at(it.first);
+		m_context.setStackOffset(0);
 		// We have to allow this for libraries, because value of the previous
 		// call is still visible in the delegatecall.
 		if (!functionType->isPayable() && !_contract.isLibrary())
 			appendCallValueCheck();
 
+		// Return tag is used to jump out of the function.
 		eth::AssemblyItem returnTag = m_context.pushNewTag();
+		// Parameter for calldataUnpacker
 		m_context << CompilerUtils::dataStartOffset;
 		appendCalldataUnpacker(functionType->parameterTypes());
 		m_context.appendJumpTo(m_context.functionEntryLabel(functionType->declaration()));
 		m_context << returnTag;
+		// Return tag and input parameters get consumed.
+		m_context.adjustStackOffset(
+			CompilerUtils(m_context).sizeOnStack(functionType->returnParameterTypes()) -
+			CompilerUtils(m_context).sizeOnStack(functionType->parameterTypes()) -
+			1
+		);
+		// Consumes the return parameters.
 		appendReturnValuePacker(functionType->returnParameterTypes(), _contract.isLibrary());
 	}
 }
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index a65549fd..9d4024c9 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -88,6 +88,7 @@ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const&
 	FunctionType accessorType(_varDecl);
 
 	TypePointers paramTypes = accessorType.parameterTypes();
+	m_context.adjustStackOffset(1 + CompilerUtils::sizeOnStack(paramTypes));
 
 	// retrieve the position of the variable
 	auto const& location = m_context.storageLocationOfVariable(_varDecl);
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index ffee5e36..da725581 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -9501,6 +9501,27 @@ BOOST_AUTO_TEST_CASE(revert)
 	BOOST_CHECK(callContractFunction("a()") == encodeArgs(u256(42)));
 }
 
+BOOST_AUTO_TEST_CASE(negative_stack_height)
+{
+	// This code was causing negative stack height during code generation
+	// because the stack height was not adjusted at the beginning of functions.
+	char const* sourceCode = R"(
+		contract C {
+			mapping(uint => Invoice) public invoices;
+			struct Invoice {
+				uint AID;
+				bool Aboola;
+				bool Aboolc;
+				bool exists;
+			}
+			function nredit(uint startindex) public constant returns(uint[500] CIDs, uint[500] dates, uint[500] RIDs, bool[500] Cboolas, uint[500] amounts){}
+			function return500InvoicesByDates(uint begindate, uint enddate, uint startindex) public constant returns(uint[500] AIDs, bool[500] Aboolas, uint[500] dates, bytes32[3][500] Abytesas, bytes32[3][500] bytesbs, bytes32[2][500] bytescs, uint[500] amounts, bool[500] Aboolbs, bool[500] Aboolcs){}
+			function return500PaymentsByDates(uint begindate, uint enddate, uint startindex) public constant returns(uint[500] BIDs, uint[500] dates, uint[500] RIDs, bool[500] Bboolas, bytes32[3][500] bytesbs,bytes32[2][500] bytescs, uint[500] amounts, bool[500] Bboolbs){}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+}
+
 BOOST_AUTO_TEST_CASE(literal_empty_string)
 {
 	char const* sourceCode = R"(