Use only a single stack slot for storage references.

7 files changed, 41 insertions, 80 deletions

diff --git a/ArrayUtils.cpp b/ArrayUtils.cpp
index f13b2817..48ee5a05 100644
--- a/ArrayUtils.cpp
+++ b/ArrayUtils.cpp
@@ -37,7 +37,7 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 	// this copies source to target and also clears target if it was larger
 	// need to leave "target_ref target_byte_off" on the stack at the end
 
-	// stack layout: [source_ref] [source_byte_off] [source length] target_ref target_byte_off (top)
+	// stack layout: [source_ref] [source length] target_ref (top)
 	solAssert(_targetType.location() == DataLocation::Storage, "");
 
 	IntegerType uint256(256);
@@ -53,16 +53,11 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 	bool haveByteOffsetTarget = !directCopy && targetBaseType->getStorageBytes() <= 16;
 	unsigned byteOffsetSize = (haveByteOffsetSource ? 1 : 0) + (haveByteOffsetTarget ? 1 : 0);
 
-	// stack: source_ref [source_byte_off] [source_length] target_ref target_byte_off
+	// stack: source_ref [source_length] target_ref
 	// store target_ref
-	// arrays always start at zero byte offset, pop offset
-	m_context << eth::Instruction::POP;
 	for (unsigned i = _sourceType.getSizeOnStack(); i > 0; --i)
 		m_context << eth::swapInstruction(i);
-	// stack: target_ref source_ref [source_byte_off] [source_length]
-	if (sourceIsStorage)
-		// arrays always start at zero byte offset, pop offset
-		m_context << eth::Instruction::POP;
+	// stack: target_ref source_ref [source_length]
 	// stack: target_ref source_ref [source_length]
 	// retrieve source length
 	if (_sourceType.location() != DataLocation::CallData || !_sourceType.isDynamicallySized())
@@ -90,7 +85,6 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 		m_context
 			<< eth::Instruction::POP << eth::Instruction::POP
 			<< eth::Instruction::POP << eth::Instruction::POP;
-		m_context << u256(0);
 		return;
 	}
 	// compute hashes (data positions)
@@ -136,13 +130,11 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 		solAssert(byteOffsetSize == 0, "Byte offset for array as base type.");
 		auto const& sourceBaseArrayType = dynamic_cast<ArrayType const&>(*sourceBaseType);
 		m_context << eth::Instruction::DUP3;
-		if (sourceIsStorage)
-			m_context << u256(0);
-		else if (sourceBaseArrayType.location() == DataLocation::Memory)
+		if (sourceBaseArrayType.location() == DataLocation::Memory)
 			m_context << eth::Instruction::MLOAD;
-		m_context << eth::dupInstruction(sourceIsStorage ? 4 : 3) << u256(0);
+		m_context << eth::Instruction::DUP3;
 		copyArrayToStorage(dynamic_cast<ArrayType const&>(*targetBaseType), sourceBaseArrayType);
