From 0dca223b45ebb9076ae4e286aee18374f335a32a Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Wed, 14 Oct 2020 13:20:20 +0200 Subject: [PATCH] Add another argument to setimmutable and the AssignImmutable opcode, allowing to modify code at any memory offset. --- libevmasm/Assembly.cpp | 5 ++- libevmasm/AssemblyItem.cpp | 2 +- libevmasm/KnownState.cpp | 6 +++- libsolidity/codegen/ContractCompiler.cpp | 3 ++ libsolidity/codegen/ir/IRGenerator.cpp | 2 +- libyul/backends/evm/EVMDialect.cpp | 11 ++++--- test/libevmasm/Assembler.cpp | 31 +++++++++++++------ ..._long_name_does_not_end_up_in_bytecode.yul | 13 +++++--- ...literal_argument_into_literal_argument.yul | 2 +- test/libyul/yulSyntaxTests/setimmutable.yul | 2 +- .../setimmutable_bad_literal.yul | 12 +++---- .../string_literal_too_long_immutable.yul | 1 + 12 files changed, 58 insertions(+), 32 deletions(-) diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index c729a4eb4..052ee5f21 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -674,15 +674,18 @@ LinkerObject const& Assembly::assemble() const case AssignImmutable: for (auto const& offset: immutableReferencesBySub[i.data()].second) { - ret.bytecode.push_back(uint8_t(Instruction::DUP1)); + ret.bytecode.push_back(uint8_t(Instruction::DUP2)); + ret.bytecode.push_back(uint8_t(Instruction::DUP2)); // TODO: should we make use of the constant optimizer methods for pushing the offsets? bytes offsetBytes = toCompactBigEndian(u256(offset)); ret.bytecode.push_back(uint8_t(Instruction::PUSH1) - 1 + offsetBytes.size()); ret.bytecode += offsetBytes; + ret.bytecode.push_back(uint8_t(Instruction::ADD)); ret.bytecode.push_back(uint8_t(Instruction::MSTORE)); } immutableReferencesBySub.erase(i.data()); ret.bytecode.push_back(uint8_t(Instruction::POP)); + ret.bytecode.push_back(uint8_t(Instruction::POP)); break; case PushDeployTimeAddress: ret.bytecode.push_back(uint8_t(Instruction::PUSH20)); diff --git a/libevmasm/AssemblyItem.cpp b/libevmasm/AssemblyItem.cpp index e70e844d6..8ff96328c 100644 --- a/libevmasm/AssemblyItem.cpp +++ b/libevmasm/AssemblyItem.cpp @@ -102,7 +102,7 @@ size_t AssemblyItem::arguments() const if (type() == Operation) return static_cast(instructionInfo(instruction()).args); else if (type() == AssignImmutable) - return 1; + return 2; else return 0; } diff --git a/libevmasm/KnownState.cpp b/libevmasm/KnownState.cpp index 3741486a9..89114b01d 100644 --- a/libevmasm/KnownState.cpp +++ b/libevmasm/KnownState.cpp @@ -93,9 +93,13 @@ KnownState::StoreOperation KnownState::feedItem(AssemblyItem const& _item, bool // can be ignored } else if (_item.type() == AssignImmutable) + { // Since AssignImmutable breaks blocks, it should be fine to only consider its changes to the stack, which - // is the same as POP. + // is the same as two POPs. + // Note that the StoreOperation for POP is generic and _copyItem is ignored. + feedItem(AssemblyItem(Instruction::POP), _copyItem); return feedItem(AssemblyItem(Instruction::POP), _copyItem); + } else if (_item.type() != Operation) { assertThrow(_item.deposit() == 1, InvalidDeposit, ""); diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 512725b2e..35780fdc2 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -203,7 +203,10 @@ size_t ContractCompiler::packIntoContractCreator(ContractDefinition const& _cont { auto slotNames = m_context.immutableVariableSlotNames(*immutable); for (auto&& slotName: slotNames | boost::adaptors::reversed) + { + m_context << u256(0); m_context.appendImmutableAssignment(slotName); + } } if (!immutables.empty()) m_context.pushSubroutineSize(m_context.runtimeSub()); diff --git a/libsolidity/codegen/ir/IRGenerator.cpp b/libsolidity/codegen/ir/IRGenerator.cpp index 6b4c5ce61..149324a0d 100644 --- a/libsolidity/codegen/ir/IRGenerator.cpp +++ b/libsolidity/codegen/ir/IRGenerator.cpp @@ -558,7 +558,7 @@ string IRGenerator::deployCode(ContractDefinition const& _contract) codecopy(0, dataoffset(""), datasize("")) <#storeImmutables> - setimmutable("", ) + setimmutable(0, "", ) return(0, datasize("")) diff --git a/libyul/backends/evm/EVMDialect.cpp b/libyul/backends/evm/EVMDialect.cpp index 2c3b6be6d..5287e5a6a 100644 --- a/libyul/backends/evm/EVMDialect.cpp +++ b/libyul/backends/evm/EVMDialect.cpp @@ -221,21 +221,22 @@ map createBuiltins(langutil::EVMVersion _evmVe )); builtins.emplace(createFunction( "setimmutable", - 2, + 3, 0, SideEffects{false, false, false, false, true, SideEffects::None, SideEffects::None, SideEffects::Write}, - {LiteralKind::String, std::nullopt}, + {std::nullopt, LiteralKind::String, std::nullopt}, []( FunctionCall const& _call, AbstractAssembly& _assembly, BuiltinContext&, std::function _visitExpression ) { - yulAssert(_call.arguments.size() == 2, ""); + yulAssert(_call.arguments.size() == 3, ""); - _visitExpression(_call.arguments[1]); + _visitExpression(_call.arguments[2]); + YulString identifier = std::get(_call.arguments[1]).value; + _visitExpression(_call.arguments[0]); _assembly.setSourceLocation(_call.location); - YulString identifier = std::get(_call.arguments.front()).value; _assembly.appendImmutableAssignment(identifier.str()); } )); diff --git a/test/libevmasm/Assembler.cpp b/test/libevmasm/Assembler.cpp index c93d7b58d..b6f668589 100644 --- a/test/libevmasm/Assembler.cpp +++ b/test/libevmasm/Assembler.cpp @@ -91,7 +91,7 @@ BOOST_AUTO_TEST_CASE(all_assembly_items) // PushDeployTimeAddress _assembly.append(PushDeployTimeAddress); // AssignImmutable. - // Note that since there is no reference to "someOtherImmutable", this will compile to a simple POP in the hex output. + // Note that since there is no reference to "someOtherImmutable", this will just compile to two POPs in the hex output. _assembly.appendImmutableAssignment("someOtherImmutable"); _assembly.append(u256(2)); _assembly.appendImmutableAssignment("someImmutable"); @@ -104,9 +104,9 @@ BOOST_AUTO_TEST_CASE(all_assembly_items) BOOST_CHECK_EQUAL( _assembly.assemble().toHex(), - "5b6001600220606f73__$bf005014d9d0f534b8fcb268bd84c491a2$__" - "60005660676022604573000000000000000000000000000000000000000050" - "60028060015250" + "5b6001600220607373__$bf005014d9d0f534b8fcb268bd84c491a2$__" + "600056606b602260497300000000000000000000000000000000000000005050" + "60028181600101525050" "00fe" "7f0000000000000000000000000000000000000000000000000000000000000000" "fe010203044266eeaa" @@ -184,8 +184,10 @@ BOOST_AUTO_TEST_CASE(immutable) shared_ptr _subAsmPtr = make_shared(_subAsm); _assembly.append(u256(42)); + _assembly.append(u256(0)); _assembly.appendImmutableAssignment("someImmutable"); _assembly.append(u256(23)); + _assembly.append(u256(0)); _assembly.appendImmutableAssignment("someOtherImmutable"); auto sub = _assembly.appendSubroutine(_subAsmPtr); @@ -198,21 +200,26 @@ BOOST_AUTO_TEST_CASE(immutable) // root.asm // assign "someImmutable" "602a" // PUSH1 42 - value for someImmutable - "80" // DUP1 + "6000" // PUSH1 0 - offset of code into which to insert the immutable + "8181" // DUP2 DUP2 "6001" // PUSH1 1 - offset of first someImmutable in sub_0 + "01" // ADD - add offset of immutable to offset of code "52" // MSTORE - "80" // DUP1 + "8181" // DUP2 DUP2 "6043" // PUSH1 67 - offset of second someImmutable in sub_0 + "01" // ADD - add offset of immutable to offset of code "52" // MSTORE - "50" // POP + "5050" // POP POP // assign "someOtherImmutable" "6017" // PUSH1 23 - value for someOtherImmutable - "80" // DUP1 + "6000" // PUSH1 0 - offset of code into which to insert the immutable + "8181" // DUP2 DUP2 "6022" // PUSH1 34 - offset of someOtherImmutable in sub_0 + "01" // ADD - add offset of immutable to offset of code "52" // MSTORE - "50" // POP + "5050" // POP POP "6063" // PUSH1 0x63 - dataSize(sub_0) - "6017" // PUSH1 0x17 - dataOffset(sub_0) + "6023" // PUSH1 0x23 - dataOffset(sub_0) "fe" // INVALID // end of root.asm // sub.asm @@ -224,8 +231,10 @@ BOOST_AUTO_TEST_CASE(immutable) _assembly.assemblyString(), " /* \"root.asm\":1:3 */\n" " 0x2a\n" + " 0x00\n" " assignImmutable(\"0x26f2c0195e9d408feff3abd77d83f2971f3c9a18d1e8a9437c7835ae4211fc9f\")\n" " 0x17\n" + " 0x00\n" " assignImmutable(\"0xc3978657661c4d8e32e3d5f42597c009f0d3859e9f9d0d94325268f9799e2bfb\")\n" " dataSize(sub_0)\n" " dataOffset(sub_0)\n" @@ -242,8 +251,10 @@ BOOST_AUTO_TEST_CASE(immutable) util::jsonCompactPrint(_assembly.assemblyJSON(indices)), "{\".code\":[" "{\"begin\":1,\"end\":3,\"name\":\"PUSH\",\"source\":0,\"value\":\"2A\"}," + "{\"begin\":1,\"end\":3,\"name\":\"PUSH\",\"source\":0,\"value\":\"0\"}," "{\"begin\":1,\"end\":3,\"name\":\"ASSIGNIMMUTABLE\",\"source\":0,\"value\":\"someImmutable\"}," "{\"begin\":1,\"end\":3,\"name\":\"PUSH\",\"source\":0,\"value\":\"17\"}," + "{\"begin\":1,\"end\":3,\"name\":\"PUSH\",\"source\":0,\"value\":\"0\"}," "{\"begin\":1,\"end\":3,\"name\":\"ASSIGNIMMUTABLE\",\"source\":0,\"value\":\"someOtherImmutable\"}," "{\"begin\":1,\"end\":3,\"name\":\"PUSH #[$]\",\"source\":0,\"value\":\"0000000000000000000000000000000000000000000000000000000000000000\"}," "{\"begin\":1,\"end\":3,\"name\":\"PUSH [$]\",\"source\":0,\"value\":\"0000000000000000000000000000000000000000000000000000000000000000\"}" diff --git a/test/libyul/objectCompiler/immutable_long_name_does_not_end_up_in_bytecode.yul b/test/libyul/objectCompiler/immutable_long_name_does_not_end_up_in_bytecode.yul index b7058289b..a208d15a5 100644 --- a/test/libyul/objectCompiler/immutable_long_name_does_not_end_up_in_bytecode.yul +++ b/test/libyul/objectCompiler/immutable_long_name_does_not_end_up_in_bytecode.yul @@ -1,6 +1,7 @@ object "a" { code { setimmutable( + 0, "long___name___that___definitely___exceeds___the___thirty___two___byte___limit", 0x1234567890123456789012345678901234567890 ) @@ -8,10 +9,12 @@ object "a" { } // ---- // Assembly: -// /* "source":152:194 */ +// /* "source":167:209 */ // 0x1234567890123456789012345678901234567890 -// /* "source":32:204 */ +// /* "source":58:59 */ +// 0x00 +// /* "source":32:219 */ // assignImmutable("0x85a5b1db611c82c46f5fa18e39ae218397536256c451e5de155a86de843a9ad6") -// Bytecode: 73123456789012345678901234567890123456789050 -// Opcodes: PUSH20 0x1234567890123456789012345678901234567890 POP -// SourceMappings: 152:42:0:-:0;32:172 +// Bytecode: 73123456789012345678901234567890123456789060005050 +// Opcodes: PUSH20 0x1234567890123456789012345678901234567890 PUSH1 0x0 POP POP +// SourceMappings: 167:42:0:-:0;58:1;32:187 diff --git a/test/libyul/yulSyntaxTests/passing_builtin_with_literal_argument_into_literal_argument.yul b/test/libyul/yulSyntaxTests/passing_builtin_with_literal_argument_into_literal_argument.yul index 4884fc265..433d36683 100644 --- a/test/libyul/yulSyntaxTests/passing_builtin_with_literal_argument_into_literal_argument.yul +++ b/test/libyul/yulSyntaxTests/passing_builtin_with_literal_argument_into_literal_argument.yul @@ -1,5 +1,5 @@ { - setimmutable(loadimmutable("abc"), "abc") + setimmutable(0, loadimmutable("abc"), "abc") } // ==== // dialect: evm diff --git a/test/libyul/yulSyntaxTests/setimmutable.yul b/test/libyul/yulSyntaxTests/setimmutable.yul index 3c37c90ff..e1d6a7491 100644 --- a/test/libyul/yulSyntaxTests/setimmutable.yul +++ b/test/libyul/yulSyntaxTests/setimmutable.yul @@ -1,5 +1,5 @@ { - setimmutable("address", 0x1234567890123456789012345678901234567890) + setimmutable(0, "address", 0x1234567890123456789012345678901234567890) } // ==== // dialect: evm diff --git a/test/libyul/yulSyntaxTests/setimmutable_bad_literal.yul b/test/libyul/yulSyntaxTests/setimmutable_bad_literal.yul index 6f19c6edb..9e4bc8032 100644 --- a/test/libyul/yulSyntaxTests/setimmutable_bad_literal.yul +++ b/test/libyul/yulSyntaxTests/setimmutable_bad_literal.yul @@ -1,11 +1,11 @@ { - setimmutable(0, 0x1234567890123456789012345678901234567890) - setimmutable(true, 0x1234567890123456789012345678901234567890) - setimmutable(false, 0x1234567890123456789012345678901234567890) + setimmutable(0, 0, 0x1234567890123456789012345678901234567890) + setimmutable(0, true, 0x1234567890123456789012345678901234567890) + setimmutable(0, false, 0x1234567890123456789012345678901234567890) } // ==== // dialect: evm // ---- -// TypeError 5859: (19-20): Function expects string literal. -// TypeError 5859: (83-87): Function expects string literal. -// TypeError 5859: (150-155): Function expects string literal. +// TypeError 5859: (22-23): Function expects string literal. +// TypeError 5859: (89-93): Function expects string literal. +// TypeError 5859: (159-164): Function expects string literal. diff --git a/test/libyul/yulSyntaxTests/string_literal_too_long_immutable.yul b/test/libyul/yulSyntaxTests/string_literal_too_long_immutable.yul index 37ce835df..98707451e 100644 --- a/test/libyul/yulSyntaxTests/string_literal_too_long_immutable.yul +++ b/test/libyul/yulSyntaxTests/string_literal_too_long_immutable.yul @@ -1,5 +1,6 @@ { setimmutable( + 0, "long___name___that___definitely___exceeds___the___thirty___two___byte___limit", 0x1234567890123456789012345678901234567890 )