Merge pull request #14176 from ethereum/internal-function-id-annotation-fix

Move AST annotation of internal function dispatch IDs to ContractDefinition

Changelog.md

@@ -25,7 +25,7 @@ Bugfixes:
 
 
 AST Changes:
- * AST: Add the ``internalFunctionID`` field to the AST nodes of functions that may be called via the internal dispatch. These IDs are always generated, but they are only used in via-IR code generation.
+ * AST: Add the ``internalFunctionIDs`` field to the AST nodes of contracts containing IDs of functions that may be called via the internal dispatch. The field is a map from function AST IDs to internal dispatch function IDs. These IDs are always generated, but they are only used in via-IR code generation.
  * AST: Add the ``usedEvents`` field to ``ContractDefinition`` which contains the AST IDs of all events emitted by the contract as well as all events defined and inherited by the contract.
 
 

libsolidity/ast/ASTAnnotations.h

@@ -168,6 +168,9 @@ struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, StructurallyDocu
 	/// List of contracts whose bytecode is referenced by this contract, e.g. through "new".
 	/// The Value represents the ast node that referenced the contract.
 	std::map<ContractDefinition const*, ASTNode const*, ASTCompareByID<ContractDefinition>> contractDependencies;
+
+	// Per-contract map from function AST IDs to internal dispatch function IDs.
+	std::map<FunctionDefinition const*, uint64_t> internalFunctionIDs;
 };
 
 struct CallableDeclarationAnnotation: DeclarationAnnotation
@@ -178,7 +181,6 @@ struct CallableDeclarationAnnotation: DeclarationAnnotation
 
 struct FunctionDefinitionAnnotation: CallableDeclarationAnnotation, StructurallyDocumentedAnnotation
 {
-	util::SetOnce<uint64_t> internalFunctionID;
 };
 
 struct EventDefinitionAnnotation: CallableDeclarationAnnotation, StructurallyDocumentedAnnotation

libsolidity/ast/ASTJsonExporter.cpp

@@ -299,6 +299,14 @@ bool ASTJsonExporter::visit(ContractDefinition const& _node)
 	if (!_node.annotation().linearizedBaseContracts.empty())
 		attributes.emplace_back("linearizedBaseContracts", getContainerIds(_node.annotation().linearizedBaseContracts));
 
+	if (!_node.annotation().internalFunctionIDs.empty())
+	{
+		Json::Value internalFunctionIDs(Json::objectValue);
+		for (auto const& [functionDefinition, internalFunctionID]: _node.annotation().internalFunctionIDs)
+			internalFunctionIDs[to_string(functionDefinition->id())] = internalFunctionID;
+		attributes.emplace_back("internalFunctionIDs", std::move(internalFunctionIDs));
+	}
+
 	setJsonNode(_node, "ContractDefinition", std::move(attributes));
 	return false;
 }
@@ -473,9 +481,6 @@ bool ASTJsonExporter::visit(FunctionDefinition const& _node)
 	if (!_node.annotation().baseFunctions.empty())
 		attributes.emplace_back(make_pair("baseFunctions", getContainerIds(_node.annotation().baseFunctions, true)));
 
-	if (_node.annotation().internalFunctionID.set())
-		attributes.emplace_back("internalFunctionID", *_node.annotation().internalFunctionID);
-
 	setJsonNode(_node, "FunctionDefinition", std::move(attributes));
 	return false;
 }

libsolidity/codegen/ir/IRGenerator.cpp

@@ -313,7 +313,7 @@ InternalDispatchMap IRGenerator::generateInternalDispatchFunctions(ContractDefin
 				solAssert(m_context.functionCollector().contains(IRNames::function(*function)), "");
 
 				cases.emplace_back(map<string, string>{
-					{"funID", to_string(*function->annotation().internalFunctionID)},
+					{"funID", to_string(m_context.mostDerivedContract().annotation().internalFunctionIDs.at(function))},
 					{"name", IRNames::function(*function)}
 				});
 			}

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -2807,7 +2807,7 @@ void IRGeneratorForStatements::assignInternalFunctionIDIfNotCalledDirectly(
 		return;
 
 	define(IRVariable(_expression).part("functionIdentifier")) <<
-		to_string(*_referencedFunction.annotation().internalFunctionID) <<
+		to_string(m_context.mostDerivedContract().annotation().internalFunctionIDs.at(&_referencedFunction)) <<
 		"\n";
 	m_context.addToInternalDispatch(_referencedFunction);
 }

libsolidity/interface/CompilerStack.cpp

