Merge pull request #1083 from chriseth/sol_structAssigndAndDeleteWithByteArray

Fixes for assigning and deleting structs containing byte arrays.

libsolidity/CompilerContext.h

@@ -48,6 +48,7 @@ public:
 	bytes const& getCompiledContract(ContractDefinition const& _contract) const;
 
 	void adjustStackOffset(int _adjustment) { m_asm.adjustDeposit(_adjustment); }
+	unsigned getStackHeight() { solAssert(m_asm.deposit() >= 0, ""); return unsigned(m_asm.deposit()); }
 
 	bool isMagicGlobal(Declaration const* _declaration) const { return m_magicGlobals.count(_declaration) != 0; }
 	bool isLocalVariable(Declaration const* _declaration) const;

libsolidity/ExpressionCompiler.cpp

@@ -1119,9 +1119,13 @@ void ExpressionCompiler::LValue::storeValue(Type const& _sourceType, Location co
 		{
 			solAssert(_sourceType.getCategory() == m_dataType->getCategory(), "");
 			if (m_dataType->getCategory() == Type::Category::ByteArray)
+			{
 				CompilerUtils(*m_context).copyByteArrayToStorage(
 							dynamic_cast<ByteArrayType const&>(*m_dataType),
 							dynamic_cast<ByteArrayType const&>(_sourceType));
+				if (_move)
+					*m_context << eth::Instruction::POP;
+			}
 			else if (m_dataType->getCategory() == Type::Category::Struct)
 			{
 				// stack layout: source_ref target_ref
@@ -1136,12 +1140,14 @@ void ExpressionCompiler::LValue::storeValue(Type const& _sourceType, Location co
 					*m_context << structType.getStorageOffsetOfMember(member.first)
 							   << eth::Instruction::DUP3 << eth::Instruction::DUP2
 							   << eth::Instruction::ADD;
+					// stack: source_ref target_ref member_offset source_member_ref
 					LValue rightHandSide(*m_context, LValueType::Storage, memberType);
 					rightHandSide.retrieveValue(_location, true);
-					// stack: source_ref target_ref offset source_value...
+					// stack: source_ref target_ref member_offset source_value...
 					*m_context << eth::dupInstruction(2 + memberType->getSizeOnStack())
 							   << eth::dupInstruction(2 + memberType->getSizeOnStack())
 							   << eth::Instruction::ADD;
+					// stack: source_ref target_ref member_offset source_value... target_member_ref
 					LValue memberLValue(*m_context, LValueType::Storage, memberType);
 					memberLValue.storeValue(*memberType, _location, true);
 					*m_context << eth::Instruction::POP;
@@ -1189,6 +1195,23 @@ void ExpressionCompiler::LValue::setToZero(Location const& _location) const
 	case LValueType::Storage:
 		if (m_dataType->getCategory() == Type::Category::ByteArray)
 			CompilerUtils(*m_context).clearByteArray(dynamic_cast<ByteArrayType const&>(*m_dataType));
+		else if (m_dataType->getCategory() == Type::Category::Struct)
+		{
+			// stack layout: ref
+			auto const& structType = dynamic_cast<StructType const&>(*m_dataType);
+			for (auto const& member: structType.getMembers())
+			{
+				// zero each member that is not a mapping
+				TypePointer const& memberType = member.second;
+				if (memberType->getCategory() == Type::Category::Mapping)
+					continue;
+				*m_context << structType.getStorageOffsetOfMember(member.first)
+					<< eth::Instruction::DUP2 << eth::Instruction::ADD;
+				LValue memberValue(*m_context, LValueType::Storage, memberType);
+				memberValue.setToZero();
+			}
+			*m_context << eth::Instruction::POP;
+		}
 		else
 		{
 			if (m_size == 0)

libsolidity/ExpressionCompiler.h

@@ -144,7 +144,7 @@ private:
 		void retrieveValue(Location const& _location, bool _remove = false) const;
 		/// Moves a value from the stack to the lvalue. Removes the value if @a _move is true.
 		/// @a _location is the source location of the expression that caused this operation.
-		/// Stack pre: [lvalue_ref] value
+		/// Stack pre: value [lvalue_ref]
 		/// Stack post if !_move: value_of(lvalue_ref)
 		void storeValue(Type const& _sourceType, Location const& _location = Location(), bool _move = false) const;
 		/// Stores zero in the lvalue.

test/SolidityEndToEndTest.cpp

@@ -2479,6 +2479,42 @@ BOOST_AUTO_TEST_CASE(struct_copy)
 	BOOST_CHECK(callContractFunction("retrieve(uint256)", 8) == encodeArgs(0, 0, 0, 0));
 }
 
+BOOST_AUTO_TEST_CASE(struct_containing_bytes_copy_and_delete)
+{
+	char const* sourceCode = R"(
+		contract c {
+			struct Struct { uint a; bytes data; uint b; }
+			Struct data1;
+			Struct data2;
+			function set(uint _a, bytes _data, uint _b) external returns (bool) {
+				data1.a = _a;
+				data1.b = _b;
+				data1.data = _data;
+				return true;
+			}
+			function copy() returns (bool) {
+				data1 = data2;
+				return true;
+			}
+			function del() returns (bool) {
+				delete data1;
+				return true;
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	string data = "123456789012345678901234567890123";
+	BOOST_CHECK(m_state.storage(m_contractAddress).empty());
+	BOOST_CHECK(callContractFunction("set(uint256,bytes,uint256)", u256(data.length()), 12, data, 13) == encodeArgs(true));
+	BOOST_CHECK(!m_state.storage(m_contractAddress).empty());
+	BOOST_CHECK(callContractFunction("copy()") == encodeArgs(true));
+	BOOST_CHECK(m_state.storage(m_contractAddress).empty());
+	BOOST_CHECK(callContractFunction("set(uint256,bytes,uint256)", u256(data.length()), 12, data, 13) == encodeArgs(true));
+	BOOST_CHECK(!m_state.storage(m_contractAddress).empty());
+	BOOST_CHECK(callContractFunction("del()") == encodeArgs(true));
+	BOOST_CHECK(m_state.storage(m_contractAddress).empty());
+}
+
 BOOST_AUTO_TEST_CASE(struct_copy_via_local)
 {
 	char const* sourceCode = R"(