From bed225c9810b3ebecaa3540a4731e1bffa25b2f7 Mon Sep 17 00:00:00 2001 From: Christian Date: Mon, 16 Feb 2015 17:33:13 +0100 Subject: [PATCH] Calldata byte arrays stored on the stack. --- libsolidity/AST.cpp | 9 ++++++++ libsolidity/Compiler.cpp | 17 +++++++++----- libsolidity/CompilerUtils.cpp | 34 +++++++++++++++++----------- libsolidity/ExpressionCompiler.cpp | 19 ++++++++++++---- libsolidity/Types.cpp | 11 +++++++-- libsolidity/Types.h | 4 ++++ test/SolidityEndToEndTest.cpp | 36 ++++++++++++++++++++++++++---- 7 files changed, 101 insertions(+), 29 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 0fafd2d12..c6d8f5c58 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -274,6 +274,15 @@ TypePointer FunctionDefinition::getType(ContractDefinition const*) const void FunctionDefinition::checkTypeRequirements() { + // change all byte arrays parameters to point to calldata + if (getVisibility() == Visibility::External) + for (ASTPointer const& var: getParameters()) + { + auto const& type = var->getType(); + solAssert(!!type, ""); + if (auto const* byteArrayType = dynamic_cast(type.get())) + var->setType(byteArrayType->copyForLocation(ByteArrayType::Location::CallData)); + } for (ASTPointer const& var: getParameters() + getReturnParameters()) if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 98c9ffaac..14acc0113 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -193,17 +193,23 @@ void Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool for (TypePointer const& type: _typeParameters) if (type->isDynamicallySized()) { - // value on stack: [memory_offset] (only if we are already in dynamic mode) + // value on stack: [calldata_offset] (only if we are already in dynamic mode) if (currentDynamicParameter == 0) // switch from static to dynamic m_context << u256(offset); + // retrieve length CompilerUtils(m_context).loadFromMemory( CompilerUtils::dataStartOffset + currentDynamicParameter * 32, IntegerType(256), !_fromMemory, c_padToWords); - // store new memory pointer - m_context << eth::Instruction::DUP2 << eth::Instruction::DUP2 << eth::Instruction::ADD; + // stack: offset length + // add 32-byte padding to copy of length + m_context << u256(32) << eth::Instruction::DUP1 << u256(31) + << eth::Instruction::DUP4 << eth::Instruction::ADD + << eth::Instruction::DIV << eth::Instruction::MUL; + // stack: offset length padded_length + m_context << eth::Instruction::DUP3 << eth::Instruction::ADD; currentDynamicParameter++; - // value on stack: offset length next_memory_offset + // stack: offset length next_calldata_offset } else if (currentDynamicParameter == 0) // we can still use static load @@ -294,8 +300,7 @@ bool Compiler::visit(FunctionDefinition const& _function) // Note that the fact that the return arguments are of increasing index is vital for this // algorithm to work. - unsigned const c_argumentsSize = (_function.getVisibility() == Declaration::Visibility::External - ? 0 : CompilerUtils::getSizeOnStack(_function.getParameters())); + unsigned const c_argumentsSize = CompilerUtils::getSizeOnStack(_function.getParameters()); unsigned const c_returnValuesSize = CompilerUtils::getSizeOnStack(_function.getReturnParameters()); unsigned const c_localVariablesSize = CompilerUtils::getSizeOnStack(_function.getLocalVariables()); diff --git a/libsolidity/CompilerUtils.cpp b/libsolidity/CompilerUtils.cpp index 0a95c3adb..047bc6d62 100644 --- a/libsolidity/CompilerUtils.cpp +++ b/libsolidity/CompilerUtils.cpp @@ -70,9 +70,12 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound if (type.getLocation() == ByteArrayType::Location::CallData) { - m_context << eth::Instruction::CALLDATASIZE << u256(0) << eth::Instruction::DUP3 - << eth::Instruction::CALLDATACOPY - << eth::Instruction::CALLDATASIZE << eth::Instruction::ADD; + // stack: target source_offset source_len + m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 << eth::Instruction::DUP5 + // stack: target source_offset source_len source_len source_offset target + << eth::Instruction::CALLDATACOPY + << eth::Instruction::DUP3 << eth::Instruction::ADD + << eth::Instruction::SWAP2 << eth::Instruction::POP << eth::Instruction::POP; } else { @@ -171,29 +174,32 @@ void CompilerUtils::copyByteArrayToStorage(ByteArrayType const& _targetType, { case ByteArrayType::Location::CallData: { - // @todo this does not take length into account. It also assumes that after "CALLDATALENGTH" we only have zeros. + // This also assumes that after "length" we only have zeros, i.e. it cannot be used to + // slice a byte array from calldata. + + // stack: source_offset source_len target_ref // fetch old length and convert to words m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD; m_context << u256(31) << eth::Instruction::ADD << u256(32) << eth::Instruction::SWAP1 << eth::Instruction::DIV; - // stack here: target_ref target_length_words + // stack here: source_offset source_len target_ref target_length_words // actual array data is stored at SHA3(storage_offset) m_context << eth::Instruction::DUP2; CompilerUtils(m_context).computeHashStatic(); // compute target_data_end m_context << eth::Instruction::DUP1 << eth::Instruction::SWAP2 << eth::Instruction::ADD << eth::Instruction::SWAP1; - // stack here: target_ref target_data_end target_data_ref + // stack here: source_offset source_len target_ref target_data_end target_data_ref // store length (in bytes) - m_context << eth::Instruction::CALLDATASIZE; - m_context << eth::Instruction::DUP1 << eth::Instruction::DUP5 << eth::Instruction::SSTORE; + m_context << eth::Instruction::DUP4 << eth::Instruction::DUP1 << eth::Instruction::DUP5 + << eth::Instruction::SSTORE; // jump to end if length is zero m_context << eth::Instruction::ISZERO; eth::AssemblyItem copyLoopEnd = m_context.newTag(); m_context.appendConditionalJumpTo(copyLoopEnd); // store start offset - m_context << u256(0); - // stack now: target_ref target_data_end target_data_ref calldata_offset + m_context << eth::Instruction::DUP5; + // stack now: source_offset source_len target_ref target_data_end target_data_ref calldata_offset eth::AssemblyItem copyLoopStart = m_context.newTag(); m_context << copyLoopStart // copy from calldata and store @@ -204,16 +210,18 @@ void CompilerUtils::copyByteArrayToStorage(ByteArrayType const& _targetType, // increment calldata_offset by 32 << eth::Instruction::SWAP1 << u256(32) << eth::Instruction::ADD // check for loop condition - << eth::Instruction::DUP1 << eth::Instruction::CALLDATASIZE << eth::Instruction::GT; + << eth::Instruction::DUP1 << eth::Instruction::DUP6 << eth::Instruction::GT; m_context.appendConditionalJumpTo(copyLoopStart); m_context << eth::Instruction::POP; m_context << copyLoopEnd; // now clear leftover bytes of the old value - // stack now: target_ref target_data_end target_data_ref + // stack now: source_offset source_len target_ref target_data_end target_data_ref clearStorageLoop(); + // stack now: source_offset source_len target_ref target_data_end - m_context << eth::Instruction::POP; + m_context << eth::Instruction::POP << eth::Instruction::SWAP2 + << eth::Instruction::POP << eth::Instruction::POP; break; } case ByteArrayType::Location::Storage: diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 7128459a4..3bf1c8c93 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -475,9 +475,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) else if (member == "gasprice") m_context << eth::Instruction::GASPRICE; else if (member == "data") - { - // nothing to store on the stack - } + m_context << u256(0) << eth::Instruction::CALLDATASIZE; else BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Unknown magic member.")); break; @@ -510,6 +508,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) m_context << m_context.getFunctionEntryLabel(*function).pushTag(); return; } + solAssert(false, "Function not found in member access."); } else if (auto enumType = dynamic_cast(type.getActualType().get())) m_context << enumType->getMemberValue(_memberAccess.getMemberName()); @@ -518,7 +517,19 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess) case Type::Category::ByteArray: { solAssert(member == "length", "Illegal bytearray member."); - m_context << eth::Instruction::SLOAD; + auto const& type = dynamic_cast(*_memberAccess.getExpression().getType()); + switch (type.getLocation()) + { + case ByteArrayType::Location::CallData: + m_context << eth::Instruction::SWAP1 << eth::Instruction::POP; + break; + case ByteArrayType::Location::Storage: + m_context << eth::Instruction::SLOAD; + break; + default: + solAssert(false, "Unsupported byte array location."); + break; + } break; } default: diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 99515a3ac..6149f34f8 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -540,12 +540,19 @@ bool ByteArrayType::operator==(Type const& _other) const unsigned ByteArrayType::getSizeOnStack() const { if (m_location == Location::CallData) - return 0; + // offset, length (stack top) + return 2; else + // offset return 1; } -const MemberList ByteArrayType::s_byteArrayMemberList = MemberList({{"length", make_shared(256)}}); +shared_ptr ByteArrayType::copyForLocation(ByteArrayType::Location _location) const +{ + return make_shared(_location); +} + +const MemberList ByteArrayType::s_byteArrayMemberList = MemberList({{"length", make_shared(256)}}); bool ContractType::operator==(Type const& _other) const { diff --git a/libsolidity/Types.h b/libsolidity/Types.h index 90812f564..b66857f0c 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -298,6 +298,10 @@ public: Location getLocation() const { return m_location; } + /// @returns a copy of this type with location changed to @a _location + /// @todo this might move as far up as Type later + std::shared_ptr copyForLocation(Location _location) const; + private: Location m_location; static const MemberList s_byteArrayMemberList; diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index 9899af29f..103b11269 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -2283,9 +2283,9 @@ BOOST_AUTO_TEST_CASE(bytes_from_calldata_to_memory) } )"; compileAndRun(sourceCode); - bytes calldata = bytes(61, 0x22) + bytes(12, 0x12); - sendMessage(calldata, false); - BOOST_CHECK(m_output == encodeArgs(dev::sha3(bytes{'a', 'b', 'c'} + calldata))); + bytes calldata1 = bytes(61, 0x22) + bytes(12, 0x12); + sendMessage(calldata1, false); + BOOST_CHECK(m_output == encodeArgs(dev::sha3(bytes{'a', 'b', 'c'} + calldata1))); } BOOST_AUTO_TEST_CASE(call_forward_bytes) @@ -2546,7 +2546,35 @@ BOOST_AUTO_TEST_CASE(external_function) } )"; compileAndRun(sourceCode); - BOOST_CHECK(callContractFunction("test(uint256,uint256)", 2, 3) == encodeArgs(2, 3+7)); + BOOST_CHECK(callContractFunction("test(uint256,uint256)", 2, 3) == encodeArgs(2+7, 3)); +} + +BOOST_AUTO_TEST_CASE(bytes_in_arguments) +{ + char const* sourceCode = R"( + contract c { + uint result; + function f(uint a, uint b) { result += a + b; } + function g(uint a) { result *= a; } + function test(uint a, bytes data1, bytes data2, uint b) external returns (uint r_a, uint r, uint r_b, uint l) { + r_a = a; + this.call(data1); + this.call(data2); + r = result; + r_b = b; + l = data1.length; + } + } + )"; + compileAndRun(sourceCode); + string innercalldata1 = asString(FixedHash<4>(dev::sha3("f(uint256,uint256)")).asBytes() + encodeArgs(8, 9)); + bytes calldata1 = encodeArgs(u256(innercalldata1.length()), 12, innercalldata1, 13); + string innercalldata2 = asString(FixedHash<4>(dev::sha3("g(uint256)")).asBytes() + encodeArgs(3)); + bytes calldata = encodeArgs( + u256(innercalldata1.length()), u256(innercalldata2.length()), + 12, innercalldata1, innercalldata2, 13); + BOOST_CHECK(callContractFunction("test(uint256,bytes,bytes,uint256)", calldata) + == encodeArgs(12, (8 + 9) * 3, 13, u256(innercalldata1.length()))); } BOOST_AUTO_TEST_SUITE_END()