From ce0a3e93f2e2a29a179d6c8d7f98b01310d7f909 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Mon, 7 Mar 2022 12:49:23 +0100 Subject: [PATCH] Store whether an evmasm Assembly is creation code. --- Changelog.md | 1 + libevmasm/Assembly.cpp | 8 ++++--- libevmasm/Assembly.h | 6 ++++- libsolidity/codegen/CompilerContext.h | 2 +- libyul/AssemblyStack.cpp | 8 +++++-- libyul/backends/evm/AbstractAssembly.h | 2 +- libyul/backends/evm/EVMObjectCompiler.cpp | 5 +++- libyul/backends/evm/EthAssemblyAdapter.cpp | 4 ++-- libyul/backends/evm/EthAssemblyAdapter.h | 2 +- libyul/backends/evm/NoOutputAssembly.cpp | 2 +- libyul/backends/evm/NoOutputAssembly.h | 2 +- test/cmdlineTests/yul_optimize_runs/output | 5 ++-- test/libevmasm/Assembler.cpp | 18 +++++++------- test/libevmasm/Optimiser.cpp | 4 ++-- .../semanticTests/array/reusing_memory.sol | 2 +- .../constructor/arrays_in_constructors.sol | 2 +- .../bytes_in_constructors_packer.sol | 2 +- .../creation_function_call_with_args.sol | 2 +- .../creation_function_call_with_salt.sol | 2 +- .../inheritance/value_for_constructor.sol | 2 +- .../salted_create_with_value.sol | 2 +- test/libyul/EVMCodeTransformTest.cpp | 2 +- test/tools/fuzzer_common.cpp | 24 ++++++++++--------- 23 files changed, 62 insertions(+), 47 deletions(-) diff --git a/Changelog.md b/Changelog.md index 181c386aa..63588731f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -8,6 +8,7 @@ Compiler Features: * JSON-AST: Added selector field for errors and events. Bugfixes: + * Yul IR Code Generation: Optimize embedded creation code with correct settings. This fixes potential mismatches between the constructor code of a contract compiled in isolation and the bytecode in ``type(C).creationCode``, resp. the bytecode used for ``new C(...)``. ### 0.8.12 (2022-02-16) diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index 55d23e283..492d434c8 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -411,13 +411,15 @@ map const& Assembly::optimiseInternal( if (m_tagReplacements) return *m_tagReplacements; + assertThrow(_settings.isCreation == m_creation, OptimizerException, "Mismatching creation settings."); + // Run optimisation for sub-assemblies. for (size_t subId = 0; subId < m_subs.size(); ++subId) { OptimiserSettings settings = _settings; - // Disable creation mode for sub-assemblies. - settings.isCreation = false; - map const& subTagReplacements = m_subs[subId]->optimiseInternal( + Assembly& sub = *m_subs[subId]; + settings.isCreation = sub.isCreation(); + map const& subTagReplacements = sub.optimiseInternal( settings, JumpdestRemover::referencedTags(m_items, subId) ); diff --git a/libevmasm/Assembly.h b/libevmasm/Assembly.h index 97807e223..270d85480 100644 --- a/libevmasm/Assembly.h +++ b/libevmasm/Assembly.h @@ -48,7 +48,7 @@ using AssemblyPointer = std::shared_ptr; class Assembly { public: - explicit Assembly(std::string _name = std::string()):m_name(std::move(_name)) { } + Assembly(bool _creation, std::string _name): m_creation(_creation), m_name(std::move(_name)) { } AssemblyItem newTag() { assertThrow(m_usedTags < 0xffffffff, AssemblyException, ""); return AssemblyItem(Tag, m_usedTags++); } AssemblyItem newPushTag() { assertThrow(m_usedTags < 0xffffffff, AssemblyException, ""); return AssemblyItem(PushTag, m_usedTags++); } @@ -157,6 +157,8 @@ public: std::vector decodeSubPath(size_t _subObjectId) const; size_t encodeSubPath(std::vector const& _subPath); + bool isCreation() const { return m_creation; } + protected: /// Does the same operations as @a optimise, but should only be applied to a sub and /// returns the replaced tags. Also takes an argument containing the tags of this assembly @@ -214,6 +216,8 @@ protected: mutable std::vector m_tagPositionsInBytecode; int m_deposit = 0; + /// True, if the assembly contains contract creation code. + bool const m_creation = false; /// Internal name of the assembly object, only used with the Yul backend /// currently std::string m_name; diff --git a/libsolidity/codegen/CompilerContext.h b/libsolidity/codegen/CompilerContext.h index cd71d5a4d..a796ac60b 100644 --- a/libsolidity/codegen/CompilerContext.h +++ b/libsolidity/codegen/CompilerContext.h @@ -65,7 +65,7 @@ public: RevertStrings _revertStrings, CompilerContext* _runtimeContext = nullptr ): - m_asm(std::make_shared()), + m_asm(std::make_shared(_runtimeContext != nullptr, std::string{})), m_evmVersion(_evmVersion), m_revertStrings(_revertStrings), m_reservedMemory{0}, diff --git a/libyul/AssemblyStack.cpp b/libyul/AssemblyStack.cpp index 30ad58d77..5d0ddd81a 100644 --- a/libyul/AssemblyStack.cpp +++ b/libyul/AssemblyStack.cpp @@ -38,6 +38,7 @@ #include #include +#include #include using namespace std; @@ -194,7 +195,10 @@ void AssemblyStack::optimize(Object& _object, bool _isCreation) yulAssert(_object.analysisInfo, ""); for (auto& subNode: _object.subObjects) if (auto subObject = dynamic_cast(subNode.get())) - optimize(*subObject, false); + { + bool isCreation = !boost::ends_with(subObject->name.str(), "_deployed"); + optimize(*subObject, isCreation); + } Dialect const& dialect = languageToDialect(m_language, m_evmVersion); unique_ptr meter; @@ -281,7 +285,7 @@ AssemblyStack::assembleEVMWithDeployed(optional _deployName) const yulAssert(m_parserResult->code, ""); yulAssert(m_parserResult->analysisInfo, ""); - evmasm::Assembly assembly; + evmasm::Assembly assembly(true, {}); EthAssemblyAdapter adapter(assembly); compileEVM(adapter, m_optimiserSettings.optimizeStackAllocation); diff --git a/libyul/backends/evm/AbstractAssembly.h b/libyul/backends/evm/AbstractAssembly.h index 66d5333e2..2d7be9001 100644 --- a/libyul/backends/evm/AbstractAssembly.h +++ b/libyul/backends/evm/AbstractAssembly.h @@ -98,7 +98,7 @@ public: /// Append the assembled size as a constant. virtual void appendAssemblySize() = 0; /// Creates a new sub-assembly, which can be referenced using dataSize and dataOffset. - virtual std::pair, SubID> createSubAssembly(std::string _name = "") = 0; + virtual std::pair, SubID> createSubAssembly(bool _creation, std::string _name = "") = 0; /// Appends the offset of the given sub-assembly or data. virtual void appendDataOffset(std::vector const& _subPath) = 0; /// Appends the size of the given sub-assembly or data. diff --git a/libyul/backends/evm/EVMObjectCompiler.cpp b/libyul/backends/evm/EVMObjectCompiler.cpp index c3c95d8bc..b60e383e6 100644 --- a/libyul/backends/evm/EVMObjectCompiler.cpp +++ b/libyul/backends/evm/EVMObjectCompiler.cpp @@ -30,6 +30,8 @@ #include #include +#include + using namespace solidity::yul; using namespace std; @@ -48,7 +50,8 @@ void EVMObjectCompiler::run(Object& _object, bool _optimize) for (auto const& subNode: _object.subObjects) if (auto* subObject = dynamic_cast(subNode.get())) { - auto subAssemblyAndID = m_assembly.createSubAssembly(subObject->name.str()); + bool isCreation = !boost::ends_with(subObject->name.str(), "_deployed"); + auto subAssemblyAndID = m_assembly.createSubAssembly(isCreation, subObject->name.str()); context.subIDs[subObject->name] = subAssemblyAndID.second; subObject->subId = subAssemblyAndID.second; compile(*subObject, *subAssemblyAndID.first, m_dialect, _optimize); diff --git a/libyul/backends/evm/EthAssemblyAdapter.cpp b/libyul/backends/evm/EthAssemblyAdapter.cpp index 1c41534b2..5d7b4cbf4 100644 --- a/libyul/backends/evm/EthAssemblyAdapter.cpp +++ b/libyul/backends/evm/EthAssemblyAdapter.cpp @@ -122,9 +122,9 @@ void EthAssemblyAdapter::appendAssemblySize() m_assembly.appendProgramSize(); } -pair, AbstractAssembly::SubID> EthAssemblyAdapter::createSubAssembly(string _name) +pair, AbstractAssembly::SubID> EthAssemblyAdapter::createSubAssembly(bool _creation, string _name) { - shared_ptr assembly{make_shared(std::move(_name))}; + shared_ptr assembly{make_shared(_creation, std::move(_name))}; auto sub = m_assembly.newSub(assembly); return {make_shared(*assembly), static_cast(sub.data())}; } diff --git a/libyul/backends/evm/EthAssemblyAdapter.h b/libyul/backends/evm/EthAssemblyAdapter.h index c11b375f6..87047ccbf 100644 --- a/libyul/backends/evm/EthAssemblyAdapter.h +++ b/libyul/backends/evm/EthAssemblyAdapter.h @@ -55,7 +55,7 @@ public: void appendJumpTo(LabelID _labelId, int _stackDiffAfter, JumpType _jumpType) override; void appendJumpToIf(LabelID _labelId, JumpType _jumpType) override; void appendAssemblySize() override; - std::pair, SubID> createSubAssembly(std::string _name = {}) override; + std::pair, SubID> createSubAssembly(bool _creation, std::string _name = {}) override; void appendDataOffset(std::vector const& _subPath) override; void appendDataSize(std::vector const& _subPath) override; SubID appendData(bytes const& _data) override; diff --git a/libyul/backends/evm/NoOutputAssembly.cpp b/libyul/backends/evm/NoOutputAssembly.cpp index 62c0ad119..e26204201 100644 --- a/libyul/backends/evm/NoOutputAssembly.cpp +++ b/libyul/backends/evm/NoOutputAssembly.cpp @@ -98,7 +98,7 @@ void NoOutputAssembly::appendAssemblySize() appendInstruction(evmasm::Instruction::PUSH1); } -pair, AbstractAssembly::SubID> NoOutputAssembly::createSubAssembly(std::string) +pair, AbstractAssembly::SubID> NoOutputAssembly::createSubAssembly(bool, std::string) { yulAssert(false, "Sub assemblies not implemented."); return {}; diff --git a/libyul/backends/evm/NoOutputAssembly.h b/libyul/backends/evm/NoOutputAssembly.h index b41d7f4a6..1103392ef 100644 --- a/libyul/backends/evm/NoOutputAssembly.h +++ b/libyul/backends/evm/NoOutputAssembly.h @@ -65,7 +65,7 @@ public: void appendJumpToIf(LabelID _labelId, JumpType _jumpType) override; void appendAssemblySize() override; - std::pair, SubID> createSubAssembly(std::string _name = "") override; + std::pair, SubID> createSubAssembly(bool _creation, std::string _name = "") override; void appendDataOffset(std::vector const& _subPath) override; void appendDataSize(std::vector const& _subPath) override; SubID appendData(bytes const& _data) override; diff --git a/test/cmdlineTests/yul_optimize_runs/output b/test/cmdlineTests/yul_optimize_runs/output index b95184440..357c796ee 100644 --- a/test/cmdlineTests/yul_optimize_runs/output +++ b/test/cmdlineTests/yul_optimize_runs/output @@ -21,7 +21,7 @@ object "RunsTest1" { Binary representation: -602580600c6000396000f3fe7fabc123450000000000000000000000000000000000000000000000000000000060005500 +600c80600c6000396000f3fe63abc1234560e01b60005500 Text representation: /* "yul_optimize_runs/input.yul":106:125 */ @@ -40,8 +40,7 @@ Text representation: stop sub_0: assembly { - /* "yul_optimize_runs/input.yul":237:257 */ - 0xabc1234500000000000000000000000000000000000000000000000000000000 + shl(0xe0, 0xabc12345) /* "yul_optimize_runs/input.yul":277:278 */ 0x00 /* "yul_optimize_runs/input.yul":270:288 */ diff --git a/test/libevmasm/Assembler.cpp b/test/libevmasm/Assembler.cpp index 78e598bca..3a3167816 100644 --- a/test/libevmasm/Assembler.cpp +++ b/test/libevmasm/Assembler.cpp @@ -58,11 +58,11 @@ BOOST_AUTO_TEST_CASE(all_assembly_items) { "root.asm", 0 }, { "sub.asm", 1 } }; - Assembly _assembly; + Assembly _assembly{false, {}}; auto root_asm = make_shared("root.asm"); _assembly.setSourceLocation({1, 3, root_asm}); - Assembly _subAsm; + Assembly _subAsm{false, {}}; auto sub_asm = make_shared("sub.asm"); _subAsm.setSourceLocation({6, 8, sub_asm}); // PushImmutable @@ -197,7 +197,7 @@ BOOST_AUTO_TEST_CASE(immutables_and_its_source_maps) { *subName, 1 } }; - auto subAsm = make_shared(); + auto subAsm = make_shared(false, string{}); for (char i = 0; i < numImmutables; ++i) { for (int r = 0; r < numActualRefs; ++r) @@ -207,7 +207,7 @@ BOOST_AUTO_TEST_CASE(immutables_and_its_source_maps) } } - Assembly assembly; + Assembly assembly{true, {}}; for (char i = 1; i <= numImmutables; ++i) { assembly.setSourceLocation({10*i, 10*i + 3+i, assemblyName}); @@ -256,11 +256,11 @@ BOOST_AUTO_TEST_CASE(immutable) { "root.asm", 0 }, { "sub.asm", 1 } }; - Assembly _assembly; + Assembly _assembly{true, {}}; auto root_asm = make_shared("root.asm"); _assembly.setSourceLocation({1, 3, root_asm}); - Assembly _subAsm; + Assembly _subAsm{false, {}}; auto sub_asm = make_shared("sub.asm"); _subAsm.setSourceLocation({6, 8, sub_asm}); _subAsm.appendImmutable("someImmutable"); @@ -349,10 +349,10 @@ BOOST_AUTO_TEST_CASE(immutable) BOOST_AUTO_TEST_CASE(subobject_encode_decode) { - Assembly assembly; + Assembly assembly{true, {}}; - shared_ptr subAsmPtr = make_shared(); - shared_ptr subSubAsmPtr = make_shared(); + shared_ptr subAsmPtr = make_shared(false, string{}); + shared_ptr subSubAsmPtr = make_shared(false, string{}); assembly.appendSubroutine(subAsmPtr); subAsmPtr->appendSubroutine(subSubAsmPtr); diff --git a/test/libevmasm/Optimiser.cpp b/test/libevmasm/Optimiser.cpp index c8bb93c41..278a7c77f 100644 --- a/test/libevmasm/Optimiser.cpp +++ b/test/libevmasm/Optimiser.cpp @@ -1250,8 +1250,8 @@ BOOST_AUTO_TEST_CASE(jumpdest_removal_subassemblies) // tag unifications (due to block deduplication) is also // visible at the super-assembly. - Assembly main; - AssemblyPointer sub = make_shared(); + Assembly main{false, {}}; + AssemblyPointer sub = make_shared(true, string{}); sub->append(u256(1)); auto t1 = sub->newTag(); diff --git a/test/libsolidity/semanticTests/array/reusing_memory.sol b/test/libsolidity/semanticTests/array/reusing_memory.sol index ee91ce7f7..ba0ca8abd 100644 --- a/test/libsolidity/semanticTests/array/reusing_memory.sol +++ b/test/libsolidity/semanticTests/array/reusing_memory.sol @@ -26,6 +26,6 @@ contract Main { // compileViaYul: also // ---- // f(uint256): 0x34 -> 0x46bddb1178e94d7f2892ff5f366840eb658911794f2c3a44c450aa2c505186c1 -// gas irOptimized: 113598 +// gas irOptimized: 113610 // gas legacy: 126596 // gas legacyOptimized: 113823 diff --git a/test/libsolidity/semanticTests/constructor/arrays_in_constructors.sol b/test/libsolidity/semanticTests/constructor/arrays_in_constructors.sol index cc303899a..21de2ac97 100644 --- a/test/libsolidity/semanticTests/constructor/arrays_in_constructors.sol +++ b/test/libsolidity/semanticTests/constructor/arrays_in_constructors.sol @@ -26,6 +26,6 @@ contract Creator { // compileViaYul: also // ---- // f(uint256,address[]): 7, 0x40, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 -> 7, 8 -// gas irOptimized: 443960 +// gas irOptimized: 444029 // gas legacy: 590683 // gas legacyOptimized: 448326 diff --git a/test/libsolidity/semanticTests/constructor/bytes_in_constructors_packer.sol b/test/libsolidity/semanticTests/constructor/bytes_in_constructors_packer.sol index ffe0fb8c2..d5e633924 100644 --- a/test/libsolidity/semanticTests/constructor/bytes_in_constructors_packer.sol +++ b/test/libsolidity/semanticTests/constructor/bytes_in_constructors_packer.sol @@ -26,6 +26,6 @@ contract Creator { // compileViaYul: also // ---- // f(uint256,bytes): 7, 0x40, 78, "abcdefghijklmnopqrstuvwxyzabcdef", "ghijklmnopqrstuvwxyzabcdefghijkl", "mnopqrstuvwxyz" -> 7, "h" -// gas irOptimized: 300804 +// gas irOptimized: 300885 // gas legacy: 428917 // gas legacyOptimized: 298128 diff --git a/test/libsolidity/semanticTests/functionCall/creation_function_call_with_args.sol b/test/libsolidity/semanticTests/functionCall/creation_function_call_with_args.sol index 685d8a7de..c4a65bb55 100644 --- a/test/libsolidity/semanticTests/functionCall/creation_function_call_with_args.sol +++ b/test/libsolidity/semanticTests/functionCall/creation_function_call_with_args.sol @@ -17,7 +17,7 @@ contract D { // compileViaYul: also // ---- // constructor(): 2 -> -// gas irOptimized: 203967 +// gas irOptimized: 203963 // gas legacy: 245842 // gas legacyOptimized: 195676 // f() -> 2 diff --git a/test/libsolidity/semanticTests/functionCall/creation_function_call_with_salt.sol b/test/libsolidity/semanticTests/functionCall/creation_function_call_with_salt.sol index a768a2836..7ed638418 100644 --- a/test/libsolidity/semanticTests/functionCall/creation_function_call_with_salt.sol +++ b/test/libsolidity/semanticTests/functionCall/creation_function_call_with_salt.sol @@ -18,7 +18,7 @@ contract D { // compileViaYul: also // ---- // constructor(): 2 -> -// gas irOptimized: 204130 +// gas irOptimized: 204126 // gas legacy: 246202 // gas legacyOptimized: 195914 // f() -> 2 diff --git a/test/libsolidity/semanticTests/inheritance/value_for_constructor.sol b/test/libsolidity/semanticTests/inheritance/value_for_constructor.sol index e6f94bdcf..462de46a7 100644 --- a/test/libsolidity/semanticTests/inheritance/value_for_constructor.sol +++ b/test/libsolidity/semanticTests/inheritance/value_for_constructor.sol @@ -42,7 +42,7 @@ contract Main { // compileViaYul: also // ---- // constructor(), 22 wei -> -// gas irOptimized: 284287 +// gas irOptimized: 284283 // gas legacy: 402045 // gas legacyOptimized: 266772 // getFlag() -> true diff --git a/test/libsolidity/semanticTests/salted_create/salted_create_with_value.sol b/test/libsolidity/semanticTests/salted_create/salted_create_with_value.sol index 4ce8b5149..eb9d93ee2 100644 --- a/test/libsolidity/semanticTests/salted_create/salted_create_with_value.sol +++ b/test/libsolidity/semanticTests/salted_create/salted_create_with_value.sol @@ -22,6 +22,6 @@ contract A { // compileViaYul: also // ---- // f(), 10 ether -> 3007, 3008, 3009 -// gas irOptimized: 272413 +// gas irOptimized: 272449 // gas legacy: 422501 // gas legacyOptimized: 287472 diff --git a/test/libyul/EVMCodeTransformTest.cpp b/test/libyul/EVMCodeTransformTest.cpp index 8986acc1b..679493f35 100644 --- a/test/libyul/EVMCodeTransformTest.cpp +++ b/test/libyul/EVMCodeTransformTest.cpp @@ -65,7 +65,7 @@ TestCase::TestResult EVMCodeTransformTest::run(ostream& _stream, string const& _ return TestResult::FatalError; } - evmasm::Assembly assembly; + evmasm::Assembly assembly{false, {}}; EthAssemblyAdapter adapter(assembly); EVMObjectCompiler::compile( *stack.parserResult(), diff --git a/test/tools/fuzzer_common.cpp b/test/tools/fuzzer_common.cpp index 82066bf52..094a84554 100644 --- a/test/tools/fuzzer_common.cpp +++ b/test/tools/fuzzer_common.cpp @@ -185,25 +185,27 @@ void FuzzerUtil::testConstantOptimizer(string const& _input, bool _quiet) if (!_quiet) cout << "Got " << numbers.size() << " inputs:" << endl; - Assembly assembly; - for (u256 const& n: numbers) - { - if (!_quiet) - cout << n << endl; - assembly.append(n); - } for (bool isCreation: {false, true}) + { + Assembly assembly{isCreation, {}}; + for (u256 const& n: numbers) + { + if (!_quiet) + cout << n << endl; + assembly.append(n); + } for (unsigned runs: {1u, 2u, 3u, 20u, 40u, 100u, 200u, 400u, 1000u}) { // Make a copy here so that each time we start with the original state. Assembly tmp = assembly; ConstantOptimisationMethod::optimiseConstants( - isCreation, - runs, - langutil::EVMVersion{}, - tmp + isCreation, + runs, + langutil::EVMVersion{}, + tmp ); } + } } void FuzzerUtil::testStandardCompiler(string const& _input, bool _quiet)