Merge pull request #1598 from wuestholz/develop

Change translation of implicit throws

14 files changed, 85 insertions, 23 deletions

diff --git a/Changelog.md b/Changelog.md
index 7c09407a..8dd1b89c 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -9,6 +9,8 @@ Features:
  * Metadata: Do not include platform in the version number.
  * Metadata: Add option to store sources as literal content.
  * Code generator: Extract array utils into low-level functions.
+ * Code generator: Internal errors (array out of bounds, etc.) now cause a reversion by using an invalid
+   instruction (0xfe) instead of an invalid jump. Invalid jump is still kept for explicit throws.
 
 Bugfixes:
  * Code generator: Allow recursive structs.
diff --git a/docs/control-structures.rst b/docs/control-structures.rst
index 3f012b12..ff9b245a 100644
--- a/docs/control-structures.rst
+++ b/docs/control-structures.rst
@@ -394,8 +394,13 @@ Currently, Solidity automatically generates a runtime exception in the following
 #. If you perform an external function call targeting a contract that contains no code.
 #. If your contract receives Ether via a public function without ``payable`` modifier (including the constructor and the fallback function).
 #. If your contract receives Ether via a public accessor function.
+#. If you call a zero-initialized variable of internal function type.
 
-Internally, Solidity performs an "invalid jump" when an exception is thrown and thus causes the EVM to revert all changes made to the state. The reason for this is that there is no safe way to continue execution, because an expected effect did not occur. Because we want to retain the atomicity of transactions, the safest thing to do is to revert all changes and make the whole transaction (or at least call) without effect.
+Internally, Solidity performs an "invalid jump" when a user-provided exception is thrown. In contrast, it performs an invalid operation
+(instruction ``0xfe``) if a runtime exception is encountered. In both cases, this causes
+the EVM to revert all changes made to the state. The reason for this is that there is no safe way to continue execution, because an expected effect
+did not occur. Because we want to retain the atomicity of transactions, the safest thing to do is to revert all changes and make the whole transaction
+(or at least call) without effect.
 
 .. index:: ! assembly, ! asm, ! evmasm
 
diff --git a/libevmasm/Instruction.cpp b/libevmasm/Instruction.cpp
index 17445c59..b0f063da 100644
--- a/libevmasm/Instruction.cpp
+++ b/libevmasm/Instruction.cpp
@@ -159,6 +159,7 @@ const std::map<std::string, Instruction> dev::solidity::c_instructions =
 	{ "CALLCODE", Instruction::CALLCODE },
 	{ "RETURN", Instruction::RETURN },
 	{ "DELEGATECALL", Instruction::DELEGATECALL },
+	{ "INVALID", Instruction::INVALID },
 	{ "SUICIDE", Instruction::SUICIDE }
 };
 
@@ -293,6 +294,7 @@ static const std::map<Instruction, InstructionInfo> c_instructionInfo =
 	{ Instruction::CALLCODE,	{ "CALLCODE",		0, 7, 1, true, Tier::Special } },
 	{ Instruction::RETURN,		{ "RETURN",			0, 2, 0, true, Tier::Zero } },
 	{ Instruction::DELEGATECALL,{ "DELEGATECALL",	0, 6, 1, true, Tier::Special } },
+	{ Instruction::INVALID,		{ "INVALID",		0, 0, 0, true, Tier::Zero } },
 	{ Instruction::SUICIDE,		{ "SUICIDE",		0, 1, 0, true, Tier::Zero } }
 };
 
diff --git a/libevmasm/Instruction.h b/libevmasm/Instruction.h
index 2dd451cd..a8a72234 100644
--- a/libevmasm/Instruction.h
+++ b/libevmasm/Instruction.h
@@ -176,6 +176,8 @@ enum class Instruction: uint8_t
 	CALLCODE,			///< message-call with another account's code only
 	RETURN,				///< halt execution returning output data
 	DELEGATECALL,		///< like CALLCODE but keeps caller's value and sender
+
+	INVALID = 0xfe,		///< invalid instruction for expressing runtime errors (e.g., division-by-zero)
 	SUICIDE = 0xff		///< halt execution and register account for later deletion
 };
 
diff --git a/libevmasm/PeepholeOptimiser.cpp b/libevmasm/PeepholeOptimiser.cpp
index 923ffa67..528ce1c4 100644
--- a/libevmasm/PeepholeOptimiser.cpp
+++ b/libevmasm/PeepholeOptimiser.cpp
@@ -199,6 +199,7 @@ struct UnreachableCode
 			it[0] != Instruction::JUMP &&
 			it[0] != Instruction::RETURN &&
 			it[0] != Instruction::STOP &&
