Don't crash on recursive native layout (#74630)

Fixes dotnet/runtimelab#163.

* We don't have a nice way to pass context around when computing the native type. Use a threadstatic. This is pretty rare.
* We had one more spot where we ran out of stack when we were trying to look for delegates. Remove the need for that code by using the generic `Marshal` API (that we recognize elsewhere). Move the code responsible for warnings to dataflow analysis.

src/coreclr/tools/Common/TypeSystem/Interop/IL/Marshaller.cs

@@ -1917,10 +1917,10 @@ protected override void AllocAndTransformManagedToNative(ILCodeStream codeStream
             codeStream.Emit(ILOpcode.brfalse, lNullPointer);
 
             codeStream.Emit(ILOpcode.call, _ilCodeStreams.Emitter.NewToken(
-                InteropTypes.GetPInvokeMarshal(Context).GetKnownMethod("GetFunctionPointerForDelegate",
-                new MethodSignature(MethodSignatureFlags.Static, 0, Context.GetWellKnownType(WellKnownType.IntPtr),
-                    new TypeDesc[] { Context.GetWellKnownType(WellKnownType.MulticastDelegate).BaseType }
-                ))));
+                InteropTypes.GetMarshal(Context).GetKnownMethod("GetFunctionPointerForDelegate",
+                new MethodSignature(MethodSignatureFlags.Static, 1, Context.GetWellKnownType(WellKnownType.IntPtr),
+                    new TypeDesc[] { Context.GetSignatureVariable(0, method: true) }
+                )).MakeInstantiatedMethod(ManagedType)));
 
             codeStream.Emit(ILOpcode.br, lDone);
 
@@ -1941,13 +1941,12 @@ protected override void TransformNativeToManaged(ILCodeStream codeStream)
             LoadNativeValue(codeStream);
             codeStream.Emit(ILOpcode.dup);
             codeStream.Emit(ILOpcode.brfalse, lNullPointer);
-            codeStream.Emit(ILOpcode.ldtoken, _ilCodeStreams.Emitter.NewToken(ManagedType));
 
             codeStream.Emit(ILOpcode.call, _ilCodeStreams.Emitter.NewToken(
-                InteropTypes.GetPInvokeMarshal(Context).GetKnownMethod("GetDelegateForFunctionPointer",
-                new MethodSignature(MethodSignatureFlags.Static, 0, Context.GetWellKnownType(WellKnownType.MulticastDelegate).BaseType,
-                    new TypeDesc[] { Context.GetWellKnownType(WellKnownType.IntPtr), Context.GetWellKnownType(WellKnownType.RuntimeTypeHandle) }
-                ))));
+                InteropTypes.GetMarshal(Context).GetKnownMethod("GetDelegateForFunctionPointer",
+                new MethodSignature(MethodSignatureFlags.Static, 1, Context.GetSignatureVariable(0, method: true),
+                    new TypeDesc[] { Context.GetWellKnownType(WellKnownType.IntPtr) }
+                )).MakeInstantiatedMethod(ManagedType)));
 
             codeStream.Emit(ILOpcode.br, lDone);
 

src/coreclr/tools/Common/TypeSystem/Interop/IL/NativeStructType.cs

@@ -160,6 +160,9 @@ public FieldDesc[] Fields
             }
         }
 
+        [ThreadStatic]
+        static Stack<MetadataType> s_typesBeingLookedAt;
+
         public NativeStructType(ModuleDesc owningModule, MetadataType managedStructType, InteropStateManager interopStateManager)
         {
             Debug.Assert(!managedStructType.IsGenericDefinition);
@@ -168,7 +171,21 @@ public NativeStructType(ModuleDesc owningModule, MetadataType managedStructType,
             ManagedStructType = managedStructType;
             _interopStateManager = interopStateManager;
             _hasInvalidLayout = false;
-            CalculateFields();
+
+            Stack<MetadataType> typesBeingLookedAt = (s_typesBeingLookedAt ??= new Stack<MetadataType>());
+            if (typesBeingLookedAt.Contains(managedStructType))
+                ThrowHelper.ThrowTypeLoadException(managedStructType);
+
+            typesBeingLookedAt.Push(managedStructType);
+            try
+            {
+                CalculateFields();
+            }
+            finally
+            {
+                MetadataType popped = typesBeingLookedAt.Pop();
+                Debug.Assert(popped == managedStructType);
+            }
         }
 
         private void CalculateFields()

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/AnalysisBasedInteropStubManager.cs

@@ -40,7 +40,7 @@ public override void AddCompilationRoots(IRootingServiceProvider rootProvider)
             }
         }
 
