From e480c7012c7cb9cfe08b6bce96a538accd9341ad Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 21 Apr 2015 10:59:48 +0200 Subject: [PATCH 1/2] bytes parameters for events and sha3. --- libsolidity/AST.cpp | 11 +-- libsolidity/CompilerUtils.cpp | 7 ++ libsolidity/CompilerUtils.h | 2 + libsolidity/ExpressionCompiler.cpp | 91 ++++++++++++++++++++--- libsolidity/ExpressionCompiler.h | 18 +++-- libsolidity/Types.cpp | 6 +- libsolidity/Types.h | 6 +- test/libsolidity/SolidityEndToEndTest.cpp | 43 +++++++++++ 8 files changed, 154 insertions(+), 30 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 78b83d064..59a7b61c2 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -450,14 +450,11 @@ void FunctionDefinition::checkTypeRequirements() { if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); + // todo delete when will be implemented arrays as parameter type in internal functions + if (getVisibility() == Visibility::Public && var->getType()->getCategory() == Type::Category::Array) + BOOST_THROW_EXCEPTION(var->createTypeError("Arrays only implemented for external functions.")); if (getVisibility() >= Visibility::Public && !(var->getType()->externalType())) - { - // todo delete when will be implemented arrays as parameter type in internal functions - if (getVisibility() == Visibility::Public && var->getType()->getCategory() == Type::Category::Array) - BOOST_THROW_EXCEPTION(var->createTypeError("Arrays only implemented for external functions.")); - else - BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed for public and external functions.")); - } + BOOST_THROW_EXCEPTION(var->createTypeError("Internal type is not allowed for public and external functions.")); } for (ASTPointer const& modifier: m_functionModifiers) modifier->checkTypeRequirements(isConstructor() ? diff --git a/libsolidity/CompilerUtils.cpp b/libsolidity/CompilerUtils.cpp index 8d3e9d2a2..07bc3cdab 100644 --- a/libsolidity/CompilerUtils.cpp +++ b/libsolidity/CompilerUtils.cpp @@ -155,6 +155,13 @@ void CompilerUtils::copyToStackTop(unsigned _stackDepth, unsigned _itemSize) m_context << eth::dupInstruction(_stackDepth); } +void CompilerUtils::moveToStackTop(unsigned _stackDepth) +{ + solAssert(_stackDepth <= 15, "Stack too deep."); + for (unsigned i = 0; i < _stackDepth; ++i) + m_context << eth::swapInstruction(1 + i); +} + void CompilerUtils::popStackElement(Type const& _type) { popStackSlots(_type.getSizeOnStack()); diff --git a/libsolidity/CompilerUtils.h b/libsolidity/CompilerUtils.h index 5b809beac..45f53e12e 100644 --- a/libsolidity/CompilerUtils.h +++ b/libsolidity/CompilerUtils.h @@ -77,6 +77,8 @@ public: /// Copies an item that occupies @a _itemSize stack slots from a stack depth of @a _stackDepth /// to the top of the stack. void copyToStackTop(unsigned _stackDepth, unsigned _itemSize); + /// Moves a single stack element (with _stackDepth items on top of it) to the top of the stack. + void moveToStackTop(unsigned _stackDepth); /// Removes the current value from the top of the stack. void popStackElement(Type const& _type); /// Removes element from the top of the stack _amount times. diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index cf6a01ec1..a11c99443 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -532,7 +532,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) case Location::SHA3: { m_context << u256(0); - appendArgumentsCopyToMemory(arguments, TypePointers(), function.padArguments()); + appendArgumentsCopyToMemory(arguments, TypePointers(), function.padArguments(), false, true); m_context << u256(0) << eth::Instruction::SHA3; break; } @@ -575,9 +575,15 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) solAssert(numIndexed <= 4, "Too many indexed arguments."); // Copy all non-indexed arguments to memory (data) m_context << u256(0); + vector> nonIndexedArgs; + TypePointers nonIndexedTypes; for (unsigned arg = 0; arg < arguments.size(); ++arg) if (!event.getParameters()[arg]->isIndexed()) - appendExpressionCopyToMemory(*function.getParameterTypes()[arg], *arguments[arg]); + { + nonIndexedArgs.push_back(arguments[arg]); + nonIndexedTypes.push_back(function.getParameterTypes()[arg]); + } + appendArgumentsCopyToMemory(nonIndexedArgs, nonIndexedTypes); m_context << u256(0) << eth::logInstruction(numIndexed); break; } @@ -1046,8 +1052,14 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio // For bare call, activate "4 byte pad exception": If the first argument has exactly 4 bytes, // do not pad it to 32 bytes. - appendArgumentsCopyToMemory(_arguments, _functionType.getParameterTypes(), - _functionType.padArguments(), bare); + // If the function takes arbitrary parameters, copy dynamic length data in place. + appendArgumentsCopyToMemory( + _arguments, + _functionType.getParameterTypes(), + _functionType.padArguments(), + bare, + _functionType.takesArbitraryParameters() + ); // CALL arguments: outSize, outOff, inSize, (already present up to here) // inOff, value, addr, gas (stack top) @@ -1089,20 +1101,72 @@ void ExpressionCompiler::appendArgumentsCopyToMemory( vector> const& _arguments, TypePointers const& _types, bool _padToWordBoundaries, - bool _padExceptionIfFourBytes + bool _padExceptionIfFourBytes, + bool _copyDynamicDataInPlace ) { solAssert(_types.empty() || _types.size() == _arguments.size(), ""); + TypePointers types = _types; + if (_types.empty()) + for (ASTPointer const& argument: _arguments) + types.push_back(argument->getType()->getRealType()); + + vector dynamicArguments; + unsigned stackSizeOfDynamicTypes = 0; for (size_t i = 0; i < _arguments.size(); ++i) { _arguments[i]->accept(*this); - TypePointer const& expectedType = _types.empty() ? _arguments[i]->getType()->getRealType() : _types[i]; - appendTypeConversion(*_arguments[i]->getType(), *expectedType, true); + TypePointer argType = types[i]->externalType(); + solAssert(!!argType, "Externalable type expected."); + if (argType->isValueType()) + appendTypeConversion(*_arguments[i]->getType(), *argType, true); + else + argType = _arguments[i]->getType()->getRealType()->externalType(); + solAssert(!!argType, "Externalable type expected."); bool pad = _padToWordBoundaries; // Do not pad if the first argument has exactly four bytes - if (i == 0 && pad && _padExceptionIfFourBytes && expectedType->getCalldataEncodedSize(false) == 4) + if (i == 0 && pad && _padExceptionIfFourBytes && argType->getCalldataEncodedSize(false) == 4) pad = false; - appendTypeMoveToMemory(*expectedType, pad); + if (!_copyDynamicDataInPlace && argType->isDynamicallySized()) + { + solAssert(argType->getCategory() == Type::Category::Array, "Unknown dynamic type."); + auto const& arrayType = dynamic_cast(*_arguments[i]->getType()); + // move memory reference to top of stack + CompilerUtils(m_context).moveToStackTop(arrayType.getSizeOnStack()); + if (arrayType.getLocation() == ArrayType::Location::CallData) + m_context << eth::Instruction::DUP2; // length is on stack + else if (arrayType.getLocation() == ArrayType::Location::Storage) + m_context << eth::Instruction::DUP3 << eth::Instruction::SLOAD; + else + { + solAssert(arrayType.getLocation() == ArrayType::Location::Memory, ""); + m_context << eth::Instruction::DUP2 << eth::Instruction::MLOAD; + } + appendTypeMoveToMemory(IntegerType(256), true); + stackSizeOfDynamicTypes += arrayType.getSizeOnStack(); + dynamicArguments.push_back(i); + } + else + appendTypeMoveToMemory(*argType, pad); + } + + // copy dynamic values to memory + unsigned dynStackPointer = stackSizeOfDynamicTypes; + // stack layout: ... + for (size_t i: dynamicArguments) + { + auto const& arrayType = dynamic_cast(*_arguments[i]->getType()); + CompilerUtils(m_context).copyToStackTop(1 + dynStackPointer, arrayType.getSizeOnStack()); + dynStackPointer -= arrayType.getSizeOnStack(); + appendTypeMoveToMemory(arrayType, true); + } + solAssert(dynStackPointer == 0, ""); + + // remove dynamic values (and retain memory pointer) + if (stackSizeOfDynamicTypes > 0) + { + m_context << eth::swapInstruction(stackSizeOfDynamicTypes); + CompilerUtils(m_context).popStackSlots(stackSizeOfDynamicTypes); } } @@ -1114,8 +1178,13 @@ void ExpressionCompiler::appendTypeMoveToMemory(Type const& _type, bool _padToWo void ExpressionCompiler::appendExpressionCopyToMemory(Type const& _expectedType, Expression const& _expression) { _expression.accept(*this); - appendTypeConversion(*_expression.getType(), _expectedType, true); - appendTypeMoveToMemory(_expectedType); + if (_expectedType.isValueType()) + { + appendTypeConversion(*_expression.getType(), _expectedType, true); + appendTypeMoveToMemory(_expectedType); + } + else + appendTypeMoveToMemory(*_expression.getType()->getRealType()); } void ExpressionCompiler::setLValueFromDeclaration(Declaration const& _declaration, Expression const& _expression) diff --git a/libsolidity/ExpressionCompiler.h b/libsolidity/ExpressionCompiler.h index 2577d21b5..35526662a 100644 --- a/libsolidity/ExpressionCompiler.h +++ b/libsolidity/ExpressionCompiler.h @@ -100,12 +100,18 @@ private: /// Appends code to call a function of the given type with the given arguments. void appendExternalFunctionCall(FunctionType const& _functionType, std::vector> const& _arguments, bool bare = false); - /// Appends code that evaluates the given arguments and moves the result to memory. The memory offset is - /// expected to be on the stack and is updated by this call. - void appendArgumentsCopyToMemory(std::vector> const& _arguments, - TypePointers const& _types = {}, - bool _padToWordBoundaries = true, - bool _padExceptionIfFourBytes = false); + /// Appends code that evaluates the given arguments and moves the result to memory encoded as + /// specified by the ABI. The memory offset is expected to be on the stack and is updated by + /// this call. If @a _padToWordBoundaries is set to false, all values are concatenated without + /// padding. If @a _copyDynamicDataInPlace is set, dynamic types is stored (without length) + /// together with fixed-length data. + void appendArgumentsCopyToMemory( + std::vector> const& _arguments, + TypePointers const& _types = {}, + bool _padToWordBoundaries = true, + bool _padExceptionIfFourBytes = false, + bool _copyDynamicDataInPlace = false + ); /// Appends code that moves a stack element of the given type to memory. The memory offset is /// expected below the stack element and is updated by this call. void appendTypeMoveToMemory(Type const& _type, bool _padToWordBoundaries = true); diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index a445d56e1..19bc134e1 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -745,8 +745,6 @@ string ArrayType::toString() const TypePointer ArrayType::externalType() const { - if (m_location != Location::CallData) - return TypePointer(); if (m_isByteArray) return shared_from_this(); if (!m_baseType->externalType()) @@ -1218,7 +1216,9 @@ string FunctionType::externalSignature(std::string const& _name) const } string ret = funcName + "("; - TypePointers externalParameterTypes = externalFunctionType()->getParameterTypes(); + FunctionTypePointer external = externalFunctionType(); + solAssert(!!external, "External function type requested."); + TypePointers externalParameterTypes = external->getParameterTypes(); for (auto it = externalParameterTypes.cbegin(); it != externalParameterTypes.cend(); ++it) { solAssert(!!(*it), "Parameter should have external type"); diff --git a/libsolidity/Types.h b/libsolidity/Types.h index ab41d4d4d..cc9c455f2 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -430,7 +430,7 @@ public: virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override; virtual TypePointer unaryOperatorResult(Token::Value _operator) const override; virtual bool operator==(Type const& _other) const override; - virtual unsigned getCalldataEncodedSize(bool _padded = true) const override + virtual unsigned getCalldataEncodedSize(bool _padded ) const override { return externalType()->getCalldataEncodedSize(_padded); } @@ -506,7 +506,7 @@ public: explicit EnumType(EnumDefinition const& _enum): m_enum(_enum) {} virtual TypePointer unaryOperatorResult(Token::Value _operator) const override; virtual bool operator==(Type const& _other) const override; - virtual unsigned getCalldataEncodedSize(bool _padded = true) const override + virtual unsigned getCalldataEncodedSize(bool _padded) const override { return externalType()->getCalldataEncodedSize(_padded); } @@ -558,7 +558,7 @@ public: /// appropriate external types of input/return parameters of current function. /// Returns an empty shared pointer if one of the input/return parameters does not have an /// external type. - virtual FunctionTypePointer externalFunctionType() const; + FunctionTypePointer externalFunctionType() const; virtual TypePointer externalType() const override { return externalFunctionType(); } explicit FunctionType(FunctionDefinition const& _function, bool _isInternal = true); diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 8b926d6cf..d6a240f0e 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2340,6 +2340,49 @@ BOOST_AUTO_TEST_CASE(event_lots_of_data) BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::sha3(string("Deposit(address,bytes32,uint256,bool)"))); } +BOOST_AUTO_TEST_CASE(event_really_lots_of_data) +{ + char const* sourceCode = R"( + contract ClientReceipt { + event Deposit(uint fixeda, bytes dynx, uint fixedb); + function deposit() { + Deposit(10, msg.data, 15); + } + } + )"; + compileAndRun(sourceCode); + callContractFunction("deposit()"); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(10, 4, 15) + FixedHash<4>(dev::sha3("deposit()")).asBytes()); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::sha3(string("Deposit(uint256,bytes,uint256)"))); +} + +BOOST_AUTO_TEST_CASE(event_really_lots_of_data_from_storage) +{ + char const* sourceCode = R"( + contract ClientReceipt { + bytes x; + event Deposit(uint fixeda, bytes dynx, uint fixedb); + function deposit() { + x.length = 3; + x[0] = "A"; + x[1] = "B"; + x[2] = "C"; + Deposit(10, x, 15); + } + } + )"; + compileAndRun(sourceCode); + callContractFunction("deposit()"); + BOOST_REQUIRE_EQUAL(m_logs.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); + BOOST_CHECK(m_logs[0].data == encodeArgs(10, 4, 15) + asBytes("ABC")); + BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); + BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::sha3(string("Deposit(uint256,bytes,uint256)"))); +} + BOOST_AUTO_TEST_CASE(empty_name_input_parameter_with_named_one) { char const* sourceCode = R"( From 0a956345433c174cc087c53c73f6c75b29bb1877 Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 21 Apr 2015 20:09:20 +0200 Subject: [PATCH 2/2] Fix regarding memory overwrite during sha3 computation. --- libsolidity/ExpressionCompiler.cpp | 13 +++++++++---- test/libsolidity/SolidityEndToEndTest.cpp | 20 +++++++++++++++++++- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index a11c99443..8c07fbd19 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -531,9 +531,12 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) break; case Location::SHA3: { - m_context << u256(0); + // we might compute a sha as part of argumentsAppendCopyToMemory, this is only a hack + // and should be removed once we have a real free memory pointer + m_context << u256(0x40); appendArgumentsCopyToMemory(arguments, TypePointers(), function.padArguments(), false, true); - m_context << u256(0) << eth::Instruction::SHA3; + m_context << u256(0x40) << eth::Instruction::SWAP1 << eth::Instruction::SUB; + m_context << u256(0x40) << eth::Instruction::SHA3; break; } case Location::Log0: @@ -574,7 +577,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) } solAssert(numIndexed <= 4, "Too many indexed arguments."); // Copy all non-indexed arguments to memory (data) - m_context << u256(0); + // Memory position is only a hack and should be removed once we have free memory pointer. + m_context << u256(0x40); vector> nonIndexedArgs; TypePointers nonIndexedTypes; for (unsigned arg = 0; arg < arguments.size(); ++arg) @@ -584,7 +588,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall) nonIndexedTypes.push_back(function.getParameterTypes()[arg]); } appendArgumentsCopyToMemory(nonIndexedArgs, nonIndexedTypes); - m_context << u256(0) << eth::logInstruction(numIndexed); + m_context << u256(0x40) << eth::Instruction::SWAP1 << eth::Instruction::SUB; + m_context << u256(0x40) << eth::logInstruction(numIndexed); break; } case Location::BlockHash: diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index d6a240f0e..596f710b9 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2378,7 +2378,7 @@ BOOST_AUTO_TEST_CASE(event_really_lots_of_data_from_storage) callContractFunction("deposit()"); BOOST_REQUIRE_EQUAL(m_logs.size(), 1); BOOST_CHECK_EQUAL(m_logs[0].address, m_contractAddress); - BOOST_CHECK(m_logs[0].data == encodeArgs(10, 4, 15) + asBytes("ABC")); + BOOST_CHECK(m_logs[0].data == encodeArgs(10, 3, 15) + asBytes("ABC")); BOOST_REQUIRE_EQUAL(m_logs[0].topics.size(), 1); BOOST_CHECK_EQUAL(m_logs[0].topics[0], dev::sha3(string("Deposit(uint256,bytes,uint256)"))); } @@ -2471,6 +2471,24 @@ BOOST_AUTO_TEST_CASE(sha3_multiple_arguments_with_string_literals) bytes{0x66, 0x6f, 0x6f}))); } +BOOST_AUTO_TEST_CASE(sha3_with_bytes) +{ + char const* sourceCode = R"( + contract c { + bytes data; + function foo() returns (bool) + { + data.length = 3; + data[0] = "f"; + data[1] = "o"; + data[2] = "o"; + return sha3(data) == sha3("foo"); + } + })"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("foo()") == encodeArgs(true)); +} + BOOST_AUTO_TEST_CASE(generic_call) { char const* sourceCode = R"**(