Merge pull request #2679 from ethereum/delegatecall

.delegatecall() should always return a boolean of execution status

6 files changed, 97 insertions, 4 deletions

diff --git a/Changelog.md b/Changelog.md
index 7e4606bd..4b3f2ddc 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -3,6 +3,7 @@
 Features:
 
 Bugfixes:
+ * Code Generator: ``.delegatecall()`` should always return execution outcome.
  * Code Generator: Provide "new account gas" for low-level ``callcode`` and ``delegatecall``.
 
 ### 0.4.14 (2017-07-31)
diff --git a/docs/bugs.json b/docs/bugs.json
index 4fd73492..ac322a48 100644
--- a/docs/bugs.json
+++ b/docs/bugs.json
@@ -1,14 +1,22 @@
 [
     {
+        "name": "DelegateCallReturnValue",
+        "summary": "The low-level .delegatecall() does not return the execution outcome, but converts the value returned by the functioned called to a boolean instead.",
+        "description": "The return value of the low-level .delegatecall() function is taken from a position in memory, where the call data or the return data resides. This value is interpreted as a boolean and put onto the stack. This means if the called function returns at least 32 zero bytes, .delegatecall() returns false even if the call was successuful.",
+        "introduced": "0.3.0",
+        "fixed": "0.4.15",
+        "severity": "low"
+    },
+    {
         "name": "ECRecoverMalformedInput",
-	"summary": "The ecrecover() builtin can return garbage for malformed input.",
+        "summary": "The ecrecover() builtin can return garbage for malformed input.",
         "description": "The ecrecover precompile does not properly signal failure for malformed input (especially in the 'v' argument) and thus the Solidity function can return data that was previously present in the return area in memory.",
         "fixed": "0.4.14",
         "severity": "medium"
     },
     {
         "name": "SkipEmptyStringLiteral",
-	"summary": "If \"\" is used in a function call, the following function arguments will not be correctly passed to the function.",
+        "summary": "If \"\" is used in a function call, the following function arguments will not be correctly passed to the function.",
         "description": "If the empty string literal \"\" is used as an argument in a function call, it is skipped by the encoder. This has the effect that the encoding of all arguments following this is shifted left by 32 bytes and thus the function call data is corrupted.",
         "fixed": "0.4.12",
         "severity": "low"
diff --git a/docs/bugs_by_version.json b/docs/bugs_by_version.json
index 1a5b4f5f..f3e42f91 100644
--- a/docs/bugs_by_version.json
+++ b/docs/bugs_by_version.json
@@ -182,6 +182,7 @@
     }, 
     "0.3.0": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -198,6 +199,7 @@
     }, 
     "0.3.1": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -213,6 +215,7 @@
     }, 
     "0.3.2": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -228,6 +231,7 @@
     }, 
     "0.3.3": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -242,6 +246,7 @@
     }, 
     "0.3.4": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -256,6 +261,7 @@
     }, 
     "0.3.5": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -270,6 +276,7 @@
     }, 
     "0.3.6": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -282,6 +289,7 @@
     }, 
     "0.4.0": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -294,6 +302,7 @@
     }, 
     "0.4.1": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -306,6 +315,7 @@
     }, 
     "0.4.10": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction"
@@ -314,6 +324,7 @@
     }, 
     "0.4.11": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral"
         ], 
@@ -321,22 +332,27 @@
     }, 
     "0.4.12": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput"
         ], 
         "released": "2017-07-03"
     }, 
     "0.4.13": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput"
         ], 
         "released": "2017-07-06"
     }, 
     "0.4.14": {
-        "bugs": [], 
+        "bugs": [
+            "DelegateCallReturnValue"
+        ], 
         "released": "2017-07-31"
     }, 
     "0.4.2": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -348,6 +364,7 @@
     }, 
     "0.4.3": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -358,6 +375,7 @@
     }, 
     "0.4.4": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -367,6 +385,7 @@
     }, 
     "0.4.5": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -377,6 +396,7 @@
     }, 
     "0.4.6": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction", 
@@ -386,6 +406,7 @@
     }, 
     "0.4.7": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction"
@@ -394,6 +415,7 @@
     }, 
     "0.4.8": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction"