+			it[0] != Instruction::INVALID &&
 			it[0] != Instruction::SUICIDE
 		)
 			return false;
diff --git a/libevmasm/SemanticInformation.cpp b/libevmasm/SemanticInformation.cpp
index 23a00d95..d3ce4735 100644
--- a/libevmasm/SemanticInformation.cpp
+++ b/libevmasm/SemanticInformation.cpp
@@ -118,6 +118,7 @@ bool SemanticInformation::altersControlFlow(AssemblyItem const& _item)
 	case Instruction::RETURN:
 	case Instruction::SUICIDE:
 	case Instruction::STOP:
+	case Instruction::INVALID:
 		return true;
 	default:
 		return false;
diff --git a/libsolidity/codegen/ArrayUtils.cpp b/libsolidity/codegen/ArrayUtils.cpp
index 4d100d1d..bdd29abd 100644
--- a/libsolidity/codegen/ArrayUtils.cpp
+++ b/libsolidity/codegen/ArrayUtils.cpp
@@ -901,7 +901,7 @@ void ArrayUtils::accessIndex(ArrayType const& _arrayType, bool _doBoundsCheck) c
 		// check out-of-bounds access
 		m_context << Instruction::DUP2 << Instruction::LT << Instruction::ISZERO;
 		// out-of-bounds access throws exception
-		m_context.appendConditionalJumpTo(m_context.errorTag());
+		m_context.appendConditionalInvalid();
 	}
 	if (location == DataLocation::CallData && _arrayType.isDynamicallySized())
 		// remove length if present
diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp
index e26f96e8..a8316109 100644
--- a/libsolidity/codegen/CompilerContext.cpp
+++ b/libsolidity/codegen/CompilerContext.cpp
@@ -70,19 +70,30 @@ void CompilerContext::callLowLevelFunction(
 	eth::AssemblyItem retTag = pushNewTag();
 	CompilerUtils(*this).moveIntoStack(_inArgs);
 
+	*this << lowLevelFunctionTag(_name, _inArgs, _outArgs, _generator);
+
+	appendJump(eth::AssemblyItem::JumpType::IntoFunction);
+	adjustStackOffset(int(_outArgs) - 1 - _inArgs);
+	*this << retTag.tag();
+}
+
+eth::AssemblyItem CompilerContext::lowLevelFunctionTag(
+	string const& _name,
+	unsigned _inArgs,
+	unsigned _outArgs,
+	function<void(CompilerContext&)> const& _generator
+)
+{
 	auto it = m_lowLevelFunctions.find(_name);
 	if (it == m_lowLevelFunctions.end())
 	{
 		eth::AssemblyItem tag = newTag().pushTag();
 		m_lowLevelFunctions.insert(make_pair(_name, tag));
 		m_lowLevelFunctionGenerationQueue.push(make_tuple(_name, _inArgs, _outArgs, _generator));
-		*this << tag;
+		return tag;
 	}
 	else
-		*this << it->second;
-	appendJump(eth::AssemblyItem::JumpType::IntoFunction);
-	adjustStackOffset(int(_outArgs) - 1 - _inArgs);
-	*this << retTag.tag();
+		return it->second;
 }
 
 void CompilerContext::appendMissingLowLevelFunctions()
@@ -215,6 +226,20 @@ CompilerContext& CompilerContext::appendJump(eth::AssemblyItem::JumpType _jumpTy
 	return *this << item;
 }
 
+CompilerContext& CompilerContext::appendInvalid()
+{
+	return *this << Instruction::INVALID;
+}
+
+CompilerContext& CompilerContext::appendConditionalInvalid()
+{
+	*this << Instruction::ISZERO;
+	eth::AssemblyItem afterTag = appendConditionalJump();
+	*this << Instruction::INVALID;
+	*this << afterTag;
+	return *this;
+}
+
 void CompilerContext::resetVisitedNodes(ASTNode const* _node)
 {
 	stack<ASTNode const*> newStack;
diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h
index f024b010..c37142c9 100644
--- a/libsolidity/codegen/CompilerContext.h
+++ b/libsolidity/codegen/CompilerContext.h
@@ -104,6 +104,16 @@ public:
 		unsigned _outArgs,
 		std::function<void(CompilerContext&)> const& _generator
 	);
+	/// Returns the tag of the named low-level function and inserts the generator into the
+	/// list of low-level-functions to be generated, unless it already exists.
+	/// Note that the generator should not assume that objects are still alive when it is called,
+	/// unless they are guaranteed to be alive for the whole run of the compiler (AST nodes, for example).
+	eth::AssemblyItem lowLevelFunctionTag(
+		std::string const& _name,
+		unsigned _inArgs,
+		unsigned _outArgs,
+		std::function<void(CompilerContext&)> const& _generator
+	);
 	/// Generates the code for missing low-level functions, i.e. calls the generators passed above.
 	void appendMissingLowLevelFunctions();
 
