diff --git a/Changelog.md b/Changelog.md index 19227259587c..0b955c0cb623 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,7 @@ Language Features: * Inline Assembly: Allow assigning to `_slot` of local storage variable pointers. * General: Deprecated `value(...)` and `gas(...)` in favor of `{value: ...}` and `{gas: ...}` + * Inline Assembly: Perform control flow analysis on inline assembly. Allows storage returns to be set in assembly only. Compiler Features: diff --git a/libevmasm/SemanticInformation.cpp b/libevmasm/SemanticInformation.cpp index b7fa069d26e3..4ea9a01aee37 100644 --- a/libevmasm/SemanticInformation.cpp +++ b/libevmasm/SemanticInformation.cpp @@ -155,6 +155,26 @@ bool SemanticInformation::terminatesControlFlow(Instruction _instruction) } } +bool SemanticInformation::reverts(AssemblyItem const& _item) +{ + if (_item.type() != Operation) + return false; + else + return reverts(_item.instruction()); +} + +bool SemanticInformation::reverts(Instruction _instruction) +{ + switch (_instruction) + { + case Instruction::INVALID: + case Instruction::REVERT: + return true; + default: + return false; + } +} + bool SemanticInformation::isDeterministic(AssemblyItem const& _item) { if (_item.type() != Operation) diff --git a/libevmasm/SemanticInformation.h b/libevmasm/SemanticInformation.h index f08c3d73d4b6..39a1b2439418 100644 --- a/libevmasm/SemanticInformation.h +++ b/libevmasm/SemanticInformation.h @@ -47,6 +47,8 @@ struct SemanticInformation static bool altersControlFlow(AssemblyItem const& _item); static bool terminatesControlFlow(AssemblyItem const& _item); static bool terminatesControlFlow(Instruction _instruction); + static bool reverts(AssemblyItem const& _item); + static bool reverts(Instruction _instruction); /// @returns false if the value put on the stack by _item depends on anything else than /// the information in the current block header, memory, storage or stack. static bool isDeterministic(AssemblyItem const& _item); diff --git a/libsolidity/analysis/ControlFlowAnalyzer.cpp b/libsolidity/analysis/ControlFlowAnalyzer.cpp index c8a824c544c2..1a001eb1e92e 100644 --- a/libsolidity/analysis/ControlFlowAnalyzer.cpp +++ b/libsolidity/analysis/ControlFlowAnalyzer.cpp @@ -37,7 +37,7 @@ bool ControlFlowAnalyzer::visit(FunctionDefinition const& _function) { auto const& functionFlow = m_cfg.functionFlow(_function); checkUninitializedAccess(functionFlow.entry, functionFlow.exit); - checkUnreachable(functionFlow.entry, functionFlow.exit, functionFlow.revert); + checkUnreachable(functionFlow.entry, functionFlow.exit, functionFlow.revert, functionFlow.transactionReturn); } return false; } @@ -137,7 +137,7 @@ void ControlFlowAnalyzer::checkUninitializedAccess(CFGNode const* _entry, CFGNod m_errorReporter.typeError( variableOccurrence->occurrence() ? - variableOccurrence->occurrence()->location() : + *variableOccurrence->occurrence() : variableOccurrence->declaration().location(), ssl, string("This variable is of storage pointer type and can be ") + @@ -148,7 +148,7 @@ void ControlFlowAnalyzer::checkUninitializedAccess(CFGNode const* _entry, CFGNod } } -void ControlFlowAnalyzer::checkUnreachable(CFGNode const* _entry, CFGNode const* _exit, CFGNode const* _revert) const +void ControlFlowAnalyzer::checkUnreachable(CFGNode const* _entry, CFGNode const* _exit, CFGNode const* _revert, CFGNode const* _transactionReturn) const { // collect all nodes reachable from the entry point std::set reachable = util::BreadthFirstSearch{{_entry}}.run( @@ -158,10 +158,10 @@ void ControlFlowAnalyzer::checkUnreachable(CFGNode const* _entry, CFGNode const* } ).visited; - // traverse all paths backwards from exit and revert + // traverse all paths backwards from exit, revert and transaction return // and extract (valid) source locations of unreachable nodes into sorted set std::set unreachable; - util::BreadthFirstSearch{{_exit, _revert}}.run( + util::BreadthFirstSearch{{_exit, _revert, _transactionReturn}}.run( [&](CFGNode const* _node, auto&& _addChild) { if (!reachable.count(_node) && _node->location.isValid()) unreachable.insert(_node->location); diff --git a/libsolidity/analysis/ControlFlowAnalyzer.h b/libsolidity/analysis/ControlFlowAnalyzer.h index 24af990ab601..a839c5679f96 100644 --- a/libsolidity/analysis/ControlFlowAnalyzer.h +++ b/libsolidity/analysis/ControlFlowAnalyzer.h @@ -36,9 +36,9 @@ class ControlFlowAnalyzer: private ASTConstVisitor private: /// Checks for uninitialized variable accesses in the control flow between @param _entry and @param _exit. void checkUninitializedAccess(CFGNode const* _entry, CFGNode const* _exit) const; - /// Checks for unreachable code, i.e. code ending in @param _exit or @param _revert + /// Checks for unreachable code, i.e. code ending in @param _exit, @param _revert or @param _transactionReturn /// that can not be reached from @param _entry. - void checkUnreachable(CFGNode const* _entry, CFGNode const* _exit, CFGNode const* _revert) const; + void checkUnreachable(CFGNode const* _entry, CFGNode const* _exit, CFGNode const* _revert, CFGNode const* _transactionReturn) const; CFG const& m_cfg; langutil::ErrorReporter& m_errorReporter; diff --git a/libsolidity/analysis/ControlFlowBuilder.cpp b/libsolidity/analysis/ControlFlowBuilder.cpp index c7504897b71b..f4b6b5d5fdee 100644 --- a/libsolidity/analysis/ControlFlowBuilder.cpp +++ b/libsolidity/analysis/ControlFlowBuilder.cpp @@ -16,6 +16,8 @@ */ #include +#include +#include using namespace solidity; using namespace solidity::langutil; @@ -26,10 +28,12 @@ ControlFlowBuilder::ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, Funct m_nodeContainer(_nodeContainer), m_currentNode(_functionFlow.entry), m_returnNode(_functionFlow.exit), - m_revertNode(_functionFlow.revert) + m_revertNode(_functionFlow.revert), + m_transactionReturnNode(_functionFlow.transactionReturn) { } + unique_ptr ControlFlowBuilder::createFunctionFlow( CFG::NodeContainer& _nodeContainer, FunctionDefinition const& _function @@ -39,6 +43,7 @@ unique_ptr ControlFlowBuilder::createFunctionFlow( functionFlow->entry = _nodeContainer.newNode(); functionFlow->exit = _nodeContainer.newNode(); functionFlow->revert = _nodeContainer.newNode(); + functionFlow->transactionReturn = _nodeContainer.newNode(); ControlFlowBuilder builder(_nodeContainer, *functionFlow); builder.appendControlFlow(_function); @@ -131,17 +136,17 @@ bool ControlFlowBuilder::visit(ForStatement const& _forStatement) if (_forStatement.condition()) appendControlFlow(*_forStatement.condition()); - auto loopExpression = newLabel(); + auto postPart = newLabel(); auto nodes = splitFlow<2>(); auto afterFor = nodes[1]; m_currentNode = nodes[0]; { - BreakContinueScope scope(*this, afterFor, loopExpression); + BreakContinueScope scope(*this, afterFor, postPart); appendControlFlow(_forStatement.body()); } - placeAndConnectLabel(loopExpression); + placeAndConnectLabel(postPart); if (auto expression = _forStatement.loopExpression()) appendControlFlow(*expression); @@ -315,8 +320,7 @@ bool ControlFlowBuilder::visit(FunctionDefinition const& _functionDefinition) appendControlFlow(*returnParameter); m_returnNode->variableOccurrences.emplace_back( *returnParameter, - VariableOccurrence::Kind::Return, - nullptr + VariableOccurrence::Kind::Return ); } @@ -345,7 +349,7 @@ bool ControlFlowBuilder::visit(Return const& _return) m_currentNode->variableOccurrences.emplace_back( *returnParameter, VariableOccurrence::Kind::Assignment, - &_return + _return.location() ); } connect(m_currentNode, m_returnNode); @@ -363,18 +367,158 @@ bool ControlFlowBuilder::visit(FunctionTypeName const& _functionTypeName) bool ControlFlowBuilder::visit(InlineAssembly const& _inlineAssembly) { - solAssert(!!m_currentNode, ""); - visitNode(_inlineAssembly); - for (auto const& ref: _inlineAssembly.annotation().externalReferences) + solAssert(!!m_currentNode && !m_inlineAssembly, ""); + + m_inlineAssembly = &_inlineAssembly; + (*this)(_inlineAssembly.operations()); + m_inlineAssembly = nullptr; + + return false; +} + +void ControlFlowBuilder::visit(yul::Statement const& _statement) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + m_currentNode->location = langutil::SourceLocation::smallestCovering(m_currentNode->location, locationOf(_statement)); + ASTWalker::visit(_statement); +} + +void ControlFlowBuilder::operator()(yul::If const& _if) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + visit(*_if.condition); + + auto nodes = splitFlow<2>(); + m_currentNode = nodes[0]; + (*this)(_if.body); + nodes[0] = m_currentNode; + mergeFlow(nodes, nodes[1]); +} + +void ControlFlowBuilder::operator()(yul::Switch const& _switch) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + visit(*_switch.expression); + + auto beforeSwitch = m_currentNode; + + auto nodes = splitFlow(_switch.cases.size()); + for (size_t i = 0u; i < _switch.cases.size(); ++i) + { + m_currentNode = nodes[i]; + (*this)(_switch.cases[i].body); + nodes[i] = m_currentNode; + } + mergeFlow(nodes); + + bool hasDefault = util::contains_if(_switch.cases, [](yul::Case const& _case) { return !_case.value; }); + if (!hasDefault) + connect(beforeSwitch, m_currentNode); +} + +void ControlFlowBuilder::operator()(yul::ForLoop const& _forLoop) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + + (*this)(_forLoop.pre); + + auto condition = createLabelHere(); + + if (_forLoop.condition) + visit(*_forLoop.condition); + + auto loopExpression = newLabel(); + auto nodes = splitFlow<2>(); + auto afterFor = nodes[1]; + m_currentNode = nodes[0]; + + { + BreakContinueScope scope(*this, afterFor, loopExpression); + (*this)(_forLoop.body); + } + + placeAndConnectLabel(loopExpression); + + (*this)(_forLoop.post); + + connect(m_currentNode, condition); + m_currentNode = afterFor; +} + +void ControlFlowBuilder::operator()(yul::Break const&) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + solAssert(m_breakJump, ""); + connect(m_currentNode, m_breakJump); + m_currentNode = newLabel(); +} + +void ControlFlowBuilder::operator()(yul::Continue const&) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + solAssert(m_continueJump, ""); + connect(m_currentNode, m_continueJump); + m_currentNode = newLabel(); +} + +void ControlFlowBuilder::operator()(yul::Identifier const& _identifier) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + auto const& externalReferences = m_inlineAssembly->annotation().externalReferences; + if (externalReferences.count(&_identifier)) { - if (auto variableDeclaration = dynamic_cast(ref.second.declaration)) + if (auto const* declaration = dynamic_cast(externalReferences.at(&_identifier).declaration)) m_currentNode->variableOccurrences.emplace_back( - *variableDeclaration, - VariableOccurrence::Kind::InlineAssembly, - &_inlineAssembly + *declaration, + VariableOccurrence::Kind::Access, + _identifier.location ); } - return true; +} + +void ControlFlowBuilder::operator()(yul::Assignment const& _assignment) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + visit(*_assignment.value); + auto const& externalReferences = m_inlineAssembly->annotation().externalReferences; + for (auto const& variable: _assignment.variableNames) + if (externalReferences.count(&variable)) + if (auto const* declaration = dynamic_cast(externalReferences.at(&variable).declaration)) + m_currentNode->variableOccurrences.emplace_back( + *declaration, + VariableOccurrence::Kind::Assignment, + variable.location + ); +} + +void ControlFlowBuilder::operator()(yul::FunctionCall const& _functionCall) +{ + using namespace yul; + solAssert(m_currentNode && m_inlineAssembly, ""); + yul::ASTWalker::operator()(_functionCall); + + if (auto const *builtinFunction = m_inlineAssembly->dialect().builtin(_functionCall.functionName.name)) + if (builtinFunction->controlFlowSideEffects.terminates) + { + if (builtinFunction->controlFlowSideEffects.reverts) + connect(m_currentNode, m_revertNode); + else + connect(m_currentNode, m_transactionReturnNode); + m_currentNode = newLabel(); + } +} + +void ControlFlowBuilder::operator()(yul::FunctionDefinition const&) +{ + solAssert(m_currentNode && m_inlineAssembly, ""); + // External references cannot be accessed from within functions, so we can ignore their control flow. + // TODO: we might still want to track if they always revert or return, though. +} + +void ControlFlowBuilder::operator()(yul::Leave const&) +{ + // This has to be implemented, if we ever decide to visit functions. + solUnimplementedAssert(false, ""); } bool ControlFlowBuilder::visit(VariableDeclaration const& _variableDeclaration) @@ -384,8 +528,7 @@ bool ControlFlowBuilder::visit(VariableDeclaration const& _variableDeclaration) m_currentNode->variableOccurrences.emplace_back( _variableDeclaration, - VariableOccurrence::Kind::Declaration, - nullptr + VariableOccurrence::Kind::Declaration ); // Handle declaration with immediate assignment. @@ -393,14 +536,13 @@ bool ControlFlowBuilder::visit(VariableDeclaration const& _variableDeclaration) m_currentNode->variableOccurrences.emplace_back( _variableDeclaration, VariableOccurrence::Kind::Assignment, - _variableDeclaration.value().get() + _variableDeclaration.value()->location() ); // Function arguments are considered to be immediately assigned as well (they are "externally assigned"). else if (_variableDeclaration.isCallableOrCatchParameter() && !_variableDeclaration.isReturnParameter()) m_currentNode->variableOccurrences.emplace_back( _variableDeclaration, - VariableOccurrence::Kind::Assignment, - nullptr + VariableOccurrence::Kind::Assignment ); return true; } @@ -434,7 +576,7 @@ bool ControlFlowBuilder::visit(VariableDeclarationStatement const& _variableDecl m_currentNode->variableOccurrences.emplace_back( *var, VariableOccurrence::Kind::Assignment, - expression + expression ? std::make_optional(expression->location()) : std::optional{} ); } } @@ -452,7 +594,7 @@ bool ControlFlowBuilder::visit(Identifier const& _identifier) static_cast(_identifier).annotation().lValueRequested ? VariableOccurrence::Kind::Assignment : VariableOccurrence::Kind::Access, - &_identifier + _identifier.location() ); return true; diff --git a/libsolidity/analysis/ControlFlowBuilder.h b/libsolidity/analysis/ControlFlowBuilder.h index ecdedfbfefd7..ee1c3ab79af0 100644 --- a/libsolidity/analysis/ControlFlowBuilder.h +++ b/libsolidity/analysis/ControlFlowBuilder.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -30,7 +31,7 @@ namespace solidity::frontend { * Modifiers are not yet applied to the functions. This is done in a second * step in the CFG class. */ -class ControlFlowBuilder: private ASTConstVisitor +class ControlFlowBuilder: private ASTConstVisitor, private yul::ASTWalker { public: static std::unique_ptr createFunctionFlow( @@ -39,7 +40,10 @@ class ControlFlowBuilder: private ASTConstVisitor ); private: - explicit ControlFlowBuilder(CFG::NodeContainer& _nodeContainer, FunctionFlow const& _functionFlow); + explicit ControlFlowBuilder( + CFG::NodeContainer& _nodeContainer, + FunctionFlow const& _functionFlow + ); // Visits for constructing the control flow. bool visit(BinaryOperation const& _operation) override; @@ -62,6 +66,17 @@ class ControlFlowBuilder: private ASTConstVisitor // Visits for filling variable occurrences. bool visit(FunctionTypeName const& _functionTypeName) override; bool visit(InlineAssembly const& _inlineAssembly) override; + void visit(yul::Statement const& _statement) override; + void operator()(yul::If const& _if) override; + void operator()(yul::Switch const& _switch) override; + void operator()(yul::ForLoop const& _for) override; + void operator()(yul::Break const&) override; + void operator()(yul::Continue const&) override; + void operator()(yul::Identifier const& _identifier) override; + void operator()(yul::Assignment const& _assignment) override; + void operator()(yul::FunctionCall const& _functionCall) override; + void operator()(yul::FunctionDefinition const& _functionDefinition) override; + void operator()(yul::Leave const& _leave) override; bool visit(VariableDeclaration const& _variableDeclaration) override; bool visit(VariableDeclarationStatement const& _variableDeclarationStatement) override; bool visit(Identifier const& _identifier) override; @@ -70,6 +85,9 @@ class ControlFlowBuilder: private ASTConstVisitor bool visitNode(ASTNode const&) override; private: + using ASTConstVisitor::visit; + using yul::ASTWalker::visit; + using yul::ASTWalker::operator(); /// Appends the control flow of @a _node to the current control flow. void appendControlFlow(ASTNode const& _node); @@ -136,6 +154,7 @@ class ControlFlowBuilder: private ASTConstVisitor CFGNode* m_currentNode = nullptr; CFGNode* m_returnNode = nullptr; CFGNode* m_revertNode = nullptr; + CFGNode* m_transactionReturnNode = nullptr; /// The current jump destination of break Statements. CFGNode* m_breakJump = nullptr; @@ -145,6 +164,8 @@ class ControlFlowBuilder: private ASTConstVisitor CFGNode* m_placeholderEntry = nullptr; CFGNode* m_placeholderExit = nullptr; + InlineAssembly const* m_inlineAssembly = nullptr; + /// Helper class that replaces the break and continue jump destinations for the /// current scope and restores the originals at the end of the scope. class BreakContinueScope diff --git a/libsolidity/analysis/ControlFlowGraph.h b/libsolidity/analysis/ControlFlowGraph.h index e40bb7623604..93e2e3b89b7f 100644 --- a/libsolidity/analysis/ControlFlowGraph.h +++ b/libsolidity/analysis/ControlFlowGraph.h @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -33,8 +34,8 @@ namespace solidity::frontend /** * Occurrence of a variable in a block of control flow. * Stores the declaration of the referenced variable, the - * kind of the occurrence and possibly the node at which - * it occurred. + * kind of the occurrence and possibly the source location + * at which it occurred. */ class VariableOccurrence { @@ -47,7 +48,7 @@ class VariableOccurrence Assignment, InlineAssembly }; - VariableOccurrence(VariableDeclaration const& _declaration, Kind _kind, ASTNode const* _occurrence): + VariableOccurrence(VariableDeclaration const& _declaration, Kind _kind, std::optional const& _occurrence = {}): m_declaration(_declaration), m_occurrenceKind(_kind), m_occurrence(_occurrence) { } @@ -57,8 +58,8 @@ class VariableOccurrence { if (m_occurrence && _rhs.m_occurrence) { - if (m_occurrence->id() < _rhs.m_occurrence->id()) return true; - if (_rhs.m_occurrence->id() < m_occurrence->id()) return false; + if (*m_occurrence < *_rhs.m_occurrence) return true; + if (*_rhs.m_occurrence < *m_occurrence) return false; } else if (_rhs.m_occurrence) return true; @@ -74,14 +75,14 @@ class VariableOccurrence VariableDeclaration const& declaration() const { return m_declaration; } Kind kind() const { return m_occurrenceKind; }; - ASTNode const* occurrence() const { return m_occurrence; } + std::optional const& occurrence() const { return m_occurrence; } private: /// Declaration of the occurring variable. VariableDeclaration const& m_declaration; /// Kind of occurrence. Kind m_occurrenceKind = Kind::Access; - /// AST node at which the variable occurred, if available (may be nullptr). - ASTNode const* m_occurrence = nullptr; + /// Source location at which the variable occurred, if available (may be nullptr). + std::optional m_occurrence; }; /** @@ -119,6 +120,10 @@ struct FunctionFlow /// This node is empty does not have any exits, but may have multiple entries /// (e.g. all assert, require, revert and throw statements). CFGNode* revert = nullptr; + /// Transaction return node. Destination node for inline assembly "return" calls. + /// This node is empty and does not have any exits, but may have multiple entries + /// (e.g. all inline assembly return calls). + CFGNode* transactionReturn = nullptr; }; class CFG: private ASTConstVisitor @@ -140,7 +145,6 @@ class CFG: private ASTConstVisitor std::vector> m_nodes; }; private: - langutil::ErrorReporter& m_errorReporter; /// Node container. diff --git a/libyul/ControlFlowSideEffects.h b/libyul/ControlFlowSideEffects.h new file mode 100644 index 000000000000..5621a122b1df --- /dev/null +++ b/libyul/ControlFlowSideEffects.h @@ -0,0 +1,38 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see . +*/ + +#pragma once + +#include + +namespace solidity::yul +{ + +/** + * Side effects of code related to control flow. + */ +struct ControlFlowSideEffects +{ + /// If true, this code terminates the control flow. + /// State may or may not be reverted as indicated by the ``reverts`` flag. + bool terminates = false; + /// If true, this code reverts all state changes in the transaction. + /// Whenever this is true, ``terminates`` has to be true as well. + bool reverts = false; +}; + +} diff --git a/libyul/Dialect.h b/libyul/Dialect.h index c137791ee9c3..a2a5961775db 100644 --- a/libyul/Dialect.h +++ b/libyul/Dialect.h @@ -22,6 +22,7 @@ #include #include +#include #include @@ -42,6 +43,7 @@ struct BuiltinFunction std::vector parameters; std::vector returns; SideEffects sideEffects; + ControlFlowSideEffects controlFlowSideEffects; /// If true, this is the msize instruction. bool isMSize = false; /// If true, can only accept literals as arguments and they cannot be moved to variables. diff --git a/libyul/backends/evm/EVMDialect.cpp b/libyul/backends/evm/EVMDialect.cpp index 27a2b85e3eaf..ea6586323ce6 100644 --- a/libyul/backends/evm/EVMDialect.cpp +++ b/libyul/backends/evm/EVMDialect.cpp @@ -52,6 +52,8 @@ pair createEVMFunction( f.parameters.resize(info.args); f.returns.resize(info.ret); f.sideEffects = EVMDialect::sideEffectsOfInstruction(_instruction); + f.controlFlowSideEffects.terminates = evmasm::SemanticInformation::terminatesControlFlow(_instruction); + f.controlFlowSideEffects.reverts = evmasm::SemanticInformation::reverts(_instruction); f.isMSize = _instruction == evmasm::Instruction::MSIZE; f.literalArguments = false; f.instruction = _instruction; diff --git a/libyul/backends/wasm/WasmDialect.cpp b/libyul/backends/wasm/WasmDialect.cpp index 7f2149adcfdd..b8dc9f32f8e1 100644 --- a/libyul/backends/wasm/WasmDialect.cpp +++ b/libyul/backends/wasm/WasmDialect.cpp @@ -99,6 +99,8 @@ WasmDialect::WasmDialect() addFunction("unreachable", {}, {}, false); m_functions["unreachable"_yulstring].sideEffects.invalidatesStorage = false; m_functions["unreachable"_yulstring].sideEffects.invalidatesMemory = false; + m_functions["unreachable"_yulstring].controlFlowSideEffects.terminates = true; + m_functions["unreachable"_yulstring].controlFlowSideEffects.reverts = true; addFunction("datasize", {i64}, {i64}, true, true); addFunction("dataoffset", {i64}, {i64}, true, true); @@ -147,7 +149,12 @@ void WasmDialect::addEthereumExternals() static string const i64{"i64"}; static string const i32{"i32"}; static string const i32ptr{"i32"}; // Uses "i32" on purpose. - struct External { string name; vector parameters; vector returns; }; + struct External { + string name; + vector parameters; + vector returns; + ControlFlowSideEffects controlFlowSideEffects = {}; + }; static vector externals{ {"getAddress", {i32ptr}, {}}, {"getExternalBalance", {i32ptr, i32ptr}, {}}, @@ -175,11 +182,11 @@ void WasmDialect::addEthereumExternals() {"log", {i32ptr, i32, i32, i32ptr, i32ptr, i32ptr, i32ptr}, {}}, {"getBlockNumber", {}, {i64}}, {"getTxOrigin", {i32ptr}, {}}, - {"finish", {i32ptr, i32}, {}}, - {"revert", {i32ptr, i32}, {}}, + {"finish", {i32ptr, i32}, {}, {true, false}}, + {"revert", {i32ptr, i32}, {}, {true, true}}, {"getReturnDataSize", {}, {i32}}, {"returnDataCopy", {i32ptr, i32, i32}, {}}, - {"selfDestruct", {i32ptr}, {}}, + {"selfDestruct", {i32ptr}, {}, {true, false}}, {"getBlockTimestamp", {}, {i64}} }; for (External const& ext: externals) @@ -193,6 +200,7 @@ void WasmDialect::addEthereumExternals() f.returns.emplace_back(YulString(p)); // TODO some of them are side effect free. f.sideEffects = SideEffects::worst(); + f.controlFlowSideEffects = ext.controlFlowSideEffects; f.isMSize = false; f.sideEffects.invalidatesStorage = (ext.name == "storageStore"); f.literalArguments = false; diff --git a/test/libsolidity/syntaxTests/controlFlow/leave_inside_function.sol b/test/libsolidity/syntaxTests/controlFlow/leave_inside_function.sol new file mode 100644 index 000000000000..245106e133f7 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/leave_inside_function.sol @@ -0,0 +1,11 @@ +contract C { + function f() public pure { + assembly { + function f() { + // Make sure this doesn't trigger the unimplemented assertion in the control flow builder. + leave + } + } + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/controlFlow/leave_outside_function.sol b/test/libsolidity/syntaxTests/controlFlow/leave_outside_function.sol new file mode 100644 index 000000000000..772a108e2151 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/leave_outside_function.sol @@ -0,0 +1,10 @@ +contract C { + function f() public pure { + assembly { + // Make sure this doesn't trigger the unimplemented assertion in the control flow builder. + leave + } + } +} +// ---- +// SyntaxError: (178-183): Keyword "leave" can only be used inside a function. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/for_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/for_err.sol new file mode 100644 index 000000000000..909d4f333e23 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/for_err.sol @@ -0,0 +1,29 @@ +contract C { + struct S { bool f; } + S s; + function f() internal pure returns (S storage c) { + assembly { + for {} eq(0,0) { c_slot := s_slot } {} + } + } + function g() internal pure returns (S storage c) { + assembly { + for {} eq(0,1) { c_slot := s_slot } {} + } + } + function h() internal pure returns (S storage c) { + assembly { + for {} eq(0,0) {} { c_slot := s_slot } + } + } + function i() internal pure returns (S storage c) { + assembly { + for {} eq(0,1) {} { c_slot := s_slot } + } + } +} +// ---- +// TypeError: (87-98): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour. +// TypeError: (228-239): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour. +// TypeError: (369-380): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour. +// TypeError: (510-521): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/for_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/for_fine.sol new file mode 100644 index 000000000000..ac3532aaf3a4 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/for_fine.sol @@ -0,0 +1,15 @@ +contract C { + struct S { bool f; } + S s; + function f() internal pure returns (S storage c) { + assembly { + for { c_slot := s_slot } iszero(0) {} {} + } + } + function g() internal pure returns (S storage c) { + assembly { + for { c_slot := s_slot } iszero(1) {} {} + } + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/if_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/if_err.sol new file mode 100644 index 000000000000..2f79ee10f237 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/if_err.sol @@ -0,0 +1,11 @@ +contract C { + struct S { bool f; } + S s; + function f(bool flag) internal pure returns (S storage c) { + assembly { + if flag { c_slot := s_slot } + } + } +} +// ---- +// TypeError: (96-107): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/returning_function.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/returning_function.sol new file mode 100644 index 000000000000..81b6bcfe7268 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/returning_function.sol @@ -0,0 +1,13 @@ +contract C { + struct S { bool f; } + S s; + function f() internal pure returns (S storage c) { + // this should warn about unreachable code, but currently function flow is ignored + assembly { + function f() { return(0, 0) } + f() + c_slot := s_slot + } + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/reverting_function.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/reverting_function.sol new file mode 100644 index 000000000000..4619584ef8d2 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/reverting_function.sol @@ -0,0 +1,13 @@ +contract C { + struct S { bool f; } + S s; + function f() internal pure returns (S storage c) { + // this could be allowed, but currently control flow for functions is not analysed + assembly { + function f() { revert(0, 0) } + f() + } + } +} +// ---- +// TypeError: (87-98): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/stub.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/stub.sol new file mode 100644 index 000000000000..e5873f3d1a35 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/stub.sol @@ -0,0 +1,10 @@ +contract C { + struct S { bool f; } + S s; + function f() internal pure returns (S storage c) { + assembly { + c_slot := s_slot + } + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_err.sol new file mode 100644 index 000000000000..0644d4b8f48b --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_err.sol @@ -0,0 +1,27 @@ +contract C { + struct S { bool f; } + S s; + function f(uint256 a) internal pure returns (S storage c) { + assembly { + switch a + case 0 { c_slot := s_slot } + } + } + function g(bool flag) internal pure returns (S storage c) { + assembly { + switch flag + case 0 { c_slot := s_slot } + case 1 { c_slot := s_slot } + } + } + function h(uint256 a) internal pure returns (S storage c) { + assembly { + switch a + case 0 { c_slot := s_slot } + default { return(0,0) } + } + } +} +// ---- +// TypeError: (96-107): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour. +// TypeError: (256-267): This variable is of storage pointer type and can be returned without prior assignment, which would lead to undefined behaviour. diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_fine.sol new file mode 100644 index 000000000000..9e73ecff1fc5 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/assembly/switch_fine.sol @@ -0,0 +1,25 @@ +contract C { + struct S { bool f; } + S s; + function f(uint256 a) internal pure returns (S storage c) { + assembly { + switch a + default { c_slot := s_slot } + } + } + function g(bool flag) internal pure returns (S storage c) { + assembly { + switch flag + case 0 { c_slot := s_slot } + default { c_slot := s_slot } + } + } + function h(uint256 a) internal pure returns (S storage c) { + assembly { + switch a + case 0 { revert(0, 0) } + default { c_slot := s_slot } + } + } +} +// ---- diff --git a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol index e73d9cde6918..89a13717e9e8 100644 --- a/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol +++ b/test/libsolidity/syntaxTests/controlFlow/uninitializedAccess/assembly.sol @@ -6,4 +6,4 @@ contract C { } } // ---- -// TypeError: (92-116): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour. +// TypeError: (107-113): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour. diff --git a/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/double_revert.sol b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/double_revert.sol new file mode 100644 index 000000000000..8a441a5b267d --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/double_revert.sol @@ -0,0 +1,17 @@ +contract C { + function f() public pure { + assembly { + revert(0, 0) + revert(0, 0) + } + } + function g() public pure { + assembly { + revert(0, 0) + } + revert(); + } +} +// ---- +// Warning: (100-112): Unreachable code. +// Warning: (222-230): Unreachable code. diff --git a/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/for_break.sol b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/for_break.sol new file mode 100644 index 000000000000..466bf4fae504 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/for_break.sol @@ -0,0 +1,13 @@ +contract C { + function f() public pure { + assembly { + for { let a := 0} lt(a,1) { a := add(a, 1) } { + break + let b := 42 + } + } + } +} +// ---- +// Warning: (103-117): Unreachable code. +// Warning: (160-171): Unreachable code. diff --git a/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/for_continue.sol b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/for_continue.sol new file mode 100644 index 000000000000..be09967f1a00 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/for_continue.sol @@ -0,0 +1,12 @@ +contract C { + function f() public pure { + assembly { + for { let a := 0} lt(a,1) { a := add(a, 1) } { + continue + let b := 42 + } + } + } +} +// ---- +// Warning: (163-174): Unreachable code. diff --git a/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/return.sol b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/return.sol new file mode 100644 index 000000000000..9b13441b45f8 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/return.sol @@ -0,0 +1,17 @@ +contract C { + function f(uint256 y) public pure returns (uint256 x) { + assembly { + return(0, 0) + x := y + } + } + function g(uint256 y) public pure returns (uint256 x) { + assembly { + return(0, 0) + } + x = y; + } +} +// ---- +// Warning: (129-135): Unreachable code. +// Warning: (274-279): Unreachable code. diff --git a/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/revert.sol b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/revert.sol new file mode 100644 index 000000000000..66b6ae382d49 --- /dev/null +++ b/test/libsolidity/syntaxTests/controlFlow/unreachableCode/assembly/revert.sol @@ -0,0 +1,17 @@ +contract C { + function f(uint256 y) public pure returns (uint256 x) { + assembly { + revert(0, 0) + x := y + } + } + function g(uint256 y) public pure returns (uint256 x) { + assembly { + revert(0, 0) + } + x = y; + } +} +// ---- +// Warning: (129-135): Unreachable code. +// Warning: (274-279): Unreachable code. diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp index 020f3d4c4430..93b5287218ad 100644 --- a/test/libyul/Parser.cpp +++ b/test/libyul/Parser.cpp @@ -539,7 +539,7 @@ BOOST_AUTO_TEST_CASE(builtins_analysis) { return _name == "builtin"_yulstring ? &f : nullptr; } - BuiltinFunction f{"builtin"_yulstring, vector(2), vector(3), {}}; + BuiltinFunction f{"builtin"_yulstring, vector(2), vector(3), {}, {}}; }; SimpleDialect dialect;