Padding for ABI types.

6 files changed, 49 insertions, 34 deletions

diff --git a/Compiler.cpp b/Compiler.cpp
index 4e5b7f55..c7d063ea 100644
--- a/Compiler.cpp
+++ b/Compiler.cpp
@@ -95,7 +95,7 @@ void Compiler::appendConstructorCall(FunctionDefinition const& _constructor)
 	// copy constructor arguments from code to memory and then to stack, they are supplied after the actual program
 	unsigned argumentSize = 0;
 	for (ASTPointer<VariableDeclaration> const& var: _constructor.getParameters())
-		argumentSize += var->getType()->getCalldataEncodedSize();
+		argumentSize += CompilerUtils::getPaddedSize(var->getType()->getCalldataEncodedSize());
 	if (argumentSize > 0)
 	{
 		m_context << u256(argumentSize);
@@ -159,9 +159,9 @@ unsigned Compiler::appendCalldataUnpacker(FunctionDefinition const& _function, b
 			BOOST_THROW_EXCEPTION(CompilerError()
 								  << errinfo_sourceLocation(var->getLocation())
 								  << errinfo_comment("Type " + var->getType()->toString() + " not yet supported."));
-		bool leftAligned = var->getType()->getCategory() == Type::Category::STRING;
-		CompilerUtils(m_context).loadFromMemory(dataOffset, numBytes, leftAligned, !_fromMemory);
-		dataOffset += numBytes;
+		bool const leftAligned = var->getType()->getCategory() == Type::Category::STRING;
+		bool const padToWords = true;
+		dataOffset += CompilerUtils(m_context).loadFromMemory(dataOffset, numBytes, leftAligned, !_fromMemory, padToWords);
 	}
 	return dataOffset;
 }
@@ -181,10 +181,11 @@ void Compiler::appendReturnValuePacker(FunctionDefinition const& _function)
 								  << errinfo_sourceLocation(parameters[i]->getLocation())
 								  << errinfo_comment("Type " + paramType.toString() + " not yet supported."));
 		CompilerUtils(m_context).copyToStackTop(stackDepth, paramType);
+		ExpressionCompiler::appendTypeConversion(m_context, paramType, paramType, true);
 		bool const leftAligned = paramType.getCategory() == Type::Category::STRING;
-		CompilerUtils(m_context).storeInMemory(dataOffset, numBytes, leftAligned);
+		bool const padToWords = true;
+		dataOffset += CompilerUtils(m_context).storeInMemory(dataOffset, numBytes, leftAligned, padToWords);
 		stackDepth -= paramType.getSizeOnStack();
-		dataOffset += numBytes;
 	}
 	// note that the stack is not cleaned up here
 	m_context << u256(dataOffset) << u256(0) << eth::Instruction::RETURN;
diff --git a/CompilerUtils.cpp b/CompilerUtils.cpp
index a5254b42..3101c1b4 100644
--- a/CompilerUtils.cpp
+++ b/CompilerUtils.cpp
@@ -33,17 +33,21 @@ namespace solidity
 
 const unsigned int CompilerUtils::dataStartOffset = 4;
 