@@ -402,6 +424,7 @@
     }, 
     "0.4.9": {
         "bugs": [
+            "DelegateCallReturnValue", 
             "ECRecoverMalformedInput", 
             "SkipEmptyStringLiteral", 
             "ConstantOptimizerSubtraction"
diff --git a/docs/control-structures.rst b/docs/control-structures.rst
index a7af69f5..796e9238 100644
--- a/docs/control-structures.rst
+++ b/docs/control-structures.rst
@@ -393,6 +393,9 @@ When exceptions happen in a sub-call, they "bubble up" (i.e. exceptions are reth
 and the low-level functions ``call``, ``delegatecall`` and ``callcode`` -- those return ``false`` in case
 of an exception instead of "bubbling up".
 
+.. warning::
+    The low-level ``call``, ``delegatecall`` and ``callcode`` will return success if the calling account is non-existent, as part of the design of EVM. Existence must be checked prior to calling if desired.
+
 Catching exceptions is not yet possible.
 
 In the following example, you can see how ``require`` can be used to easily check conditions on inputs
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index 2c8cfd76..521d485f 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -1560,7 +1560,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 		utils().moveToStackTop(gasValueSize, _functionType.selfType()->sizeOnStack());
 
 	auto funKind = _functionType.kind();
-	bool returnSuccessCondition = funKind == FunctionType::Kind::BareCall || funKind == FunctionType::Kind::BareCallCode;
+	bool returnSuccessCondition = funKind == FunctionType::Kind::BareCall || funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::BareDelegateCall;
 	bool isCallCode = funKind == FunctionType::Kind::BareCallCode || funKind == FunctionType::Kind::CallCode;
 	bool isDelegateCall = funKind == FunctionType::Kind::BareDelegateCall || funKind == FunctionType::Kind::DelegateCall;
 
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 2c9dfad9..e6c816f4 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -9897,6 +9897,64 @@ BOOST_AUTO_TEST_CASE(inlineasm_empty_let)
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(0), u256(0)));
 }
 
+BOOST_AUTO_TEST_CASE(bare_call_invalid_address)
+{
+	char const* sourceCode = R"(
+		contract C {
+			/// Calling into non-existant account is successful (creates the account)
+			function f() external constant returns (bool) {
+				return address(0x4242).call();
+			}
+			function g() external constant returns (bool) {
+				return address(0x4242).callcode();
+			}
+			function h() external constant returns (bool) {
+				return address(0x4242).delegatecall();
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 0, "C");
+	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(1)));
+	BOOST_CHECK(callContractFunction("g()") == encodeArgs(u256(1)));
+	BOOST_CHECK(callContractFunction("h()") == encodeArgs(u256(1)));
+}
+
+BOOST_AUTO_TEST_CASE(delegatecall_return_value)
+{
+	char const* sourceCode = R"DELIMITER(
+		contract C {
+			uint value;
+			function set(uint _value) external {
+				value = _value;
+			}
+			function get() external constant returns (uint) {
+				return value;
+			}
+			function get_delegated() external constant returns (bool) {
+				return this.delegatecall(bytes4(sha3("get()")));
+			}
+			function assert0() external constant {
+				assert(value == 0);
+			}
+			function assert0_delegated() external constant returns (bool) {
+				return this.delegatecall(bytes4(sha3("assert0()")));
+			}
+		}
+	)DELIMITER";
+	compileAndRun(sourceCode, 0, "C");
+	BOOST_CHECK(callContractFunction("get()") == encodeArgs(u256(0)));
+	BOOST_CHECK(callContractFunction("assert0_delegated()") == encodeArgs(u256(1)));
+	BOOST_CHECK(callContractFunction("get_delegated()") == encodeArgs(u256(1)));
+	BOOST_CHECK(callContractFunction("set(uint256)", u256(1)) == encodeArgs());
+	BOOST_CHECK(callContractFunction("get()") == encodeArgs(u256(1)));
+	BOOST_CHECK(callContractFunction("assert0_delegated()") == encodeArgs(u256(0)));
+	BOOST_CHECK(callContractFunction("get_delegated()") == encodeArgs(u256(1)));
+	BOOST_CHECK(callContractFunction("set(uint256)", u256(42)) == encodeArgs());
+	BOOST_CHECK(callContractFunction("get()") == encodeArgs(u256(42)));
+	BOOST_CHECK(callContractFunction("assert0_delegated()") == encodeArgs(u256(0)));
+	BOOST_CHECK(callContractFunction("get_delegated()") == encodeArgs(u256(1)));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }