Merge pull request #2437 from ethereum/warnDoubleCopyStorage

Warn about copies in storage that might overwrite unexpectedly.

2 files changed, 106 insertions, 0 deletions

diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 823a8eda..ffee5e36 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -4483,6 +4483,38 @@ BOOST_AUTO_TEST_CASE(array_copy_including_mapping)
 	BOOST_CHECK(storageEmpty(m_contractAddress));
 }
 
+BOOST_AUTO_TEST_CASE(swap_in_storage_overwrite)
+{
+	// This tests a swap in storage which does not work as one
+	// might expect because we do not have temporary storage.
+	// (x, y) = (y, x) is the same as
+	// y = x;
+	// x = y;
+	char const* sourceCode = R"(
+		contract c {
+			struct S { uint a; uint b; }
+			S public x;
+			S public y;
+			function set() {
+				x.a = 1; x.b = 2;
+				y.a = 3; y.b = 4;
+			}
+			function swap() {
+				(x, y) = (y, x);
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(0), u256(0)));
+	BOOST_CHECK(callContractFunction("y()") == encodeArgs(u256(0), u256(0)));
+	BOOST_CHECK(callContractFunction("set()") == encodeArgs());
+	BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(1), u256(2)));
+	BOOST_CHECK(callContractFunction("y()") == encodeArgs(u256(3), u256(4)));
+	BOOST_CHECK(callContractFunction("swap()") == encodeArgs());
+	BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(1), u256(2)));
+	BOOST_CHECK(callContractFunction("y()") == encodeArgs(u256(1), u256(2)));
+}
+
 BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base)
 {
 	char const* sourceCode = R"(
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index eb1cf0dc..b489be22 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -5816,6 +5816,80 @@ BOOST_AUTO_TEST_CASE(pure_statement_check_for_regular_for_loop)
 	success(text);
 }
 
+BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies)
+{
+	char const* text = R"(
+		contract C {
+			struct S { uint a; uint b; }
+			S x; S y;
+			function f() {
+				(x, y) = (y, x);
+			}
+		}
+	)";
+	CHECK_WARNING(text, "This assignment performs two copies to storage.");
+}
+
+BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies_fill_right)
+{
+	char const* text = R"(
+		contract C {
+			struct S { uint a; uint b; }
+			S x; S y;
+			function f() {
+				(x, y, ) = (y, x, 1, 2);
+			}
+		}
+	)";
+	CHECK_WARNING(text, "This assignment performs two copies to storage.");
+}
+
+BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies_fill_left)
+{
+	char const* text = R"(
+		contract C {
+			struct S { uint a; uint b; }
+			S x; S y;
+			function f() {
+				(,x, y) = (1, 2, y, x);
+			}
+		}
+	)";
+	CHECK_WARNING(text, "This assignment performs two copies to storage.");
+}
+
+BOOST_AUTO_TEST_CASE(nowarn_swap_memory)
+{
+	char const* text = R"(
+		contract C {
+			struct S { uint a; uint b; }
+			function f() {
+				S memory x;
+				S memory y;
+				(x, y) = (y, x);
+			}
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
+BOOST_AUTO_TEST_CASE(nowarn_swap_storage_pointers)
+{
+	char const* text = R"(
+		contract C {
+			struct S { uint a; uint b; }
+			S x; S y;
+			function f() {
+				S storage x_local = x;
+				S storage y_local = y;
+				S storage z_local = x;
+				(x, y_local, x_local, z_local) = (y, x_local, y_local, y);
+			}
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
 BOOST_AUTO_TEST_CASE(warn_unused_local)
 {
 	char const* text = R"(