Move the metadata update related APIs to the MetadataUpdater class (#…

…54590)

Move the metadata update related APIs to the MetadataUpdater class

The old APIs AssemblyExtensions.ApplyUpdate() and AssemblyExtensions.GetApplyUpdateCapabilities() will be removed
after all the references to them have been changed to the new APIs.

Add the new IsSupported API described in issue #51159.

Change the tests to use the MetadataUpdater APIs.

Fix the ApplyUpdate qcalls and icalls

Add the ILLink substitutions for MetadataUpdater.IsSupported property

Change the old APIs to call the new ones

Update mono's MetadataUpdater.IsSupported property

Update feature switch doc

Fixed the argument checking in coreclr's MetadataUpdater.ApplyUpdate().

docs/workflow/trimming/feature-switches.md

@@ -22,6 +22,7 @@ configurations but their defaults might vary as any SDK can set the defaults dif
 | EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization | System.ComponentModel.TypeConverter.EnableUnsafeBinaryFormatterInDesigntimeLicenseContextSerialization | BinaryFormatter serialization support is trimmed when set to false. |
 | BuiltInComInteropSupport | System.Runtime.InteropServices.BuiltInComInterop.IsSupported | Built-in COM support is trimmed when set to false. |
 | EnableCppCLIHostActivation | System.Runtime.InteropServices.EnableCppCLIHostActivation | C++/CLI host activation code is disabled when set to false and related functionality can be trimmed. |
+| MetadataUpdaterSupport | System.Reflection.Metadata.MetadataUpdater.IsSupported | Metadata update related code to be trimmed when set to false |
 | _EnableConsumingManagedCodeFromNativeHosting | System.Runtime.InteropServices.EnableConsumingManagedCodeFromNativeHosting | Getting a managed function from native hosting is disabled when set to false and related functionality can be trimmed. |
 
 Any feature-switch which defines property can be set in csproj file or

src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj

@@ -189,6 +189,7 @@
     <Compile Include="$(BclSourcesRoot)\System\Reflection\MdImport.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\MemberInfo.Internal.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\Metadata\AssemblyExtensions.cs" />
+    <Compile Include="$(BclSourcesRoot)\System\Reflection\Metadata\MetadataUpdater.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\MethodBase.CoreCLR.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\RtFieldInfo.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimeAssembly.cs" />

src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/AssemblyExtensions.cs

@@ -41,9 +41,6 @@ public static unsafe bool TryGetRawMetadata(this Assembly assembly, out byte* bl
             return InternalTryGetRawMetadata(new QCallAssembly(ref rtAsm), ref blob, ref length);
         }
 
-        [DllImport(RuntimeHelpers.QCall)]
-        private static extern unsafe void ApplyUpdate(QCallAssembly assembly, byte* metadataDelta, int metadataDeltaLength, byte* ilDelta, int ilDeltaLength, byte* pdbDelta, int pdbDeltaLength);
-
         /// <summary>
         /// Updates the specified assembly using the provided metadata, IL and PDB deltas.
         /// </summary>
@@ -61,30 +58,12 @@ public static unsafe bool TryGetRawMetadata(this Assembly assembly, out byte* bl
         /// <exception cref="NotSupportedException">The update could not be applied.</exception>
         public static void ApplyUpdate(Assembly assembly, ReadOnlySpan<byte> metadataDelta, ReadOnlySpan<byte> ilDelta, ReadOnlySpan<byte> pdbDelta)
         {
-            if (assembly == null)
-            {
-                throw new ArgumentNullException(nameof(assembly));
-            }
-
-            RuntimeAssembly? runtimeAssembly = assembly as RuntimeAssembly;
-            if (runtimeAssembly == null)
-            {
-                throw new ArgumentException(SR.Argument_MustBeRuntimeAssembly);
-            }
-
-            unsafe
-            {
-                RuntimeAssembly rtAsm = runtimeAssembly;
-                fixed (byte* metadataDeltaPtr = metadataDelta, ilDeltaPtr = ilDelta, pdbDeltaPtr = pdbDelta)
-                {
-                    ApplyUpdate(new QCallAssembly(ref rtAsm), metadataDeltaPtr, metadataDelta.Length, ilDeltaPtr, ilDelta.Length, pdbDeltaPtr, pdbDelta.Length);
-                }
-            }
+            MetadataUpdater.ApplyUpdate(assembly, metadataDelta, ilDelta, pdbDelta);
         }
 
         internal static string GetApplyUpdateCapabilities()
         {
-            return "Baseline AddMethodToExistingType AddStaticFieldToExistingType AddInstanceFieldToExistingType NewTypeDefinition ChangeCustomAttributes";
+            return MetadataUpdater.GetCapabilities();
         }
     }
 }

src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs

