Fix issues where the module index GUID table isn't properly computed (#59375)

* Fix issues where the module index GUID table isn't properly computed
- Since there are two possible places where the module indices must be up to date, ensure that they are finalized before either Node writes its data
- Ensure that module indices are precomputed when needed from inlinee data and the InstanceEntryPointTableNode

diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
index 588d026..becd276 100644 (file)
--- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
+++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs
@@ -394,7 +394,7 @@ namespace Internal.JitInterface
             _methodCodeNode.InitializeDebugLocInfos(_debugLocInfos);
             _methodCodeNode.InitializeDebugVarInfos(_debugVarInfos);
 #if READYTORUN
-            _methodCodeNode.InitializeInliningInfo(_inlinedMethods.ToArray());
+            _methodCodeNode.InitializeInliningInfo(_inlinedMethods.ToArray(), _compilation.NodeFactory);
 
             // Detect cases where the instruction set support used is a superset of the baseline instruction set specification
             var baselineSupport = _compilation.InstructionSetSupport;

diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.cs
index 548abb4..c63e589 100644 (file)
--- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.cs
+++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/InstanceEntryPointTableNode.cs
@@ -17,22 +17,58 @@ using Internal.TypeSystem.Ecma;
 
 namespace ILCompiler.DependencyAnalysis.ReadyToRun
 {
-    public class InstanceEntryPointTableNode : HeaderTableNode
+    public class InstanceEntryPointTableNode : HeaderTableNode, ISignatureEmitter
     {
         private readonly NodeFactory _factory;
+        private bool _materializedSignature;
 
         public InstanceEntryPointTableNode(NodeFactory factory)
             : base(factory.Target)
         {
             _factory = factory;
+            _factory.ManifestMetadataTable.RegisterEmitter(this);
         }
-        
+
+        public void MaterializeSignature()
+        {
+            if (!_materializedSignature)
+            {
+                if (_factory.CompilationModuleGroup.IsInputBubble)
+                {
+                    foreach (MethodWithGCInfo method in _factory.EnumerateCompiledMethods(null, CompiledMethodCategory.Instantiated))
+                    {
+                        BuildSignatureForMethod(method, _factory);
+                    }
+                }
+
+                _materializedSignature = true;
+            }
+        }
+
         public override void AppendMangledName(NameMangler nameMangler, Utf8StringBuilder sb)
         {
             sb.Append(nameMangler.CompilationUnitPrefix);
             sb.Append("__ReadyToRunInstanceEntryPointTable");
         }
 
+        private ArraySignatureBuilder BuildSignatureForMethod(MethodWithGCInfo method, NodeFactory factory)
+        {
+            // In composite R2R format, always enforce owning type to let us share generic instantiations among modules
+
+            EcmaMethod typicalMethod = (EcmaMethod)method.Method.GetTypicalMethodDefinition();
+            ModuleToken moduleToken = new ModuleToken(typicalMethod.Module, typicalMethod.Handle);
+
+            ArraySignatureBuilder signatureBuilder = new ArraySignatureBuilder();
+            signatureBuilder.EmitMethodSignature(
+                new MethodWithToken(method.Method, moduleToken, constrainedType: null, unboxing: false, context: null),
+                enforceDefEncoding: true,
+                enforceOwningType: _factory.CompilationModuleGroup.EnforceOwningType(moduleToken.Module),
+                factory.SignatureContext,
+                isInstantiatingStub: false);
+
+            return signatureBuilder;
+        }
+
         public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
         {
             if (relocsOnly)
@@ -55,17 +91,7 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
                 int methodIndex = factory.RuntimeFunctionsTable.GetIndex(method);
 
-                // In composite R2R format, always enforce owning type to let us share generic instantiations among modules
-                EcmaMethod typicalMethod = (EcmaMethod)method.Method.GetTypicalMethodDefinition();
-                ModuleToken moduleToken = new ModuleToken(typicalMethod.Module, typicalMethod.Handle);
-
-                ArraySignatureBuilder signatureBuilder = new ArraySignatureBuilder();
-                signatureBuilder.EmitMethodSignature(
-                    new MethodWithToken(method.Method, moduleToken, constrainedType: null, unboxing: false, context: null),
-                    enforceDefEncoding: true,
-                    enforceOwningType: _factory.CompilationModuleGroup.EnforceOwningType(moduleToken.Module),
-                    factory.SignatureContext,
-                    isInstantiatingStub: false);
+                ArraySignatureBuilder signatureBuilder = BuildSignatureForMethod(method, factory);
                 byte[] signature = signatureBuilder.ToArray();
                 BlobVertex signatureBlob;
                 if (!uniqueSignatures.TryGetValue(signature, out signatureBlob))

diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs
index 80f80b2..ca6b4fc 100644 (file)
--- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs
+++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ManifestMetadataTableNode.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Concurrent;
 using System.Linq;
 using System.Reflection;
 using System.Reflection.Metadata;
@@ -64,6 +65,11 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
         private int _nextModuleId;
 
         /// <summary>
+        /// Modules which need to exist in set of modules visible
+        /// </summary>
+        private ConcurrentBag<EcmaModule> _modulesWhichMustBeIndexable = new ConcurrentBag<EcmaModule>();
+
+        /// <summary>
         /// Set to true after GetData has been called. After that, ModuleToIndex may be called no more.
         /// </summary>
         private bool _emissionCompleted;
@@ -136,6 +142,19 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             return ModuleToIndexInternal(module);
         }
 
+        public void EnsureModuleIndexable(ModuleDesc module)
+        {
+            if (_emissionCompleted)
+            {
+                throw new InvalidOperationException("Adding a new assembly after signatures have been materialized.");
+            }
+
+            if (module is EcmaModule ecmaModule && _nodeFactory.CompilationModuleGroup.VersionsWithModule(ecmaModule))
+            {
+                _modulesWhichMustBeIndexable.Add(ecmaModule);
+            }
+        }
+
         private int ModuleToIndexInternal(EcmaModule module)
         {
             AssemblyName assemblyName = module.Assembly.GetName();
@@ -170,13 +189,8 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             sb.Append("ManifestMetadataTableNode");
         }
 
-        public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
+        private void ComputeLastSetOfModuleIndices()
         {
-            if (relocsOnly)
-            {
-                return new ObjectData(Array.Empty<byte>(), null, 1, null);
-            }
-
             if (!_emissionCompleted)
             {
                 foreach (ISignatureEmitter emitter in _signatureEmitters)
@@ -184,8 +198,25 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                     emitter.MaterializeSignature();
                 }
 
+                EcmaModule [] moduleArray = _modulesWhichMustBeIndexable.ToArray();
+                Array.Sort(moduleArray, (EcmaModule moduleA, EcmaModule moduleB) => moduleA.CompareTo(moduleB));
+                foreach (var module in moduleArray)
+                {
+                    ModuleToIndex(module);
+                }
+
                 _emissionCompleted = true;
             }
+        }
+
+        public override ObjectData GetData(NodeFactory factory, bool relocsOnly = false)
+        {
+            if (relocsOnly)
+            {
+                return new ObjectData(Array.Empty<byte>(), null, 1, null);
+            }
+
+            ComputeLastSetOfModuleIndices();
 
             MetadataBuilder metadataBuilder = new MetadataBuilder();
 
@@ -277,6 +308,8 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
 
         internal byte[] GetManifestAssemblyMvidTableData()
         {
+            ComputeLastSetOfModuleIndices();
+
             byte[] manifestAssemblyMvidTable = new byte[ManifestAssemblyMvidTableSize];
             for (int i = 0; i < _manifestAssemblyMvids.Count; i++)
             {

diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs
index 0ef6142..cad838b 100644 (file)
--- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs
+++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/MethodWithGCInfo.cs
@@ -8,6 +8,7 @@ using System.Diagnostics;
 using Internal.JitInterface;
 using Internal.Text;
 using Internal.TypeSystem;
+using Internal.TypeSystem.Ecma;
 
 namespace ILCompiler.DependencyAnalysis.ReadyToRun
 {
@@ -45,6 +46,34 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
                 InitializeFrameInfos(Array.Empty<FrameInfo>());
             }
             _lateTriggeredCompilation = context.CompilationCurrentPhase != 0;
+            RegisterInlineeModuleIndices(context);
+        }
+
+        private void RegisterInlineeModuleIndices(NodeFactory factory)
+        {
+            if (_inlinedMethods != null)
+            {
+                foreach (var inlinee in _inlinedMethods)
+                {
+                    MethodDesc inlineeDefinition = inlinee.GetTypicalMethodDefinition();
+                    if (!(inlineeDefinition is EcmaMethod ecmaInlineeDefinition))
+                    {
+                        // We don't record non-ECMA methods because they don't have tokens that
+                        // diagnostic tools could reason about anyway.
+                        continue;
+                    }
+
+                    if (!factory.CompilationModuleGroup.VersionsWithMethodBody(inlinee))
+                    {
+                        // We cannot record inlining info across version bubble as cross-bubble assemblies
+                        // are not guaranteed to preserve token values. Only non-versionable methods may be
+                        // inlined across the version bubble.
+                        Debug.Assert(inlinee.IsNonVersionable());
+                        continue;
+                    }
+                    factory.ManifestMetadataTable.EnsureModuleIndexable(ecmaInlineeDefinition.Module);
+                }
+            }
         }
 
         public override int DependencyPhaseForDeferredStaticComputation => _lateTriggeredCompilation ? 2 : 0;
@@ -316,10 +345,12 @@ namespace ILCompiler.DependencyAnalysis.ReadyToRun
             return comparer.Compare(_method, otherNode._method);
         }
 
-        public void InitializeInliningInfo(MethodDesc[] inlinedMethods)
+        public void InitializeInliningInfo(MethodDesc[] inlinedMethods, NodeFactory factory)
         {
             Debug.Assert(_inlinedMethods == null);
             _inlinedMethods = inlinedMethods;
+            if (this.Marked)
+                RegisterInlineeModuleIndices(factory);
         }
 
         public int Offset => 0;