-void CompilerUtils::loadFromMemory(unsigned _offset, unsigned _bytes, bool _leftAligned, bool _fromCalldata)
+unsigned CompilerUtils::loadFromMemory(unsigned _offset, unsigned _bytes, bool _leftAligned,
+									   bool _fromCalldata, bool _padToWordBoundaries)
 {
 	if (_bytes == 0)
 	{
 		m_context << u256(0);
-		return;
+		return 0;
 	}
 	eth::Instruction load = _fromCalldata ? eth::Instruction::CALLDATALOAD : eth::Instruction::MLOAD;
 	solAssert(_bytes <= 32, "Memory load of more than 32 bytes requested.");
-	if (_bytes == 32)
+	if (_bytes == 32 || _padToWordBoundaries)
+	{
 		m_context << u256(_offset) << load;
+		return 32;
+	}
 	else
 	{
 		// load data and add leading or trailing zeros by dividing/multiplying depending on alignment
@@ -54,21 +58,24 @@ void CompilerUtils::loadFromMemory(unsigned _offset, unsigned _bytes, bool _left
 		m_context << u256(_offset) << load << eth::Instruction::DIV;
 		if (_leftAligned)
 			m_context << eth::Instruction::MUL;
+		return _bytes;
 	}
 }
 
-void CompilerUtils::storeInMemory(unsigned _offset, unsigned _bytes, bool _leftAligned)
+unsigned CompilerUtils::storeInMemory(unsigned _offset, unsigned _bytes, bool _leftAligned,
+									  bool _padToWordBoundaries)
 {
 	if (_bytes == 0)
 	{
 		m_context << eth::Instruction::POP;
-		return;
+		return 0;
 	}
 	solAssert(_bytes <= 32, "Memory store of more than 32 bytes requested.");
-	if (_bytes != 32 && !_leftAligned)
+	if (_bytes != 32 && !_leftAligned && !_padToWordBoundaries)
 		// shift the value accordingly before storing
 		m_context << (u256(1) << ((32 - _bytes) * 8)) << eth::Instruction::MUL;
 	m_context << u256(_offset) << eth::Instruction::MSTORE;
+	return _padToWordBoundaries ? 32 : _bytes;
 }
 
 void CompilerUtils::moveToStackVariable(VariableDeclaration const& _variable)
diff --git a/CompilerUtils.h b/CompilerUtils.h
index 6bd8d315..b5a69528 100644
--- a/CompilerUtils.h
+++ b/CompilerUtils.h
@@ -40,12 +40,23 @@ public:
 	/// @param _bytes number of bytes to load
 	/// @param _leftAligned if true, store left aligned on stack (otherwise right aligned)
 	/// @param _fromCalldata if true, load from calldata, not from memory
-	void loadFromMemory(unsigned _offset, unsigned _bytes = 32, bool _leftAligned = false, bool _fromCalldata = false);
+	/// @param _padToWordBoundaries if true, assume the data is padded to word (32 byte) boundaries
+	/// @returns the number of bytes consumed in memory (can be different from _bytes if
+	///          _padToWordBoundaries is true)
+	unsigned loadFromMemory(unsigned _offset, unsigned _bytes = 32, bool _leftAligned = false,
+							bool _fromCalldata = false, bool _padToWordBoundaries = false);
 	/// Stores data from stack in memory.
 	/// @param _offset offset in memory
 	/// @param _bytes number of bytes to store
 	/// @param _leftAligned if true, data is left aligned on stack (otherwise right aligned)
-	void storeInMemory(unsigned _offset, unsigned _bytes = 32, bool _leftAligned = false);
+	/// @param _padToWordBoundaries if true, pad the data to word (32 byte) boundaries
+	/// @returns the number of bytes written to memory (can be different from _bytes if
+	///          _padToWordBoundaries is true)
+	unsigned storeInMemory(unsigned _offset, unsigned _bytes = 32, bool _leftAligned = false,
+						   bool _padToWordBoundaries = false);
+	/// @returns _size rounded up to the next multiple of 32 (the number of bytes occupied in the
+	///          padded calldata)
+	static unsigned getPaddedSize(unsigned _size) { return ((_size + 31) / 32) * 32; }
 
 	/// Moves the value that is at the top of the stack to a stack variable.
 	void moveToStackVariable(VariableDeclaration const& _variable);
diff --git a/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index 5667098d..b12b8e67 100644
--- a/ExpressionCompiler.cpp
+++ b/ExpressionCompiler.cpp
@@ -41,11 +41,11 @@ void ExpressionCompiler::compileExpression(CompilerContext& _context, Expression
 	_expression.accept(compiler);
 }
 
-void ExpressionCompiler::appendTypeConversion(CompilerContext& _context,
-											  Type const& _typeOnStack, Type const& _targetType)
+void ExpressionCompiler::appendTypeConversion(CompilerContext& _context, Type const& _typeOnStack,
+											  Type const& _targetType, bool _cleanupNeeded)
 {
 	ExpressionCompiler compiler(_context);
-	compiler.appendTypeConversion(_typeOnStack, _targetType);
+	compiler.appendTypeConversion(_typeOnStack, _targetType, _cleanupNeeded);
 }
 
 bool ExpressionCompiler::visit(Assignment const& _assignment)
@@ -295,7 +295,6 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			FunctionCallOptions options;
 			options.bare = true;
 			options.obtainAddress = [&]() { m_context << contractAddress; };
-			options.packDensely = false;
 			appendExternalFunctionCall(function, arguments, options);
 			break;
 		}
@@ -327,15 +326,15 @@ bool ExpressionCompiler::visit(NewExpression const& _newExpression)
 	for (unsigned i = 0; i < arguments.size(); ++i)
 	{
 		arguments[i]->accept(*this);
-		appendTypeConversion(*arguments[i]->getType(), *types[i]);
+		appendTypeConversion(*arguments[i]->getType(), *types[i], true);
 		unsigned const numBytes = types[i]->getCalldataEncodedSize();
 		if (numBytes > 32)
 			BOOST_THROW_EXCEPTION(CompilerError()
 								  << errinfo_sourceLocation(arguments[i]->getLocation())
 								  << errinfo_comment("Type " + types[i]->toString() + " not yet supported."));
 		bool const leftAligned = types[i]->getCategory() == Type::Category::STRING;
-		CompilerUtils(m_context).storeInMemory(dataOffset, numBytes, leftAligned);
-		dataOffset += numBytes;
+		bool const padToWords = true;
+		dataOffset += CompilerUtils(m_context).storeInMemory(dataOffset, numBytes, leftAligned, padToWords);
 	}
 	// size, offset, endowment
 	m_context << u256(dataOffset) << u256(0) << u256(0) << eth::Instruction::CREATE;