@@ -0,0 +1,63 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+namespace System.Reflection.Metadata
+{
+    public static class MetadataUpdater
+    {
+        [DllImport(RuntimeHelpers.QCall)]
+        private static extern unsafe void ApplyUpdate(QCallAssembly assembly, byte* metadataDelta, int metadataDeltaLength, byte* ilDelta, int ilDeltaLength, byte* pdbDelta, int pdbDeltaLength);
+
+        [DllImport(RuntimeHelpers.QCall)]
+        private static extern unsafe bool IsApplyUpdateSupported();
+
+        /// <summary>
+        /// Updates the specified assembly using the provided metadata, IL and PDB deltas.
+        /// </summary>
+        /// <remarks>
+        /// Currently executing methods will continue to use the existing IL. New executions of modified methods will
+        /// use the new IL. Different runtimes may have different limitations on what kinds of changes are supported,
+        /// and runtimes make no guarantees as to the state of the assembly and process if the delta includes
+        /// unsupported changes.
+        /// </remarks>
+        /// <param name="assembly">The assembly to update.</param>
+        /// <param name="metadataDelta">The metadata changes to be applied.</param>
+        /// <param name="ilDelta">The IL changes to be applied.</param>
+        /// <param name="pdbDelta">The PDB changes to be applied.</param>
+        /// <exception cref="ArgumentException">The assembly argument is not a runtime assembly.</exception>
+        /// <exception cref="ArgumentNullException">The assembly argument is null.</exception>
+        /// <exception cref="InvalidOperationException">The assembly is not editable.</exception>
+        /// <exception cref="NotSupportedException">The update could not be applied.</exception>
+        public static void ApplyUpdate(Assembly assembly, ReadOnlySpan<byte> metadataDelta, ReadOnlySpan<byte> ilDelta, ReadOnlySpan<byte> pdbDelta)
+        {
+            if (assembly is not RuntimeAssembly runtimeAssembly)
+            {
+                if (assembly is null) throw new ArgumentNullException(nameof(assembly));
+                throw new ArgumentException(SR.Argument_MustBeRuntimeAssembly);
+            }
+
+            unsafe
+            {
+                RuntimeAssembly rtAsm = runtimeAssembly;
+                fixed (byte* metadataDeltaPtr = metadataDelta, ilDeltaPtr = ilDelta, pdbDeltaPtr = pdbDelta)
+                {
+                    ApplyUpdate(new QCallAssembly(ref rtAsm), metadataDeltaPtr, metadataDelta.Length, ilDeltaPtr, ilDelta.Length, pdbDeltaPtr, pdbDelta.Length);
+                }
+            }
+        }
+
+        /// <summary>
+        /// Returns the metadata update capabilities.
+        /// </summary>
+        internal static string GetCapabilities() => "Baseline AddMethodToExistingType AddStaticFieldToExistingType AddInstanceFieldToExistingType NewTypeDefinition ChangeCustomAttributes";
+
+        /// <summary>
+        /// Returns true if the apply assembly update is enabled and available.
+        /// </summary>
+        public static bool IsSupported { get; } = IsApplyUpdateSupported();
+    }
+}

src/coreclr/vm/assemblynative.cpp

@@ -1450,3 +1450,21 @@ void QCALLTYPE AssemblyNative::ApplyUpdate(
 
     END_QCALL;
 }
+
+// static
+BOOL QCALLTYPE AssemblyNative::IsApplyUpdateSupported()
+{
+    QCALL_CONTRACT;
+
+    BOOL result = false;
+
+    BEGIN_QCALL;
+
+#ifdef EnC_SUPPORTED
+    BOOL result = CORDebuggerAttached() || g_pConfig->ForceEnc() || g_pConfig->DebugAssembliesModifiable();
+#endif
+
+    END_QCALL;
+
+    return result;
+}

src/coreclr/vm/assemblynative.hpp

@@ -129,6 +129,7 @@ class AssemblyNative
     static void QCALLTYPE TraceSatelliteSubdirectoryPathProbed(LPCWSTR filePath, HRESULT hr);
 
     static void QCALLTYPE ApplyUpdate(QCall::AssemblyHandle assembly, UINT8* metadataDelta, INT32 metadataDeltaLength, UINT8* ilDelta, INT32 ilDeltaLength, UINT8* pdbDelta, INT32 pdbDeltaLength);
+    static BOOL QCALLTYPE IsApplyUpdateSupported();
 };
 
 #endif

src/coreclr/vm/ecalllist.h

@@ -438,7 +438,11 @@ FCFuncEnd()
 
 FCFuncStart(gAssemblyExtensionsFuncs)
     QCFuncElement("InternalTryGetRawMetadata", AssemblyNative::InternalTryGetRawMetadata)
