Properly compute source mappings for immutables.

Changelog.md

@@ -27,6 +27,7 @@ Important Bugfixes:
 
 Bugfixes:
  * AST: Export ``canonicalName`` for ``UserDefinedValueTypeDefinition`` and ``ContractDefinition``.
+ * Code Generator: Fixes source mappings for immutables.
 
 
 

libevmasm/Assembly.cpp

@@ -675,8 +675,11 @@ LinkerObject const& Assembly::assemble() const
 			break;
 		case PushImmutable:
 			ret.bytecode.push_back(static_cast<uint8_t>(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)
 			{

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 <n> ADD MSTORE)* (PUSH <n> 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;

libevmasm/AssemblyItem.h

@@ -27,6 +27,7 @@
 #include <liblangutil/SourceLocation.h>
 #include <libsolutil/Common.h>
 #include <libsolutil/Assertions.h>
+#include <optional>
 #include <iostream>
 #include <sstream>
 
@@ -169,7 +170,8 @@ public:
 
 	size_t m_modifierDepth = 0;
 
-	void setImmutableOccurrences(size_t _n) const { m_immutableOccurrences = std::make_shared<size_t>(_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<u256> m_pushedValue;
 	/// Number of PushImmutable's with the same hash. Only used for AssignImmutable.
-	mutable std::shared_ptr<size_t> m_immutableOccurrences;
+	mutable std::optional<size_t> m_immutableOccurrences;
 };
 
 inline size_t bytesRequired(AssemblyItems const& _items, size_t _addressLength)

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 <a> PUSH <b>
+					(numActualRefs - 1) * 5 +  // DUP DUP PUSH <n> ADD MTOSRE
+					3                          // PUSH <n> ADD MSTORkhbE
+				) * numImmutables;
+
+			auto constexpr NumSubs = 1;
+			auto constexpr NumOpcodesWithoutMappings =
+				NumSubs +                  // PUSH <addr> for every sub assembly
+				1;                         // INVALID
+
+			auto assemblyName = make_shared<string>("root.asm");
+			auto subName = make_shared<string>("sub.asm");
+
+			map<string, unsigned> indices = {
+				{ *assemblyName, 0 },
+				{ *subName, 1 }
+			};
+
+			auto subAsm = make_shared<Assembly>();
+			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<string, unsigned> indices = {