From fdf791149d5150d1cbab01f32aba6c7460e0785b Mon Sep 17 00:00:00 2001 From: Christian Date: Fri, 23 Jan 2015 02:35:27 +0100 Subject: [PATCH] Compilation of function modifiers. --- libsolidity/AST.cpp | 5 +- libsolidity/AST.h | 8 +-- libsolidity/Compiler.cpp | 80 ++++++++++++++++++++------ libsolidity/Compiler.h | 13 ++++- libsolidity/CompilerContext.cpp | 16 +++--- libsolidity/CompilerContext.h | 8 +-- libsolidity/NameAndTypeResolver.cpp | 3 +- test/SolidityEndToEndTest.cpp | 70 ++++++++++++++++++---- test/SolidityExpressionCompiler.cpp | 5 +- test/SolidityNameAndTypeResolution.cpp | 11 ++++ 10 files changed, 167 insertions(+), 52 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 85d7db6ec..cc7da7154 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -224,7 +224,7 @@ string FunctionDefinition::getCanonicalSignature() const Declaration::LValueType VariableDeclaration::getLValueType() const { - if (dynamic_cast(getScope())) + if (dynamic_cast(getScope()) || dynamic_cast(getScope())) return Declaration::LValueType::LOCAL; else return Declaration::LValueType::STORAGE; @@ -291,7 +291,8 @@ void Return::checkTypeRequirements() { if (!m_expression) return; - solAssert(m_returnParameters, "Return parameters not assigned."); + if (!m_returnParameters) + BOOST_THROW_EXCEPTION(createTypeError("Return arguments not allowed.")); if (m_returnParameters->getParameters().size() != 1) BOOST_THROW_EXCEPTION(createTypeError("Different number of arguments in return statement " "than in returns declaration.")); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index ded30e9ce..e047f9761 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -722,12 +722,8 @@ public: virtual void accept(ASTConstVisitor& _visitor) const override; virtual void checkTypeRequirements() override; - void setFunctionReturnParameters(ParameterList const& _parameters) { m_returnParameters = &_parameters; } - ParameterList const& getFunctionReturnParameters() const - { - solAssert(m_returnParameters, ""); - return *m_returnParameters; - } + void setFunctionReturnParameters(ParameterList const* _parameters) { m_returnParameters = _parameters; } + ParameterList const* getFunctionReturnParameters() const { return m_returnParameters; } Expression const* getExpression() const { return m_expression.get(); } private: diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 5a434a71b..fa8eb7758 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -77,6 +77,7 @@ void Compiler::packIntoContractCreator(ContractDefinition const& _contract, Comp std::vector const& bases = _contract.getLinearizedBaseContracts(); for (ContractDefinition const* contract: bases) { + //TODO include modifiers if (FunctionDefinition const* constructor = contract->getConstructor()) nodesUsedInConstructors.insert(constructor); for (ASTPointer const& base: contract->getBaseContracts()) @@ -150,11 +151,7 @@ void Compiler::appendBaseConstructorCall(FunctionDefinition const& _constructor, FunctionType constructorType(_constructor); eth::AssemblyItem returnLabel = m_context.pushNewTag(); for (unsigned i = 0; i < _arguments.size(); ++i) - { - compileExpression(*_arguments[i]); - ExpressionCompiler::appendTypeConversion(m_context, *_arguments[i]->getType(), - *constructorType.getParameterTypes()[i]); - } + compileExpression(*_arguments[i], constructorType.getParameterTypes()[i]); m_context.appendJumpTo(m_context.getFunctionEntryLabel(_constructor)); m_context << returnLabel; } @@ -280,20 +277,28 @@ bool Compiler::visit(FunctionDefinition const& _function) m_returnTag = m_context.newTag(); m_breakTags.clear(); m_continueTags.clear(); + m_stackCleanupForReturn = 0; + m_currentFunction = &_function; + m_modifierDepth = 0; m_context << m_context.getFunctionEntryLabel(_function); // stack upon entry: [return address] [arg0] [arg1] ... [argn] // reserve additional slots: [retarg0] ... [retargm] [localvar0] ... [localvarp] + unsigned parametersSize = CompilerUtils::getSizeOnStack(_function.getParameters()); + m_context.adjustStackOffset(parametersSize); for (ASTPointer const& variable: _function.getParameters()) - m_context.addVariable(*variable); + { + m_context.addVariable(*variable, parametersSize); + parametersSize -= variable->getType()->getSizeOnStack(); + } for (ASTPointer const& variable: _function.getReturnParameters()) m_context.addAndInitializeVariable(*variable); for (VariableDeclaration const* localVariable: _function.getLocalVariables()) m_context.addAndInitializeVariable(*localVariable); - _function.getBody().accept(*this); + appendModifierOrFunctionCode(); m_context << m_returnTag; @@ -420,13 +425,15 @@ bool Compiler::visit(Return const& _return) //@todo modifications are needed to make this work with functions returning multiple values if (Expression const* expression = _return.getExpression()) { - compileExpression(*expression); - VariableDeclaration const& firstVariable = *_return.getFunctionReturnParameters().getParameters().front(); - ExpressionCompiler::appendTypeConversion(m_context, *expression->getType(), *firstVariable.getType()); - + solAssert(_return.getFunctionReturnParameters(), "Invalid return parameters pointer."); + VariableDeclaration const& firstVariable = *_return.getFunctionReturnParameters()->getParameters().front(); + compileExpression(*expression, firstVariable.getType()); CompilerUtils(m_context).moveToStackVariable(firstVariable); } + for (unsigned i = 0; i < m_stackCleanupForReturn; ++i) + m_context << eth::Instruction::POP; m_context.appendJumpTo(m_returnTag); + m_context.adjustStackOffset(m_stackCleanupForReturn); return false; } @@ -434,10 +441,7 @@ bool Compiler::visit(VariableDefinition const& _variableDefinition) { if (Expression const* expression = _variableDefinition.getExpression()) { - compileExpression(*expression); - ExpressionCompiler::appendTypeConversion(m_context, - *expression->getType(), - *_variableDefinition.getDeclaration().getType()); + compileExpression(*expression, _variableDefinition.getDeclaration().getType()); CompilerUtils(m_context).moveToStackVariable(_variableDefinition.getDeclaration()); } return false; @@ -451,9 +455,53 @@ bool Compiler::visit(ExpressionStatement const& _expressionStatement) return false; } -void Compiler::compileExpression(Expression const& _expression) +bool Compiler::visit(PlaceholderStatement const&) +{ + ++m_modifierDepth; + appendModifierOrFunctionCode(); + --m_modifierDepth; +} + +void Compiler::appendModifierOrFunctionCode() +{ + solAssert(m_currentFunction, ""); + if (m_modifierDepth >= m_currentFunction->getModifiers().size()) + m_currentFunction->getBody().accept(*this); + else + { + ASTPointer const& modifierInvocation = m_currentFunction->getModifiers()[m_modifierDepth]; + + // TODO get the most derived override of the modifier + ModifierDefinition const* modifier = dynamic_cast( + modifierInvocation->getName()->getReferencedDeclaration()); + solAssert(!!modifier, "Modifier not found."); + solAssert(modifier->getParameters().size() == modifierInvocation->getArguments().size(), ""); + for (unsigned i = 0; i < modifier->getParameters().size(); ++i) + { + m_context.addVariable(*modifier->getParameters()[i]); + compileExpression(*modifierInvocation->getArguments()[i], + modifier->getParameters()[i]->getType()); + } + for (VariableDeclaration const* localVariable: modifier->getLocalVariables()) + m_context.addAndInitializeVariable(*localVariable); + + unsigned const c_stackSurplus = CompilerUtils::getSizeOnStack(modifier->getParameters()) + + CompilerUtils::getSizeOnStack(modifier->getLocalVariables()); + m_stackCleanupForReturn += c_stackSurplus; + + modifier->getBody().accept(*this); + + for (unsigned i = 0; i < c_stackSurplus; ++i) + m_context << eth::Instruction::POP; + m_stackCleanupForReturn -= c_stackSurplus; + } +} + +void Compiler::compileExpression(Expression const& _expression, TypePointer const& _targetType) { ExpressionCompiler::compileExpression(m_context, _expression, m_optimize); + if (_targetType) + ExpressionCompiler::appendTypeConversion(m_context, *_expression.getType(), *_targetType); } } diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index 2bae6b397..83bee904a 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -31,7 +31,8 @@ namespace solidity { class Compiler: private ASTConstVisitor { public: - explicit Compiler(bool _optimize = false): m_optimize(_optimize), m_context(), m_returnTag(m_context.newTag()) {} + explicit Compiler(bool _optimize = false): m_optimize(_optimize), m_context(), + m_returnTag(m_context.newTag()) {} void compileContract(ContractDefinition const& _contract, std::map const& _contracts); @@ -70,8 +71,13 @@ private: virtual bool visit(Return const& _return) override; virtual bool visit(VariableDefinition const& _variableDefinition) override; virtual bool visit(ExpressionStatement const& _expressionStatement) override; + virtual bool visit(PlaceholderStatement const&) override; - void compileExpression(Expression const& _expression); + /// Appends one layer of function modifier code of the current function, or the function + /// body itself if the last modifier was reached. + void appendModifierOrFunctionCode(); + + void compileExpression(Expression const& _expression, TypePointer const& _targetType = TypePointer()); bool const m_optimize; CompilerContext m_context; @@ -79,6 +85,9 @@ private: std::vector m_breakTags; ///< tag to jump to for a "break" statement std::vector m_continueTags; ///< tag to jump to for a "continue" statement eth::AssemblyItem m_returnTag; ///< tag to jump to for a "return" statement + unsigned m_modifierDepth = 0; + FunctionDefinition const* m_currentFunction; + unsigned m_stackCleanupForReturn; ///< this number of stack elements need to be removed before jump to m_returnTag }; } diff --git a/libsolidity/CompilerContext.cpp b/libsolidity/CompilerContext.cpp index 27ec3efd5..96d1840e5 100644 --- a/libsolidity/CompilerContext.cpp +++ b/libsolidity/CompilerContext.cpp @@ -43,10 +43,11 @@ void CompilerContext::addStateVariable(VariableDeclaration const& _declaration) m_stateVariablesSize += _declaration.getType()->getStorageSize(); } -void CompilerContext::addVariable(VariableDeclaration const& _declaration) +void CompilerContext::addVariable(VariableDeclaration const& _declaration, + unsigned _offsetToCurrent) { - m_localVariables[&_declaration] = m_localVariablesSize; - m_localVariablesSize += _declaration.getType()->getSizeOnStack(); + solAssert(m_asm.deposit() >= 0 && unsigned(m_asm.deposit()) >= _offsetToCurrent, ""); + m_localVariables[&_declaration] = unsigned(m_asm.deposit()) - _offsetToCurrent; } void CompilerContext::addAndInitializeVariable(VariableDeclaration const& _declaration) @@ -56,7 +57,6 @@ void CompilerContext::addAndInitializeVariable(VariableDeclaration const& _decla int const size = _declaration.getType()->getSizeOnStack(); for (int i = 0; i < size; ++i) *this << u256(0); - m_asm.adjustDeposit(-size); } void CompilerContext::addFunction(FunctionDefinition const& _function) @@ -75,7 +75,7 @@ bytes const& CompilerContext::getCompiledContract(const ContractDefinition& _con bool CompilerContext::isLocalVariable(Declaration const* _declaration) const { - return m_localVariables.count(_declaration) > 0; + return m_localVariables.count(_declaration); } eth::AssemblyItem CompilerContext::getFunctionEntryLabel(FunctionDefinition const& _function) const @@ -96,17 +96,17 @@ unsigned CompilerContext::getBaseStackOffsetOfVariable(Declaration const& _decla { auto res = m_localVariables.find(&_declaration); solAssert(res != m_localVariables.end(), "Variable not found on stack."); - return m_localVariablesSize - res->second - 1; + return res->second; } unsigned CompilerContext::baseToCurrentStackOffset(unsigned _baseOffset) const { - return _baseOffset + m_asm.deposit(); + return m_asm.deposit() - _baseOffset - 1; } unsigned CompilerContext::currentToBaseStackOffset(unsigned _offset) const { - return -baseToCurrentStackOffset(-_offset); + return m_asm.deposit() - _offset - 1; } u256 CompilerContext::getStorageLocationOfVariable(const Declaration& _declaration) const diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index cde992d58..39d8c6f6b 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -42,7 +42,7 @@ public: void addMagicGlobal(MagicVariableDeclaration const& _declaration); void addStateVariable(VariableDeclaration const& _declaration); void startNewFunction() { m_localVariables.clear(); m_asm.setDeposit(0); } - void addVariable(VariableDeclaration const& _declaration); + void addVariable(VariableDeclaration const& _declaration, unsigned _offsetToCurrent = 0); void addAndInitializeVariable(VariableDeclaration const& _declaration); void addFunction(FunctionDefinition const& _function); @@ -59,7 +59,7 @@ public: eth::AssemblyItem getFunctionEntryLabel(FunctionDefinition const& _function) const; /// @returns the entry label of the given function and takes overrides into account. eth::AssemblyItem getVirtualFunctionEntryLabel(FunctionDefinition const& _function) const; - /// Returns the distance of the given local variable from the top of the local variable stack. + /// Returns the distance of the given local variable from the bottom of the stack (of the current function). unsigned getBaseStackOffsetOfVariable(Declaration const& _declaration) const; /// If supplied by a value returned by @ref getBaseStackOffsetOfVariable(variable), returns /// the distance of that variable from the current top of the stack. @@ -112,10 +112,8 @@ private: u256 m_stateVariablesSize = 0; /// Storage offsets of state variables std::map m_stateVariables; - /// Offsets of local variables on the stack (relative to stack base). + /// Positions of local variables on the stack. std::map m_localVariables; - /// Sum of stack sizes of local variables - unsigned m_localVariablesSize; /// Labels pointing to the entry points of funcitons. std::map m_functionEntryLabels; /// Labels pointing to the entry points of function overrides. diff --git a/libsolidity/NameAndTypeResolver.cpp b/libsolidity/NameAndTypeResolver.cpp index ea3b43c25..4ca48165d 100644 --- a/libsolidity/NameAndTypeResolver.cpp +++ b/libsolidity/NameAndTypeResolver.cpp @@ -310,8 +310,7 @@ void ReferencesResolver::endVisit(VariableDeclaration& _variable) bool ReferencesResolver::visit(Return& _return) { - solAssert(m_returnParameters, "Return parameters not set."); - _return.setFunctionReturnParameters(*m_returnParameters); + _return.setFunctionReturnParameters(m_returnParameters); return true; } diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index 8bce1788a..c25b8ca5d 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -1653,22 +1653,72 @@ BOOST_AUTO_TEST_CASE(constructor_argument_overriding) BOOST_AUTO_TEST_CASE(function_modifier) { char const* sourceCode = R"( - contract BaseBase { - uint m_a; - function BaseBase(uint a) { - m_a = a; - } + contract C { + function getOne() nonFree returns (uint r) { return 1; } + modifier nonFree { if (msg.value > 0) _ } } - contract Base is BaseBase(2) { } - contract Derived is Base, BaseBase(3) { - function getA() returns (uint r) { return m_a; } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("getOne()") == encodeArgs(0)); + BOOST_CHECK(callContractFunctionWithValue("getOne()", 1) == encodeArgs(1)); +} + +BOOST_AUTO_TEST_CASE(function_modifier_local_variables) +{ + char const* sourceCode = R"( + contract C { + modifier mod1 { var a = 1; var b = 2; _ } + modifier mod2(bool a) { if (a) return; else _ } + function f(bool a) mod1 mod2(a) returns (uint r) { return 3; } } )"; - compileAndRun(sourceCode, 0, "Derived"); - BOOST_CHECK(callContractFunction("getA()") == encodeArgs(3)); + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f(bool)", true) == encodeArgs(0)); + BOOST_CHECK(callContractFunction("f(bool)", false) == encodeArgs(3)); } +BOOST_AUTO_TEST_CASE(function_modifier_loop) +{ + char const* sourceCode = R"( + contract C { + modifier repeat(uint count) { for (var i = 0; i < count; ++i) _ } + function f() repeat(10) returns (uint r) { r += 1; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(10)); +} + +BOOST_AUTO_TEST_CASE(function_modifier_multi_invocation) +{ + char const* sourceCode = R"( + contract C { + modifier repeat(bool twice) { if (twice) _ _ } + function f(bool twice) repeat(twice) returns (uint r) { r += 1; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f(bool)", false) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("f(bool)", true) == encodeArgs(2)); +} + +BOOST_AUTO_TEST_CASE(function_modifier_multi_with_return) +{ + // Here, the explicit return prevents the second execution + char const* sourceCode = R"( + contract C { + modifier repeat(bool twice) { if (twice) _ _ } + function f(bool twice) repeat(twice) returns (uint r) { r += 1; return r; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f(bool)", false) == encodeArgs(1)); + BOOST_CHECK(callContractFunction("f(bool)", true) == encodeArgs(1)); +} + + // modifier overriding +// functions called by modifiers used by constructor need to be pulled into the creation context BOOST_AUTO_TEST_SUITE_END() diff --git a/test/SolidityExpressionCompiler.cpp b/test/SolidityExpressionCompiler.cpp index d50dc253e..06c252db3 100644 --- a/test/SolidityExpressionCompiler.cpp +++ b/test/SolidityExpressionCompiler.cpp @@ -118,8 +118,11 @@ bytes compileFirstExpression(const string& _sourceCode, vector> _ CompilerContext context; for (vector const& function: _functions) context.addFunction(dynamic_cast(resolveDeclaration(function, resolver))); + unsigned parametersSize = _localVariables.size(); // assume they are all one slot on the stack + context.adjustStackOffset(parametersSize); for (vector const& variable: _localVariables) - context.addVariable(dynamic_cast(resolveDeclaration(variable, resolver))); + context.addVariable(dynamic_cast(resolveDeclaration(variable, resolver)), + parametersSize--); ExpressionCompiler::compileExpression(context, *extractor.getExpression()); diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index c195539f0..69559b4a1 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -573,6 +573,17 @@ BOOST_AUTO_TEST_CASE(function_overrides_modifier) BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); } +BOOST_AUTO_TEST_CASE(modifier_returns_value) +{ + char const* text = R"( + contract A { + function f(uint a) mod(2) returns (uint r) {} + modifier mod(uint a) { return 7; } + } + )"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); +} + BOOST_AUTO_TEST_SUITE_END() }