From 9bcbd93ac59a19320fd56e27c58a6283f2450666 Mon Sep 17 00:00:00 2001 From: Valentin Wüstholz Date: Sun, 22 Jan 2017 20:49:12 +0100 Subject: Change translation of implicit throws (issue #1589). This adds a new invalid instruction that is used for encoding implicit throws that are emitted by the compiler. This makes it possible to distinguish such runtime errors from user-provided, explicit throws. --- libevmasm/Instruction.cpp | 2 ++ libevmasm/Instruction.h | 2 ++ libsolidity/codegen/ArrayUtils.cpp | 2 +- libsolidity/codegen/CompilerContext.cpp | 12 ++++++++++++ libsolidity/codegen/CompilerContext.h | 4 ++++ libsolidity/codegen/CompilerUtils.cpp | 6 +++--- libsolidity/codegen/ContractCompiler.cpp | 8 +++++--- libsolidity/codegen/ExpressionCompiler.cpp | 12 ++++++------ test/libsolidity/Assembly.cpp | 4 ++-- test/libsolidity/SolidityExpressionCompiler.cpp | 14 ++++++++++++-- 10 files changed, 49 insertions(+), 17 deletions(-) diff --git a/libevmasm/Instruction.cpp b/libevmasm/Instruction.cpp index 17445c59..ea5b5a10 100644 --- a/libevmasm/Instruction.cpp +++ b/libevmasm/Instruction.cpp @@ -154,6 +154,7 @@ const std::map dev::solidity::c_instructions = { "LOG2", Instruction::LOG2 }, { "LOG3", Instruction::LOG3 }, { "LOG4", Instruction::LOG4 }, + { "INVALID", Instruction::INVALID }, { "CREATE", Instruction::CREATE }, { "CALL", Instruction::CALL }, { "CALLCODE", Instruction::CALLCODE }, @@ -288,6 +289,7 @@ static const std::map c_instructionInfo = { Instruction::LOG2, { "LOG2", 0, 4, 0, true, Tier::Special } }, { Instruction::LOG3, { "LOG3", 0, 5, 0, true, Tier::Special } }, { Instruction::LOG4, { "LOG4", 0, 6, 0, true, Tier::Special } }, + { Instruction::INVALID, { "INVALID", 0, 0, 0, true, Tier::Zero } }, { Instruction::CREATE, { "CREATE", 0, 3, 1, true, Tier::Special } }, { Instruction::CALL, { "CALL", 0, 7, 1, true, Tier::Special } }, { Instruction::CALLCODE, { "CALLCODE", 0, 7, 1, true, Tier::Special } }, diff --git a/libevmasm/Instruction.h b/libevmasm/Instruction.h index 2dd451cd..7432f04d 100644 --- a/libevmasm/Instruction.h +++ b/libevmasm/Instruction.h @@ -171,6 +171,8 @@ enum class Instruction: uint8_t LOG3, ///< Makes a log entry; 3 topics. LOG4, ///< Makes a log entry; 4 topics. + INVALID = 0xef, ///< invalid instruction for expressing runtime errors (e.g., division-by-zero) + CREATE = 0xf0, ///< create a new account with associated code CALL, ///< message-call into an account CALLCODE, ///< message-call with another account's code only 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..45450350 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -215,6 +215,18 @@ CompilerContext& CompilerContext::appendJump(eth::AssemblyItem::JumpType _jumpTy return *this << item; } +CompilerContext& CompilerContext::appendInvalid() +{ + return *this << Instruction::INVALID; +} + +CompilerContext& CompilerContext::appendConditionalInvalid() +{ + eth::AssemblyItem falseTag = appendConditionalJump(); + eth::AssemblyItem endTag = appendJumpToNew(); + return *this << falseTag << Instruction::INVALID << endTag; +} + void CompilerContext::resetVisitedNodes(ASTNode const* _node) { stack newStack; diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index f024b010..58d6cb2a 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -127,6 +127,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..67877bbf 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(_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(_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,7 @@ void CompilerUtils::pushZeroValue(Type const& _type) { if (funType->location() == FunctionType::Location::Internal) { - m_context << m_context.errorTag(); + m_context.appendInvalid(); return; } } diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 9dc1fb37..56d03a05 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()); + eth::AssemblyItem falseTag = a.appendJumpI(); + eth::AssemblyItem endTag = a.appendJump().tag(); + a << falseTag << Instruction::INVALID << endTag; //@todo adjust for larger return values, make this dynamic. a << u256(0x20) << u256(0) << Instruction::RETURN; return make_shared(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..aed3c854 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -116,8 +116,8 @@ BOOST_AUTO_TEST_CASE(location_test) shared_ptr n = make_shared(""); AssemblyItems items = compileContract(sourceCode); vector locations = - vector(18, SourceLocation(2, 75, n)) + - vector(27, SourceLocation(20, 72, n)) + + vector(17, SourceLocation(2, 75, n)) + + vector(32, SourceLocation(20, 72, n)) + vector{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} + vector(2, SourceLocation(58, 67, n)) + vector(3, SourceLocation(20, 72, n)); diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp index 0c5a09c3..ca630169 100644 --- a/test/libsolidity/SolidityExpressionCompiler.cpp +++ b/test/libsolidity/SolidityExpressionCompiler.cpp @@ -337,13 +337,23 @@ BOOST_AUTO_TEST_CASE(arithmetics) byte(Instruction::ADD), byte(Instruction::DUP2), byte(Instruction::ISZERO), - byte(Instruction::PUSH1), 0x0, + byte(Instruction::PUSH1), 0x1e, byte(Instruction::JUMPI), + byte(Instruction::PUSH1), 0x20, + byte(Instruction::JUMP), + byte(Instruction::JUMPDEST), + byte(Instruction::INVALID), + byte(Instruction::JUMPDEST), byte(Instruction::MOD), byte(Instruction::DUP2), byte(Instruction::ISZERO), - byte(Instruction::PUSH1), 0x0, + byte(Instruction::PUSH1), 0x2a, byte(Instruction::JUMPI), + byte(Instruction::PUSH1), 0x2c, + byte(Instruction::JUMP), + byte(Instruction::JUMPDEST), + byte(Instruction::INVALID), + byte(Instruction::JUMPDEST), byte(Instruction::DIV), byte(Instruction::MUL)}); BOOST_CHECK_EQUAL_COLLECTIONS(code.begin(), code.end(), expectation.begin(), expectation.end()); -- cgit From 5b7cc018f0b256fb42f7bee38ad8d1ec4e2ec634 Mon Sep 17 00:00:00 2001 From: Valentin Wüstholz Date: Mon, 23 Jan 2017 10:46:50 +0100 Subject: Address feedback from code review. --- docs/control-structures.rst | 2 +- libsolidity/codegen/CompilerContext.cpp | 6 +++--- libsolidity/codegen/ContractCompiler.cpp | 6 +++--- test/libsolidity/Assembly.cpp | 2 +- test/libsolidity/SolidityExpressionCompiler.cpp | 12 ++++-------- 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/docs/control-structures.rst b/docs/control-structures.rst index 3f012b12..a398d857 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -395,7 +395,7 @@ Currently, Solidity automatically generates a runtime exception in the following #. 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. -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 (i.e., non-existent) operation 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/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 45450350..3bb6c953 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -222,9 +222,9 @@ CompilerContext& CompilerContext::appendInvalid() CompilerContext& CompilerContext::appendConditionalInvalid() { - eth::AssemblyItem falseTag = appendConditionalJump(); - eth::AssemblyItem endTag = appendJumpToNew(); - return *this << falseTag << Instruction::INVALID << endTag; + *this << Instruction::ISZERO; + eth::AssemblyItem afterTag = appendConditionalJump(); + return *this << Instruction::INVALID << afterTag; } void CompilerContext::resetVisitedNodes(ASTNode const* _node) diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 56d03a05..3ca2f375 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -918,9 +918,9 @@ eth::AssemblyPointer ContractCompiler::cloneRuntime() a << Instruction::DELEGATECALL; //Propagate error condition (if DELEGATECALL pushes 0 on stack). a << Instruction::ISZERO; - eth::AssemblyItem falseTag = a.appendJumpI(); - eth::AssemblyItem endTag = a.appendJump().tag(); - a << falseTag << Instruction::INVALID << endTag; + a << Instruction::ISZERO; + eth::AssemblyItem afterTag = a.appendJumpI(); + a << Instruction::INVALID << afterTag; //@todo adjust for larger return values, make this dynamic. a << u256(0x20) << u256(0) << Instruction::RETURN; return make_shared(a); diff --git a/test/libsolidity/Assembly.cpp b/test/libsolidity/Assembly.cpp index aed3c854..497bfc77 100644 --- a/test/libsolidity/Assembly.cpp +++ b/test/libsolidity/Assembly.cpp @@ -117,7 +117,7 @@ BOOST_AUTO_TEST_CASE(location_test) AssemblyItems items = compileContract(sourceCode); vector locations = vector(17, SourceLocation(2, 75, n)) + - vector(32, SourceLocation(20, 72, n)) + + vector(30, SourceLocation(20, 72, n)) + vector{SourceLocation(42, 51, n), SourceLocation(65, 67, n)} + vector(2, SourceLocation(58, 67, n)) + vector(3, SourceLocation(20, 72, n)); diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp index ca630169..a769776e 100644 --- a/test/libsolidity/SolidityExpressionCompiler.cpp +++ b/test/libsolidity/SolidityExpressionCompiler.cpp @@ -337,21 +337,17 @@ BOOST_AUTO_TEST_CASE(arithmetics) byte(Instruction::ADD), byte(Instruction::DUP2), byte(Instruction::ISZERO), - byte(Instruction::PUSH1), 0x1e, + byte(Instruction::ISZERO), + byte(Instruction::PUSH1), 0x1d, byte(Instruction::JUMPI), - byte(Instruction::PUSH1), 0x20, - byte(Instruction::JUMP), - byte(Instruction::JUMPDEST), byte(Instruction::INVALID), byte(Instruction::JUMPDEST), byte(Instruction::MOD), byte(Instruction::DUP2), byte(Instruction::ISZERO), - byte(Instruction::PUSH1), 0x2a, + byte(Instruction::ISZERO), + byte(Instruction::PUSH1), 0x26, byte(Instruction::JUMPI), - byte(Instruction::PUSH1), 0x2c, - byte(Instruction::JUMP), - byte(Instruction::JUMPDEST), byte(Instruction::INVALID), byte(Instruction::JUMPDEST), byte(Instruction::DIV), -- cgit From c2b3d8bcd28a3047a832cf813df14a97a5b01daa Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Jan 2017 15:57:49 +0100 Subject: Change code for INVALID opcode to 0xfe. --- libevmasm/Instruction.cpp | 4 ++-- libevmasm/Instruction.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/libevmasm/Instruction.cpp b/libevmasm/Instruction.cpp index ea5b5a10..b0f063da 100644 --- a/libevmasm/Instruction.cpp +++ b/libevmasm/Instruction.cpp @@ -154,12 +154,12 @@ const std::map dev::solidity::c_instructions = { "LOG2", Instruction::LOG2 }, { "LOG3", Instruction::LOG3 }, { "LOG4", Instruction::LOG4 }, - { "INVALID", Instruction::INVALID }, { "CREATE", Instruction::CREATE }, { "CALL", Instruction::CALL }, { "CALLCODE", Instruction::CALLCODE }, { "RETURN", Instruction::RETURN }, { "DELEGATECALL", Instruction::DELEGATECALL }, + { "INVALID", Instruction::INVALID }, { "SUICIDE", Instruction::SUICIDE } }; @@ -289,12 +289,12 @@ static const std::map c_instructionInfo = { Instruction::LOG2, { "LOG2", 0, 4, 0, true, Tier::Special } }, { Instruction::LOG3, { "LOG3", 0, 5, 0, true, Tier::Special } }, { Instruction::LOG4, { "LOG4", 0, 6, 0, true, Tier::Special } }, - { Instruction::INVALID, { "INVALID", 0, 0, 0, true, Tier::Zero } }, { Instruction::CREATE, { "CREATE", 0, 3, 1, true, Tier::Special } }, { Instruction::CALL, { "CALL", 0, 7, 1, true, Tier::Special } }, { 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 7432f04d..a8a72234 100644 --- a/libevmasm/Instruction.h +++ b/libevmasm/Instruction.h @@ -171,13 +171,13 @@ enum class Instruction: uint8_t LOG3, ///< Makes a log entry; 3 topics. LOG4, ///< Makes a log entry; 4 topics. - INVALID = 0xef, ///< invalid instruction for expressing runtime errors (e.g., division-by-zero) - CREATE = 0xf0, ///< create a new account with associated code CALL, ///< message-call into an account 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 }; -- cgit From ae2b59d18a181bfbcb563b91866611d2e5e55b41 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Jan 2017 15:59:29 +0100 Subject: Fix optimizer with regards to INVALID instruction. --- libevmasm/PeepholeOptimiser.cpp | 1 + libevmasm/SemanticInformation.cpp | 1 + 2 files changed, 2 insertions(+) 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; -- cgit From 390bebaaf9e2af51c7e2f72337d1e7b23f51486a Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Jan 2017 15:59:48 +0100 Subject: Split line. --- libsolidity/codegen/CompilerContext.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 3bb6c953..7577a606 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -224,7 +224,9 @@ CompilerContext& CompilerContext::appendConditionalInvalid() { *this << Instruction::ISZERO; eth::AssemblyItem afterTag = appendConditionalJump(); - return *this << Instruction::INVALID << afterTag; + *this << Instruction::INVALID; + *this << afterTag; + return *this; } void CompilerContext::resetVisitedNodes(ASTNode const* _node) -- cgit From d9fbb83861153499b4aec5525db85ec59445abd1 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Jan 2017 16:35:51 +0100 Subject: Allow inserting low-level functions without calling them. --- libsolidity/codegen/CompilerContext.cpp | 21 ++++++++++++++++----- libsolidity/codegen/CompilerContext.h | 10 ++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 7577a606..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 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() diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index 58d6cb2a..c37142c9 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -104,6 +104,16 @@ public: unsigned _outArgs, std::function 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 const& _generator + ); /// Generates the code for missing low-level functions, i.e. calls the generators passed above. void appendMissingLowLevelFunctions(); -- cgit From a98fa41897950b84b0217c9ce3c79c20009d0c8d Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Jan 2017 16:24:03 +0100 Subject: Uninitialized internal function should call INVALID. --- libsolidity/codegen/CompilerUtils.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp index 67877bbf..477f021a 100644 --- a/libsolidity/codegen/CompilerUtils.cpp +++ b/libsolidity/codegen/CompilerUtils.cpp @@ -807,7 +807,9 @@ void CompilerUtils::pushZeroValue(Type const& _type) { if (funType->location() == FunctionType::Location::Internal) { - m_context.appendInvalid(); + m_context << m_context.lowLevelFunctionTag("$invalidFunction", 0, 0, [](CompilerContext& _context) { + _context.appendInvalid(); + }); return; } } -- cgit From 7660736aa269c69a69ef728924f566d72661638a Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 26 Jan 2017 16:28:44 +0100 Subject: Document special case of zero-initialized internal function. --- docs/control-structures.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/control-structures.rst b/docs/control-structures.rst index a398d857..d7005717 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 a user-provided exception is thrown. In contrast, it performs an invalid (i.e., non-existent) operation 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. +Internally, Solidity performs an "invalid jump" when a user-provided exception is thrown. In contrast, it performs an invalid operation +(code ``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 -- cgit From bff8fc23e6cc602511b52aaa665e63b948eba068 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 27 Jan 2017 10:18:53 +0100 Subject: Changelog and review suggestions. --- Changelog.md | 2 ++ docs/control-structures.rst | 2 +- libsolidity/codegen/ContractCompiler.cpp | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Changelog.md b/Changelog.md index 71a1e096..c87d1647 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 d7005717..ff9b245a 100644 --- a/docs/control-structures.rst +++ b/docs/control-structures.rst @@ -397,7 +397,7 @@ Currently, Solidity automatically generates a runtime exception in the following #. If you call a zero-initialized variable of internal function type. Internally, Solidity performs an "invalid jump" when a user-provided exception is thrown. In contrast, it performs an invalid operation -(code ``0xfe``) if a runtime exception is encountered. In both cases, this causes +(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. diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 3ca2f375..4d33927d 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -919,7 +919,7 @@ eth::AssemblyPointer ContractCompiler::cloneRuntime() //Propagate error condition (if DELEGATECALL pushes 0 on stack). a << Instruction::ISZERO; a << Instruction::ISZERO; - eth::AssemblyItem afterTag = a.appendJumpI(); + 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; -- cgit