From 7107ef13a7a3cd65cb805645f8e445fe692145bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Wed, 11 Mar 2020 21:15:12 +0100 Subject: [PATCH 1/2] [yul-phaser] Program: Add parseObject() --- tools/yulPhaser/Program.cpp | 41 +++++++++++++++++++++++++++++++++++++ tools/yulPhaser/Program.h | 4 ++++ 2 files changed, 45 insertions(+) diff --git a/tools/yulPhaser/Program.cpp b/tools/yulPhaser/Program.cpp index c397cd1f4..7dcfddd79 100644 --- a/tools/yulPhaser/Program.cpp +++ b/tools/yulPhaser/Program.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -136,6 +137,46 @@ variant, ErrorList> Program::parseSource(Dialect const& _diale return variant, ErrorList>(move(ast)); } +variant, ErrorList> Program::parseObject(Dialect const& _dialect, CharStream _source) +{ + ErrorList errors; + ErrorReporter errorReporter(errors); + auto scanner = make_shared(move(_source)); + + ObjectParser parser(errorReporter, _dialect); + shared_ptr object = parser.parse(scanner, false); + if (object == nullptr || !errorReporter.errors().empty()) + // NOTE: It's possible to get errors even if the returned object is non-null. + // For example when there are errors in a nested object. + return errors; + + Object* deployedObject = nullptr; + if (object->subObjects.size() > 0) + for (auto& subObject: object->subObjects) + // solc --ir produces an object with a subobject of the same name as the outer object + // but suffixed with "_deployed". + // The other object references the nested one which makes analysis fail. Below we try to + // extract just the nested one for that reason. This is just a heuristic. If there's no + // subobject with such a suffix we fall back to accepting the whole object as is. + if (subObject != nullptr && subObject->name.str() == object->name.str() + "_deployed") + { + deployedObject = dynamic_cast(subObject.get()); + if (deployedObject != nullptr) + break; + } + Object* selectedObject = (deployedObject != nullptr ? deployedObject : object.get()); + + // NOTE: I'm making a copy of the whole AST to get unique_ptr rather than shared_ptr. + // This is a slight performance hit but it's much less than the parsing itself. + // unique_ptr lets me be sure that two Program instances can never share the AST by mistake. + // The public API of the class does not provide access to the smart pointer so it won't be hard + // to switch to shared_ptr if the copying turns out to be an issue (though it would be better + // to refactor ObjectParser and Object to use unique_ptr instead). + auto astCopy = make_unique(get(ASTCopier{}(*selectedObject->code))); + + return variant, ErrorList>(move(astCopy)); +} + variant, ErrorList> Program::analyzeAST(Dialect const& _dialect, Block const& _ast) { ErrorList errors; diff --git a/tools/yulPhaser/Program.h b/tools/yulPhaser/Program.h index 6da9751b9..df44f2ba4 100644 --- a/tools/yulPhaser/Program.h +++ b/tools/yulPhaser/Program.h @@ -98,6 +98,10 @@ private: yul::Dialect const& _dialect, langutil::CharStream _source ); + static std::variant, langutil::ErrorList> parseObject( + yul::Dialect const& _dialect, + langutil::CharStream _source + ); static std::variant, langutil::ErrorList> analyzeAST( yul::Dialect const& _dialect, yul::Block const& _ast From 29186f9951097a262ad97f0eba5afd2b07f911c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Wed, 11 Mar 2020 21:16:01 +0100 Subject: [PATCH 2/2] [yul-phaser] Program: Switch from using parseCode() to parseObject() --- test/yulPhaser/Program.cpp | 126 ++++++++++++++++++++++++++++++++++++ tools/yulPhaser/Program.cpp | 17 +---- tools/yulPhaser/Program.h | 4 -- 3 files changed, 127 insertions(+), 20 deletions(-) diff --git a/test/yulPhaser/Program.cpp b/test/yulPhaser/Program.cpp index 5d4e012f3..e69f0fc5c 100644 --- a/test/yulPhaser/Program.cpp +++ b/test/yulPhaser/Program.cpp @@ -189,6 +189,132 @@ BOOST_AUTO_TEST_CASE(load_should_throw_InvalidProgram_if_program_cant_be_analyze BOOST_TEST(holds_alternative(Program::load(sourceStream))); } +BOOST_AUTO_TEST_CASE(load_should_accept_yul_objects_as_input) +{ + string sourceCode( + "object \"C_178\" {\n" + " code {\n" + " mstore(64, 128)\n" + " if iszero(calldatasize()) {}\n" + " revert(0, 0)\n" + " }\n" + "}\n" + ); + CharStream sourceStream(sourceCode, current_test_case().p_name); + auto programOrErrors = Program::load(sourceStream); + + BOOST_TEST(holds_alternative(programOrErrors)); +} + +BOOST_AUTO_TEST_CASE(load_should_return_errors_if_analysis_of_object_code_fails) +{ + string sourceCode( + "object \"C_178\" {\n" + " code {\n" + " return(0, datasize(\"C_178_deployed\"))\n" + " }\n" + "}\n" + ); + CharStream sourceStream(sourceCode, current_test_case().p_name); + auto programOrErrors = Program::load(sourceStream); + + BOOST_TEST(holds_alternative(programOrErrors)); +} + +BOOST_AUTO_TEST_CASE(load_should_return_errors_if_parsing_of_nested_object_fails) +{ + string sourceCode( + "object \"C_178\" {\n" + " code {\n" + " return(0, datasize(\"C_178_deployed\"))\n" + " }\n" + " object \"duplicate_name\" {\n" + " code {\n" + " mstore(64, 128)\n" + " }\n" + " }\n" + " object \"duplicate_name\" {\n" + " code {\n" + " mstore(64, 128)\n" + " }\n" + " }\n" + "}\n" + ); + CharStream sourceStream(sourceCode, current_test_case().p_name); + auto programOrErrors = Program::load(sourceStream); + + BOOST_TEST(holds_alternative(programOrErrors)); +} + +BOOST_AUTO_TEST_CASE(load_should_extract_nested_object_with_deployed_suffix_if_present) +{ + string sourceCode( + "object \"C_178\" {\n" + " code {\n" + " return(0, datasize(\"C_178_deployed\"))\n" + " }\n" + " object \"C_178_deployed\" {\n" + " code {\n" + " mstore(64, 128)\n" + " if iszero(calldatasize()) {}\n" + " revert(0, 0)\n" + " }\n" + " }\n" + "}\n" + ); + CharStream sourceStream(sourceCode, current_test_case().p_name); + auto programOrErrors = Program::load(sourceStream); + + BOOST_TEST(holds_alternative(programOrErrors)); +} + +BOOST_AUTO_TEST_CASE(load_should_fall_back_to_parsing_the_whole_object_if_there_is_no_subobject_with_the_right_name) +{ + string sourceCode( + "object \"C_178\" {\n" + " code {\n" + " mstore(64, 128)\n" + " }\n" + " object \"subobject\" {\n" + " code {\n" + " if iszero(calldatasize()) {}\n" + " revert(0, 0)\n" + " }\n" + " }\n" + " object \"C_177_deployed\" {\n" + " code {\n" + " if iszero(calldatasize()) {}\n" + " revert(0, 0)\n" + " }\n" + " }\n" + "}\n" + ); + CharStream sourceStream(sourceCode, current_test_case().p_name); + auto programOrErrors = Program::load(sourceStream); + + BOOST_TEST(holds_alternative(programOrErrors)); + + Block const& parentBlock = skipRedundantBlocks(get(programOrErrors).ast()); + BOOST_TEST(parentBlock.statements.size() == 1); + BOOST_TEST(holds_alternative(parentBlock.statements[0])); +} + +BOOST_AUTO_TEST_CASE(load_should_ignore_data_in_objects) +{ + string sourceCode( + "object \"C_178\" {\n" + " code {\n" + " mstore(64, 128)\n" + " }\n" + " data \"C_178_deployed\" hex\"4123\"\n" + "}\n" + ); + CharStream sourceStream(sourceCode, current_test_case().p_name); + auto programOrErrors = Program::load(sourceStream); + + BOOST_TEST(holds_alternative(programOrErrors)); +} + BOOST_AUTO_TEST_CASE(optimise) { string sourceCode( diff --git a/tools/yulPhaser/Program.cpp b/tools/yulPhaser/Program.cpp index 7dcfddd79..a457750d2 100644 --- a/tools/yulPhaser/Program.cpp +++ b/tools/yulPhaser/Program.cpp @@ -78,7 +78,7 @@ variant Program::load(CharStream& _sourceCode) // ASSUMPTION: parseSource() rewinds the stream on its own Dialect const& dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{}); - variant, ErrorList> astOrErrors = parseSource(dialect, _sourceCode); + variant, ErrorList> astOrErrors = parseObject(dialect, _sourceCode); if (holds_alternative(astOrErrors)) return get(astOrErrors); @@ -122,21 +122,6 @@ string Program::toJson() const return jsonPrettyPrint(serializedAst); } -variant, ErrorList> Program::parseSource(Dialect const& _dialect, CharStream _source) -{ - ErrorList errors; - ErrorReporter errorReporter(errors); - auto scanner = make_shared(move(_source)); - Parser parser(errorReporter, _dialect); - - unique_ptr ast = parser.parse(scanner, false); - if (ast == nullptr) - return errors; - - assert(errorReporter.errors().empty()); - return variant, ErrorList>(move(ast)); -} - variant, ErrorList> Program::parseObject(Dialect const& _dialect, CharStream _source) { ErrorList errors; diff --git a/tools/yulPhaser/Program.h b/tools/yulPhaser/Program.h index df44f2ba4..10fed6de2 100644 --- a/tools/yulPhaser/Program.h +++ b/tools/yulPhaser/Program.h @@ -94,10 +94,6 @@ private: m_nameDispenser(_dialect, *m_ast, {}) {} - static std::variant, langutil::ErrorList> parseSource( - yul::Dialect const& _dialect, - langutil::CharStream _source - ); static std::variant, langutil::ErrorList> parseObject( yul::Dialect const& _dialect, langutil::CharStream _source