-		m_context << eth::Instruction::POP << eth::Instruction::POP;
+		m_context << eth::Instruction::POP;
 	}
 	else if (directCopy)
 	{
@@ -235,7 +227,6 @@ void ArrayUtils::copyArrayToStorage(ArrayType const& _targetType, ArrayType cons
 	// stack: target_ref target_data_end target_data_pos_updated
 	clearStorageLoop(*targetBaseType);
 	m_context << eth::Instruction::POP;
-	m_context << u256(0);
 }
 
 void ArrayUtils::copyArrayToMemory(const ArrayType& _sourceType, bool _padToWordBoundaries) const
@@ -365,7 +356,6 @@ void ArrayUtils::copyArrayToMemory(const ArrayType& _sourceType, bool _padToWord
 		u256 storageSize = _sourceType.getBaseType()->getStorageSize();
 		solAssert(storageSize > 1 || (storageSize == 1 && storageBytes > 0), "");
 
-		m_context << eth::Instruction::POP; // remove offset, arrays always start new slot
 		retrieveLength(_sourceType);
 		// stack here: memory_offset storage_offset length
 		// jump to end if length is zero
diff --git a/ArrayUtils.h b/ArrayUtils.h
index c047fdcc..80ffc008 100644
--- a/ArrayUtils.h
+++ b/ArrayUtils.h
@@ -41,8 +41,8 @@ public:
 
 	/// Copies an array to an array in storage. The arrays can be of different types only if
 	/// their storage representation is the same.
-	/// Stack pre: source_reference [source_byte_offset/source_length] target_reference target_byte_offset
-	/// Stack post: target_reference target_byte_offset
+	/// Stack pre: source_reference [source_length] target_reference
+	/// Stack post: target_reference
 	void copyArrayToStorage(ArrayType const& _targetType, ArrayType const& _sourceType) const;
 	/// Copies the data part of an array (which cannot be dynamically nested) from anywhere
 	/// to a given position in memory.
diff --git a/CompilerUtils.cpp b/CompilerUtils.cpp
index 297e6aa0..e62f59e2 100644
--- a/CompilerUtils.cpp
+++ b/CompilerUtils.cpp
@@ -229,7 +229,7 @@ void CompilerUtils::encodeToMemory(
 				if (arrayType.location() == DataLocation::CallData)
 					m_context << eth::Instruction::DUP2; // length is on stack
 				else if (arrayType.location() == DataLocation::Storage)
-					m_context << eth::Instruction::DUP3 << eth::Instruction::SLOAD;
+					m_context << eth::Instruction::DUP2 << eth::Instruction::SLOAD;
 				else
 				{
 					solAssert(arrayType.location() == DataLocation::Memory, "");
@@ -416,13 +416,6 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 			{
 				// stack: <source ref> (variably sized)
 				unsigned stackSize = typeOnStack.getSizeOnStack();
-				bool fromStorage = (typeOnStack.location() == DataLocation::Storage);
-				if (fromStorage)
-				{
-					stackSize--;
-					// remove storage offset, as requested by ArrayUtils::retrieveLength
-					m_context << eth::Instruction::POP;
-				}
 				ArrayUtils(m_context).retrieveLength(typeOnStack);
 
 				// allocate memory
@@ -446,8 +439,6 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 				{
 					solAssert(typeOnStack.getBaseType()->isValueType(), "");
 					copyToStackTop(2 + stackSize, stackSize);
-					if (fromStorage)
-						m_context << u256(0); // add byte offset again
 					ArrayUtils(m_context).copyArrayToMemory(typeOnStack);
 				}
 				else
@@ -462,6 +453,8 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 					copyToStackTop(3 + stackSize, stackSize);
 					copyToStackTop(2 + stackSize, 1);
 					ArrayUtils(m_context).accessIndex(typeOnStack, false);
+					if (typeOnStack.location() == DataLocation::Storage)
+						StorageItem(m_context, *typeOnStack.getBaseType()).retrieveValue(SourceLocation(), true);
 					convertType(*typeOnStack.getBaseType(), *targetType.getBaseType(), _cleanupNeeded);
 					storeInMemoryDynamic(*targetType.getBaseType(), true);
 					m_context << eth::Instruction::SWAP1 << u256(1) << eth::Instruction::ADD;
@@ -512,8 +505,7 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 			if (typeOnStack.location() != DataLocation::Memory)
 			{
 				solAssert(typeOnStack.location() == DataLocation::Storage, "");
-				// stack: <source ref> <source byte offset>
-				m_context << eth::Instruction::POP;
+				// stack: <source ref>
 				m_context << typeOnStack.memorySize();
 				allocateMemory();
 				m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2;
diff --git a/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index 05835818..d949265d 100644
--- a/ExpressionCompiler.cpp
+++ b/ExpressionCompiler.cpp
@@ -714,7 +714,6 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 		{
 		case DataLocation::Storage:
 		{
-			m_context << eth::Instruction::POP; // structs always align to new slot
 			pair<u256, unsigned> const& offsets = type.getStorageOffsetsOfMember(member);
 			m_context << offsets.first << eth::Instruction::ADD << u256(offsets.second);
 			setLValueToStorageItem(_memberAccess);
@@ -792,8 +791,6 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
 	Type const& baseType = *_indexAccess.getBaseExpression().getType();
 	if (baseType.getCategory() == Type::Category::Mapping)
 	{
-		// storage byte offset is ignored for mappings, it should be zero.
-		m_context << eth::Instruction::POP;
 		// stack: storage_base_ref
 		Type const& keyType = *dynamic_cast<MappingType const&>(baseType).getKeyType();
 		m_context << u256(0); // memory position
@@ -812,10 +809,6 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
 		ArrayType const& arrayType = dynamic_cast<ArrayType const&>(baseType);
 		solAssert(_indexAccess.getIndexExpression(), "Index expression expected.");
 
-		// remove storage byte offset
-		if (arrayType.location() == DataLocation::Storage)
-			m_context << eth::Instruction::POP;
-
 		_indexAccess.getIndexExpression()->accept(*this);
 		// stack layout: <base_ref> [<length>] <index>
 		ArrayUtils(m_context).accessIndex(arrayType);
diff --git a/LValue.cpp b/LValue.cpp
index 82701ea9..c754bbfc 100644
--- a/LValue.cpp
+++ b/LValue.cpp
@@ -152,7 +152,14 @@ void StorageItem::retrieveValue(SourceLocation const&, bool _remove) const
 {
 	// stack: storage_key storage_offset
 	if (!m_dataType.isValueType())
-		return; // no distinction between value and reference for non-value types
+	{
+		solAssert(m_dataType.getSizeOnStack() == 1, "Invalid storage ref size.");
+		if (_remove)
+			m_context << eth::Instruction::POP; // remove byte offset
+		else
+			m_context << eth::Instruction::DUP2;
+		return;
+	}
 	if (!_remove)
 		CompilerUtils(m_context).copyToStackTop(sizeOnStack(), sizeOnStack());
 	if (m_dataType.getStorageBytes() == 32)
@@ -236,16 +243,18 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc
 			"Wrong type conversation for assignment.");
 		if (m_dataType.getCategory() == Type::Category::Array)
 		{
+			m_context << eth::Instruction::POP; // remove byte offset
 			ArrayUtils(m_context).copyArrayToStorage(
 						dynamic_cast<ArrayType const&>(m_dataType),
 						dynamic_cast<ArrayType const&>(_sourceType));
 			if (_move)
-				utils.popStackElement(m_dataType);
+				m_context << eth::Instruction::POP;
 		}
 		else if (m_dataType.getCategory() == Type::Category::Struct)
 		{
-			// stack layout: source_ref [source_offset] target_ref target_offset
-			// note that we have structs, so offsets should be zero and are ignored
+			// stack layout: source_ref target_ref target_offset
+			// note that we have structs, so offset should be zero and are ignored
+			m_context << eth::Instruction::POP;
 			auto const& structType = dynamic_cast<StructType const&>(m_dataType);
 			auto const& sourceType = dynamic_cast<StructType const&>(_sourceType);
 			solAssert(
@@ -262,44 +271,37 @@ void StorageItem::storeValue(Type const& _sourceType, SourceLocation const& _loc
 				TypePointer sourceMemberType = sourceType.getMemberType(member.name);
 				if (sourceType.location() == DataLocation::Storage)
 				{
-					// stack layout: source_ref source_offset target_ref target_offset
+					// stack layout: source_ref target_ref
 					pair<u256, unsigned> const& offsets = sourceType.getStorageOffsetsOfMember(member.name);
-					m_context << offsets.first << eth::Instruction::DUP5 << eth::Instruction::ADD;
+					m_context << offsets.first << eth::Instruction::DUP3 << eth::Instruction::ADD;
 					m_context << u256(offsets.second);
-					// stack: source_ref source_off target_ref target_off source_member_ref source_member_off
+					// stack: source_ref target_ref source_member_ref source_member_off
 					StorageItem(m_context, *sourceMemberType).retrieveValue(_location, true);
-					// stack: source_ref source_off target_ref target_off source_value...
+					// stack: source_ref target_ref source_value...
 				}
 				else
 				{
 					solAssert(sourceType.location() == DataLocation::Memory, "");
-					// stack layout: source_ref target_ref target_offset
+					// stack layout: source_ref target_ref
 					TypePointer sourceMemberType = sourceType.getMemberType(member.name);
 					m_context << sourceType.memoryOffsetOfMember(member.name);
-					m_context << eth::Instruction::DUP4 << eth::Instruction::ADD;
+					m_context << eth::Instruction::DUP3 << eth::Instruction::ADD;
 					MemoryItem(m_context, *sourceMemberType).retrieveValue(_location, true);
-					// stack layout: source_ref target_ref target_offset source_value...
+					// stack layout: source_ref target_ref source_value...
 				}
 				unsigned stackSize = sourceMemberType->getSizeOnStack();
 				pair<u256, unsigned> const& offsets = structType.getStorageOffsetsOfMember(member.name);
-				m_context << eth::dupInstruction(2 + stackSize) << offsets.first << eth::Instruction::ADD;
+				m_context << eth::dupInstruction(1 + stackSize) << offsets.first << eth::Instruction::ADD;
 				m_context << u256(offsets.second);
-				// stack: source_ref [source_off] target_ref target_off source_value... target_member_ref target_member_byte_off
+				// stack: source_ref target_ref target_off source_value... target_member_ref target_member_byte_off
 				StorageItem(m_context, *memberType).storeValue(*sourceMemberType, _location, true);
 			}
-			// stack layout: source_ref [source_offset] target_ref target_offset
-			unsigned sourceStackSize = sourceType.getSizeOnStack();
+			// stack layout: source_ref target_ref
+			solAssert(sourceType.getSizeOnStack() == 1, "Unexpected source size.");
 			if (_move)
-				utils.popStackSlots(2 + sourceType.getSizeOnStack());
-			else if (sourceType.getSizeOnStack() >= 1)
-			{
-				// remove the source ref
-				solAssert(sourceStackSize <= 2, "Invalid stack size.");
-				m_context << eth::swapInstruction(sourceStackSize);
-				if (sourceStackSize == 2)
-					m_context << eth::Instruction::POP;
-				m_context << eth::Instruction::SWAP2 << eth::Instruction::POP;
-			}
+				utils.popStackSlots(2);
+			else
+				m_context << eth::Instruction::SWAP1 << eth::Instruction::POP;
 		}
 		else
 			BOOST_THROW_EXCEPTION(
@@ -429,8 +431,6 @@ StorageArrayLength::StorageArrayLength(CompilerContext& _compilerContext, const
 	m_arrayType(_arrayType)
 {
 	solAssert(m_arrayType.isDynamicallySized(), "");
-	// storage byte offset must be zero
-	m_context << eth::Instruction::POP;
 }
 
 void StorageArrayLength::retrieveValue(SourceLocation const&, bool _remove) const
diff --git a/Types.cpp b/Types.cpp
index 3ea9caa7..0ecb01d2 100644
--- a/Types.cpp
+++ b/Types.cpp
@@ -823,11 +823,9 @@ unsigned ArrayType::getSizeOnStack() const
 	if (m_location == DataLocation::CallData)
 		// offset [length] (stack top)
 		return 1 + (isDynamicallySized() ? 1 : 0);
-	else if (m_location == DataLocation::Storage)
-		// storage_key storage_offset
-		return 2;
 	else
-		// offset
+		// storage slot or memory offset
+		// byte offset inside storage value is omitted
 		return 1;
 }
 
@@ -1035,14 +1033,6 @@ bool StructType::canLiveOutsideStorage() const
 	return true;
 }
 
-unsigned StructType::getSizeOnStack() const
-{
-	if (location() == DataLocation::Storage)
-		return 2; // slot and offset
-	else
-		return 1;
-}
-
 string StructType::toString(bool _short) const
 {
 	string ret = "struct " + m_struct.getName();
diff --git a/Types.h b/Types.h
index e17a262c..e0f8785a 100644
--- a/Types.h
+++ b/Types.h
@@ -298,7 +298,6 @@ public:
 
 	virtual bool canBeStored() const override { return false; }
 	virtual bool canLiveOutsideStorage() const override { return false; }
-	virtual unsigned getSizeOnStack() const override { return 1; }
 
 	virtual std::string toString(bool _short) const override;
 	virtual u256 literalValue(Literal const* _literal) const override;
@@ -580,7 +579,6 @@ public:
 	u256 memorySize() const;
 	virtual u256 getStorageSize() const override;
 	virtual bool canLiveOutsideStorage() const override;
-	virtual unsigned getSizeOnStack() const override;
 	virtual std::string toString(bool _short) const override;
 
 	virtual MemberList const& getMembers() const override;
@@ -616,7 +614,6 @@ public:
 	{
 		return externalType()->getCalldataEncodedSize(_padded);
 	}
-	virtual unsigned getSizeOnStack() const override { return 1; }
 	virtual unsigned getStorageBytes() const override;
 	virtual bool canLiveOutsideStorage() const override { return true; }
 	virtual std::string toString(bool _short) const override;
@@ -812,7 +809,6 @@ public:
 
 	virtual bool operator==(Type const& _other) const override;
 	virtual std::string toString(bool _short) const override;
-	virtual unsigned getSizeOnStack() const override { return 2; }
 	virtual bool canLiveOutsideStorage() const override { return false; }
 
 	TypePointer const& getKeyType() const { return m_keyType; }