Modify gas and value for external function call.

libsolidity/ExpressionCompiler.cpp

@@ -232,27 +232,41 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 		}
 		case Location::EXTERNAL:
 		case Location::BARE:
+			_functionCall.getExpression().accept(*this);
+			appendExternalFunctionCall(function, arguments, function.getLocation() == Location::BARE);
+			break;
+		case Location::SET_GAS:
 		{
-			FunctionCallOptions options;
-			options.bare = function.getLocation() == Location::BARE;
-			options.obtainAddress = [&]() { _functionCall.getExpression().accept(*this); };
-			appendExternalFunctionCall(function, arguments, options);
+			// stack layout: contract_address function_id [gas] [value]
+			_functionCall.getExpression().accept(*this);
+			arguments.front()->accept(*this);
+			appendTypeConversion(*arguments.front()->getType(), IntegerType(256), true);
+			// Note that function is not the original function, but the ".gas" function.
+			// Its values of gasSet and valueSet is equal to the original function's though.
+			unsigned stackDepth = (function.gasSet() ? 1 : 0) + (function.valueSet() ? 1 : 0);
+			if (stackDepth > 0)
+				m_context << eth::swapInstruction(stackDepth);
+			if (function.gasSet())
+				m_context << eth::Instruction::POP;
 			break;
 		}
+		case Location::SET_VALUE:
+			// stack layout: contract_address function_id [gas] [value]
+			_functionCall.getExpression().accept(*this);
+			// Note that function is not the original function, but the ".value" function.
+			// Its values of gasSet and valueSet is equal to the original function's though.
+			if (function.valueSet())
+				m_context << eth::Instruction::POP;
+			arguments.front()->accept(*this);
+			break;
 		case Location::SEND:
-		{
-			FunctionCallOptions options;
-			options.bare = true;
-			options.obtainAddress = [&]() { _functionCall.getExpression().accept(*this); };
-			options.obtainValue = [&]()
-			{
-				arguments.front()->accept(*this);
-				appendTypeConversion(*arguments.front()->getType(),
-									 *function.getParameterTypes().front(), true);
-			};
-			appendExternalFunctionCall(FunctionType(TypePointers{}, TypePointers{}, Location::EXTERNAL), {}, options);
+			// TODO set gas to min
+			_functionCall.getExpression().accept(*this);
+			arguments.front()->accept(*this);
+			appendTypeConversion(*arguments.front()->getType(),
+								 *function.getParameterTypes().front(), true);
+			appendExternalFunctionCall(FunctionType(TypePointers{}, TypePointers{}, Location::EXTERNAL, false, true), {}, true);
 			break;
-		}
 		case Location::SUICIDE:
 			arguments.front()->accept(*this);
 			appendTypeConversion(*arguments.front()->getType(), *function.getParameterTypes().front(), true);
@@ -289,11 +303,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			static const map<Location, u256> contractAddresses{{Location::ECRECOVER, 1},
 															   {Location::SHA256, 2},
 															   {Location::RIPEMD160, 3}};
-			u256 contractAddress = contractAddresses.find(function.getLocation())->second;
-			FunctionCallOptions options;
-			options.bare = true;
-			options.obtainAddress = [&]() { m_context << contractAddress; };
-			appendExternalFunctionCall(function, arguments, options);
+			m_context << contractAddresses.find(function.getLocation())->second;
+			appendExternalFunctionCall(function, arguments, true);
 			break;
 		}
 		default:
@@ -370,6 +381,10 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 		else
 			BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Invalid member access to integer."));
 		break;
+	case Type::Category::FUNCTION:
+		solAssert(!!_memberAccess.getExpression().getType()->getMemberType(member),
+				 "Invalid member access to function.");
+		break;
 	case Type::Category::MAGIC:
 		// we can ignore the kind of magic and only look at the name of the member
 		if (member == "coinbase")
@@ -646,15 +661,25 @@ void ExpressionCompiler::appendHighBitsCleanup(IntegerType const& _typeOnStack)
 
 void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functionType,
 													vector<ASTPointer<Expression const>> const& _arguments,