-        public override void AddDependenciesDueToPInvoke(ref DependencyList dependencies, NodeFactory factory, MethodDesc method)
+        public override void AddDependenciesDueToMethodCodePresence(ref DependencyList dependencies, NodeFactory factory, MethodDesc method)
         {
         }
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/Dataflow/ReflectionMethodBodyScanner.cs

@@ -317,6 +317,7 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet
 
                             ParameterMetadata returnParamMetadata = Array.Find(paramMetadata, m => m.Index == 0);
 
+                            bool aotUnsafeDelegate = IsAotUnsafeDelegate(calledMethod.Signature.ReturnType);
                             bool comDangerousMethod = IsComInterop(returnParamMetadata.MarshalAsDescriptor, calledMethod.Signature.ReturnType);
                             for (int paramIndex = 0; paramIndex < calledMethod.Signature.Length; paramIndex++)
                             {
@@ -327,9 +328,15 @@ public override bool HandleCall(MethodIL callingMethodBody, MethodDesc calledMet
                                         marshalAsDescriptor = paramMetadata[metadataIndex].MarshalAsDescriptor;
                                 }
 
+                                aotUnsafeDelegate |= IsAotUnsafeDelegate(calledMethod.Signature[paramIndex]);
                                 comDangerousMethod |= IsComInterop(marshalAsDescriptor, calledMethod.Signature[paramIndex]);
                             }
 
+                            if (aotUnsafeDelegate)
+                            {
+                                diagnosticContext.AddDiagnostic(DiagnosticId.CorrectnessOfAbstractDelegatesCannotBeGuaranteed, calledMethod.GetDisplayName());
+                            }
+
                             if (comDangerousMethod)
                             {
                                 diagnosticContext.AddDiagnostic(DiagnosticId.CorrectnessOfCOMCannotBeGuaranteed, calledMethod.GetDisplayName());
@@ -563,6 +570,13 @@ void AddReturnValue(MultiValue value)
             }
         }
 
