diff options
| -rw-r--r-- | docs/contracts.rst | 40 | ||||
| -rw-r--r-- | docs/types.rst | 2 | ||||
| -rw-r--r-- | libsolidity/codegen/ContractCompiler.cpp | 73 | ||||
| -rw-r--r-- | libsolidity/codegen/ContractCompiler.h | 7 | ||||
| -rw-r--r-- | libsolidity/codegen/ExpressionCompiler.cpp | 13 | ||||
| -rw-r--r-- | solc/CommandLineInterface.cpp | 28 | ||||
| -rw-r--r-- | test/libsolidity/SolidityEndToEndTest.cpp | 157 | ||||
| -rw-r--r-- | test/libsolidity/SolidityExpressionCompiler.cpp | 8 |
8 files changed, 270 insertions, 58 deletions
diff --git a/docs/contracts.rst b/docs/contracts.rst index 3d592ecf..5f370951 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -314,14 +314,40 @@ inheritable properties of contracts and may be overridden by derived contracts. } } + contract Mutex { + bool locked; + modifier noReentrancy() { + if (locked) throw; + locked = true; + _ + locked = false; + } + + /// This function is protected by a mutex, which means that + /// reentrant calls from within msg.sender.call cannot call f again. + /// The `return 7` statement assigns 7 to the return value but still + /// executes the statement `locked = false` in the modifier. + function f() noReentrancy returns (uint) { + if (!msg.sender.call()) throw; + return 7; + } + } + Multiple modifiers can be applied to a function by specifying them in a -whitespace-separated list and will be evaluated in order. Explicit returns from -a modifier or function body immediately leave the whole function, while control -flow reaching the end of a function or modifier body continues after the "_" in -the preceding modifier. Arbitrary expressions are allowed for modifier -arguments and in this context, all symbols visible from the function are -visible in the modifier. Symbols introduced in the modifier are not visible in -the function (as they might change by overriding). +whitespace-separated list and will be evaluated in order. + +.. warning:: + In an earlier version of Solidity, ``return`` statements in functions + having modifiers behaved differently. + +Explicit returns from a modifier or function body only leave the current +modifier or function body. Return variables are assigned and +control flow continues after the "_" in the preceding modifier. + +Arbitrary expressions are allowed for modifier arguments and in this context, +all symbols visible from the function are visible in the modifier. Symbols +introduced in the modifier are not visible in the function (as they might +change by overriding). .. index:: ! constant diff --git a/docs/types.rst b/docs/types.rst index 31f6b53d..d6445ed9 100644 --- a/docs/types.rst +++ b/docs/types.rst @@ -57,6 +57,8 @@ Operators: Division always truncates (it just maps to the DIV opcode of the EVM), but it does not truncate if both operators are :ref:`literals<rational_literals>` (or literal expressions). +Division by zero and modulus with zero throws an exception. + .. index:: address, balance, send, call, callcode, delegatecall .. _address: diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index bcfd33f2..715852be 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -431,16 +431,16 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) if (auto c = m_context.nextConstructor(dynamic_cast<ContractDefinition const&>(*_function.scope()))) appendBaseConstructor(*c); - m_returnTag = m_context.newTag(); + solAssert(m_returnTags.empty(), ""); m_breakTags.clear(); m_continueTags.clear(); m_stackCleanupForReturn = 0; m_currentFunction = &_function; - m_modifierDepth = 0; + m_modifierDepth = -1; appendModifierOrFunctionCode(); - m_context << m_returnTag; + solAssert(m_returnTags.empty(), ""); // Now we need to re-shuffle the stack. For this we keep a record of the stack layout // that shows the target positions of the elements, where "-1" denotes that this element needs @@ -695,7 +695,7 @@ bool ContractCompiler::visit(Return const& _return) } for (unsigned i = 0; i < m_stackCleanupForReturn; ++i) m_context << Instruction::POP; - m_context.appendJumpTo(m_returnTag); + m_context.appendJumpTo(m_returnTags.back()); m_context.adjustStackOffset(m_stackCleanupForReturn); return false; } @@ -755,9 +755,7 @@ bool ContractCompiler::visit(PlaceholderStatement const& _placeholderStatement) { StackHeightChecker checker(m_context); CompilerContext::LocationSetter locationSetter(m_context, _placeholderStatement); - ++m_modifierDepth; appendModifierOrFunctionCode(); - --m_modifierDepth; checker.check(); return true; } @@ -775,10 +773,15 @@ void ContractCompiler::appendMissingFunctions() void ContractCompiler::appendModifierOrFunctionCode() { solAssert(m_currentFunction, ""); + unsigned stackSurplus = 0; + Block const* codeBlock = nullptr; + + m_modifierDepth++; + if (m_modifierDepth >= m_currentFunction->modifiers().size()) { solAssert(m_currentFunction->isImplemented(), ""); - m_currentFunction->body().accept(*this); + codeBlock = &m_currentFunction->body(); } else { @@ -786,37 +789,45 @@ void ContractCompiler::appendModifierOrFunctionCode() // constructor call should be excluded if (dynamic_cast<ContractDefinition const*>(modifierInvocation->name()->annotation().referencedDeclaration)) - { - ++m_modifierDepth; appendModifierOrFunctionCode(); - --m_modifierDepth; - return; - } - - ModifierDefinition const& modifier = m_context.functionModifier(modifierInvocation->name()->name()); - CompilerContext::LocationSetter locationSetter(m_context, modifier); - solAssert(modifier.parameters().size() == modifierInvocation->arguments().size(), ""); - for (unsigned i = 0; i < modifier.parameters().size(); ++i) + else { - m_context.addVariable(*modifier.parameters()[i]); - compileExpression( - *modifierInvocation->arguments()[i], - modifier.parameters()[i]->annotation().type - ); + ModifierDefinition const& modifier = m_context.functionModifier(modifierInvocation->name()->name()); + CompilerContext::LocationSetter locationSetter(m_context, modifier); + solAssert(modifier.parameters().size() == modifierInvocation->arguments().size(), ""); + for (unsigned i = 0; i < modifier.parameters().size(); ++i) + { + m_context.addVariable(*modifier.parameters()[i]); + compileExpression( + *modifierInvocation->arguments()[i], + modifier.parameters()[i]->annotation().type + ); + } + for (VariableDeclaration const* localVariable: modifier.localVariables()) + appendStackVariableInitialisation(*localVariable); + + stackSurplus = + CompilerUtils::sizeOnStack(modifier.parameters()) + + CompilerUtils::sizeOnStack(modifier.localVariables()); + codeBlock = &modifier.body(); + + codeBlock = &modifier.body(); } - for (VariableDeclaration const* localVariable: modifier.localVariables()) - appendStackVariableInitialisation(*localVariable); + } + + if (codeBlock) + { + m_returnTags.push_back(m_context.newTag()); - unsigned const c_stackSurplus = CompilerUtils::sizeOnStack(modifier.parameters()) + - CompilerUtils::sizeOnStack(modifier.localVariables()); - m_stackCleanupForReturn += c_stackSurplus; + codeBlock->accept(*this); - modifier.body().accept(*this); + solAssert(!m_returnTags.empty(), ""); + m_context << m_returnTags.back(); + m_returnTags.pop_back(); - for (unsigned i = 0; i < c_stackSurplus; ++i) - m_context << Instruction::POP; - m_stackCleanupForReturn -= c_stackSurplus; + CompilerUtils(m_context).popStackSlots(stackSurplus); } + m_modifierDepth--; } void ContractCompiler::appendStackVariableInitialisation(VariableDeclaration const& _variable) diff --git a/libsolidity/codegen/ContractCompiler.h b/libsolidity/codegen/ContractCompiler.h index d1517e88..0799a543 100644 --- a/libsolidity/codegen/ContractCompiler.h +++ b/libsolidity/codegen/ContractCompiler.h @@ -40,11 +40,9 @@ class ContractCompiler: private ASTConstVisitor public: explicit ContractCompiler(CompilerContext& _context, bool _optimise): m_optimise(_optimise), - m_context(_context), - m_returnTag(eth::Tag, u256(-1)) + m_context(_context) { m_context = CompilerContext(); - m_returnTag = m_context.newTag(); } void compileContract( @@ -122,7 +120,8 @@ private: CompilerContext& m_context; std::vector<eth::AssemblyItem> m_breakTags; ///< tag to jump to for a "break" statement std::vector<eth::AssemblyItem> m_continueTags; ///< tag to jump to for a "continue" statement - eth::AssemblyItem m_returnTag; ///< tag to jump to for a "return" statement + /// Tag to jump to for a "return" statement, needs to be stacked because of modifiers. + std::vector<eth::AssemblyItem> m_returnTags; unsigned m_modifierDepth = 0; FunctionDefinition const* m_currentFunction = nullptr; unsigned m_stackCleanupForReturn = 0; ///< this number of stack elements need to be removed before jump to m_returnTag diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp index e8350931..4a81e27d 100644 --- a/libsolidity/codegen/ExpressionCompiler.cpp +++ b/libsolidity/codegen/ExpressionCompiler.cpp @@ -1324,11 +1324,18 @@ void ExpressionCompiler::appendArithmeticOperatorCode(Token::Value _operator, Ty m_context << Instruction::MUL; break; case Token::Div: - m_context << (c_isSigned ? Instruction::SDIV : Instruction::DIV); - break; case Token::Mod: - m_context << (c_isSigned ? Instruction::SMOD : Instruction::MOD); + { + // Test for division by zero + m_context << Instruction::DUP2 << Instruction::ISZERO; + m_context.appendConditionalJumpTo(m_context.errorTag()); + + if (_operator == Token::Div) + m_context << (c_isSigned ? Instruction::SDIV : Instruction::DIV); + else + m_context << (c_isSigned ? Instruction::SMOD : Instruction::MOD); break; + } case Token::Exp: m_context << Instruction::EXP; break; diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp index ec87b891..d1834794 100644 --- a/solc/CommandLineInterface.cpp +++ b/solc/CommandLineInterface.cpp @@ -310,21 +310,18 @@ void CommandLineInterface::handleFormal() void CommandLineInterface::readInputFilesAndConfigureRemappings() { + vector<string> inputFiles; + bool addStdin = false; if (!m_args.count("input-file")) - { - string s; - while (!cin.eof()) - { - getline(cin, s); - m_sourceCodes[g_stdinFileName].append(s + '\n'); - } - } + addStdin = true; else for (string path: m_args["input-file"].as<vector<string>>()) { auto eq = find(path.begin(), path.end(), '='); if (eq != path.end()) path = string(eq + 1, path.end()); + else if (path == "-") + addStdin = true; else { auto infile = boost::filesystem::path(path); @@ -345,6 +342,15 @@ void CommandLineInterface::readInputFilesAndConfigureRemappings() } m_allowedDirectories.push_back(boost::filesystem::path(path).remove_filename()); } + if (addStdin) + { + string s; + while (!cin.eof()) + { + getline(cin, s); + m_sourceCodes[g_stdinFileName].append(s + '\n'); + } + } } bool CommandLineInterface::parseLibraryOption(string const& _input) @@ -399,9 +405,9 @@ bool CommandLineInterface::parseArguments(int _argc, char** _argv) po::options_description desc( R"(solc, the Solidity commandline compiler. Usage: solc [options] [input_file...] -Compiles the given Solidity input files (or the standard input if none given) and -outputs the components specified in the options at standard output or in files in -the output directory, if specified. +Compiles the given Solidity input files (or the standard input if none given or +"-" is used as a file name) and outputs the components specified in the options +at standard output or in files in the output directory, if specified. Example: solc --bin -o /tmp/solcoutput contract.sol Allowed options)", diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 8aab3cb9..4e6f68b0 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -2390,7 +2390,8 @@ BOOST_AUTO_TEST_CASE(function_modifier_multi_invocation) BOOST_AUTO_TEST_CASE(function_modifier_multi_with_return) { - // Here, the explicit return prevents the second execution + // Note that return sets the return variable and jumps to the end of the current function or + // modifier code block. char const* sourceCode = R"( contract C { modifier repeat(bool twice) { if (twice) _ _ } @@ -2399,7 +2400,7 @@ BOOST_AUTO_TEST_CASE(function_modifier_multi_with_return) )"; compileAndRun(sourceCode); BOOST_CHECK(callContractFunction("f(bool)", false) == encodeArgs(1)); - BOOST_CHECK(callContractFunction("f(bool)", true) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("f(bool)", true) == encodeArgs(2)); } BOOST_AUTO_TEST_CASE(function_modifier_overriding) @@ -6208,6 +6209,27 @@ BOOST_AUTO_TEST_CASE(addmod_mulmod) BOOST_CHECK(callContractFunction("test()") == encodeArgs(u256(0))); } +BOOST_AUTO_TEST_CASE(divisiod_by_zero) +{ + char const* sourceCode = R"( + contract C { + function div(uint a, uint b) returns (uint) { + return a / b; + } + function mod(uint a, uint b) returns (uint) { + return a % b; + } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("div(uint256,uint256)", 7, 2) == encodeArgs(u256(3))); + // throws + BOOST_CHECK(callContractFunction("div(uint256,uint256)", 7, 0) == encodeArgs()); + BOOST_CHECK(callContractFunction("mod(uint256,uint256)", 7, 2) == encodeArgs(u256(1))); + // throws + BOOST_CHECK(callContractFunction("mod(uint256,uint256)", 7, 0) == encodeArgs()); +} + BOOST_AUTO_TEST_CASE(string_allocation_bug) { char const* sourceCode = R"( @@ -6880,6 +6902,137 @@ BOOST_AUTO_TEST_CASE(create_dynamic_array_with_zero_length) BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(7))); } +BOOST_AUTO_TEST_CASE(return_does_not_skip_modifier) +{ + char const* sourceCode = R"( + contract C { + uint public x; + modifier setsx { + _ + x = 9; + } + function f() setsx returns (uint) { + return 2; + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(0))); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(2))); + BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(9))); +} + +BOOST_AUTO_TEST_CASE(break_in_modifier) +{ + char const* sourceCode = R"( + contract C { + uint public x; + modifier run() { + for (uint i = 0; i < 10; i++) { + _ + break; + } + } + function f() run { + x++; + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(0))); + BOOST_CHECK(callContractFunction("f()") == encodeArgs()); + BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(1))); +} + +BOOST_AUTO_TEST_CASE(stacked_return_with_modifiers) +{ + char const* sourceCode = R"( + contract C { + uint public x; + modifier run() { + for (uint i = 0; i < 10; i++) { + _ + break; + } + } + function f() run { + x++; + } + } + )"; + compileAndRun(sourceCode, 0, "C"); + BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(0))); + BOOST_CHECK(callContractFunction("f()") == encodeArgs()); + BOOST_CHECK(callContractFunction("x()") == encodeArgs(u256(1))); +} + +BOOST_AUTO_TEST_CASE(mutex) +{ + char const* sourceCode = R"( + contract mutexed { + bool locked; + modifier protected { + if (locked) throw; + locked = true; + _ + locked = false; + } + } + contract Fund is mutexed { + uint shares; + function Fund() { shares = msg.value; } + function withdraw(uint amount) protected returns (uint) { + // NOTE: It is very bad practice to write this function this way. + // Please refer to the documentation of how to do this properly. + if (amount > shares) throw; + if (!msg.sender.call.value(amount)()) throw; + shares -= amount; + return shares; + } + function withdrawUnprotected(uint amount) returns (uint) { + // NOTE: It is very bad practice to write this function this way. + // Please refer to the documentation of how to do this properly. + if (amount > shares) throw; + if (!msg.sender.call.value(amount)()) throw; + shares -= amount; + return shares; + } + } + contract Attacker { + Fund public fund; + uint callDepth; + bool protected; + function setProtected(bool _protected) { protected = _protected; } + function Attacker(Fund _fund) { fund = _fund; } + function attack() returns (uint) { + callDepth = 0; + return attackInternal(); + } + function attackInternal() internal returns (uint) { + if (protected) + return fund.withdraw(10); + else + return fund.withdrawUnprotected(10); + } + function() { + callDepth++; + if (callDepth < 4) + attackInternal(); + } + } + )"; + compileAndRun(sourceCode, 500, "Fund"); + auto fund = m_contractAddress; + BOOST_CHECK_EQUAL(balanceAt(fund), 500); + compileAndRun(sourceCode, 0, "Attacker", encodeArgs(u160(fund))); + BOOST_CHECK(callContractFunction("setProtected(bool)", true) == encodeArgs()); + BOOST_CHECK(callContractFunction("attack()") == encodeArgs()); + BOOST_CHECK_EQUAL(balanceAt(fund), 500); + BOOST_CHECK(callContractFunction("setProtected(bool)", false) == encodeArgs()); + BOOST_CHECK(callContractFunction("attack()") == encodeArgs(u256(460))); + BOOST_CHECK_EQUAL(balanceAt(fund), 460); +} + BOOST_AUTO_TEST_CASE(failing_ecrecover_invalid_input) { // ecrecover should return zero for malformed input diff --git a/test/libsolidity/SolidityExpressionCompiler.cpp b/test/libsolidity/SolidityExpressionCompiler.cpp index 967b2907..e9a05745 100644 --- a/test/libsolidity/SolidityExpressionCompiler.cpp +++ b/test/libsolidity/SolidityExpressionCompiler.cpp @@ -323,7 +323,15 @@ BOOST_AUTO_TEST_CASE(arithmetics) byte(Instruction::OR), byte(Instruction::SUB), byte(Instruction::ADD), + byte(Instruction::DUP2), + byte(Instruction::ISZERO), + byte(Instruction::PUSH1), 0x2, + byte(Instruction::JUMPI), byte(Instruction::MOD), + byte(Instruction::DUP2), + byte(Instruction::ISZERO), + byte(Instruction::PUSH1), 0x2, + byte(Instruction::JUMPI), byte(Instruction::DIV), byte(Instruction::MUL)}); BOOST_CHECK_EQUAL_COLLECTIONS(code.begin(), code.end(), expectation.begin(), expectation.end()); |