Merge pull request #4062 from ethereum/revert-variable

Fix revert with reason coming from a string variable

4 files changed, 45 insertions, 2 deletions

diff --git a/Changelog.md b/Changelog.md
index 6f6f672c..ce0adfd7 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -15,10 +15,10 @@ Features:
  * Type Checker: Warn about wildcard tuple assignments (this will turn into an error with version 0.5.0).
 
 Bugfixes:
+ * Code Generator: Fix ``revert`` with reason coming from a state or local string variable.
  * Type Checker: Show proper error when trying to ``emit`` a non-event.
  * Type Checker: Warn about empty tuple components (this will turn into an error with version 0.5.0).
 
-
 ### 0.4.23 (2018-04-19)
 
 Features:
diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp
index a39e799c..d9f17263 100644
--- a/libsolidity/codegen/CompilerUtils.cpp
+++ b/libsolidity/codegen/CompilerUtils.cpp
@@ -89,7 +89,6 @@ void CompilerUtils::revertWithStringData(Type const& _argumentType)
 	abiEncode({_argumentType.shared_from_this()}, {make_shared<ArrayType>(DataLocation::Memory, true)});
 	toSizeAfterFreeMemoryPointer();
 	m_context << Instruction::REVERT;
-	m_context.adjustStackOffset(_argumentType.sizeOnStack());
 }
 
 unsigned CompilerUtils::loadFromMemory(
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index a8222e21..4bcc1fa9 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -933,7 +933,11 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				// condition was not met, flag an error
 				m_context.appendInvalid();
 			else if (arguments.size() > 1)
+			{
 				utils().revertWithStringData(*arguments.at(1)->annotation().type);
+				// Here, the argument is consumed, but in the other branch, it is still there.
+				m_context.adjustStackOffset(arguments.at(1)->annotation().type->sizeOnStack());
+			}
 			else
 				m_context.appendRevert();
 			// the success branch
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 4b09d253..2db2aadd 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -10660,12 +10660,24 @@ BOOST_AUTO_TEST_CASE(revert_with_cause)
 {
 	char const* sourceCode = R"(
 		contract D {
+			string constant msg1 = "test1234567890123456789012345678901234567890";
+			string msg2 = "test1234567890123456789012345678901234567890";
 			function f() public {
 				revert("test123");
 			}
 			function g() public {
 				revert("test1234567890123456789012345678901234567890");
 			}
+			function h() public {
+				revert(msg1);
+			}
+			function i() public {
+				revert(msg2);
+			}
+			function j() public {
+				string memory msg3 = "test1234567890123456789012345678901234567890";
+				revert(msg3);
+			}
 		}
 		contract C {
 			D d = new D();
@@ -10686,6 +10698,15 @@ BOOST_AUTO_TEST_CASE(revert_with_cause)
 			function g() public returns (bool, bytes) {
 				return forward(address(d), msg.data);
 			}
+			function h() public returns (bool, bytes) {
+				return forward(address(d), msg.data);
+			}
+			function i() public returns (bool, bytes) {
+				return forward(address(d), msg.data);
+			}
+			function j() public returns (bool, bytes) {
+				return forward(address(d), msg.data);
+			}
 		}
 	)";
 	compileAndRun(sourceCode, 0, "C");
@@ -10693,6 +10714,9 @@ BOOST_AUTO_TEST_CASE(revert_with_cause)
 	bytes const errorSignature = bytes{0x08, 0xc3, 0x79, 0xa0};
 	ABI_CHECK(callContractFunction("f()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 7, "test123") + bytes(28, 0) : bytes());
 	ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes());
+	ABI_CHECK(callContractFunction("h()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes());
+	ABI_CHECK(callContractFunction("i()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes());
+	ABI_CHECK(callContractFunction("j()"), haveReturndata ? encodeArgs(0, 0x40, 0x84) + errorSignature + encodeArgs(0x20, 44, "test1234567890123456789012345678901234567890") + bytes(28, 0): bytes());
 }
 
 BOOST_AUTO_TEST_CASE(require_with_message)
@@ -10701,6 +10725,7 @@ BOOST_AUTO_TEST_CASE(require_with_message)
 		contract D {
 			bool flag = false;
 			string storageError = "abc";
+			string constant constantError = "abc";
 			function f(uint x) public {
 				require(x > 7, "failed");
 			}
@@ -10718,6 +10743,13 @@ BOOST_AUTO_TEST_CASE(require_with_message)
 			function h() public {
 				require(false, storageError);
 			}
+			function i() public {
+				require(false, constantError);
+			}
+			function j() public {
+				string memory errMsg = "msg";
+				require(false, errMsg);
+			}
 		}
 		contract C {
 			D d = new D();
@@ -10741,6 +10773,12 @@ BOOST_AUTO_TEST_CASE(require_with_message)
 			function h() public returns (bool, bytes) {
 				return forward(address(d), msg.data);
 			}
+			function i() public returns (bool, bytes) {
+				return forward(address(d), msg.data);
+			}
+			function j() public returns (bool, bytes) {
+				return forward(address(d), msg.data);
+			}
 		}
 	)";
 	compileAndRun(sourceCode, 0, "C");
@@ -10751,6 +10789,8 @@ BOOST_AUTO_TEST_CASE(require_with_message)
 	ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(1, 0x40, 0) : bytes());
 	ABI_CHECK(callContractFunction("g()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 18, "only on second run") + bytes(28, 0) : bytes());
 	ABI_CHECK(callContractFunction("h()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 3, "abc") + bytes(28, 0): bytes());
+	ABI_CHECK(callContractFunction("i()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 3, "abc") + bytes(28, 0): bytes());
+	ABI_CHECK(callContractFunction("j()"), haveReturndata ? encodeArgs(0, 0x40, 0x64) + errorSignature + encodeArgs(0x20, 3, "msg") + bytes(28, 0): bytes());
 }
 
 BOOST_AUTO_TEST_CASE(bubble_up_error_messages)