+FCFuncEnd()
+
+FCFuncStart(gMetadataUpdaterFuncs)
     QCFuncElement("ApplyUpdate", AssemblyNative::ApplyUpdate)
+    QCFuncElement("IsApplyUpdateSupported", AssemblyNative::IsApplyUpdateSupported)
 FCFuncEnd()
 
 FCFuncStart(gAssemblyLoadContextFuncs)
@@ -1121,11 +1125,8 @@ FCClassElement("ArgIterator", "System", gVarArgFuncs)
 FCClassElement("Array", "System", gArrayFuncs)
 FCClassElement("Assembly", "System.Reflection", gAssemblyFuncs)
 FCClassElement("AssemblyBuilder", "System.Reflection.Emit", gAssemblyBuilderFuncs)
-
 FCClassElement("AssemblyExtensions", "System.Reflection.Metadata", gAssemblyExtensionsFuncs)
-
 FCClassElement("AssemblyLoadContext", "System.Runtime.Loader", gAssemblyLoadContextFuncs)
-
 FCClassElement("AssemblyName", "System.Reflection", gAssemblyNameFuncs)
 FCClassElement("Buffer", "System", gBufferFuncs)
 FCClassElement("CLRConfig", "System", gClrConfig)
@@ -1168,6 +1169,7 @@ FCClassElement("Math", "System", gMathFuncs)
 FCClassElement("MathF", "System", gMathFFuncs)
 FCClassElement("MdUtf8String", "System", gMdUtf8String)
 FCClassElement("MetadataImport", "System.Reflection", gMetaDataImport)
+FCClassElement("MetadataUpdater", "System.Reflection.Metadata", gMetadataUpdaterFuncs)
 FCClassElement("MissingMemberException", "System",  gMissingMemberExceptionFuncs)
 FCClassElement("MngdFixedArrayMarshaler", "System.StubHelpers", gMngdFixedArrayMarshalerFuncs)
 FCClassElement("MngdNativeArrayMarshaler", "System.StubHelpers", gMngdNativeArrayMarshalerFuncs)

src/libraries/System.Private.CoreLib/src/ILLink/ILLink.LinkAttributes.Shared.xml

@@ -29,8 +29,9 @@
     <type fullname="System.Diagnostics.DebuggerVisualizerAttribute">
       <attribute internal="RemoveAttributeInstances" />
     </type>
+  </assembly>
 
-    <!-- Hot reload attributes-->
+  <assembly fullname="System.Private.CoreLib" feature="System.Reflection.Metadata.MetadataUpdater.IsSupported" featurevalue="false">
     <type fullname="System.Reflection.Metadata.MetadataUpdateHandlerAttribute">
       <attribute internal="RemoveAttributeInstances" />
     </type>

src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Substitutions.Shared.xml

@@ -27,5 +27,8 @@
     <type fullname="System.Resources.ResourceReader" feature="System.Resources.ResourceManager.AllowCustomResourceTypes" featurevalue="false">
       <method signature="System.Boolean get_AllowCustomResourceTypes()" body="stub" value="false" />
     </type>
+    <type fullname="System.Reflection.Metadata.MetadataUpdater" feature="System.Reflection.Metadata.MetadataUpdater.IsSupported" featurevalue="false">
+      <method signature="System.Boolean get_IsSupported()" body="stub" value="false" />
+    </type>
   </assembly>
 </linker>

src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs

@@ -12,6 +12,11 @@ public static partial class AssemblyExtensions
         public unsafe static bool TryGetRawMetadata(this System.Reflection.Assembly assembly, out byte* blob, out int length) { throw null; }
         public static void ApplyUpdate(Assembly assembly, ReadOnlySpan<byte> metadataDelta, ReadOnlySpan<byte> ilDelta, ReadOnlySpan<byte> pdbDelta) { throw null; }
     }
+    public static partial class MetadataUpdater
+    {
+        public static void ApplyUpdate(Assembly assembly, ReadOnlySpan<byte> metadataDelta, ReadOnlySpan<byte> ilDelta, ReadOnlySpan<byte> pdbDelta) { throw null; }
+        public static bool IsSupported { get { throw null; } }
+    }
     [System.AttributeUsage(System.AttributeTargets.Assembly, AllowMultiple = true)]
     public sealed class MetadataUpdateHandlerAttribute : System.Attribute
     {

src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs

@@ -83,5 +83,51 @@ void ClassWithCustomAttributes()
                 Assert.Equal(attrType, cattrs[0].GetType());
             });
         }
