[hot_reload] Allow benign Param table updates (#56800)

* bump hotreload-utils to one that uses the WatchHotReloadService

   necessary for producing nice deltas for code with lambdas

* allow benign param updates

   as long as the param name and sequence number stays the same, the edit is allowed.

* [enc] Add test of changes to lambda and enclosing method

diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml
index 50d8643..f5a7cb4 100644 (file)
--- a/eng/Version.Details.xml
+++ b/eng/Version.Details.xml
@@ -218,9 +218,9 @@
       <Uri>https://dev.azure.com/dnceng/internal/_git/dotnet-optimization</Uri>
       <Sha>f8742ac0820e98221e7eec5f13989d9538579399</Sha>
     </Dependency>
-    <Dependency Name="Microsoft.DotNet.HotReload.Utils.Generator.BuildTool" Version="1.0.1-alpha.0.21369.1">
+    <Dependency Name="Microsoft.DotNet.HotReload.Utils.Generator.BuildTool" Version="1.0.2-alpha.0.21402.1">
       <Uri>https://github.com/dotnet/hotreload-utils</Uri>
-      <Sha>589840a79ed6335a310a011ee9e244e660798cf5</Sha>
+      <Sha>312d3ada595616e44ad827f64231109dc4ea1431</Sha>
     </Dependency>
     <Dependency Name="System.Runtime.Numerics.TestData" Version="6.0.0-beta.21371.1">
       <Uri>https://github.com/dotnet/runtime-assets</Uri>

diff --git a/eng/Versions.props b/eng/Versions.props
index bc6adff..3c0fe9d 100644 (file)
--- a/eng/Versions.props
+++ b/eng/Versions.props
@@ -154,7 +154,7 @@
     <MicrosoftNETTestSdkVersion>16.9.0-preview-20201201-01</MicrosoftNETTestSdkVersion>
     <MicrosoftDotNetXHarnessTestRunnersXunitVersion>1.0.0-prerelease.21373.1</MicrosoftDotNetXHarnessTestRunnersXunitVersion>
     <MicrosoftDotNetXHarnessCLIVersion>1.0.0-prerelease.21373.1</MicrosoftDotNetXHarnessCLIVersion>
-    <MicrosoftDotNetHotReloadUtilsGeneratorBuildToolVersion>1.0.1-alpha.0.21369.1</MicrosoftDotNetHotReloadUtilsGeneratorBuildToolVersion>
+    <MicrosoftDotNetHotReloadUtilsGeneratorBuildToolVersion>1.0.2-alpha.0.21402.1</MicrosoftDotNetHotReloadUtilsGeneratorBuildToolVersion>
     <XUnitVersion>2.4.1</XUnitVersion>
     <XUnitRunnerVisualStudioVersion>2.4.2</XUnitRunnerVisualStudioVersion>
     <CoverletCollectorVersion>1.3.0</CoverletCollectorVersion>

diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange.cs
new file mode 100644 (file)
index 0000000..9024582
--- /dev/null
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange.cs
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+using System;
+
+namespace System.Reflection.Metadata.ApplyUpdate.Test
+{
+    public class LambdaBodyChange {
+
+        public LambdaBodyChange () {}
+
+        public string MethodWithLambda () {
+            Func<string,string> fn = static (s) => "OLD " + s;
+            return fn("STRING");
+        }
+    }
+}

diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v1.cs
new file mode 100644 (file)
index 0000000..470c22c
--- /dev/null
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v1.cs
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+using System;
+
+namespace System.Reflection.Metadata.ApplyUpdate.Test
+{
+    public class LambdaBodyChange {
+
+        public LambdaBodyChange () {}
+
+        public string MethodWithLambda () {
+            Func<string,string> fn = static (s) => s + " STRING";
+            return fn("NEW");
+        }
+    }
+}

diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v2.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v2.cs
new file mode 100644 (file)
index 0000000..f98d0a8
--- /dev/null
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v2.cs
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+using System;
+
+namespace System.Reflection.Metadata.ApplyUpdate.Test
+{
+    public class LambdaBodyChange {
+
+        public LambdaBodyChange () {}
+
+        public string MethodWithLambda () {
+            Func<string,string> fn = static (s) => s + " STRING!";
+            return fn("NEWEST");
+        }
+    }
+}

diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange.csproj
new file mode 100644 (file)
index 0000000..7a8ab7f
--- /dev/null
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange.csproj
@@ -0,0 +1,12 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <RootNamespace>System.Runtime.Loader.Tests</RootNamespace>
+    <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
+    <TestRuntime>true</TestRuntime>
+    <DeltaScript>deltascript.json</DeltaScript>
+    <SkipTestUtilitiesReference>true</SkipTestUtilitiesReference>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="LambdaBodyChange.cs" />
+  </ItemGroup>
+</Project>

diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/deltascript.json
new file mode 100644 (file)
index 0000000..6f020cd
--- /dev/null
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/deltascript.json
@@ -0,0 +1,7 @@
+{
+    "changes": [
+        {"document": "LambdaBodyChange.cs", "update": "LambdaBodyChange_v1.cs"},
+        {"document": "LambdaBodyChange.cs", "update": "LambdaBodyChange_v2.cs"},
+    ]
+}
+

diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
index c8cf4b4..7b19a79 100644 (file)
--- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
+++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
@@ -40,6 +40,33 @@ namespace System.Reflection.Metadata
         }
 
         [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] 
