Bytecode difference caused by SSA transform

Changelog

Review comments

Unique vector

Repro test

Author: nikola-matic

Changelog.md

@@ -9,6 +9,7 @@ Compiler Features:
 Bugfixes:
  * SMTChecker: Fix internal error on mapping access caused by too strong requirements on sort compatibility of the index and mapping domain.
  * SMTChecker: Fix internal error when using bitwise operators with an array element as argument.
+ * Yul Optimizer: Fix the order of assignments generated by ``SSATransform`` being dependent on AST IDs, sometimes resulting in different (but equivalent) bytecode when unrelated files were added to the compilation pipeline.
 
 
 ### 0.8.25 (2024-03-14)

libsolutil/CommonData.h

@@ -343,6 +343,49 @@ void joinMap(std::map<K, V>& _a, std::map<K, V>&& _b, F _conflictSolver)
 	}
 }
 
+template<typename T>
+class UniqueVector
+{
+public:
+	std::vector<T> const& contents() const { return m_contents; }
+	size_t size() const { return m_contents.size(); }
+	bool empty() const { return m_contents.empty(); }
+	auto begin() const { return m_contents.begin(); }
+	auto end() const { return m_contents.end(); }
+	void clear() { m_contents.clear(); }
+	bool contains(T const& _value) const
+	{
+		return std::find(m_contents.begin(), m_contents.end(), _value) != m_contents.end();
+	}
+
+	void pushBack(T _value)
+	{
+		if (!contains(_value))
+			m_contents.emplace_back(std::move(_value));
+	}
+
+	void pushBack(UniqueVector<T> const& _values)
+	{
+		for (auto&& value: _values)
+			pushBack(value);
+	}
+
+	void removeAll(std::vector<T> const& _values)
+	{
+		for (auto const& value: _values)
+			m_contents.erase(std::find(m_contents.begin(), m_contents.end(), value));
+	}
+
+private:
+	std::vector<T> m_contents;
+};
+
+template<typename T>
+void swap(UniqueVector<T>& _lhs, UniqueVector<T>& _rhs)
+{
+	std::swap(_lhs.contents(), _rhs.contents());
+}
+
 namespace detail
 {
 

libyul/optimiser/SSATransform.cpp

@@ -166,23 +166,23 @@ class IntroduceControlFlowSSA: public ASTModifier
 	std::set<YulString> const& m_variablesToReplace;
 	/// Variables (that are to be replaced) currently in scope.
 	std::set<YulString> m_variablesInScope;
-	/// Set of variables that do not have a specific value.
-	std::set<YulString> m_variablesToReassign;
+	/// Variables that do not have a specific value.
+	util::UniqueVector<YulString> m_variablesToReassign;
 	TypeInfo const& m_typeInfo;
 };
 
 void IntroduceControlFlowSSA::operator()(FunctionDefinition& _function)
 {
 	std::set<YulString> varsInScope;
 	std::swap(varsInScope, m_variablesInScope);
-	std::set<YulString> toReassign;
+	util::UniqueVector<YulString> toReassign;
 	std::swap(toReassign, m_variablesToReassign);
 
 	for (auto const& param: _function.parameters)
 		if (m_variablesToReplace.count(param.name))
 		{
 			m_variablesInScope.insert(param.name);
-			m_variablesToReassign.insert(param.name);
+			m_variablesToReassign.pushBack(param.name);
 		}
 
 	ASTModifier::operator()(_function);
@@ -196,8 +196,8 @@ void IntroduceControlFlowSSA::operator()(ForLoop& _for)
 	yulAssert(_for.pre.statements.empty(), "For loop init rewriter not run.");
 
 	for (auto const& var: assignedVariableNames(_for.body) + assignedVariableNames(_for.post))
-		if (m_variablesInScope.count(var))
-			m_variablesToReassign.insert(var);
+		if (util::contains(m_variablesInScope,var))
+			m_variablesToReassign.pushBack(var);
 
 	(*this)(_for.body);
 	(*this)(_for.post);
@@ -207,26 +207,26 @@ void IntroduceControlFlowSSA::operator()(Switch& _switch)
 {
 	yulAssert(m_variablesToReassign.empty(), "");
 
-	std::set<YulString> toReassign;
+	util::UniqueVector<YulString> toReassign;
 	for (auto& c: _switch.cases)
 	{
 		(*this)(c.body);
-		toReassign += m_variablesToReassign;
+		toReassign.pushBack(m_variablesToReassign);
 	}
 
-	m_variablesToReassign += toReassign;
+	m_variablesToReassign.pushBack(toReassign);
 }
 
 void IntroduceControlFlowSSA::operator()(Block& _block)
 {
-	std::set<YulString> variablesDeclaredHere;
-	std::set<YulString> assignedVariables;
+	util::UniqueVector<YulString> variablesDeclaredHere;
+	util::UniqueVector<YulString> assignedVariables;
 
 	util::iterateReplacing(
 		_block.statements,
 		[&](Statement& _s) -> std::optional<std::vector<Statement>>
 		{
-		std::vector<Statement> toPrepend;
+			std::vector<Statement> toPrepend;
 			for (YulString toReassign: m_variablesToReassign)
 			{
 				YulString newName = m_nameDispenser.newName(toReassign);
@@ -235,7 +235,7 @@ void IntroduceControlFlowSSA::operator()(Block& _block)
 					{TypedName{debugDataOf(_s), newName, m_typeInfo.typeOfVariable(toReassign)}},
 					std::make_unique<Expression>(Identifier{debugDataOf(_s), toReassign})
 				});
-				assignedVariables.insert(toReassign);
+				assignedVariables.pushBack(toReassign);
 			}
 			m_variablesToReassign.clear();
 
@@ -245,7 +245,7 @@ void IntroduceControlFlowSSA::operator()(Block& _block)
 				for (auto const& var: varDecl.variables)
 					if (m_variablesToReplace.count(var.name))
 					{
-						variablesDeclaredHere.insert(var.name);
+						variablesDeclaredHere.pushBack(var.name);
 						m_variablesInScope.insert(var.name);
 					}
 			}
@@ -254,7 +254,7 @@ void IntroduceControlFlowSSA::operator()(Block& _block)
 				Assignment& assignment = std::get<Assignment>(_s);
 				for (auto const& var: assignment.variableNames)
 					if (m_variablesToReplace.count(var.name))
-						assignedVariables.insert(var.name);
+						assignedVariables.pushBack(var.name);
 			}
 			else
 				visit(_s);