+
+        class NonRuntimeAssembly : Assembly
+        {
+        }
+
+        [Fact]
+        public static void ApplyUpdateInvalidParameters()
+        {
+            // Dummy delta arrays
+            var metadataDelta = new byte[20];
+            var ilDelta = new byte[20];
+
+            // Assembly can't be null
+            Assert.Throws<ArgumentNullException>("assembly", () =>
+                MetadataUpdater.ApplyUpdate(null, new ReadOnlySpan<byte>(metadataDelta), new ReadOnlySpan<byte>(ilDelta), ReadOnlySpan<byte>.Empty));
+
+            // Tests fail on non-runtime assemblies
+            Assert.Throws<ArgumentException>(() =>
+                MetadataUpdater.ApplyUpdate(new NonRuntimeAssembly(), new ReadOnlySpan<byte>(metadataDelta), new ReadOnlySpan<byte>(ilDelta), ReadOnlySpan<byte>.Empty));
+
+            // Tests that this assembly isn't not editable
+            Assert.Throws<InvalidOperationException>(() =>
+                MetadataUpdater.ApplyUpdate(typeof(AssemblyExtensions).Assembly, new ReadOnlySpan<byte>(metadataDelta), new ReadOnlySpan<byte>(ilDelta), ReadOnlySpan<byte>.Empty));
+        }
+
+        [Fact]
+        public static void GetCapabilities()
+        {
+            var ty = typeof(System.Reflection.Metadata.MetadataUpdater);
+            var mi = ty.GetMethod("GetCapabilities", BindingFlags.NonPublic | BindingFlags.Static, Array.Empty<Type>());
+
+            Assert.NotNull(mi);
+
+            var result = mi.Invoke(null, null);
+
+            Assert.NotNull(result);
+            Assert.Equal(typeof(string), result.GetType());
+        }
+
+        [Fact]
+        [ActiveIssue("Returns true on mono", TestRuntimes.Mono)]
+        public static void IsSupported()
+        {
+            bool result = MetadataUpdater.IsSupported;
+            Assert.False(result);
+        }
     }
 }

src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs

@@ -48,8 +48,8 @@ internal static bool CheckSupportedMonoConfiguration()
 
         internal static bool HasApplyUpdateCapabilities()
         {
-            var ty = typeof(AssemblyExtensions);
-            var mi = ty.GetMethod("GetApplyUpdateCapabilities", BindingFlags.NonPublic | BindingFlags.Static, Array.Empty<Type>());
+            var ty = typeof(MetadataUpdater);
+            var mi = ty.GetMethod("GetCapabilities", BindingFlags.NonPublic | BindingFlags.Static, Array.Empty<Type>());
 
             if (mi == null)
                 return false;
@@ -83,15 +83,15 @@ internal static void ApplyUpdate (System.Reflection.Assembly assm)
             byte[] dil_data = System.IO.File.ReadAllBytes(dil_name);
             byte[] dpdb_data = null; // TODO also use the dpdb data
 
-            AssemblyExtensions.ApplyUpdate(assm, dmeta_data, dil_data, dpdb_data);
+            MetadataUpdater.ApplyUpdate(assm, dmeta_data, dil_data, dpdb_data);
         }
 
         internal static bool UseRemoteExecutor => !IsModifiableAssembliesSet;
 
         internal static void AddRemoteInvokeOptions (ref RemoteInvokeOptions options)
         {
-                options = options ?? new RemoteInvokeOptions();
-                options.StartInfo.EnvironmentVariables.Add(DotNetModifiableAssembliesSwitch, DotNetModifiableAssembliesValue);
+            options = options ?? new RemoteInvokeOptions();
+            options.StartInfo.EnvironmentVariables.Add(DotNetModifiableAssembliesSwitch, DotNetModifiableAssembliesValue);
         }
 
         /// Run the given test case, which applies updates to the given assembly.

src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj

@@ -12,7 +12,6 @@
   <ItemGroup>
     <Compile Include="ApplyUpdateTest.cs" />
     <Compile Include="ApplyUpdateUtil.cs" />
-    <Compile Include="AssemblyExtensionsTest.cs" />
     <Compile Include="AssemblyLoadContextTest.cs" />
     <Compile Include="CollectibleAssemblyLoadContextTest.cs" />
     <Compile Include="ContextualReflection.cs" />

src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj

@@ -239,6 +239,7 @@
       <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\TypeBuilderInstantiation.cs" />
       <Compile Include="$(BclSourcesRoot)\System\Reflection\Emit\UnmanagedMarshal.cs" />
       <Compile Include="$(BclSourcesRoot)\System\Reflection\Metadata\AssemblyExtensions.cs" />
+      <Compile Include="$(BclSourcesRoot)\System\Reflection\Metadata\MetadataUpdater.cs" />
       <Compile Include="$(BclSourcesRoot)\System\Resources\ManifestBasedResourceGroveler.Mono.cs" />
       <Compile Include="$(BclSourcesRoot)\System\Runtime\GCSettings.Mono.cs" />
       <Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\DependentHandle.cs" />