@@ -127,6 +137,10 @@ public:
 	eth::AssemblyItem appendJumpToNew() { return m_asm->appendJump().tag(); }
 	/// Appends a JUMP to a tag already on the stack
 	CompilerContext&  appendJump(eth::AssemblyItem::JumpType _jumpType = eth::AssemblyItem::JumpType::Ordinary);
+	/// Appends an INVALID instruction
+	CompilerContext&  appendInvalid();
+	/// Appends a conditional INVALID instruction
+	CompilerContext&  appendConditionalInvalid();
 	/// Returns an "ErrorTag"
 	eth::AssemblyItem errorTag() { return m_asm->errorTag(); }
 	/// Appends a JUMP to a specific tag
diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp
index caf3b1ac..477f021a 100644
--- a/libsolidity/codegen/CompilerUtils.cpp
+++ b/libsolidity/codegen/CompilerUtils.cpp
@@ -468,7 +468,7 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 			EnumType const& enumType = dynamic_cast<decltype(enumType)>(_typeOnStack);
 			solAssert(enumType.numberOfMembers() > 0, "empty enum should have caused a parser error.");
 			m_context << u256(enumType.numberOfMembers() - 1) << Instruction::DUP2 << Instruction::GT;
-			m_context.appendConditionalJumpTo(m_context.errorTag());
+			m_context.appendConditionalInvalid();
 			enumOverflowCheckPending = false;
 		}
 		break;
@@ -497,7 +497,7 @@ void CompilerUtils::convertType(Type const& _typeOnStack, Type const& _targetTyp
 			EnumType const& enumType = dynamic_cast<decltype(enumType)>(_targetType);
 			solAssert(enumType.numberOfMembers() > 0, "empty enum should have caused a parser error.");
 			m_context << u256(enumType.numberOfMembers() - 1) << Instruction::DUP2 << Instruction::GT;
-			m_context.appendConditionalJumpTo(m_context.errorTag());
+			m_context.appendConditionalInvalid();
 			enumOverflowCheckPending = false;
 		}
 		else if (targetTypeCategory == Type::Category::FixedPoint)
@@ -807,7 +807,9 @@ void CompilerUtils::pushZeroValue(Type const& _type)
 	{
 		if (funType->location() == FunctionType::Location::Internal)
 		{
-			m_context << m_context.errorTag();
+			m_context << m_context.lowLevelFunctionTag("$invalidFunction", 0, 0, [](CompilerContext& _context) {
+				_context.appendInvalid();
+			});
 			return;
 		}
 	}
diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp
index 9dc1fb37..4d33927d 100644
--- a/libsolidity/codegen/ContractCompiler.cpp
+++ b/libsolidity/codegen/ContractCompiler.cpp
@@ -106,7 +106,7 @@ void ContractCompiler::appendCallValueCheck()
 {
 	// Throw if function is not payable but call contained ether.
 	m_context << Instruction::CALLVALUE;
-	m_context.appendConditionalJumpTo(m_context.errorTag());
+	m_context.appendConditionalInvalid();
 }
 
 void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _contract)
@@ -271,7 +271,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
 		appendReturnValuePacker(FunctionType(*fallback).returnParameterTypes(), _contract.isLibrary());
 	}
 	else
