diff options
author | Valentin Wüstholz <wuestholz@gmail.com> | 2017-01-23 17:46:50 +0800 |
---|---|---|
committer | chriseth <c@ethdev.com> | 2017-01-26 23:39:07 +0800 |
commit | 5b7cc018f0b256fb42f7bee38ad8d1ec4e2ec634 (patch) | |
tree | 95f65ad06ee940ad3db7a73d6c4cb29c603cbe3c | |
parent | 9bcbd93ac59a19320fd56e27c58a6283f2450666 (diff) | |
download | dexon-solidity-5b7cc018f0b256fb42f7bee38ad8d1ec4e2ec634.tar.gz dexon-solidity-5b7cc018f0b256fb42f7bee38ad8d1ec4e2ec634.tar.zst dexon-solidity-5b7cc018f0b256fb42f7bee38ad8d1ec4e2ec634.zip |
Address feedback from code review.
-rw-r--r-- | docs/control-structures.rst | 2 | ||||
-rw-r--r-- | libsolidity/codegen/CompilerContext.cpp | 6 | ||||
-rw-r--r-- | libsolidity/codegen/ContractCompiler.cpp | 6 | ||||
-rw-r--r-- | test/libsolidity/Assembly.cpp | 2 | ||||
-rw-r--r-- | 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<eth::Assembly>(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<SourceLocation> locations = vector<SourceLocation>(17, SourceLocation(2, 75, n)) + - vector<SourceLocation>(32, SourceLocation(20, 72, 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 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), |