Increase precision and safety of the NonVersionableAttribute (#70327)

* Increase precision and safety of the NonVersionableAttribute
- NonVersionable now only unconditionally affects whether or not the IL is reported as inlined in the inlining data
- Primitive types are now implicitly considered to be NonVersionable
- Classes defined in CoreLib may now be considered to be NonVersionable
  - Classes now marked are String, RawData, and RawArrayData, which are all tied into intrinsics in crossgen2
- NFloat is now marked as NonVersionable so that some of its methods may actually be treated as NonVersionable
- There is now an additional predicate that the IL within the NonVersionable method must satisfy a series of tests to ensure that it can safely be inlined without adding additional tokens for the logic to depend on. See below for the details of the predicate

Collectively these changes make it so that even if we change our inlining rules, the NonVersionable marked methods will successfully pass through Crossgen, without generating requiring tokens from their original modules to be present in the final image.

The rules are:
                // 1. ldfld, ldflda, and stfld to instance fields of NonVersionable structs and NonVersionable classes
                // 2. cpobj, initobj, ldobj, stobj, ldelem, ldelema or sizeof, to NonVersionable structures, signature variables, pointers, function pointers, byrefs, classes, or arrays
                // 3. stelem, to NonVersionable structures
                // In addition, the method must not have any EH.
                // The method may only have locals which are NonVersionable structures, or classes

* Address code review feedback

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

@@ -7,6 +7,7 @@
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Runtime.Serialization;
+using System.Runtime.Versioning;
 
 namespace System.Runtime.CompilerServices
 {
@@ -399,6 +400,7 @@ public GCFrameRegistration(void* allocation, uint elemCount, bool areByRefs = tr
     }
     // Helper class to assist with unsafe pinning of arbitrary objects.
     // It's used by VM code.
+    [NonVersionable] // This only applies to field layout
     internal sealed class RawData
     {
         public byte Data;
@@ -412,6 +414,7 @@ internal sealed class RawData
     // The BaseSize of an array includes all the fields before the array data,
     // including the sync block and method table. The reference to RawData.Data
     // points at the number of components, skipping over these two pointer-sized fields.
+    [NonVersionable] // This only applies to field layout
     internal sealed class RawArrayData
     {
         public uint Length; // Array._numComponents padded to IntPtr

src/coreclr/tools/Common/Compiler/TypeExtensions.cs

@@ -731,28 +731,6 @@ public static bool ContainsSignatureVariables(this Instantiation instantiation,
             return false;
         }
 
-        /// <summary>
-        /// Return true when the type in question is marked with the NonVersionable attribute.
-        /// </summary>
-        /// <param name="type">Type to check</param>
-        /// <returns>True when the type is marked with the non-versionable custom attribute, false otherwise.</returns>
-        public static bool IsNonVersionable(this MetadataType type)
-        {
-            return type.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute");
-        }
-
-        /// <summary>
-        /// Return true when the method is marked as non-versionable. Non-versionable methods
-        /// may be freely inlined into ReadyToRun images even when they don't reside in the
-        /// same version bubble as the module being compiled.
-        /// </summary>
-        /// <param name="method">Method to check</param>
-        /// <returns>True when the method is marked as non-versionable, false otherwise.</returns>
-        public static bool IsNonVersionable(this MethodDesc method)
-        {
-            return method.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute");
-        }
-
         /// <summary>
         /// Returns true if <paramref name="method"/> is an actual native entrypoint.
         /// There's a distinction between when a method reports it's a PInvoke in the metadata

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/R2RTypeExtensions.cs

@@ -0,0 +1,56 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using Internal.IL;
+using Internal.TypeSystem;
+using Internal.TypeSystem.Ecma;
+
+using Debug = System.Diagnostics.Debug;
+
+namespace ILCompiler
+{
+    public static class R2RTypeExtensions
+    {
+        /// <summary>
+        /// Return true when the type in question is marked with the NonVersionable attribute. Primitive types are implicitly NonVersionable
+        /// </summary>
+        /// <param name="type">Type to check</param>
+        /// <returns>True when the type is marked with the non-versionable custom attribute and meets the criteria
+        /// for a non-versionable type, false otherwise.</returns>
+        public static bool IsNonVersionable(this MetadataType type)
+        {
+            bool result = type.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute");
+
+            if (!type.IsValueType)
+            {
+                if (type.BaseType != type.Context.GetWellKnownType(WellKnownType.Object)) // Only types that derive directly from Object can be non-versionable
+                    result = false;
+
+                // Only reference types defined in System.Private.CoreLib are eligible for the non-versionable processing
+                // This allows for extremely careful vetting of said types
+                if (type.Module != type.Context.SystemModule)
+                    result = false;
+            }
+            else if (type.IsPrimitive)
+            {
+                // The primitive types are all NonVersionable
+                result = true;
+            }
+
+            return result;
+        }
+
+        /// <summary>
+        /// Return true when the method is marked as non-versionable. Non-versionable methods
+        /// may be freely inlined into ReadyToRun images even when they don't reside in the
+        /// same version bubble as the module being compiled.
+        /// </summary>
+        /// <param name="method">Method to check</param>
+        /// <returns>True when the method is marked as non-versionable, false otherwise.</returns>
+        public static bool IsNonVersionable(this MethodDesc method)
+        {
+            return method.HasCustomAttribute("System.Runtime.Versioning", "NonVersionableAttribute");
+        }
+    }
+}

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs

@@ -471,13 +471,6 @@ private bool IsLayoutFixedInCurrentVersionBubbleInternal(TypeDesc type)
 
             if (!NodeFactory.CompilationModuleGroup.VersionsWithModule(defType.Module))
             {
-                if (!type.IsValueType)
-                {
-                    // Eventually, we may respect the non-versionable attribute for reference types too. For now, we are going
-                    // to play it safe and ignore it.
-                    return false;
-                }
-
                 // Valuetypes with non-versionable attribute are candidates for fixed layout. Reject the rest.
                 return type is MetadataType metadataType && metadataType.IsNonVersionable();
             }

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCompilationModuleGroupBase.cs

@@ -6,10 +6,12 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Threading;
+using ILCompiler.DependencyAnalysis;
+using ILCompiler.DependencyAnalysis.ReadyToRun;
+using Internal.IL;
 using Internal.TypeSystem;
 using Internal.TypeSystem.Ecma;
 using Internal.TypeSystem.Interop;
-using ILCompiler.DependencyAnalysis.ReadyToRun;
 using Debug = System.Diagnostics.Debug;
 
 namespace ILCompiler
@@ -39,6 +41,7 @@ public abstract class ReadyToRunCompilationModuleGroupBase : CompilationModuleGr
         private readonly Dictionary<ModuleDesc, CompilationUnitIndex> _moduleCompilationUnits = new Dictionary<ModuleDesc, CompilationUnitIndex>();
         private CompilationUnitIndex _nextCompilationUnit = CompilationUnitIndex.FirstDynamicallyAssigned;
         private ModuleTokenResolver _tokenResolver = null;
+        private ConcurrentDictionary<EcmaMethod, bool> _tokenTranslationFreeNonVersionable = new ConcurrentDictionary<EcmaMethod, bool>();
 
         public ReadyToRunCompilationModuleGroupBase(ReadyToRunCompilationModuleGroupConfig config)
         {
@@ -342,11 +345,163 @@ public sealed override bool CanInline(MethodDesc callerMethod, MethodDesc callee
             // (because otherwise we may not be able to encode its tokens)
             // and if the callee is either in the same version bubble or is marked as non-versionable.
             bool canInline = VersionsWithMethodBody(callerMethod) &&
-                (VersionsWithMethodBody(calleeMethod) || calleeMethod.IsNonVersionable());
+                (VersionsWithMethodBody(calleeMethod) || IsNonVersionableWithILTokensThatDoNotNeedTranslation(calleeMethod));
 
             return canInline;
         }
 
+        public bool IsNonVersionableWithILTokensThatDoNotNeedTranslation(MethodDesc method)
+        {
+            if (!method.IsNonVersionable())
+                return false;
+
+            return _tokenTranslationFreeNonVersionable.GetOrAdd((EcmaMethod)method.GetTypicalMethodDefinition(), IsNonVersionableWithILTokensThatDoNotNeedTranslationUncached);
+        }
+
+        private bool IsNonVersionableWithILTokensThatDoNotNeedTranslationUncached(EcmaMethod method)
+        {
+            bool result = false;
+            try
+            {
+
+                // Validate that there are no tokens in the IL other than tokens associated with the following
+                // instructions with the 
+                // 1. ldfld, ldflda, and stfld to instance fields of NonVersionable structs and NonVersionable classes
+                // 2. cpobj, initobj, ldobj, stobj, ldelem, ldelema or sizeof, to NonVersionable structures, signature variables, pointers, function pointers, byrefs, classes, or arrays
+                // 3. stelem, to NonVersionable structures
+                // In addition, the method must not have any EH.
+                // The method may only have locals which are NonVersionable structures, or classes
+
+                MethodIL methodIL = new ReadyToRunILProvider().GetMethodIL(method);
+                if (methodIL.GetExceptionRegions().Length > 0)
+                    return false;
+
+                foreach (var local in methodIL.GetLocals())
+                {
+                    if (local.Type.IsPrimitive)
+                        continue;
+
+                    if (local.Type.IsArray)
+                        continue;
+
+                    if (local.Type.IsSignatureVariable)
+                        continue;
+                    MetadataType metadataType = local.Type as MetadataType;
+
+                    if (metadataType == null)
+                        return false;
+
+                    if (metadataType.IsValueType)
+                    {
+                        if (metadataType.IsNonVersionable())
+                            continue;
+                        else
+                            return false;
+                    }
+                }
+
+                ILReader ilReader = new ILReader(methodIL.GetILBytes());
+                while (ilReader.HasNext)
+                {
+                    ILOpcode opcode = ilReader.ReadILOpcode();
+                    switch (opcode)
+                    {
+                        case ILOpcode.ldfld:
+                        case ILOpcode.ldflda:
+                        case ILOpcode.stfld:
+                            {
+                                int token = ilReader.ReadILToken();
+                                FieldDesc field = methodIL.GetObject(token) as FieldDesc;
+                                if (field == null)
+                                    return false;
+                                if (field.IsStatic)
+                                    return false;
+                                MetadataType owningMetadataType = (MetadataType)field.OwningType;
+                                if (!owningMetadataType.IsNonVersionable())
+                                    return false;
+                                break;
+                            }
+
+                        case ILOpcode.ldelem:
+                        case ILOpcode.ldelema:
+                        case ILOpcode.stobj:
+                        case ILOpcode.ldobj:
+                        case ILOpcode.initobj:
+                        case ILOpcode.cpobj:
+                        case ILOpcode.sizeof_:
+                            {
+                                int token = ilReader.ReadILToken();
+                                TypeDesc type = methodIL.GetObject(token) as TypeDesc;
+                                if (type == null)
+                                    return false;
+
+                                MetadataType metadataType = type as MetadataType;
+                                if (metadataType == null)
+                                    continue; // Types which are not metadata types are all well defined in size
+
+                                if (!metadataType.IsValueType)
+                                    continue; // Reference types are all well defined in size for the sizeof instruction
+
+                                if (metadataType.IsNonVersionable())
+                                    continue;
+                                return false;
+                            }
+
+                        case ILOpcode.stelem:
+                            {
+                                int token = ilReader.ReadILToken();
+                                MetadataType type = methodIL.GetObject(token) as MetadataType;
+                                if (type == null)
+                                    return false;
+
+                                if (!type.IsValueType)
+                                    return false;
+                                if (!type.IsNonVersionable())
+                                    return false;
+                                break;
+                            }
+
+                        // IL instructions which refer to tokens which are not safe for NonVersionable methods
+                        case ILOpcode.box:
+                        case ILOpcode.call:
+                        case ILOpcode.calli:
+                        case ILOpcode.callvirt:
+                        case ILOpcode.castclass:
+                        case ILOpcode.jmp:
+                        case ILOpcode.isinst:
+                        case ILOpcode.ldstr:
+                        case ILOpcode.ldsfld:
+                        case ILOpcode.ldsflda:
+                        case ILOpcode.ldtoken:
+                        case ILOpcode.ldvirtftn:
+                        case ILOpcode.ldftn:
+                        case ILOpcode.mkrefany:
+                        case ILOpcode.newarr:
+                        case ILOpcode.newobj:
+                        case ILOpcode.refanyval:
+                        case ILOpcode.stsfld:
+                        case ILOpcode.unbox:
+                        case ILOpcode.unbox_any:
+                        case ILOpcode.constrained:
+                            return false;
+
+                        default:
+                            // Unless its a opcode known to be permitted with a 
+                            ilReader.Skip(opcode);
+                            break;
+                    }
+                }
+
+                result = true;
+            }
+            catch (TypeSystemException)
+            {
+                return false;
+            }
+
+            return result;
+        }
+
         public sealed override bool GeneratesPInvoke(MethodDesc method)
         {
             return !Marshaller.IsMarshallingRequired(method);

src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj

@@ -192,6 +192,7 @@
     <Compile Include="Compiler\PettisHansenSort\CallGraphNode.cs" />
     <Compile Include="Compiler\PettisHansenSort\DisjointSetForest.cs" />
     <Compile Include="Compiler\PettisHansenSort\PettisHansen.cs" />
+    <Compile Include="Compiler\R2RTypeExtensions.cs" />
     <Compile Include="IBC\IBCDataModel.cs" />
     <Compile Include="IBC\IBCDataReader.cs" />
     <Compile Include="IBC\MIbcProfileParser.cs" />

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/NFloat.cs

@@ -24,6 +24,7 @@ namespace System.Runtime.InteropServices
     /// <summary>Defines an immutable value type that represents a floating type that has the same size as the native integer size.</summary>
     /// <remarks>It is meant to be used as an exchange type at the managed/unmanaged boundary to accurately represent in managed code unmanaged APIs that use a type alias for C or C++'s <c>float</c> on 32-bit platforms or <c>double</c> on 64-bit platforms, such as the CGFloat type in libraries provided by Apple.</remarks>
     [Intrinsic]
+    [NonVersionable] // This only applies to field layout
     public readonly struct NFloat
         : IBinaryFloatingPointIeee754<NFloat>,
           IMinMaxValue<NFloat>

src/libraries/System.Private.CoreLib/src/System/String.cs

@@ -21,6 +21,7 @@ namespace System
     // positions (indices) are zero-based.
 
     [Serializable]
+    [NonVersionable] // This only applies to field layout
     [System.Runtime.CompilerServices.TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")]
     public sealed partial class String : IComparable, IEnumerable, IConvertible, IEnumerable<char>, IComparable<string?>, IEquatable<string?>, ICloneable
     {
@@ -482,7 +483,6 @@ public char[] ToCharArray(int startIndex, int length)
             return chars;
         }
 
-        [NonVersionable]
         public static bool IsNullOrEmpty([NotNullWhen(false)] string? value)
         {
             return value == null || value.Length == 0;