[hot_reload] Add support for row modifications; CustomAttribute updat…

…es (#55445)

This fixes #55097 - which allows us to support C# nullability analysis once again in hot reload deltas.

Specifically we allow EnC deltas to include modifications of existing rows in the CustomAttribute table as long as the Parent and Type columns stay the same (that is: a custom attribute modification that still applies to the same element - and uses the same custom attribute constructor, but may have a different value).

To support this, we add a new BaselineInfo:any_modified_rows array that keeps track for each table whether any rows have been modified (as opposed to added) by any EnC delta. When the runtime calls mono_metadata_decode_row, if there have been any deltas that modified a row in the requested table, we call hot_reload_effective_table_slow which find the latest delta that modified that row. If there have only been additions, we stop at the first delta that added the row we're looking for, if there are modifications, we look through all the deltas and return the latest one.

* [hot_reload] Add test for updating custom attribute ctor values

   Just changing the arguments of a custom attribute constructor should generate an update to the CustomAttributes table with the same parent and .ctor.  That kind of change should be allowed by Mono and CoreCLR

* [hot_reload] Allow custom attribute row modifications if Parent and Type unchanged.

   Allows updates to the constructor arguments (or property values)

* [hot_reload] Implement table lookup of modified rows

   Add a bool array on the base image to keep track of whether each table had any row modifications (as opposed to row additions) in any generation.

   If there was a modification, take the slow path in mono_image_effective_table even if the index we're looking at is in the base image.

   Update hot_reload_effective_table_slow so that if there was a modified row, we look at all the deltas to see if there's an even later update to that row.  (When there are only additions, keep same behavior as before - only look
as far as the generation that added the row we wanted to find).

   Also refine the assertion in hot_reload_relative_delta_index to properly account for EnCMap entries that correspond to modifications - in that case we might stop searching a metadata delta before we hit the end of the table if the
EnCmap entries start pointing to rows that are past the one we wanted to look up.

* Update the CustomAttributeUpdates test to check attribute value

   Check that we get the updated custom attribute string property value.

* Re-enable nullability for hot reload tests

   Mono can now deal with the custom attributes modifications that Roslyn emits

src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.CustomAttributeUpdate/CustomAttributeUpdate.cs

@@ -0,0 +1,35 @@
+// 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
+{
+    [AttributeUsage (AttributeTargets.Method, AllowMultiple=true)]
+    public class MyAttribute : Attribute
+    {
+        public MyAttribute (string stringValue) { StringValue = stringValue; }
+
+        public MyAttribute (Type typeValue) { TypeValue = typeValue; }
+
+        public MyAttribute (int x) { IntValue = x; }
+
+        public string StringValue { get; set; }
+        public Type TypeValue {get; set; }
+        public int IntValue {get; set; }
+    }
+
+    public class ClassWithCustomAttributeUpdates
+    {
+        [MyAttribute ("abcd")]
+        public static string Method1 () => null;
+
+        [MyAttribute (typeof(Exception))]
+        public static string Method2 () => null;
+
+        [MyAttribute (42, StringValue = "hijkl", TypeValue = typeof(Type))]
+        [MyAttribute (17, StringValue = "", TypeValue = typeof(object))]
+        public static string Method3 () => null;
+
+    }
+}

src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.CustomAttributeUpdate/CustomAttributeUpdate_v1.cs

@@ -0,0 +1,35 @@
+// 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
+{
+    [AttributeUsage (AttributeTargets.Method, AllowMultiple=true)]
+    public class MyAttribute : Attribute
+    {
+        public MyAttribute (string stringValue) { StringValue = stringValue; }
+
+        public MyAttribute (Type typeValue) { TypeValue = typeValue; }
+
+        public MyAttribute (int x) { IntValue = x; }
+
+        public string StringValue { get; set; }
+        public Type TypeValue {get; set; }
+        public int IntValue {get; set; }
+    }
+
+    public class ClassWithCustomAttributeUpdates
+    {
+        [MyAttribute ("rstuv")]
+        public static string Method1 () => null;
+
+        [MyAttribute (typeof(ArgumentException))]
+        public static string Method2 () => null;
+
+        [MyAttribute (2042, StringValue = "qwerty", TypeValue = typeof(byte[]))]
+        [MyAttribute (17, StringValue = "", TypeValue = typeof(object))]
+        public static string Method3 () => null;
+
+    }
+}

src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.CustomAttributeUpdate/System.Reflection.Metadata.ApplyUpdate.Test.CustomAttributeUpdate.csproj