+        static bool IsAotUnsafeDelegate(TypeDesc parameterType)
+        {
+            TypeSystemContext context = parameterType.Context;
+            return parameterType.IsWellKnownType(Internal.TypeSystem.WellKnownType.MulticastDelegate)
+                    || parameterType == context.GetWellKnownType(Internal.TypeSystem.WellKnownType.MulticastDelegate).BaseType;
+        }
+
         static bool IsComInterop(MarshalAsDescriptor? marshalInfoProvider, TypeDesc parameterType)
         {
             // This is best effort. One can likely find ways how to get COM without triggering these alarms.

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/CodeBasedDependencyAlgorithm.cs

@@ -22,7 +22,7 @@ public static void AddDependenciesDueToMethodCodePresence(ref DependencyList dep
         {
             factory.MetadataManager.GetDependenciesDueToMethodCodePresence(ref dependencies, factory, method, methodIL);
 
-            factory.InteropStubManager.AddDependenciesDueToPInvoke(ref dependencies, factory, method);
+            factory.InteropStubManager.AddDependenciesDueToMethodCodePresence(ref dependencies, factory, method);
 
             if (method.OwningType is MetadataType mdType)
                 ModuleUseBasedDependencyAlgorithm.AddDependenciesDueToModuleUse(ref dependencies, factory, mdType.Module);

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/EmptyInteropStubManager.cs

@@ -21,7 +21,7 @@ public override PInvokeILProvider CreatePInvokeILProvider()
             return null;
         }
 
-        public override void AddDependenciesDueToPInvoke(ref DependencyList dependencies, NodeFactory factory, MethodDesc method)
+        public override void AddDependenciesDueToMethodCodePresence(ref DependencyList dependencies, NodeFactory factory, MethodDesc method)
         {
         }
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/InteropStubManager.cs

@@ -16,7 +16,7 @@ namespace ILCompiler
     /// </summary>
     public abstract class InteropStubManager : ICompilationRootProvider
     {
-        public abstract void AddDependenciesDueToPInvoke(ref DependencyList dependencies, NodeFactory factory, MethodDesc method);
+        public abstract void AddDependenciesDueToMethodCodePresence(ref DependencyList dependencies, NodeFactory factory, MethodDesc method);
 
         public abstract void AddInterestingInteropConstructedTypeDependencies(ref DependencyList dependencies, NodeFactory factory, TypeDesc type);
 

src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/UsageBasedInteropStubManager.cs

@@ -28,21 +28,8 @@ public UsageBasedInteropStubManager(InteropStateManager interopStateManager, PIn
             _logger = logger;
         }
 
-        public override void AddDependenciesDueToPInvoke(ref DependencyList dependencies, NodeFactory factory, MethodDesc method)
+        public override void AddDependenciesDueToMethodCodePresence(ref DependencyList dependencies, NodeFactory factory, MethodDesc method)
         {
-            if (method.IsPInvoke && method.OwningType is MetadataType type && MarshalHelpers.IsRuntimeMarshallingEnabled(type.Module))
-            {
-                dependencies = dependencies ?? new DependencyList();
-
-                MethodSignature methodSig = method.Signature;
-                AddParameterMarshallingDependencies(ref dependencies, factory, method, methodSig.ReturnType);
-
-                for (int i = 0; i < methodSig.Length; i++)
-                {
-                    AddParameterMarshallingDependencies(ref dependencies, factory, method, methodSig[i]);
-                }
-            }
-
             if (method.HasInstantiation)
             {
                 dependencies = dependencies ?? new DependencyList();
@@ -50,48 +37,6 @@ public override void AddDependenciesDueToPInvoke(ref DependencyList dependencies
             }
         }
 
-        private void AddParameterMarshallingDependencies(ref DependencyList dependencies, NodeFactory factory, MethodDesc method, TypeDesc type)
-        {
-            if (type.IsDelegate)
-            {
-                dependencies.Add(factory.DelegateMarshallingData((DefType)type), "Delegate marshaling");
-            }
-
-            TypeSystemContext context = type.Context;
-            if ((type.IsWellKnownType(WellKnownType.MulticastDelegate)
-                    || type == context.GetWellKnownType(WellKnownType.MulticastDelegate).BaseType))
-            {
-                // If we hit this p/invoke as part of delegate marshalling (i.e. this is a delegate
-                // that has another delegate in the signature), blame the delegate type, not the marshalling thunk.
-                // This should ideally warn from the use site (e.g. where GetDelegateForFunctionPointer
-                // is called) but it's currently hard to get a warning from those spots and this guarantees
-                // we won't miss a spot (e.g. a p/invoke that has a delegate and that delegate contains
-                // a System.Delegate parameter).
-                MethodDesc reportedMethod = method;
-                if (reportedMethod is Internal.IL.Stubs.DelegateMarshallingMethodThunk delegateThunkMethod)
-                {
-                    reportedMethod = delegateThunkMethod.InvokeMethod;
-                }
-
-                _logger.LogWarning(reportedMethod, DiagnosticId.CorrectnessOfAbstractDelegatesCannotBeGuaranteed, DiagnosticUtilities.GetMethodSignatureDisplayName(method));
-            }
-
-            // struct may contain delegate fields, hence we need to add dependencies for it
-            if (type.IsByRef)
-                type = ((ParameterizedType)type).ParameterType;
-
-            if (MarshalHelpers.IsStructMarshallingRequired(type))
-            {
-                foreach (FieldDesc field in type.GetFields())
-                {
-                    if (field.IsStatic)
-                        continue;
-
-                    AddParameterMarshallingDependencies(ref dependencies, factory, method, field.FieldType);
-                }
-            }
-        }
-
         public override void AddInterestingInteropConstructedTypeDependencies(ref DependencyList dependencies, NodeFactory factory, TypeDesc type)
         {
             if (type.IsDelegate)