From ec2393d3b656291636f827f81b067caac17bad89 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 9 Jul 2020 14:24:49 +0200 Subject: [PATCH 1/2] Refactor interpreter. --- test/libyul/EwasmTranslationTest.cpp | 4 +- test/libyul/YulInterpreterTest.cpp | 3 +- test/tools/ossfuzz/yulFuzzerCommon.cpp | 3 +- test/tools/yulInterpreter/Interpreter.cpp | 81 +++++++++++------------ test/tools/yulInterpreter/Interpreter.h | 48 +++++++------- test/tools/yulrun.cpp | 5 +- 6 files changed, 69 insertions(+), 75 deletions(-) diff --git a/test/libyul/EwasmTranslationTest.cpp b/test/libyul/EwasmTranslationTest.cpp index 8ef5ba0a0..9ef203dd0 100644 --- a/test/libyul/EwasmTranslationTest.cpp +++ b/test/libyul/EwasmTranslationTest.cpp @@ -99,11 +99,9 @@ string EwasmTranslationTest::interpret() InterpreterState state; state.maxTraceSize = 10000; state.maxSteps = 100000; - WasmDialect dialect; - Interpreter interpreter(state, dialect); try { - interpreter(*m_object->code); + Interpreter::run(state, WasmDialect{}, *m_object->code); } catch (InterpreterTerminatedGeneric const&) { diff --git a/test/libyul/YulInterpreterTest.cpp b/test/libyul/YulInterpreterTest.cpp index 9925b9546..fa84c0869 100644 --- a/test/libyul/YulInterpreterTest.cpp +++ b/test/libyul/YulInterpreterTest.cpp @@ -88,10 +88,9 @@ string YulInterpreterTest::interpret() InterpreterState state; state.maxTraceSize = 10000; state.maxSteps = 10000; - Interpreter interpreter(state, EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion{})); try { - interpreter(*m_ast); + Interpreter::run(state, EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion{}), *m_ast); } catch (InterpreterTerminatedGeneric const&) { diff --git a/test/tools/ossfuzz/yulFuzzerCommon.cpp b/test/tools/ossfuzz/yulFuzzerCommon.cpp index 214caf3f8..3682eef48 100644 --- a/test/tools/ossfuzz/yulFuzzerCommon.cpp +++ b/test/tools/ossfuzz/yulFuzzerCommon.cpp @@ -44,12 +44,11 @@ yulFuzzerUtil::TerminationReason yulFuzzerUtil::interpret( 0xc7, 0x60, 0x5f, 0x7c, 0xcd, 0xfb, 0x92, 0xcd, 0x8e, 0xf3, 0x9b, 0xe4, 0x4f, 0x6c, 0x14, 0xde }; - Interpreter interpreter(state, _dialect); TerminationReason reason = TerminationReason::None; try { - interpreter(*_ast); + Interpreter::run(state, _dialect, *_ast); } catch (StepLimitReached const&) { diff --git a/test/tools/yulInterpreter/Interpreter.cpp b/test/tools/yulInterpreter/Interpreter.cpp index 5f0861d14..cd4fa5d69 100644 --- a/test/tools/yulInterpreter/Interpreter.cpp +++ b/test/tools/yulInterpreter/Interpreter.cpp @@ -64,6 +64,12 @@ void InterpreterState::dumpTraceAndState(ostream& _out) const _out << " " << slot.first.hex() << ": " << slot.second.hex() << endl; } +void Interpreter::run(InterpreterState& _state, Dialect const& _dialect, Block const& _ast) +{ + Scope scope; + Interpreter{_state, _dialect, scope}(_ast); +} + void Interpreter::operator()(ExpressionStatement const& _expressionStatement) { evaluateMulti(_expressionStatement.expression); @@ -94,8 +100,7 @@ void Interpreter::operator()(VariableDeclaration const& _declaration) YulString varName = _declaration.variables.at(i).name; solAssert(!m_variables.count(varName), ""); m_variables[varName] = values.at(i); - solAssert(!m_scopes.back().count(varName), ""); - m_scopes.back().emplace(varName, nullptr); + m_scope->names.emplace(varName, nullptr); } } @@ -128,8 +133,8 @@ void Interpreter::operator()(ForLoop const& _forLoop) { solAssert(_forLoop.condition, ""); - openScope(); - ScopeGuard g([this]{ closeScope(); }); + enterScope(_forLoop.pre); + ScopeGuard g([this]{ leaveScope(); }); for (auto const& statement: _forLoop.pre.statements) { @@ -176,14 +181,13 @@ void Interpreter::operator()(Block const& _block) m_state.trace.emplace_back("Interpreter execution step limit reached."); throw StepLimitReached(); } - openScope(); + enterScope(_block); // Register functions. for (auto const& statement: _block.statements) if (holds_alternative(statement)) { FunctionDefinition const& funDef = std::get(statement); - solAssert(!m_scopes.back().count(funDef.name), ""); - m_scopes.back().emplace(funDef.name, &funDef); + m_scope->names.emplace(funDef.name, &funDef); } for (auto const& statement: _block.statements) @@ -193,29 +197,41 @@ void Interpreter::operator()(Block const& _block) break; } - closeScope(); + leaveScope(); } u256 Interpreter::evaluate(Expression const& _expression) { - ExpressionEvaluator ev(m_state, m_dialect, m_variables, m_scopes); + ExpressionEvaluator ev(m_state, m_dialect, *m_scope, m_variables); ev.visit(_expression); return ev.value(); } vector Interpreter::evaluateMulti(Expression const& _expression) { - ExpressionEvaluator ev(m_state, m_dialect, m_variables, m_scopes); + ExpressionEvaluator ev(m_state, m_dialect, *m_scope, m_variables); ev.visit(_expression); return ev.values(); } -void Interpreter::closeScope() +void Interpreter::enterScope(Block const& _block) { - for (auto const& [var, funDeclaration]: m_scopes.back()) + if (!m_scope->subScopes.count(&_block)) + m_scope->subScopes[&_block] = make_unique(Scope{ + {}, + {}, + m_scope + }); + m_scope = m_scope->subScopes[&_block].get(); +} + +void Interpreter::leaveScope() +{ + for (auto const& [var, funDeclaration]: m_scope->names) if (!funDeclaration) - solAssert(m_variables.erase(var) == 1, ""); - m_scopes.pop_back(); + m_variables.erase(var); + m_scope = m_scope->parent; + yulAssert(m_scope, ""); } void ExpressionEvaluator::operator()(Literal const& _literal) @@ -253,10 +269,15 @@ void ExpressionEvaluator::operator()(FunctionCall const& _funCall) return; } - auto [functionScopes, fun] = findFunctionAndScope(_funCall.functionName.name); + Scope* scope = &m_scope; + for (; scope; scope = scope->parent) + if (scope->names.count(_funCall.functionName.name)) + break; + yulAssert(scope, ""); - solAssert(fun, "Function not found."); - solAssert(m_values.size() == fun->parameters.size(), ""); + FunctionDefinition const* fun = scope->names.at(_funCall.functionName.name); + yulAssert(fun, "Function not found."); + yulAssert(m_values.size() == fun->parameters.size(), ""); map variables; for (size_t i = 0; i < fun->parameters.size(); ++i) variables[fun->parameters.at(i).name] = m_values.at(i); @@ -264,7 +285,7 @@ void ExpressionEvaluator::operator()(FunctionCall const& _funCall) variables[fun->returnVariables.at(i).name] = 0; m_state.controlFlowState = ControlFlowState::Default; - Interpreter interpreter(m_state, m_dialect, variables, functionScopes); + Interpreter interpreter(m_state, m_dialect, *scope, std::move(variables)); interpreter(fun->body); m_state.controlFlowState = ControlFlowState::Default; @@ -297,27 +318,3 @@ void ExpressionEvaluator::evaluateArgs(vector const& _expr) m_values = std::move(values); std::reverse(m_values.begin(), m_values.end()); } - -pair< - vector>, - FunctionDefinition const* -> ExpressionEvaluator::findFunctionAndScope(YulString _functionName) const -{ - FunctionDefinition const* fun = nullptr; - std::vector> newScopes; - for (auto const& scope: m_scopes) - { - // Copy over all functions. - newScopes.emplace_back(); - for (auto const& [name, funDef]: scope) - if (funDef) - newScopes.back().emplace(name, funDef); - // Stop at the called function. - if (scope.count(_functionName)) - { - fun = scope.at(_functionName); - break; - } - } - return {move(newScopes), fun}; -} diff --git a/test/tools/yulInterpreter/Interpreter.h b/test/tools/yulInterpreter/Interpreter.h index f50cbe6c4..3c9619d21 100644 --- a/test/tools/yulInterpreter/Interpreter.h +++ b/test/tools/yulInterpreter/Interpreter.h @@ -96,23 +96,37 @@ struct InterpreterState void dumpTraceAndState(std::ostream& _out) const; }; +/** + * Scope structure built and maintained during execution. + */ +struct Scope +{ + /// Used for variables and functions. Value is nullptr for variables. + std::map names; + std::map> subScopes; + Scope* parent = nullptr; +}; + /** * Yul interpreter. */ class Interpreter: public ASTWalker { public: + static void run(InterpreterState& _state, Dialect const& _dialect, Block const& _ast); + Interpreter( InterpreterState& _state, Dialect const& _dialect, - std::map _variables = {}, - std::vector> _scopes = {} + Scope& _scope, + std::map _variables = {} ): m_dialect(_dialect), m_state(_state), m_variables(std::move(_variables)), - m_scopes(std::move(_scopes)) - {} + m_scope(&_scope) + { + } void operator()(ExpressionStatement const& _statement) override; void operator()(Assignment const& _assignment) override; @@ -136,18 +150,14 @@ private: /// Evaluates the expression and returns its value. std::vector evaluateMulti(Expression const& _expression); - void openScope() { m_scopes.emplace_back(); } - /// Unregisters variables and functions. - void closeScope(); + void enterScope(Block const& _block); + void leaveScope(); Dialect const& m_dialect; InterpreterState& m_state; /// Values of variables. std::map m_variables; - /// Scopes of variables and functions. Used for lookup, clearing at end of blocks - /// and passing over the visible functions across function calls. - /// The pointer is nullptr if and only if the key is a variable. - std::vector> m_scopes; + Scope* m_scope; }; /** @@ -159,13 +169,13 @@ public: ExpressionEvaluator( InterpreterState& _state, Dialect const& _dialect, - std::map const& _variables, - std::vector> const& _scopes + Scope& _scope, + std::map const& _variables ): m_state(_state), m_dialect(_dialect), m_variables(_variables), - m_scopes(_scopes) + m_scope(_scope) {} void operator()(Literal const&) override; @@ -184,19 +194,11 @@ private: /// stores it in m_value. void evaluateArgs(std::vector const& _expr); - /// Finds the function called @a _functionName in the current scope stack and returns - /// the function's scope stack (with variables removed) and definition. - std::pair< - std::vector>, - FunctionDefinition const* - > findFunctionAndScope(YulString _functionName) const; - InterpreterState& m_state; Dialect const& m_dialect; /// Values of variables. std::map const& m_variables; - /// Stack of scopes in the current context. - std::vector> const& m_scopes; + Scope& m_scope; /// Current value of the expression std::vector m_values; }; diff --git a/test/tools/yulrun.cpp b/test/tools/yulrun.cpp index 1e82fba7f..494115540 100644 --- a/test/tools/yulrun.cpp +++ b/test/tools/yulrun.cpp @@ -88,11 +88,10 @@ void interpret(string const& _source) InterpreterState state; state.maxTraceSize = 10000; - Dialect const& dialect(EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion{})); - Interpreter interpreter(state, dialect); try { - interpreter(*ast); + Dialect const& dialect(EVMDialect::strictAssemblyForEVMObjects(langutil::EVMVersion{})); + Interpreter::run(state, dialect, *ast); } catch (InterpreterTerminatedGeneric const&) { From 3cf5ed9514211d18d5171f9ddbf849c850fcd1e4 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 9 Jul 2020 14:56:34 +0200 Subject: [PATCH 2/2] Use plain strings for comparison. --- .../EwasmBuiltinInterpreter.cpp | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp index b09fb5a87..1d8e29e91 100644 --- a/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp +++ b/test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp @@ -76,11 +76,12 @@ u256 EwasmBuiltinInterpreter::evalBuiltin(YulString _fun, vector const& _a for (u256 const& a: _arguments) arg.emplace_back(uint64_t(a & uint64_t(-1))); - if (_fun == "datasize"_yulstring) + string fun = _fun.str(); + if (fun == "datasize") return u256(keccak256(h256(_arguments.at(0)))) & 0xfff; - else if (_fun == "dataoffset"_yulstring) + else if (fun == "dataoffset") return u256(keccak256(h256(_arguments.at(0) + 2))) & 0xfff; - else if (_fun == "datacopy"_yulstring) + else if (fun == "datacopy") { // This is identical to codecopy. if (accessMemory(_arguments.at(0), _arguments.at(2))) @@ -93,49 +94,49 @@ u256 EwasmBuiltinInterpreter::evalBuiltin(YulString _fun, vector const& _a ); return 0; } - else if (_fun == "i32.drop"_yulstring || _fun == "i64.drop"_yulstring || _fun == "nop"_yulstring) + else if (fun == "i32.drop" || fun == "i64.drop" || fun == "nop") return {}; - else if (_fun == "i32.wrap_i64"_yulstring) + else if (fun == "i32.wrap_i64") return arg.at(0) & uint32_t(-1); - else if (_fun == "i64.extend_i32_u"_yulstring) + else if (fun == "i64.extend_i32_u") // Return the same as above because everything is u256 anyway. return arg.at(0) & uint32_t(-1); - else if (_fun == "unreachable"_yulstring) + else if (fun == "unreachable") { logTrace(evmasm::Instruction::INVALID, {}); throw ExplicitlyTerminated(); } - else if (_fun == "i64.store"_yulstring) + else if (fun == "i64.store") { accessMemory(arg[0], 8); writeMemoryWord(arg[0], arg[1]); return 0; } - else if (_fun == "i64.store8"_yulstring || _fun == "i32.store8"_yulstring) + else if (fun == "i64.store8" || fun == "i32.store8") { accessMemory(arg[0], 1); writeMemoryByte(arg[0], static_cast(arg[1] & 0xff)); return 0; } - else if (_fun == "i64.load"_yulstring) + else if (fun == "i64.load") { accessMemory(arg[0], 8); return readMemoryWord(arg[0]); } - else if (_fun == "i32.store"_yulstring) + else if (fun == "i32.store") { accessMemory(arg[0], 4); writeMemoryHalfWord(arg[0], arg[1]); return 0; } - else if (_fun == "i32.load"_yulstring) + else if (fun == "i32.load") { accessMemory(arg[0], 4); return readMemoryHalfWord(arg[0]); } - string prefix = _fun.str(); + string prefix = fun; string suffix; auto dot = prefix.find("."); if (dot != string::npos)