@@ -268,9 +268,10 @@ void IntroduceControlFlowSSA::operator()(Block& _block)
 			}
 		}
 	);
-	m_variablesToReassign += assignedVariables;
-	m_variablesInScope -= variablesDeclaredHere;
-	m_variablesToReassign -= variablesDeclaredHere;
+
+	m_variablesToReassign.pushBack(assignedVariables);
+	m_variablesInScope -= variablesDeclaredHere.contents();
+	m_variablesToReassign.removeAll(variablesDeclaredHere.contents());
 }
 
 /**

scripts/common/cmdline_helpers.py

@@ -11,19 +11,23 @@
 from bytecodecompare.prepare_report import parse_cli_output
 
 
-DEFAULT_PREAMBLE = dedent("""
+DEFAULT_PREAMBLE = dedent(
+    """
     // SPDX-License-Identifier: UNLICENSED
     pragma solidity >=0.0;
-""")
+"""
+)
 
 
 def inside_temporary_dir(prefix):
     """
     Creates a temporary directory, enters the directory and executes the function inside it.
     Restores the previous working directory after executing the function.
     """
+
     def tmp_dir_decorator(fn):
         previous_dir = os.getcwd()
+
         def f(*args, **kwargs):
             try:
                 tmp_dir = mkdtemp(prefix=prefix)
@@ -33,35 +37,49 @@ def f(*args, **kwargs):
                 return result
             finally:
                 os.chdir(previous_dir)
+
         return f
+
     return tmp_dir_decorator
 
 
-def solc_bin_report(solc_binary: str, input_files: List[Path], via_ir: bool) -> FileReport:
+def solc_bin_report(
+    solc_binary: str,
+    input_files: List[Path],
+    via_ir: bool,
+    optimize: bool = False,
+    yul_optimizations: Optional[str] = None,
+) -> FileReport:
     """
     Runs the solidity compiler binary
     """
 
     output = subprocess.check_output(
-        [solc_binary, '--bin'] +
-        input_files +
-        (['--via-ir'] if via_ir else []),
-        encoding='utf8',
+        [solc_binary, "--bin"]
+        + input_files
+        + (["--via-ir"] if via_ir else [])
+        + (["--optimize"] if optimize else [])
+        + (["--yul-optimizations", yul_optimizations] if yul_optimizations else []),
+        encoding="utf8",
     )
-    return parse_cli_output('', output, 0)
+    return parse_cli_output("", output, 0)
 
 
-def save_bytecode(bytecode_path: Path, reports: FileReport, contract: Optional[str] = None):
-    with open(bytecode_path, 'w', encoding='utf8') as f:
+def save_bytecode(
+    bytecode_path: Path, reports: FileReport, contract: Optional[str] = None
+):
+    with open(bytecode_path, "w", encoding="utf8") as f:
         for report in reports.contract_reports:
             if contract is None or report.contract_name == contract:
-                bytecode = report.bytecode if report.bytecode is not None else '<NO BYTECODE>'
-                f.write(f'{report.contract_name}: {bytecode}\n')
+                bytecode = (
+                    report.bytecode if report.bytecode is not None else "<NO BYTECODE>"
+                )
+                f.write(f"{report.contract_name}: {bytecode}\n")
 
 
 def add_preamble(source_path: Path, preamble: str = DEFAULT_PREAMBLE):
-    for source in source_path.glob('**/*.sol'):
-        with open(source, 'r+', encoding='utf8') as f:
+    for source in source_path.glob("**/*.sol"):
+        with open(source, "r+", encoding="utf8") as f:
             content = f.read()
             f.seek(0, 0)
             f.write(preamble + content)

