From 0407b1c69769b2328c152684ad76cfce9d3dbdd4 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 29 Dec 2021 16:04:37 +0100 Subject: [PATCH] Improved code generation for simple modifiers. --- libsolidity/analysis/SyntaxChecker.cpp | 22 +++++ libsolidity/analysis/SyntaxChecker.h | 4 + libsolidity/ast/ASTAnnotations.h | 3 + libsolidity/codegen/CompilerContext.cpp | 6 ++ libsolidity/codegen/ContractCompiler.cpp | 121 +++++++++++++++++++---- libsolidity/codegen/ContractCompiler.h | 3 + 6 files changed, 142 insertions(+), 17 deletions(-) diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index e47080cb4..e31417643 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -179,6 +179,7 @@ bool SyntaxChecker::visit(PragmaDirective const& _pragma) bool SyntaxChecker::visit(ModifierDefinition const&) { m_placeholderFound = false; + m_modifierIsSimple = true; return true; } @@ -186,6 +187,17 @@ void SyntaxChecker::endVisit(ModifierDefinition const& _modifier) { if (_modifier.isImplemented() && !m_placeholderFound) m_errorReporter.syntaxError(2883_error, _modifier.body().location(), "Modifier body does not contain '_'."); + + if (!m_placeholderFound) + m_modifierIsSimple = false; + if (_modifier.isImplemented() && ( + _modifier.body().statements().empty() || + !dynamic_cast(_modifier.body().statements().back().get()) + )) + m_modifierIsSimple = false; + if (m_modifierIsSimple) + _modifier.annotation().simpleModifier = true; + m_placeholderFound = false; } @@ -266,6 +278,12 @@ bool SyntaxChecker::visit(Break const& _breakStatement) return true; } +bool SyntaxChecker::visit(Return const&) +{ + m_modifierIsSimple = false; + return true; +} + bool SyntaxChecker::visit(Throw const& _throwStatement) { m_errorReporter.syntaxError( @@ -357,6 +375,10 @@ bool SyntaxChecker::visit(PlaceholderStatement const& _placeholder) "The placeholder statement \"_\" cannot be used inside an \"unchecked\" block." ); + // Two placeholders make it non-simple. + if (m_placeholderFound) + m_modifierIsSimple = false; + m_placeholderFound = true; return true; } diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h index ca36ed992..6d47830d4 100644 --- a/libsolidity/analysis/SyntaxChecker.h +++ b/libsolidity/analysis/SyntaxChecker.h @@ -77,6 +77,7 @@ private: bool visit(Continue const& _continueStatement) override; bool visit(Break const& _breakStatement) override; + bool visit(Return const& _returnStatement) override; bool visit(Throw const& _throwStatement) override; @@ -100,6 +101,9 @@ private: /// Flag that indicates whether a function modifier actually contains '_'. bool m_placeholderFound = false; + /// Indicates that the current modifier is "simple" which means it does not contain + /// a "return" statement and the only placeholder is the last statement. + bool m_modifierIsSimple = false; /// Flag that indicates whether some version pragma was present. bool m_versionPragmaFound = false; diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index 53931e426..f4cf5ae86 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -192,6 +192,9 @@ struct ErrorDefinitionAnnotation: CallableDeclarationAnnotation, StructurallyDoc struct ModifierDefinitionAnnotation: CallableDeclarationAnnotation, StructurallyDocumentedAnnotation { + /// If true, indicates that the modifier does not contain a "return" statement and only + /// a single `_` (placeholder) as the last statament. This means it can be called as a function. + bool simpleModifier = false; }; struct VariableDeclarationAnnotation: DeclarationAnnotation, StructurallyDocumentedAnnotation diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp index 700f53d1f..7ee31d02d 100644 --- a/libsolidity/codegen/CompilerContext.cpp +++ b/libsolidity/codegen/CompilerContext.cpp @@ -601,6 +601,12 @@ evmasm::AssemblyItem CompilerContext::FunctionCompilationQueue::entryLabel( params = CompilerUtils::sizeOnStack(functionType.parameterTypes()); returns = CompilerUtils::sizeOnStack(functionType.returnParameterTypes()); } + else if (auto const* modifier = dynamic_cast(&_declaration)) + { + solAssert(modifier->annotation().simpleModifier, "Can only call simple modifiers as functions."); + params = CompilerUtils::sizeOnStack(modifier->parameters()); + } + /// TODO What else can be there apart from function and modifiers? // some name that cannot clash with yul function names. string labelName = "@" + _declaration.name() + "_" + to_string(_declaration.id()); diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 5abc64636..7a347908a 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -705,6 +705,62 @@ bool ContractCompiler::visit(FunctionDefinition const& _function) return false; } +bool ContractCompiler::visit(ModifierDefinition const& _modifier) +{ + solAssert(_modifier.isImplemented(), ""); + solAssert(_modifier.annotation().simpleModifier, "Direct visit to non-simple modifier."); + + CompilerContext::LocationSetter locationSetter(m_context, _modifier); + + m_context.startFunction(_modifier); + + // stack upon entry: [return address] [arg0] [arg1] ... [argn] + + unsigned parametersSize = CompilerUtils::sizeOnStack(_modifier.parameters()); + // adding 1 for return address. + m_context.setStackOffset(static_cast(parametersSize) + 1); + + for (ASTPointer const& variable: _modifier.parameters()) + { + m_context.addVariable(*variable, parametersSize); + parametersSize -= variable->annotation().type->sizeOnStack(); + } + + solAssert(m_returnTags.empty(), ""); + m_breakTags.clear(); + m_continueTags.clear(); + m_currentFunction = nullptr; + m_scopeStackHeight.clear(); + m_modifierDepth = 0; + m_context.setModifierDepth(0); + m_context.setArithmetic(Arithmetic::Checked); + + m_context.setUseABICoderV2(*_modifier.sourceUnit().annotation().useABICoderV2); + + // This should not be used because we should not have a return statement. + solAssert(m_returnTags.empty(), ""); + + solAssert(m_ignoredPlaceholders == 0); + _modifier.body().accept(*this); + solAssert(m_ignoredPlaceholders == 1); + m_ignoredPlaceholders = 0; + + m_context.setModifierDepth(0); + + CompilerUtils(m_context).popStackSlots(CompilerUtils::sizeOnStack(_modifier.parameters())); + + for (ASTPointer const& variable: _modifier.parameters()) + m_context.removeVariable(*variable); + + solAssert(m_context.numberOfLocalVariables() == 0, ""); + + m_context.appendJump(evmasm::AssemblyItem::JumpType::OutOfFunction); + solAssert(m_context.stackHeight() == 0); + + return false; +} + + bool ContractCompiler::visit(InlineAssembly const& _inlineAssembly) { unsigned startStackHeight = m_context.stackHeight(); @@ -1280,6 +1336,8 @@ bool ContractCompiler::visit(Break const& _breakStatement) bool ContractCompiler::visit(Return const& _return) { + solAssert(m_currentFunction, "Used return inside simple modifier."); + CompilerContext::LocationSetter locationSetter(m_context, _return); if (Expression const* expression = _return.expression()) { @@ -1381,12 +1439,20 @@ bool ContractCompiler::visit(ExpressionStatement const& _expressionStatement) bool ContractCompiler::visit(PlaceholderStatement const& _placeholderStatement) { - StackHeightChecker checker(m_context); - CompilerContext::LocationSetter locationSetter(m_context, _placeholderStatement); - solAssert(m_context.arithmetic() == Arithmetic::Checked, "Placeholder cannot be used inside checked block."); - appendModifierOrFunctionCode(); - solAssert(m_context.arithmetic() == Arithmetic::Checked, "Arithmetic not reset to 'checked'."); - checker.check(); + if (m_currentFunction) + { + StackHeightChecker checker(m_context); + CompilerContext::LocationSetter locationSetter(m_context, _placeholderStatement); + solAssert(m_context.arithmetic() == Arithmetic::Checked, "Placeholder cannot be used inside checked block."); + appendModifierOrFunctionCode(); + solAssert(m_context.arithmetic() == Arithmetic::Checked, "Arithmetic not reset to 'checked'."); + checker.check(); + } + else + // We are generating code for a simple modifier, so we just register the placeholder but + // do not generate any code. + m_ignoredPlaceholders++; + return true; } @@ -1416,6 +1482,8 @@ void ContractCompiler::endVisit(Block const& _block) void ContractCompiler::appendMissingFunctions() { + // Note that `function` can also be a modifier, but if it is a modifier, + // it is a "simple" modifier, i.e. no `return` and only a single `_` at the very end. while (Declaration const* function = m_context.nextFunctionToCompile()) { m_context.setStackOffset(0); @@ -1465,18 +1533,37 @@ void ContractCompiler::appendModifierOrFunctionCode() modifierInvocation->arguments() ? *modifierInvocation->arguments() : std::vector>(); solAssert(modifier.parameters().size() == modifierArguments.size(), ""); - for (unsigned i = 0; i < modifier.parameters().size(); ++i) - { - m_context.addVariable(*modifier.parameters()[i]); - addedVariables.push_back(modifier.parameters()[i].get()); - compileExpression( - *modifierArguments[i], - modifier.parameters()[i]->annotation().type - ); - } - stackSurplus = CompilerUtils::sizeOnStack(modifier.parameters()); - codeBlock = &modifier.body(); + if (modifier.annotation().simpleModifier) + { + AssemblyItem returnLabel = m_context.pushNewTag(); + for (unsigned i = 0; i < modifier.parameters().size(); ++i) + compileExpression( + *modifierArguments[i], + modifier.parameters()[i]->annotation().type + ); + m_context.appendJumpTo( + m_context.functionEntryLabel(modifier), + evmasm::AssemblyItem::JumpType::IntoFunction + ); + m_context << returnLabel.tag(); + m_context.adjustStackOffset(-static_cast(CompilerUtils::sizeOnStack(modifier.parameters())) - 1); + appendModifierOrFunctionCode(); + } + else + { + for (unsigned i = 0; i < modifier.parameters().size(); ++i) + { + m_context.addVariable(*modifier.parameters()[i]); + addedVariables.push_back(modifier.parameters()[i].get()); + compileExpression( + *modifierArguments[i], + modifier.parameters()[i]->annotation().type + ); + } + stackSurplus = CompilerUtils::sizeOnStack(modifier.parameters()); + codeBlock = &modifier.body(); + } } } diff --git a/libsolidity/codegen/ContractCompiler.h b/libsolidity/codegen/ContractCompiler.h index 74a2e3b5f..0f25a43b6 100644 --- a/libsolidity/codegen/ContractCompiler.h +++ b/libsolidity/codegen/ContractCompiler.h @@ -105,6 +105,7 @@ private: bool visit(VariableDeclaration const& _variableDeclaration) override; bool visit(FunctionDefinition const& _function) override; + bool visit(ModifierDefinition const& _modifier) override; bool visit(InlineAssembly const& _inlineAssembly) override; bool visit(TryStatement const& _tryStatement) override; void handleCatch(std::vector> const& _catchClauses); @@ -155,6 +156,8 @@ private: /// Tag to jump to for a "return" statement and the stack height after freeing the local function or modifier variables. /// Needs to be stacked because of modifiers. std::vector> m_returnTags; + /// Number of placeholder statements that were ignored since we generated code for a simple modifier. + size_t m_ignoredPlaceholders = 0; unsigned m_modifierDepth = 0; FunctionDefinition const* m_currentFunction = nullptr;