@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <RootNamespace>System.Runtime.Loader.Tests</RootNamespace>
+    <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
+    <TestRuntime>true</TestRuntime>
+    <DeltaScript>deltascript.json</DeltaScript>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="CustomAttributeUpdate.cs" />
+  </ItemGroup>
+</Project>

src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.CustomAttributeUpdate/deltascript.json

@@ -0,0 +1,6 @@
+{
+    "changes": [
+        {"document": "CustomAttributeUpdate.cs", "update": "CustomAttributeUpdate_v1.cs"},
+    ]
+}
+

src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.MethodBody1/System.Reflection.Metadata.ApplyUpdate.Test.MethodBody1.csproj

@@ -5,7 +5,6 @@
     <TestRuntime>true</TestRuntime>
     <DeltaScript>deltascript.json</DeltaScript>
     <SkipTestUtilitiesReference>true</SkipTestUtilitiesReference>
-    <Nullable>disable</Nullable>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="MethodBody1.cs" />

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

@@ -83,6 +83,35 @@ void ClassWithCustomAttributes()
             });
         }
 
+        [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))]
+        public void CustomAttributeUpdates()
+        {
+            // Test that _modifying_ custom attribute constructor/property argumments works as expected.
+            // For this test, we don't change which constructor is called, or how many custom attributes there are.
+            ApplyUpdateUtil.TestCase(static () =>
+            {
+                var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributeUpdates).Assembly;
+
+                ApplyUpdateUtil.ApplyUpdate(assm);
+                ApplyUpdateUtil.ClearAllReflectionCaches();
+
+                // Just check the updated value on one method
+
+                Type attrType = typeof(System.Reflection.Metadata.ApplyUpdate.Test.MyAttribute);
+                Type ty = assm.GetType("System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributeUpdates");
+                Assert.NotNull(ty);
+                MethodInfo mi = ty.GetMethod(nameof(System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributeUpdates.Method1), BindingFlags.Public | BindingFlags.Static);
+                Assert.NotNull(mi);
+                var cattrs = Attribute.GetCustomAttributes(mi, attrType);
+                Assert.NotNull(cattrs);
+                Assert.Equal(1, cattrs.Length);
+                Assert.NotNull(cattrs[0]);
+                Assert.Equal(attrType, cattrs[0].GetType());
+                string p = (cattrs[0] as System.Reflection.Metadata.ApplyUpdate.Test.MyAttribute).StringValue;
+                Assert.Equal("rstuv", p);
+            });
+        }
+
         class NonRuntimeAssembly : Assembly
         {
         }

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

@@ -39,6 +39,7 @@
     <ProjectReference Include="LoaderLinkTest.Dynamic\LoaderLinkTest.Dynamic.csproj" />
     <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" />
 
   </ItemGroup>
   <ItemGroup Condition="'$(TargetOS)' == 'Browser'">
@@ -58,6 +59,9 @@
       <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)" />
     </ItemGroup>
   </Target>
 

src/mono/mono/component/hot_reload-stub.c

@@ -59,6 +59,9 @@ hot_reload_stub_table_bounds_check (MonoImage *base_image, int table_index, int
 static gboolean
 hot_reload_stub_delta_heap_lookup (MonoImage *base_image, MetadataHeapGetterFunc get_heap, uint32_t orig_index, MonoImage **image_out, uint32_t *index_out);
 
+static gboolean
+hot_reload_stub_has_modified_rows (const MonoTableInfo *table);
+
 static MonoComponentHotReload fn_table = {
 	{ MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available },
 	&hot_reload_stub_set_fastpath_data,
@@ -75,6 +78,7 @@ static MonoComponentHotReload fn_table = {
 	&hot_reload_stub_get_updated_method_rva,
 	&hot_reload_stub_table_bounds_check,
 	&hot_reload_stub_delta_heap_lookup,
+	&hot_reload_stub_has_modified_rows,
 };
 
 static bool
@@ -172,6 +176,11 @@ hot_reload_stub_delta_heap_lookup (MonoImage *base_image, MetadataHeapGetterFunc
 	g_assert_not_reached ();
 }
 
+static gboolean
+hot_reload_stub_has_modified_rows (const MonoTableInfo *table){
+	return FALSE;
+}
+
 MONO_COMPONENT_EXPORT_ENTRYPOINT
 MonoComponentHotReload *
 mono_component_hot_reload_init (void)