From 897292439c4c21c47a7b69509086283d0443d394 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Tue, 21 Sep 2021 17:52:08 +0200 Subject: [PATCH] Properly compute source mappings for immutables. --- Changelog.md | 1 + libevmasm/Assembly.cpp | 4 ++ libevmasm/AssemblyItem.cpp | 19 ++++++++- libevmasm/AssemblyItem.h | 6 ++- test/libevmasm/Assembler.cpp | 83 ++++++++++++++++++++++++++++++++++++ 5 files changed, 109 insertions(+), 4 deletions(-) diff --git a/Changelog.md b/Changelog.md index 058ecc2e0..bd0fa5554 100644 --- a/Changelog.md +++ b/Changelog.md @@ -27,6 +27,7 @@ Important Bugfixes: Bugfixes: * AST: Export ``canonicalName`` for ``UserDefinedValueTypeDefinition`` and ``ContractDefinition``. + * Code Generator: Fixes source mappings for immutables. diff --git a/libevmasm/Assembly.cpp b/libevmasm/Assembly.cpp index a2c679683..da4e8ec29 100644 --- a/libevmasm/Assembly.cpp +++ b/libevmasm/Assembly.cpp @@ -675,8 +675,11 @@ LinkerObject const& Assembly::assemble() const break; case PushImmutable: ret.bytecode.push_back(static_cast(Instruction::PUSH32)); + // Maps keccak back to the "identifier" string of that immutable. ret.immutableReferences[i.data()].first = m_immutables.at(i.data()); + // Record the bytecode offset of the PUSH32 argument. ret.immutableReferences[i.data()].second.emplace_back(ret.bytecode.size()); + // Advance bytecode by 32 bytes (default initialized). ret.bytecode.resize(ret.bytecode.size() + 32); break; case VerbatimBytecode: @@ -684,6 +687,7 @@ LinkerObject const& Assembly::assemble() const break; case AssignImmutable: { + // Expect 2 elements on stack (source, dest_base) auto const& offsets = immutableReferencesBySub[i.data()].second; for (size_t i = 0; i < offsets.size(); ++i) { diff --git a/libevmasm/AssemblyItem.cpp b/libevmasm/AssemblyItem.cpp index a34870eda..d913ebc36 100644 --- a/libevmasm/AssemblyItem.cpp +++ b/libevmasm/AssemblyItem.cpp @@ -88,9 +88,11 @@ size_t AssemblyItem::bytesRequired(size_t _addressLength) const return 1 + 32; case AssignImmutable: if (m_immutableOccurrences) - return 1 + (3 + 32) * *m_immutableOccurrences; + // (DUP DUP PUSH ADD MSTORE)* (PUSH ADD MSTORE) + return (*m_immutableOccurrences - 1) * (5 + 32) + (3 + 32); else - return 1 + (3 + 32) * 1024; // 1024 occurrences are beyond the maximum code size anyways. + // POP POP + return 2; case VerbatimBytecode: return std::get<2>(*m_verbatimBytecode).size(); default: @@ -334,6 +336,7 @@ std::string AssemblyItem::computeSourceMapping( int prevSourceIndex = -1; int prevModifierDepth = -1; char prevJump = 0; + for (auto const& item: _items) { if (!ret.empty()) @@ -402,6 +405,18 @@ std::string AssemblyItem::computeSourceMapping( } } + // Append empty items if this AssignImmutable was referenced more than once. + // For n immutable occurrences the first (n - 1) occurrences will + // generate 5 opcodes and the last will generate 3 opcodes, + // because it is reusing the 2 top-most elements on the stack. + if (item.immutableOccurrences()) + { + auto const n = (item.immutableOccurrences() - 1) * 5 + 3; + ret += string(n - 1, ';'); + } + else if (item.type() == AssemblyItemType::AssignImmutable) + ret += ';'; // two POP's + prevStart = location.start; prevLength = length; prevSourceIndex = sourceIndex; diff --git a/libevmasm/AssemblyItem.h b/libevmasm/AssemblyItem.h index f1b2b5043..9da2aa433 100644 --- a/libevmasm/AssemblyItem.h +++ b/libevmasm/AssemblyItem.h @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -169,7 +170,8 @@ public: size_t m_modifierDepth = 0; - void setImmutableOccurrences(size_t _n) const { m_immutableOccurrences = std::make_shared(_n); } + void setImmutableOccurrences(size_t _n) const { m_immutableOccurrences = _n; } + size_t immutableOccurrences() const noexcept { return m_immutableOccurrences.value_or(0); } private: AssemblyItemType m_type; @@ -184,7 +186,7 @@ private: /// e.g. PushSubSize, PushTag, PushSub, etc. mutable std::shared_ptr m_pushedValue; /// Number of PushImmutable's with the same hash. Only used for AssignImmutable. - mutable std::shared_ptr m_immutableOccurrences; + mutable std::optional m_immutableOccurrences; }; inline size_t bytesRequired(AssemblyItems const& _items, size_t _addressLength) diff --git a/test/libevmasm/Assembler.cpp b/test/libevmasm/Assembler.cpp index 0c1e4dbad..e7024f252 100644 --- a/test/libevmasm/Assembler.cpp +++ b/test/libevmasm/Assembler.cpp @@ -165,6 +165,89 @@ BOOST_AUTO_TEST_CASE(all_assembly_items) ); } +BOOST_AUTO_TEST_CASE(immutables_and_its_source_maps) +{ + // Tests for 1, 2, 3 number of immutables. + for (int numImmutables = 1; numImmutables <= 3; ++numImmutables) + { + BOOST_TEST_MESSAGE("NumImmutables: "s + to_string(numImmutables)); + // Tests for the cases 1, 2, 3 occurrences of an immutable reference. + for (int numActualRefs = 1; numActualRefs <= 3; ++numActualRefs) + { + BOOST_TEST_MESSAGE("NumActualRefs: "s + to_string(numActualRefs)); + auto const NumExpectedMappings = + ( + 2 + // PUSH PUSH + (numActualRefs - 1) * 5 + // DUP DUP PUSH ADD MTOSRE + 3 // PUSH ADD MSTORkhbE + ) * numImmutables; + + auto constexpr NumSubs = 1; + auto constexpr NumOpcodesWithoutMappings = + NumSubs + // PUSH for every sub assembly + 1; // INVALID + + auto assemblyName = make_shared("root.asm"); + auto subName = make_shared("sub.asm"); + + map indices = { + { *assemblyName, 0 }, + { *subName, 1 } + }; + + auto subAsm = make_shared(); + for (int8_t i = 0; i < numImmutables; ++i) + { + for (int r = 0; r < numActualRefs; ++r) + { + subAsm->setSourceLocation(SourceLocation{10*i, 10*i + 6 + r, subName}); + subAsm->appendImmutable(string(1, 'a' + i)); // "a", "b", ... + } + } + + Assembly assembly; + for (int8_t i = 1; i <= numImmutables; ++i) + { + assembly.setSourceLocation({10*i, 10*i + 3+i, assemblyName}); + assembly.append(u256(0x71)); // immutble value + assembly.append(u256(0)); // target... modules? + assembly.appendImmutableAssignment(string(1, 'a' + i - 1)); + } + + assembly.appendSubroutine(subAsm); + + checkCompilation(assembly); + + string const sourceMappings = AssemblyItem::computeSourceMapping(assembly.items(), indices); + auto const numberOfMappings = std::count(sourceMappings.begin(), sourceMappings.end(), ';'); + + LinkerObject const& obj = assembly.assemble(); + string const disassembly = disassemble(obj.bytecode, "\n"); + auto const numberOfOpcodes = std::count(disassembly.begin(), disassembly.end(), '\n'); + + #if 0 // {{{ debug prints + { + LinkerObject const& subObj = assembly.sub(0).assemble(); + string const subDisassembly = disassemble(subObj.bytecode, "\n"); + cout << '\n'; + cout << "### immutables: " << numImmutables << ", refs: " << numActualRefs << '\n'; + cout << " - srcmap: \"" << sourceMappings << "\"\n"; + cout << " - src mappings: " << numberOfMappings << '\n'; + cout << " - opcodes: " << numberOfOpcodes << '\n'; + cout << " - subs: " << assembly.numSubs() << '\n'; + cout << " - sub opcodes " << std::count(subDisassembly.begin(), subDisassembly.end(), '\n') << '\n'; + cout << " - sub srcmaps " << AssemblyItem::computeSourceMapping(subAsm->items(), indices) << '\n'; + cout << " - main bytecode:\n\t" << disassemble(obj.bytecode, "\n\t"); + cout << "\r - sub bytecode:\n\t" << disassemble(subObj.bytecode, "\n\t"); + } + #endif // }}} + + BOOST_REQUIRE_EQUAL(NumExpectedMappings, numberOfMappings); + BOOST_REQUIRE_EQUAL(NumExpectedMappings, numberOfOpcodes - NumOpcodesWithoutMappings); + } + } +} + BOOST_AUTO_TEST_CASE(immutable) { map indices = {