Merge pull request #15262 from ethereum/subObjectNonDeterminism

Order yul subobjects by order of reference.

Author: ekpyron

Changelog.md

@@ -19,6 +19,7 @@ Bugfixes:
  * SMTChecker: Fix formatting of unary minus expressions in invariants.
  * SMTChecker: Fix internal compiler error when reporting proved targets for BMC engine.
  * TypeChecker: Fix segfault when assigning nested tuple to tuple.
+ * Yul IR Code Generation: Deterministic order of Yul subobjects.
  * Yul Optimizer: Name simplification could lead to forbidden identifiers with a leading and/or trailing dot, e.g., ``x._`` would get simplified into ``x.``.
  * Yul Parser: Fix segfault when parsing very long location comments.
 

libsolidity/codegen/ir/IRGenerationContext.h

@@ -143,7 +143,8 @@ class IRGenerationContext
 
 	RevertStrings revertStrings() const { return m_revertStrings; }
 
-	std::set<ContractDefinition const*, ASTNode::CompareByID>& subObjectsCreated() { return m_subObjects; }
+	util::UniqueVector<ContractDefinition const*> const& subObjectsCreated() const { return m_subObjects; }
+	void addSubObject(ContractDefinition const* _contractDefinition) { m_subObjects.pushBack(_contractDefinition); }
 
 	bool memoryUnsafeInlineAssemblySeen() const { return m_memoryUnsafeInlineAssemblySeen; }
 	void setMemoryUnsafeInlineAssemblySeen() { m_memoryUnsafeInlineAssemblySeen = true; }
@@ -195,7 +196,7 @@ class IRGenerationContext
 	/// It will fail at runtime but the code must still compile.
 	InternalDispatchMap m_internalDispatchMap;
 
-	std::set<ContractDefinition const*, ASTNode::CompareByID> m_subObjects;
+	util::UniqueVector<ContractDefinition const*> m_subObjects;
 
 	langutil::DebugInfoSelection m_debugInfoSelection = {};
 	langutil::CharStreamProvider const* m_soliditySourceProvider = nullptr;

libsolidity/codegen/ir/IRGenerator.cpp

@@ -101,10 +101,10 @@ std::string IRGenerator::generate(
 	std::map<ContractDefinition const*, std::string_view const> const& _otherYulSources
 )
 {
-	auto subObjectSources = [&_otherYulSources](std::set<ContractDefinition const*, ASTNode::CompareByID> const& subObjects) -> std::string
+	auto subObjectSources = [&_otherYulSources](UniqueVector<ContractDefinition const*> const& _subObjects) -> std::string
 	{
 		std::string subObjectsSources;
-		for (ContractDefinition const* subObject: subObjects)
+		for (ContractDefinition const* subObject: _subObjects)
 			subObjectsSources += _otherYulSources.at(subObject);
 		return subObjectsSources;
 	};

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -1559,7 +1559,7 @@ void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall)
 
 		ContractDefinition const* contract =
 			&dynamic_cast<ContractType const&>(*functionType->returnParameterTypes().front()).contractDefinition();