test/cmdlineTests/~bytecode_equivalence_with_unused_contracts/inputs.sol

@@ -0,0 +1,13 @@
+==== Source: A.sol ====
+contract DummyContract1 {}
+contract DummyContract2 {}
+contract DummyContract3 {}
+
+==== Source: B.sol ====
+contract B {
+    function f(uint8 a_0, uint8 a_1, uint8 a_2) public pure {
+        a_1 = 1;
+        a_2 = 2;
+        a_0;
+    }
+}

test/cmdlineTests/~bytecode_equivalence_with_unused_contracts/test.py

@@ -0,0 +1,65 @@
+#!/usr/bin/env python3
+
+import os
+import sys
+from pathlib import Path
+from textwrap import dedent
+
+# pylint: disable=wrong-import-position
+PROJECT_ROOT = Path(__file__).parents[3]
+sys.path.insert(0, str(PROJECT_ROOT / "scripts"))
+
+from common.cmdline_helpers import add_preamble
+from common.cmdline_helpers import inside_temporary_dir
+from common.cmdline_helpers import save_bytecode
+from common.cmdline_helpers import solc_bin_report
+from common.git_helpers import git_diff
+from splitSources import split_sources
+
+
+@inside_temporary_dir(Path(__file__).parent.name)
+def test_bytecode_equivalence():
+    source_file_path = Path(__file__).parent / "inputs.sol"
+    split_sources(source_file_path, suppress_output=True)
+    add_preamble(Path.cwd())
+
+    solc_binary = os.environ.get("SOLC")
+    if solc_binary is None:
+        raise RuntimeError(
+            dedent(
+                """\
+            `solc` compiler not found.
+            Please ensure you set the SOLC environment variable
+            with the correct path to the compiler's binary.
+        """
+            )
+        )
+
+    # Repro for https://github.com/ethereum/solidity/issues/14829
+    save_bytecode(
+        Path("B.bin"),
+        solc_bin_report(
+            solc_binary,
+            [Path("B.sol")],
+            via_ir=True,
+            optimize=True,
+            yul_optimizations="a:",
+        ),
+        contract="B",
+    )
+    save_bytecode(
+        Path("AB.bin"),
+        solc_bin_report(
+            solc_binary,
+            [Path("A.sol"), Path("B.sol")],
+            via_ir=True,
+            optimize=True,
+            yul_optimizations="a:",
+        ),
+        contract="B",
+    )
+    return git_diff(Path("B.bin"), Path("AB.bin"))
+
+
+if __name__ == "__main__":
+    sys.exit(test_bytecode_equivalence())

test/libyul/yulOptimizerTests/ssaTransform/function.yul

@@ -12,15 +12,15 @@
 // {
 //     function f(a, b) -> c, d
 //     {
-//         let b_5 := b
-//         let a_6 := a
-//         let b_1 := add(b_5, a_6)
+//         let a_5 := a
+//         let b_6 := b
+//         let b_1 := add(b_6, a_5)
 //         b := b_1
 //         let c_2 := add(c, b_1)
 //         c := c_2
 //         let d_3 := add(d, c_2)
 //         d := d_3
-//         let a_4 := add(a_6, d_3)
+//         let a_4 := add(a_5, d_3)
 //         a := a_4
 //     }
 // }