From 72bc435bcc690f6de3cd0b4c5b6df4f987bf7923 Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Tue, 22 Jun 2021 19:28:38 -0700 Subject: [PATCH] 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 https://github.com/dotnet/runtime/issues/51159. Change the tests to use the MetadataUpdater APIs. --- .../System.Private.CoreLib.csproj | 1 + .../Reflection/Metadata/MetadataUpdater.cs | 63 ++++++++++++++++++ .../ref/System.Runtime.Loader.cs | 6 ++ .../tests/ApplyUpdateTest.cs | 38 +++++++++++ .../tests/ApplyUpdateUtil.cs | 16 ++--- .../tests/AssemblyExtensionsTest.cs | 48 -------------- .../tests/System.Runtime.Loader.Tests.csproj | 1 - .../System.Private.CoreLib.csproj | 1 + .../Reflection/Metadata/MetadataUpdater.cs | 65 +++++++++++++++++++ 9 files changed, 179 insertions(+), 60 deletions(-) create mode 100644 src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs delete mode 100644 src/libraries/System.Runtime.Loader/tests/AssemblyExtensionsTest.cs create mode 100644 src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs diff --git a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj index 626f0eec24b22..97671f56e5dc0 100644 --- a/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -189,6 +189,7 @@ + diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs new file mode 100644 index 0000000000000..22e6ceccd9052 --- /dev/null +++ b/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); + + /// + /// Updates the specified assembly using the provided metadata, IL and PDB deltas. + /// + /// + /// 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. + /// + /// The assembly to update. + /// The metadata changes to be applied. + /// The IL changes to be applied. + /// The PDB changes to be applied. + /// The assembly argument is null. + /// The update could not be applied. + public static void ApplyUpdate(Assembly assembly, ReadOnlySpan metadataDelta, ReadOnlySpan ilDelta, ReadOnlySpan 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); + } + } + } + + /// + /// Returns the metadata update capabilities. + /// + public static string GetCapabilities() => "Baseline AddMethodToExistingType AddStaticFieldToExistingType AddInstanceFieldToExistingType NewTypeDefinition"; + + /// + /// Returns true if the apply assembly update is enabled and available. + /// + public static bool IsSupported => Debugger.IsAttached || Environment.GetEnvironmentVariable("DOTNET_MODIFIABLE_ASSEMBLIES") != "" || Environment.GetEnvironmentVariable("COMPlus_ForceEnc") == "1"; + } +} diff --git a/src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs b/src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs index 3aaa591f62411..2d70d94551b13 100644 --- a/src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs +++ b/src/libraries/System.Runtime.Loader/ref/System.Runtime.Loader.cs @@ -12,6 +12,12 @@ 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 metadataDelta, ReadOnlySpan ilDelta, ReadOnlySpan pdbDelta) { throw null; } } + public static partial class MetadataUpdater + { + public static void ApplyUpdate(Assembly assembly, ReadOnlySpan metadataDelta, ReadOnlySpan ilDelta, ReadOnlySpan pdbDelta) { throw null; } + public static string GetCapabilities() { throw null; } + public static bool IsSupported { get { throw null; } } + } [System.AttributeUsage(System.AttributeTargets.Assembly, AllowMultiple = true)] public sealed class MetadataUpdateHandlerAttribute : System.Attribute { diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 2a953d19eece8..b4b79202605a2 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -83,5 +83,43 @@ 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("assembly", () => + MetadataUpdater.ApplyUpdate(null, new ReadOnlySpan(metadataDelta), new ReadOnlySpan(ilDelta), ReadOnlySpan.Empty)); + + // Tests fail on non-runtime assemblies + Assert.Throws(() => + MetadataUpdater.ApplyUpdate(new NonRuntimeAssembly(), new ReadOnlySpan(metadataDelta), new ReadOnlySpan(ilDelta), ReadOnlySpan.Empty)); + + // Tests that this assembly isn't not editable + Assert.Throws(() => + MetadataUpdater.ApplyUpdate(typeof(AssemblyExtensions).Assembly, new ReadOnlySpan(metadataDelta), new ReadOnlySpan(ilDelta), ReadOnlySpan.Empty)); + } + + [Fact] + public static void GetCapabilities() + { + string result = MetadataUpdater.GetCapabilities(); + Assert.NotNull(result); + } + + [Fact] + public static void IsSupported() + { + bool result = MetadataUpdater.IsSupported; + Assert.False(result); + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs index ac734d35456aa..49e14a58e8d83 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateUtil.cs @@ -48,16 +48,10 @@ internal static bool CheckSupportedMonoConfiguration() internal static bool HasApplyUpdateCapabilities() { - var ty = typeof(AssemblyExtensions); - var mi = ty.GetMethod("GetApplyUpdateCapabilities", BindingFlags.NonPublic | BindingFlags.Static, Array.Empty()); - - if (mi == null) - return false; - - var caps = mi.Invoke(null, null); + string caps = MetadataUpdater.GetCapabilities(); // any non-empty string, assumed to be at least "baseline" - return caps is string {Length: > 0}; + return caps.Length > 0; } private static System.Collections.Generic.Dictionary assembly_count = new(); @@ -83,15 +77,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. diff --git a/src/libraries/System.Runtime.Loader/tests/AssemblyExtensionsTest.cs b/src/libraries/System.Runtime.Loader/tests/AssemblyExtensionsTest.cs deleted file mode 100644 index 5208b573c75d2..0000000000000 --- a/src/libraries/System.Runtime.Loader/tests/AssemblyExtensionsTest.cs +++ /dev/null @@ -1,48 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Xunit; - -namespace System.Reflection.Metadata -{ - public class AssemblyExtensionsTest - { - 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("assembly", () => - AssemblyExtensions.ApplyUpdate(null, new ReadOnlySpan(metadataDelta), new ReadOnlySpan(ilDelta), ReadOnlySpan.Empty)); - - // Tests fail on non-runtime assemblies - Assert.Throws(() => - AssemblyExtensions.ApplyUpdate(new NonRuntimeAssembly(), new ReadOnlySpan(metadataDelta), new ReadOnlySpan(ilDelta), ReadOnlySpan.Empty)); - - // Tests that this assembly isn't not editable - Assert.Throws(() => - AssemblyExtensions.ApplyUpdate(typeof(AssemblyExtensions).Assembly, new ReadOnlySpan(metadataDelta), new ReadOnlySpan(ilDelta), ReadOnlySpan.Empty)); - } - - [Fact] - public void GetApplyUpdateCapabilitiesIsCallable() - { - var ty = typeof(System.Reflection.Metadata.AssemblyExtensions); - var mi = ty.GetMethod("GetApplyUpdateCapabilities", BindingFlags.NonPublic | BindingFlags.Static, Array.Empty()); - - Assert.NotNull(mi); - - var result = mi.Invoke(null, null); - - Assert.NotNull(result); - Assert.Equal(typeof(string), result.GetType()); - } - } -} diff --git a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj index dc1a2cccd21a5..eeae81e75cc68 100644 --- a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj +++ b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj @@ -12,7 +12,6 @@ - diff --git a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj index 61ad598697e04..803d83464ea12 100644 --- a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -239,6 +239,7 @@ + diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs new file mode 100644 index 0000000000000..6dbcdc783caa0 --- /dev/null +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs @@ -0,0 +1,65 @@ +// 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; + +namespace System.Reflection.Metadata +{ + public static class MetadataUpdater + { + /// + /// Updates the specified assembly using the provided metadata, IL and PDB deltas. + /// + /// + /// 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. + /// + /// The assembly to update. + /// The metadata changes to be applied. + /// The IL changes to be applied. + /// The PDB changes to be applied. + /// The assembly argument is null. + /// The update could not be applied. + public static void ApplyUpdate(Assembly assembly, ReadOnlySpan metadataDelta, ReadOnlySpan ilDelta, ReadOnlySpan pdbDelta) + { + if (assembly is not RuntimeAssembly runtimeAssembly) + { + if (assembly is null) throw new ArgumentNullException(nameof(assembly)); + throw new ArgumentException(SR.Argument_MustBeRuntimeAssembly); + } + + // System.Private.CoreLib is not editable + if (runtimeAssembly == typeof(AssemblyExtensions).Assembly) + throw new InvalidOperationException (SR.InvalidOperation_AssemblyNotEditable); + + unsafe + { + IntPtr monoAssembly = runtimeAssembly.GetUnderlyingNativeHandle (); + fixed (byte* metadataDeltaPtr = metadataDelta, ilDeltaPtr = ilDelta, pdbDeltaPtr = pdbDelta) + { + ApplyUpdate_internal(monoAssembly, metadataDeltaPtr, metadataDelta.Length, ilDeltaPtr, ilDelta.Length, pdbDeltaPtr, pdbDelta.Length); + } + } + } + + private static Lazy s_ApplyUpdateCapabilities = new Lazy(() => InitializeApplyUpdateCapabilities()); + + public static string GetCapabilities() => s_ApplyUpdateCapabilities.Value; + + private static string InitializeApplyUpdateCapabilities() + { + return ApplyUpdateEnabled() != 0 ? "Baseline" : string.Empty ; + } + + public static bool IsSupported => Debugger.IsAttached || Environment.GetEnvironmentVariable("DOTNET_MODIFIABLE_ASSEMBLIES") != ""; + + [MethodImpl (MethodImplOptions.InternalCall)] + private static extern int ApplyUpdateEnabled (); + + [MethodImpl (MethodImplOptions.InternalCall)] + private static unsafe extern void ApplyUpdate_internal (IntPtr base_assm, byte* dmeta_bytes, int dmeta_length, byte *dil_bytes, int dil_length, byte *dpdb_bytes, int dpdb_length); + } +}