Calldata byte arrays stored on the stack.

6 files changed, 69 insertions, 25 deletions

diff --git a/AST.cpp b/AST.cpp
index 0fafd2d1..c6d8f5c5 100644
--- a/AST.cpp
+++ b/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<VariableDeclaration> const& var: getParameters())
+		{
+			auto const& type = var->getType();
+			solAssert(!!type, "");
+			if (auto const* byteArrayType = dynamic_cast<ByteArrayType const*>(type.get()))
+				var->setType(byteArrayType->copyForLocation(ByteArrayType::Location::CallData));
+		}
 	for (ASTPointer<VariableDeclaration> const& var: getParameters() + getReturnParameters())
 		if (!var->getType()->canLiveOutsideStorage())
 			BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage."));
diff --git a/Compiler.cpp b/Compiler.cpp
index 98c9ffaa..14acc011 100644
--- a/Compiler.cpp
+++ b/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/CompilerUtils.cpp b/CompilerUtils.cpp
index 0a95c3ad..047bc6d6 100644
--- a/CompilerUtils.cpp
+++ b/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/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index 7128459a..3bf1c8c9 100644
--- a/ExpressionCompiler.cpp
+++ b/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<EnumType const*>(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<ByteArrayType const&>(*_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/Types.cpp b/Types.cpp
index 99515a3a..6149f34f 100644
--- a/Types.cpp
+++ b/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<IntegerType >(256)}});
+shared_ptr<ByteArrayType> ByteArrayType::copyForLocation(ByteArrayType::Location _location) const
+{
+	return make_shared<ByteArrayType>(_location);
+}
+
+const MemberList ByteArrayType::s_byteArrayMemberList = MemberList({{"length", make_shared<IntegerType>(256)}});
 
 bool ContractType::operator==(Type const& _other) const
 {
diff --git a/Types.h b/Types.h
index 90812f56..b66857f0 100644
--- a/Types.h
+++ b/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<ByteArrayType> copyForLocation(Location _location) const;
+
 private:
 	Location m_location;
 	static const MemberList s_byteArrayMemberList;