From 46caff4597a1cafbafe0cbcc9fb7e7478dd2a0e3 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 23 Jun 2017 17:27:57 +0200 Subject: [PATCH 1/4] Test for negative stack height. --- test/libsolidity/SolidityEndToEndTest.cpp | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index ffee5e36f..da725581b 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"( From 168f64f4cb55a7055261a4c66ca54f496e96b503 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 23 Jun 2017 17:20:07 +0200 Subject: [PATCH 2/4] Fix negative stack size checks. --- Changelog.md | 1 + libevmasm/Assembly.cpp | 1 + libsolidity/codegen/ContractCompiler.cpp | 10 ++++++++++ libsolidity/codegen/ExpressionCompiler.cpp | 1 + 4 files changed, 13 insertions(+) diff --git a/Changelog.md b/Changelog.md index c12afcd20..8b44934d1 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 27199b7b2..597fdae19 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 977a2c7ce..61a90050f 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -267,12 +267,16 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac m_context << notFound; if (fallback) { + m_context.setStackOffset(0); if (!fallback->isPayable()) appendCallValueCheck(); eth::AssemblyItem returnTag = m_context.pushNewTag(); fallback->accept(*this); m_context << returnTag; + m_context.adjustStackOffset( + CompilerUtils(m_context).sizeOnStack(FunctionType(*fallback).returnParameterTypes()) - 1 + ); appendReturnValuePacker(FunctionType(*fallback).returnParameterTypes(), _contract.isLibrary()); } else @@ -285,6 +289,7 @@ 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()) @@ -295,6 +300,11 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac appendCalldataUnpacker(functionType->parameterTypes()); m_context.appendJumpTo(m_context.functionEntryLabel(functionType->declaration())); m_context << returnTag; + m_context.adjustStackOffset( + CompilerUtils(m_context).sizeOnStack(functionType->returnParameterTypes()) - + CompilerUtils(m_context).sizeOnStack(functionType->parameterTypes()) - + 1 + ); appendReturnValuePacker(functionType->returnParameterTypes(), _contract.isLibrary()); } } diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index a65549fd8..9d4024c98 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); From ef9a7b2144993e097da6bde7675abd5651bf64cc Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 27 Jun 2017 14:29:04 +0200 Subject: [PATCH 3/4] Stack adjustment and code generation for fallback function. This assumes that the fallback function does not have return parameters. --- libsolidity/codegen/ContractCompiler.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 61a90050f..b6352b394 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -271,13 +271,15 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac 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; - m_context.adjustStackOffset( - CompilerUtils(m_context).sizeOnStack(FunctionType(*fallback).returnParameterTypes()) - 1 - ); - 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(); From 6a708b0cfe245499f85f7260f7267b399c9a7fcb Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 28 Jun 2017 16:55:20 +0100 Subject: [PATCH 4/4] Document appendFunctionSelector --- libsolidity/codegen/ContractCompiler.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index b6352b394..74b07d4d8 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -297,16 +297,20 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac 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()); } }