-													FunctionCallOptions const& _options)
+													bool bare)
 {
 	solAssert(_arguments.size() == _functionType.getParameterTypes().size(), "");
 
-	_options.obtainAddress();
-	if (!_options.bare)
+	// Assumed stack content here:
+	// <stack top>
+	// value [if _functionType.valueSet()]
+	// gas [if _functionType.gasSet()]
+	// function identifier [unless options.bare]
+	// contract address
+
+	unsigned gasValueSize = (_functionType.gasSet() ? 1 : 0) + (_functionType.valueSet() ? 1 : 0);
+	if (!bare)
+	{
+		m_context << eth::dupInstruction(gasValueSize + 1);
 		CompilerUtils(m_context).storeInMemory(0, CompilerUtils::dataStartOffset);
+	}
 
-	unsigned dataOffset = _options.bare ? 0 : CompilerUtils::dataStartOffset; // reserve 4 bytes for the function's hash identifier
+	unsigned dataOffset = bare ? 0 : CompilerUtils::dataStartOffset; // reserve 4 bytes for the function's hash identifier
 	for (unsigned i = 0; i < _arguments.size(); ++i)
 	{
 		_arguments[i]->accept(*this);
@@ -676,16 +701,25 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio
 	unsigned retSize = firstType ? CompilerUtils::getPaddedSize(firstType->getCalldataEncodedSize()) : 0;
 	// CALL arguments: outSize, outOff, inSize, inOff, value, addr, gas (stack top)
 	m_context << u256(retSize) << u256(0) << u256(dataOffset) << u256(0);
-	if (_options.obtainValue)
-		_options.obtainValue();
+	if (_functionType.valueSet())
+		m_context << eth::dupInstruction(5);
 	else
 		m_context << u256(0);
-	m_context << eth::dupInstruction(6); //copy contract address
+	m_context << eth::dupInstruction(6 + gasValueSize + (bare ? 0 : 1)); //copy contract address
 
-	m_context << u256(25) << eth::Instruction::GAS << eth::Instruction::SUB
-			  << eth::Instruction::CALL
-			  << eth::Instruction::POP // @todo do not ignore failure indicator
-			  << eth::Instruction::POP; // pop contract address
+	if (_functionType.gasSet())
+		m_context << eth::dupInstruction(7 + (_functionType.valueSet() ? 1 : 0));
+	else
+		m_context << u256(25) << eth::Instruction::GAS << eth::Instruction::SUB;
+	m_context << eth::Instruction::CALL
+			  << eth::Instruction::POP; // @todo do not ignore failure indicator
+	if (_functionType.valueSet())
+		m_context << eth::Instruction::POP;
+	if (_functionType.gasSet())
+		m_context << eth::Instruction::POP;
+	if (!bare)
+		m_context << eth::Instruction::POP;
+	m_context << eth::Instruction::POP; // pop contract address
 
 	if (retSize > 0)
 	{

libsolidity/ExpressionCompiler.h

@@ -87,21 +87,9 @@ private:
 	//// Appends code that cleans higher-order bits for integer types.
 	void appendHighBitsCleanup(IntegerType const& _typeOnStack);
 
-	/// Additional options used in appendExternalFunctionCall.
-	struct FunctionCallOptions
-	{
-		FunctionCallOptions() {}
-		/// Invoked to copy the address to the stack
-		std::function<void()> obtainAddress;
-		/// Invoked to copy the ethe value to the stack (if not specified, value is 0).
-		std::function<void()> obtainValue;
-		/// If true, do not prepend function index to call data
-		bool bare = false;
-	};
-
 	/// Appends code to call a function of the given type with the given arguments.
 	void appendExternalFunctionCall(FunctionType const& _functionType, std::vector<ASTPointer<Expression const>> const& _arguments,
-									FunctionCallOptions const& _options = FunctionCallOptions());
+									bool bare = false);
 
 	/**
 	 * Helper class to store and retrieve lvalues to and from various locations.

libsolidity/Types.cpp

@@ -561,6 +561,21 @@ FunctionType::FunctionType(FunctionDefinition const& _function, bool _isInternal
 	m_location = _isInternal ? Location::INTERNAL : Location::EXTERNAL;
 }
 
+FunctionType::FunctionType(TypePointers const& _parameterTypes, TypePointers const& _returnParameterTypes,
+						   FunctionType::Location _location, bool _gasSet, bool _valueSet):
+	m_parameterTypes(_parameterTypes), m_returnParameterTypes(_returnParameterTypes),
+	m_location(_location), m_gasSet(_gasSet), m_valueSet(_valueSet)
+{
+	if (m_location == Location::EXTERNAL)
+		m_sizeOnStack = 2;
+	else if (m_location == Location::INTERNAL || m_location == Location::BARE)
+		m_sizeOnStack = 1;
+	if (m_gasSet)
+		m_sizeOnStack++;
+	if (m_valueSet)
+		m_sizeOnStack++;
+}
+
 bool FunctionType::operator==(Type const& _other) const
 {
 	if (_other.getCategory() != getCategory())
@@ -580,6 +595,9 @@ bool FunctionType::operator==(Type const& _other) const
 	if (!equal(m_returnParameterTypes.cbegin(), m_returnParameterTypes.cend(),
 			   other.m_returnParameterTypes.cbegin(), typeCompare))
 		return false;
+	//@todo this is ugly, but cannot be prevented right now
+	if (m_gasSet != other.m_gasSet || m_valueSet != other.m_valueSet)
+		return false;
 	return true;
 }
 
@@ -595,17 +613,42 @@ string FunctionType::toString() const
 }
 
 unsigned FunctionType::getSizeOnStack() const
+{
+	unsigned size = 0;
+	if (m_location == Location::EXTERNAL)
+		size = 2;
+	else if (m_location == Location::INTERNAL || m_location == Location::BARE)
+		size = 1;
+	if (m_gasSet)
+		size++;
+	if (m_valueSet)
+		size++;
+	return size;
+}
+
+MemberList const& FunctionType::getMembers() const
 {
 	switch (m_location)
 	{
-	case Location::INTERNAL:
-		return 1;
 	case Location::EXTERNAL:
-		return 2;
+	case Location::ECRECOVER:
+	case Location::SHA256:
+	case Location::RIPEMD160:
 	case Location::BARE:
-		return 1;
+		if (!m_members)
+		{
+			map<string, TypePointer> members{
+				{"gas", make_shared<FunctionType>(parseElementaryTypeVector({"uint"}),
+												  TypePointers{copyAndSetGasOrValue(true, false)},
+												  Location::SET_GAS, m_gasSet, m_valueSet)},
+				{"value", make_shared<FunctionType>(parseElementaryTypeVector({"uint"}),
+													TypePointers{copyAndSetGasOrValue(false, true)},
+													Location::SET_VALUE, m_gasSet, m_valueSet)}};
+			m_members.reset(new MemberList(members));
+		}
+		return *m_members;
 	default:
-		return 0;
+		return EmptyMemberList;
 	}
 }
 
@@ -628,6 +671,13 @@ TypePointers FunctionType::parseElementaryTypeVector(strings const& _types)
 	return pointers;
 }
 
+TypePointer FunctionType::copyAndSetGasOrValue(bool _setGas, bool _setValue) const
+{
+	return make_shared<FunctionType>(m_parameterTypes, m_returnParameterTypes, m_location,
+									 m_gasSet || _setGas, m_valueSet || _setValue);
+}
+
+
 bool MappingType::operator==(Type const& _other) const
 {
 	if (_other.getCategory() != getCategory())

libsolidity/Types.h

@@ -345,10 +345,15 @@ class FunctionType: public Type
 {
 public:
 	/// The meaning of the value(s) on the stack referencing the function:
-	/// INTERNAL: jump tag, EXTERNAL: contract address + function index,
+	/// INTERNAL: jump tag, EXTERNAL: contract address + function identifier,
 	/// BARE: contract address (non-abi contract call)
 	/// OTHERS: special virtual function, nothing on the stack
-	enum class Location { INTERNAL, EXTERNAL, SEND, SHA3, SUICIDE, ECRECOVER, SHA256, RIPEMD160, LOG0, LOG1, LOG2, LOG3, LOG4, BARE };
+	enum class Location { INTERNAL, EXTERNAL, SEND,
+						  SHA3, SUICIDE,
+						  ECRECOVER, SHA256, RIPEMD160,
+						  LOG0, LOG1, LOG2, LOG3, LOG4,
+						  SET_GAS, SET_VALUE,
+						  BARE };
 
 	virtual Category getCategory() const override { return Category::FUNCTION; }
 	explicit FunctionType(FunctionDefinition const& _function, bool _isInternal = true);
@@ -357,9 +362,8 @@ public:
 		FunctionType(parseElementaryTypeVector(_parameterTypes), parseElementaryTypeVector(_returnParameterTypes),
 					 _location) {}
 	FunctionType(TypePointers const& _parameterTypes, TypePointers const& _returnParameterTypes,
-				 Location _location = Location::INTERNAL):
-		m_parameterTypes(_parameterTypes), m_returnParameterTypes(_returnParameterTypes),
-		m_location(_location) {}
+				 Location _location = Location::INTERNAL,
+				 bool _gasSet = false, bool _valueSet = false);
 
 	TypePointers const& getParameterTypes() const { return m_parameterTypes; }
 	TypePointers const& getReturnParameterTypes() const { return m_returnParameterTypes; }
@@ -370,16 +374,28 @@ public:
 	virtual u256 getStorageSize() const override { BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Storage size of non-storable function type requested.")); }
 	virtual bool canLiveOutsideStorage() const override { return false; }
 	virtual unsigned getSizeOnStack() const override;
+	virtual MemberList const& getMembers() const override;
 
 	Location const& getLocation() const { return m_location; }
 	std::string getCanonicalSignature() const;
 
+	bool gasSet() const { return m_gasSet; }
+	bool valueSet() const { return m_valueSet; }
+
+	/// @returns a copy of this type, where gas or value are set manually. This will never set one
+	/// of the parameters to fals.
+	TypePointer copyAndSetGasOrValue(bool _setGas, bool _setValue) const;
+
 private:
 	static TypePointers parseElementaryTypeVector(strings const& _types);
 
 	TypePointers m_parameterTypes;
 	TypePointers m_returnParameterTypes;
 	Location m_location;
+	unsigned m_sizeOnStack = 0;
+	bool m_gasSet = false; ///< true iff the gas value to be used is on the stack
+	bool m_valueSet = false; ///< true iff the value to be sent is on the stack
+	mutable std::unique_ptr<MemberList> m_members;
 };
 
 /**

test/SolidityEndToEndTest.cpp

@@ -1295,7 +1295,85 @@ BOOST_AUTO_TEST_CASE(contracts_as_addresses)
 		}
 	)";
 	compileAndRun(sourceCode, 20);
-	BOOST_REQUIRE(callContractFunction("getBalance()") == toBigEndian(u256(20 - 5)) + toBigEndian(u256(5)));
+	BOOST_REQUIRE(callContractFunction("getBalance()") == encodeArgs(u256(20 - 5), u256(5)));
+}
+
+BOOST_AUTO_TEST_CASE(gas_and_value_basic)
+{
+	char const* sourceCode = R"(
+		contract helper {
+			bool flag;
+			function getBalance() returns (uint256 myBalance) {
+				return this.balance;
+			}
+			function setFlag() { flag = true; }
+			function getFlag() returns (bool fl) { return flag; }
+		}
+		contract test {
+			helper h;
+			function test() { h = new helper(); }
+			function sendAmount(uint amount) returns (uint256 bal) {
+				return h.getBalance.value(amount)();
+			}
+			function outOfGas() returns (bool flagBefore, bool flagAfter, uint myBal) {
+				flagBefore = h.getFlag();
+				h.setFlag.gas(2)(); // should fail due to OOG, return value can be garbage
+				flagAfter = h.getFlag();
+				myBal = this.balance;
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 20);
+	BOOST_REQUIRE(callContractFunction("sendAmount(uint256)", 5) == encodeArgs(5));
+	// call to helper should not succeed but amount should be transferred anyway
+	BOOST_REQUIRE(callContractFunction("outOfGas()", 5) == encodeArgs(false, false, 20 - 5));
+}
+
+BOOST_AUTO_TEST_CASE(value_complex)
+{
+	char const* sourceCode = R"(
+		contract helper {
+			function getBalance() returns (uint256 myBalance) {
+				return this.balance;
+			}
+		}
+		contract test {
+			helper h;
+			function test() { h = new helper(); }
+			function sendAmount(uint amount) returns (uint256 bal) {
+				var x1 = h.getBalance.value(amount);
+				uint someStackElement = 20;
+				var x2 = x1.gas(1000);
+				return x2.value(amount + 3)();// overwrite value
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 20);
+	BOOST_REQUIRE(callContractFunction("sendAmount(uint256)", 5) == encodeArgs(8));
+}
+
+BOOST_AUTO_TEST_CASE(value_insane)
+{
+	char const* sourceCode = R"(
+		contract helper {
+			function getBalance() returns (uint256 myBalance) {
+				return this.balance;
+			}
+		}
+		contract test {
+			helper h;
+			function test() { h = new helper(); }
+			function sendAmount(uint amount) returns (uint256 bal) {
+				var x1 = h.getBalance.value;
+				uint someStackElement = 20;
+				var x2 = x1(amount).gas;
+				var x3 = x2(1000).value;
+				return x3(amount + 3)();// overwrite value
+			}
+		}
+	)";
+	compileAndRun(sourceCode, 20);
+	BOOST_REQUIRE(callContractFunction("sendAmount(uint256)", 5) == encodeArgs(8));
 }
 
 BOOST_AUTO_TEST_SUITE_END()