-		m_context.subObjectsCreated().insert(contract);
+		m_context.addSubObject(contract);
 
 		Whiskers t(R"(let <memPos> := <allocateUnbounded>()
 			let <memEnd> := add(<memPos>, datasize("<object>"))
@@ -1947,7 +1947,7 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
 			auto const& contractType = dynamic_cast<ContractType const&>(*arg);
 			solAssert(!contractType.isSuper());
 			ContractDefinition const& contract = contractType.contractDefinition();
-			m_context.subObjectsCreated().insert(&contract);
+			m_context.addSubObject(&contract);
 			appendCode() << Whiskers(R"(
 				let <size> := datasize("<objectName>")
 				let <result> := <allocationFunction>(add(<size>, 32))

test/cmdlineTests/ir_subobject_order/args

@@ -0,0 +1 @@
+--ir-optimized --optimize --debug-info none

test/cmdlineTests/ir_subobject_order/input.sol

@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-3.0
+pragma solidity >0.0.0;
+
+contract B { constructor() payable { assembly { revert(0,0) } } }
+contract A { constructor() payable { assembly { revert(0,0) } } }
+contract C {
+    // The subobject order should be determined by reference, not AST ID,
+    // So the subobject for A should precede the subobject for B.
+	A a = new A();
+	B b = new B();
+	fallback() external payable {
+		assembly { revert(0,0) }
+	}
+}

test/cmdlineTests/ir_subobject_order/output

@@ -0,0 +1,95 @@
+Optimized IR:
+/// @use-src 0:"ir_subobject_order/input.sol"
+object "A_13" {
+    code { { revert(0, 0) } }
+    /// @use-src 0:"ir_subobject_order/input.sol"
+    object "A_13_deployed" {
+        code { { revert(0, 0) } }
+        data ".metadata" hex"<BYTECODE REMOVED>"
+    }
+}
+
+Optimized IR:
+/// @use-src 0:"ir_subobject_order/input.sol"
+object "B_7" {
+    code { { revert(0, 0) } }
+    /// @use-src 0:"ir_subobject_order/input.sol"
+    object "B_7_deployed" {
+        code { { revert(0, 0) } }
+        data ".metadata" hex"<BYTECODE REMOVED>"
+    }
+}
+
+Optimized IR:
+/// @use-src 0:"ir_subobject_order/input.sol"
+object "C_33" {
+    code {
+        {
+            let _1 := memoryguard(0x80)
+            mstore(64, _1)
+            if callvalue() { revert(0, 0) }
+            let _2 := datasize("A_13")
+            let _3 := add(_1, _2)
+            if or(gt(_3, sub(shl(64, 1), 1)), lt(_3, _1))
+            {
+                mstore(0, shl(224, 0x4e487b71))
+                mstore(4, 0x41)
+                revert(0, 0x24)
+            }
+            datacopy(_1, dataoffset("A_13"), _2)
+            let expr_address := create(0, _1, sub(_3, _1))
+            if iszero(expr_address)
+            {
+                let pos := mload(64)
+                returndatacopy(pos, 0, returndatasize())
+                revert(pos, returndatasize())
+            }
+            sstore(0, or(and(sload(0), not(sub(shl(160, 1), 1))), and(expr_address, sub(shl(160, 1), 1))))
+            let _4 := mload(64)
+            let _5 := datasize("B_7")
+            let _6 := add(_4, _5)
+            if or(gt(_6, sub(shl(64, 1), 1)), lt(_6, _4))
+            {
+                mstore(0, shl(224, 0x4e487b71))
+                mstore(4, 0x41)
+                revert(0, 0x24)
+            }
+            datacopy(_4, dataoffset("B_7"), _5)
+            let expr_address_1 := create(0, _4, sub(_6, _4))
+            if iszero(expr_address_1)
+            {
+                let pos_1 := mload(64)
+                returndatacopy(pos_1, 0, returndatasize())
+                revert(pos_1, returndatasize())
+            }
+            sstore(0x01, or(and(sload(0x01), not(sub(shl(160, 1), 1))), and(expr_address_1, sub(shl(160, 1), 1))))
+            let _7 := mload(64)
+            let _8 := datasize("C_33_deployed")
+            codecopy(_7, dataoffset("C_33_deployed"), _8)
+            return(_7, _8)
+        }
+    }
+    /// @use-src 0:"ir_subobject_order/input.sol"
+    object "C_33_deployed" {
+        code { { revert(0, 0) } }
+        data ".metadata" hex"<BYTECODE REMOVED>"
+    }
+    /// @use-src 0:"ir_subobject_order/input.sol"
+    object "A_13" {
+        code { { revert(0, 0) } }
+        /// @use-src 0:"ir_subobject_order/input.sol"
+        object "A_13_deployed" {
+            code { { revert(0, 0) } }
+            data ".metadata" hex"<BYTECODE REMOVED>"
+        }
+    }
+    /// @use-src 0:"ir_subobject_order/input.sol"
+    object "B_7" {
+        code { { revert(0, 0) } }
+        /// @use-src 0:"ir_subobject_order/input.sol"
+        object "B_7_deployed" {
+            code { { revert(0, 0) } }
+            data ".metadata" hex"<BYTECODE REMOVED>"
+        }
+    }
+}