@@ -634,22 +633,20 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio
 	{
 		_arguments[i]->accept(*this);
 		Type const& type = *_functionType.getParameterTypes()[i];
-		appendTypeConversion(*_arguments[i]->getType(), type);
-		unsigned const numBytes = _options.packDensely ? type.getCalldataEncodedSize() : 32;
+		appendTypeConversion(*_arguments[i]->getType(), type, true);
+		unsigned const numBytes = type.getCalldataEncodedSize();
 		if (numBytes == 0 || numBytes > 32)
 			BOOST_THROW_EXCEPTION(CompilerError()
 								  << errinfo_sourceLocation(_arguments[i]->getLocation())
 								  << errinfo_comment("Type " + type.toString() + " not yet supported."));
 		bool const leftAligned = type.getCategory() == Type::Category::STRING;
-		CompilerUtils(m_context).storeInMemory(dataOffset, numBytes, leftAligned);
-		dataOffset += numBytes;
+		bool const padToWords = true;
+		dataOffset += CompilerUtils(m_context).storeInMemory(dataOffset, numBytes, leftAligned, padToWords);
 	}
 	//@todo only return the first return value for now
 	Type const* firstType = _functionType.getReturnParameterTypes().empty() ? nullptr :
 							_functionType.getReturnParameterTypes().front().get();
-	unsigned retSize = firstType ? firstType->getCalldataEncodedSize() : 0;
-	if (!_options.packDensely && retSize > 0)
-		retSize = 32;
+	unsigned retSize = firstType ? CompilerUtils::getPaddedSize(firstType->getCalldataEncodedSize()) : 0;
 	// CALL arguments: outSize, outOff, inSize, inOff, value, addr, gas (stack top)
 	m_context << u256(retSize) << u256(0) << u256(dataOffset) << u256(0);
 	if (_options.obtainValue)
@@ -666,7 +663,7 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio
 	if (retSize > 0)
 	{
 		bool const leftAligned = firstType->getCategory() == Type::Category::STRING;
-		CompilerUtils(m_context).loadFromMemory(0, retSize, leftAligned);
+		CompilerUtils(m_context).loadFromMemory(0, retSize, leftAligned, false, true);
 	}
 }
 
diff --git a/ExpressionCompiler.h b/ExpressionCompiler.h
index abd7f775..98f58c85 100644
--- a/ExpressionCompiler.h
+++ b/ExpressionCompiler.h
@@ -51,7 +51,8 @@ public:
 	static void compileExpression(CompilerContext& _context, Expression const& _expression, bool _optimize = false);
 
 	/// Appends code to remove dirty higher order bits in case of an implicit promotion to a wider type.
-	static void appendTypeConversion(CompilerContext& _context, Type const& _typeOnStack, Type const& _targetType);
+	static void appendTypeConversion(CompilerContext& _context, Type const& _typeOnStack,
+									 Type const& _targetType, bool _cleanupNeeded = false);
 
 private:
 	explicit ExpressionCompiler(CompilerContext& _compilerContext, bool _optimize = false):
@@ -96,9 +97,6 @@ private:
 		std::function<void()> obtainValue;
 		/// If true, do not prepend function index to call data
 		bool bare = false;
-		/// If false, use calling convention that all arguments and return values are packed as
-		/// 32 byte values with padding.
-		bool packDensely = true;
 	};
 
 	/// Appends code to call a function of the given type with the given arguments.
diff --git a/Types.h b/Types.h
index 8c800fa0..69f69ef6 100644
--- a/Types.h
+++ b/Types.h
@@ -115,7 +115,8 @@ public:
 	virtual bool operator!=(Type const& _other) const { return !this->operator ==(_other); }
 
 	/// @returns number of bytes used by this type when encoded for CALL, or 0 if the encoding
-	/// is not a simple big-endian encoding or the type cannot be stored on the stack.
+	/// is not a simple big-endian encoding or the type cannot be stored in calldata.
+	/// Note that irrespective of this size, each calldata element is padded to a multiple of 32 bytes.
 	virtual unsigned getCalldataEncodedSize() const { return 0; }
 	/// @returns number of bytes required to hold this value in storage.
 	/// For dynamically "allocated" types, it returns the size of the statically allocated head,