-		m_context.appendJumpTo(m_context.errorTag());
+		m_context.appendInvalid();
 
 	for (auto const& it: interfaceFunctions)
 	{
@@ -918,7 +918,9 @@ eth::AssemblyPointer ContractCompiler::cloneRuntime()
 	a << Instruction::DELEGATECALL;
 	//Propagate error condition (if DELEGATECALL pushes 0 on stack).
 	a << Instruction::ISZERO;
-	a.appendJumpI(a.errorTag());
+	a << Instruction::ISZERO;
+	eth::AssemblyItem afterTag = a.appendJumpI().tag();
+	a << Instruction::INVALID << afterTag;
 	//@todo adjust for larger return values, make this dynamic.
 	a << u256(0x20) << u256(0) << Instruction::RETURN;
 	return make_shared<eth::Assembly>(a);
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index bda4e04d..b66a3e12 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -585,7 +585,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			m_context << Instruction::CREATE;
 			// Check if zero (out of stack or not enough balance).
 			m_context << Instruction::DUP1 << Instruction::ISZERO;
-			m_context.appendConditionalJumpTo(m_context.errorTag());
+			m_context.appendConditionalInvalid();
 			if (function.valueSet())
 				m_context << swapInstruction(1) << Instruction::POP;
 			break;
@@ -1234,7 +1234,7 @@ bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
 		m_context << u256(fixedBytesType.numBytes());
 		m_context << Instruction::DUP2 << Instruction::LT << Instruction::ISZERO;
 		// out-of-bounds access throws exception
-		m_context.appendConditionalJumpTo(m_context.errorTag());
+		m_context.appendConditionalInvalid();
 
 		m_context << Instruction::BYTE;
 		m_context << (u256(1) << (256 - 8)) << Instruction::MUL;
@@ -1416,7 +1416,7 @@ void ExpressionCompiler::appendArithmeticOperatorCode(Token::Value _operator, Ty
 	{
 		// Test for division by zero
 		m_context << Instruction::DUP2 << Instruction::ISZERO;
-		m_context.appendConditionalJumpTo(m_context.errorTag());
+		m_context.appendConditionalInvalid();
 
 		if (_operator == Token::Div)
 			m_context << (c_isSigned ? Instruction::SDIV : Instruction::DIV);
@@ -1477,7 +1477,7 @@ void ExpressionCompiler::appendShiftOperatorCode(Token::Value _operator, Type co
 	if (c_amountSigned)
 	{
 		m_context << u256(0) << Instruction::DUP3 << Instruction::SLT;
-		m_context.appendConditionalJumpTo(m_context.errorTag());
+		m_context.appendConditionalInvalid();
 	}
 
 	switch (_operator)
@@ -1663,7 +1663,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	if (funKind == FunctionKind::External || funKind == FunctionKind::CallCode || funKind == FunctionKind::DelegateCall)
 	{
 		m_context << Instruction::DUP1 << Instruction::EXTCODESIZE << Instruction::ISZERO;
-		m_context.appendConditionalJumpTo(m_context.errorTag());
+		m_context.appendConditionalInvalid();
 		existenceChecked = true;
 	}
 
@@ -1699,7 +1699,7 @@ void ExpressionCompiler::appendExternalFunctionCall(
 	{
 		//Propagate error condition (if CALL pushes 0 on stack).
 		m_context << Instruction::ISZERO;
-		m_context.appendConditionalJumpTo(m_context.errorTag());
+		m_context.appendConditionalInvalid();
 	}
 
 	utils().popStackSlots(remainsSize);
diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp
index 155dd5c9..497bfc77 100644
--- a/test/libsolidity/Assembly.cpp
+++ b/test/libsolidity/Assembly.cpp
@@ -116,8 +116,8 @@ BOOST_AUTO_TEST_CASE(location_test)
 	shared_ptr<string const> n = make_shared<string>("");
 	AssemblyItems items = compileContract(sourceCode);
 	vector<SourceLocation> locations =
-		vector<SourceLocation>(18, SourceLocation(2, 75, n)) +
-		vector<SourceLocation>(27, SourceLocation(20, 72, n)) +
+		vector<SourceLocation>(17, SourceLocation(2, 75, n)) +
+		vector<SourceLocation>(30, SourceLocation(20, 72, n)) +
 		vector<SourceLocation>{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} +
 		vector<SourceLocation>(2, SourceLocation(58, 67, n)) +
 		vector<SourceLocation>(3, SourceLocation(20, 72, n));
diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp
index 0c5a09c3..a769776e 100644
--- a/test/libsolidity/SolidityExpressionCompiler.cpp
+++ b/test/libsolidity/SolidityExpressionCompiler.cpp
@@ -337,13 +337,19 @@ BOOST_AUTO_TEST_CASE(arithmetics)
 					   byte(Instruction::ADD),
 					   byte(Instruction::DUP2),
 					   byte(Instruction::ISZERO),
-					   byte(Instruction::PUSH1), 0x0,
+					   byte(Instruction::ISZERO),
+					   byte(Instruction::PUSH1), 0x1d,
 					   byte(Instruction::JUMPI),
+					   byte(Instruction::INVALID),
+					   byte(Instruction::JUMPDEST),
 					   byte(Instruction::MOD),
 					   byte(Instruction::DUP2),
 					   byte(Instruction::ISZERO),
-					   byte(Instruction::PUSH1), 0x0,
+					   byte(Instruction::ISZERO),
+					   byte(Instruction::PUSH1), 0x26,
 					   byte(Instruction::JUMPI),
+					   byte(Instruction::INVALID),
+					   byte(Instruction::JUMPDEST),
 					   byte(Instruction::DIV),
 					   byte(Instruction::MUL)});
 	BOOST_CHECK_EQUAL_COLLECTIONS(code.begin(), code.end(), expectation.begin(), expectation.end());