@@ -1246,7 +1246,6 @@ void CompilerStack::storeContractDefinitions()
 
 void CompilerStack::annotateInternalFunctionIDs()
 {
-	uint64_t internalFunctionID = 1;
 	for (Source const* source: m_sourceOrder)
 	{
 		if (!source->ast)
@@ -1254,20 +1253,23 @@ void CompilerStack::annotateInternalFunctionIDs()
 
 		for (ContractDefinition const* contract: ASTNode::filteredNodes<ContractDefinition>(source->ast->nodes()))
 		{
+			uint64_t internalFunctionID = 1;
 			ContractDefinitionAnnotation& annotation = contract->annotation();
 
 			if (auto const* deployTimeInternalDispatch = util::valueOrNullptr((*annotation.deployedCallGraph)->edges, CallGraph::SpecialNode::InternalDispatch))
 				for (auto const& node: *deployTimeInternalDispatch)
 					if (auto const* callable = get_if<CallableDeclaration const*>(&node))
 						if (auto const* function = dynamic_cast<FunctionDefinition const*>(*callable))
-							if (!function->annotation().internalFunctionID.set())
-								function->annotation().internalFunctionID = internalFunctionID++;
+						{
+							solAssert(contract->annotation().internalFunctionIDs.count(function) == 0);
+							contract->annotation().internalFunctionIDs[function] = internalFunctionID++;
+						}
 			if (auto const* creationTimeInternalDispatch = util::valueOrNullptr((*annotation.creationCallGraph)->edges, CallGraph::SpecialNode::InternalDispatch))
 				for (auto const& node: *creationTimeInternalDispatch)
 					if (auto const* callable = get_if<CallableDeclaration const*>(&node))
 						if (auto const* function = dynamic_cast<FunctionDefinition const*>(*callable))
 							// Make sure the function already got an ID since it also occurs in the deploy-time internal dispatch.
-							solAssert(function->annotation().internalFunctionID.set());
+							solAssert(contract->annotation().internalFunctionIDs.count(function) != 0);
 		}
 	}
 }

test/libsolidity/ASTJSON/ast_internal_function_different_ids_export.sol

@@ -0,0 +1,38 @@
+==== Source: L ====
+
+function free1() {}
+function free2() {}
+library L {
+    function g() internal {}
+    function h() internal {}
+}
+
+==== Source: A ====
+
+import "L";
+contract A {
+    function f() public {
+        (L.g)();
+        (free2)();
+        (L.h)();
+    }
+}
+contract B {
+    function f() public {
+        (L.h)();
+        (free2)();
+    }
+}
+
+==== Source: C ====
+
+import "L";
+contract C {
+    function f() public {
+        (L.g)();
+        (free2)();
+        (L.h)();
+    }
+}
+
+// ----

test/libsolidity/ASTJSON/ast_internal_function_id_export.json

@@ -41,7 +41,6 @@
       },
       "id": 4,
       "implemented": true,
-      "internalFunctionID": 1,
       "kind": "freeFunction",
       "modifiers": [],
       "name": "free1",
@@ -77,7 +76,6 @@
       },
       "id": 8,
       "implemented": true,
-      "internalFunctionID": 2,
       "kind": "freeFunction",
       "modifiers": [],
       "name": "free2",
@@ -146,6 +144,13 @@
       "contractKind": "library",
       "fullyImplemented": true,
       "id": 53,
+      "internalFunctionIDs":
+      {
+        "20": 3,
+        "24": 4,
+        "4": 1,
+        "8": 2
+      },
       "linearizedBaseContracts":
       [
         53
@@ -201,7 +206,6 @@
           },
           "id": 20,
           "implemented": true,
-          "internalFunctionID": 3,
           "kind": "function",
           "modifiers": [],
           "name": "inr1",
@@ -237,7 +241,6 @@
           },
           "id": 24,
           "implemented": true,
-          "internalFunctionID": 4,
           "kind": "function",
           "modifiers": [],
           "name": "inr2",
@@ -579,6 +582,15 @@
       "contractKind": "contract",
       "fullyImplemented": true,
       "id": 128,
+      "internalFunctionIDs":
+      {
+        "20": 3,
+        "24": 4,
+        "4": 1,
+        "69": 5,
+        "73": 6,
+        "8": 2
+      },
       "linearizedBaseContracts":
       [
         128
@@ -706,7 +718,6 @@
           },
           "id": 69,
           "implemented": true,
-          "internalFunctionID": 5,
           "kind": "function",
           "modifiers": [],
           "name": "inr1",
@@ -742,7 +753,6 @@
           },
           "id": 73,
           "implemented": true,
-          "internalFunctionID": 6,
           "kind": "function",
           "modifiers": [],
           "name": "inr2",
@@ -1405,6 +1415,15 @@
       "contractKind": "contract",
       "fullyImplemented": true,
       "id": 141,
+      "internalFunctionIDs":
+      {
+        "20": 3,
+        "24": 4,
+        "4": 1,
+        "69": 5,
+        "73": 6,
+        "8": 2
+      },
       "linearizedBaseContracts":
       [
         141,