+        void LambdaBodyChange()
+        {
+            ApplyUpdateUtil.TestCase(static () =>
+            {
+                var assm = typeof (ApplyUpdate.Test.LambdaBodyChange).Assembly;
+
+                var o = new ApplyUpdate.Test.LambdaBodyChange ();
+                var r = o.MethodWithLambda();
+
+                Assert.Equal("OLD STRING", r);
+
+                ApplyUpdateUtil.ApplyUpdate(assm);
+
+                r = o.MethodWithLambda();
+
+                Assert.Equal("NEW STRING", r);
+
+                ApplyUpdateUtil.ApplyUpdate(assm);
+
+                r = o.MethodWithLambda();
+
+                Assert.Equal("NEWEST STRING!", r);
+            });
+        }
+
+        [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))]
         [ActiveIssue("https://github.com/dotnet/runtime/issues/52993", TestRuntimes.Mono)]
         void ClassWithCustomAttributes()
         {

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 714d83d..bd9f445 100644 (file)
--- a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj
+++ b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj
@@ -35,6 +35,7 @@
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.MethodBody1\System.Reflection.Metadata.ApplyUpdate.Test.MethodBody1.csproj" />
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributes\System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributes.csproj" />
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.CustomAttributeUpdate\System.Reflection.Metadata.ApplyUpdate.Test.CustomAttributeUpdate.csproj" />
+    <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange\System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange.csproj" />
 
   </ItemGroup>
   <ItemGroup Condition="'$(TargetOS)' == 'Browser'">
@@ -49,6 +50,9 @@
       <TrimmerRootAssembly Condition="$([System.String]::Copy('%(ResolvedFileToPublish.FileName)%(ResolvedFileToPublish.Extension)').EndsWith('System.Reflection.Metadata.ApplyUpdate.Test.MethodBody1.dll'))" Include="%(ResolvedFileToPublish.FullPath)" />
       <TrimmerRootAssembly Condition="$([System.String]::Copy('%(ResolvedFileToPublish.FileName)%(ResolvedFileToPublish.Extension)').EndsWith('System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributes.dll'))" Include="%(ResolvedFileToPublish.FullPath)" />
       <TrimmerRootAssembly Condition="$([System.String]::Copy('%(ResolvedFileToPublish.FileName)%(ResolvedFileToPublish.Extension)').EndsWith('System.Reflection.Metadata.ApplyUpdate.Test.CustomAttributeUpdate.dll'))" Include="%(ResolvedFileToPublish.FullPath)" />
+      <TrimmerRootAssembly
+          Condition="$([System.String]::Copy('%(ResolvedFileToPublish.FileName)%(ResolvedFileToPublish.Extension)').EndsWith('System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange.dll'))"
+          Include="%(ResolvedFileToPublish.FullPath)" />
     </ItemGroup>
   </Target>
 

diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c
index 1d7bc77..0e7117e 100644 (file)
--- a/src/mono/mono/component/hot_reload.c
+++ b/src/mono/mono/component/hot_reload.c
@@ -1167,6 +1167,32 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, gconstpointer
                                break;
                        }
                }
+               case MONO_TABLE_PARAM: {
+                       /* FIXME: this should get the current table size, not the base stable size */
+                       if (token_index <= table_info_get_rows (&image_base->tables [token_table])) {
+                               /* We only allow modifications where the parameter name doesn't change. */
+                               uint32_t base_param [MONO_PARAM_SIZE];
+                               uint32_t upd_param [MONO_PARAM_SIZE];
+                               int mapped_token = hot_reload_relative_delta_index (image_dmeta, mono_metadata_make_token (token_table, token_index));
+                               g_assert (mapped_token != -1);
+                               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x PARAM update.  mapped index = 0x%08x\n", i, log_token, mapped_token);
+
+                               mono_metadata_decode_row (&image_dmeta->tables [MONO_TABLE_PARAM], mapped_token - 1, upd_param, MONO_PARAM_SIZE);
+                               mono_metadata_decode_row (&image_base->tables [MONO_TABLE_PARAM], token_index - 1, base_param, MONO_PARAM_SIZE);
+
+                               const char *base_name = mono_metadata_string_heap (image_base, base_param [MONO_PARAM_NAME]);
+                               const char *upd_name = mono_metadata_string_heap (image_base, upd_param [MONO_PARAM_NAME]);
+                               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x: 0x%08x PARAM update: seq = %d (base = %d), name = '%s' (base = '%s')\n", i, log_token, upd_param [MONO_PARAM_SEQUENCE], base_param [MONO_PARAM_SEQUENCE], upd_name, base_name);
+                               if (strcmp (base_name, upd_name) != 0 || base_param [MONO_PARAM_SEQUENCE] != upd_param [MONO_PARAM_SEQUENCE]) {
+                                       mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing PARAM table cols.", i, log_token);
+                                       mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing PARAM table cols. token=0x%08x", log_token);
+                                       unsupported_edits = TRUE;
+                                       continue;
+                               }
+                               break;
+                       } else
+                               break; /* added a row. ok */
+               }
                default:
                        /* FIXME: this bounds check is wrong for cumulative updates - need to look at the DeltaInfo:count.prev_gen_rows */
                        if (token_index <= table_info_get_rows (&image_base->tables [token_table])) {
@@ -1341,6 +1367,10 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen
                        /* ok, pass1 checked for disallowed modifications */
                        break;
                }
+               case MONO_TABLE_PARAM: {
+                       /* ok, pass1 checked for disallowed modifications */
+                       break;
+               }
                default: {
                        g_assert (token_index > table_info_get_rows (&image_base->tables [token_table]));
                        if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE))