[hot_reload] Add instance fields (#76462)

* [hot_reload] Add new AddInstanceField test

* Add AddInstanceFieldToExistingType capability to runtime

* Add instance field: reflection and nested struct

* make TypedReference work with added fields

* Add FieldInfo.SetValue testcase

* make FieldInfo.SetValue work

* Implement LDFLDA opcode for added fields; add test

* Disable test on CoreCLR

*adding a wasm debugger test

* make the debugger test a bit more interesting

* implement a debugger test that gets/sets instance fields

* Clear the debugger type cache on EnC update

   we only need to clear the type on which fields or methods were added. but right now we just clear the whole cache

* [sdb] implement get/set field for added instance fields

* [handles] remove the field getter/setter macros

   They had no callers, and now we don't do things this way - we assume the enclosing object had been pinned by a stack reference.

* [metadata-update] comments on asserts

* implement mono_field_get_addr added field support

* remove last use of MONO_HANDLE_SET_FIELD_REF

* WeakAttribute is not picked up from added fields

* Add a test for an array with RVA

* Impl mono_metadata_field_info_full

* mono_class_get_field_token

* mono_field_get_rva

* Add SetValueDirect/GetValueDirect test (failing)

* Implement GetValueDirect - hot reload test passes now

* Added test for auto property; crashes in ves_icall_RuntimePropertyInfo_get_property_info

* add_props

   Note: we're not doing anything with the new PropertyMap row

   Note2: we get MethodSemantics rows for properties that got updated,
   and we're currently ignoring them.  Need to check that this is
   reasonable. (A test would be to use reflection to grab a getter or
   setter whose method body was changed and then try and invoke it and
   verify that we're calling the correct method.)

* fixup comment - repeated MethodSemantics rows can happen

* added properties iteration

* protect callers of mono_class_get_property_token

* make mono_class_get_property_token work for added props

* add test for adding instance event

* basic event reflection works

* Fire the new event, too

* make reflection MetadataToken work

* remove unused ifdefs and fix whitespace


Fixes #63643

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

@@ -0,0 +1,51 @@
+// 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 AddInstanceField
+    {
+        public AddInstanceField () {
+        }
+
+        public string GetStringField => _stringField;
+        public double GetDoubleField => _doubleField;
+
+        private string _stringField;
+        private double _doubleField;
+
+        private int[] _intArrayFieldWithInit = new[] { 2, 4, 6, 8, 10, 12 };
+
+        public void TestMethod () {
+            _stringField = "abcd";
+            _doubleField = 3.14159;
+        }
+
+        public int GetIntArrayLength() => _intArrayFieldWithInit?.Length ?? -1;
+        public int GetIntArrayElt(int i) => _intArrayFieldWithInit[i];
+
+        public void IncRefDouble (ref double d)
+        {
+            d += 1.0;
+        }
+
+        public string GetStringProp => string.Empty;
+
+        public event EventHandler<double> ExistingEvent;
+
+        public double Accumulator;
+
+        private void AccumHandler (object sender, double value) => Accumulator += value;
+
+        public double FireEvents() {
+            Accumulator = 0.0;
+            ExistingEvent += AccumHandler;
+            ExistingEvent(this, 123.0);
+            ExistingEvent -= AccumHandler;
+
+            return Accumulator;
+        }
+    }
+}

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

@@ -0,0 +1,89 @@
+// 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 AddInstanceField
+    {
+        public AddInstanceField () {
+            _doubleField2 = 5.5;
+            _stringField2 = "New Initial Value";
+            NewStructField = new NewStruct {
+                D = -1985.0,
+                O = new int[2] { 15, 17 },
+            };
+            // a little bit ldflda testing
+            IncRefDouble (ref NewStructField.D);
+            IncRefDouble (ref _doubleField2);
+
+            AddedStringAutoProp = "abcd";
+
+            AddedEvent += MyHandler;
+
+            void MyHandler (object sender, double data) {
+            }
+        }
+
+        public void IncRefDouble (ref double d)
+        {
+            d += 1.0;
+        }
+
+        public string GetStringField => _stringField2;
+        public double GetDoubleField => _doubleField2;
+
+        private string _stringField;
+        private string _stringField2;
+        private double _doubleField;
+        private double _doubleField2;
+
+        private int[] _intArrayFieldWithInit = new[] { 2, 4, 6, 8, 10, 12 };
+        private int[] _intArrayFieldWithInit2 = new[] { 1, 3, 5, 7, 9, 11 };
+
+        public void TestMethod () {
+            _stringField = "spqr";
+            _stringField2 = "4567";
+            _doubleField = 2.71828;
+            _doubleField2 = 0.707106;
+            AddedStringAutoProp = AddedStringAutoProp + "Test";
+        }
+
+        public int GetIntArrayLength() => _intArrayFieldWithInit2?.Length ?? -1;
+        public int GetIntArrayElt(int i) => _intArrayFieldWithInit2[i];
+
+        public struct NewStruct
+        {
+            public double D;
+            public object O;
+        }
+
+        public NewStruct NewStructField;
+
+        public string GetStringProp => AddedStringAutoProp;
+
+        public string AddedStringAutoProp { get; set; }
+
+        public event EventHandler<double> ExistingEvent;
+        public event EventHandler<double> AddedEvent;
+
+        public double Accumulator;
+
+        private void AccumHandler (object sender, double value) => Accumulator += value;
+
+        public double FireEvents() {
+            Accumulator = 0.0;
+            ExistingEvent += AccumHandler;
+            ExistingEvent(this, 123.0);
+            ExistingEvent -= AccumHandler;
+
+            AddedEvent += AccumHandler;
+            AddedEvent(this, 123.0);
+            AddedEvent -= AccumHandler;
+
+            return Accumulator;
+        }
+
+    }
+}

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

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

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

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

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

@@ -308,6 +308,113 @@ public static void TestAddStaticField()
             });
         }
 
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/76702", TestRuntimes.CoreCLR)]
+        [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
+        public static void TestAddInstanceField()
+        {
+            // Test that adding a new instance field to an existing class is supported
+            ApplyUpdateUtil.TestCase(static () =>
+            {
+                var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField).Assembly;
+
+                var x1 = new System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField();
+
+                x1.TestMethod();
+
+                Assert.Equal ("abcd", x1.GetStringField);
+                Assert.Equal (3.14159, x1.GetDoubleField);
+
+                ApplyUpdateUtil.ApplyUpdate(assm);
+
+                x1.TestMethod();
+
+                Assert.Equal ("4567", x1.GetStringField);
+                Assert.Equal (0.707106, x1.GetDoubleField);
+
+                Assert.Equal (-1, x1.GetIntArrayLength ()); // new field on existing object is initially null
+
+                var x2 = new System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField();
+
+                Assert.Equal ("New Initial Value", x2.GetStringField);
+                Assert.Equal (6.5, x2.GetDoubleField);
+
+                Assert.Equal (6, x2.GetIntArrayLength());
+                Assert.Equal (7, x2.GetIntArrayElt (3));
+
+                // now check that reflection can get/set the new fields
+                var fi = x2.GetType().GetField("NewStructField");
+
+                Assert.NotNull(fi);
+
+                var s = fi.GetValue (x2);
+
+                Assert.NotNull(x2);
+
+                var fid = fi.FieldType.GetField("D");
+                Assert.NotNull(fid);
+                Assert.Equal(-1984.0, fid.GetValue(s));
+                var tr = TypedReference.MakeTypedReference (x2, new FieldInfo[] {fi});
+                fid.SetValueDirect(tr, (object)34567.0);
+                Assert.Equal (34567.0, fid.GetValueDirect (tr));
+
+                fi = x2.GetType().GetField("_doubleField2", BindingFlags.NonPublic | BindingFlags.Instance);
+
+                Assert.NotNull(fi);
+
+                fi.SetValue(x2, 65535.01);
+                Assert.Equal(65535.01, x2.GetDoubleField);
+
+                tr = __makeref(x2);
+                fi.SetValueDirect (tr, 32768.2);
+                Assert.Equal (32768.2, x2.GetDoubleField);
+                Assert.Equal ((object)32768.2, fi.GetValueDirect (tr));
+
+                Assert.Equal("abcd", x2.GetStringProp);
+
+                var propInfo = x2.GetType().GetProperty("AddedStringAutoProp", BindingFlags.Public | BindingFlags.Instance);
+
+                Assert.NotNull(propInfo);
+                Assert.Equal("abcd", propInfo.GetMethod.Invoke (x2, new object[] {}));
+
+                x2.TestMethod();
+
+                Assert.Equal("abcdTest", x2.GetStringProp);
+
+                var addedPropToken = propInfo.MetadataToken;
+
+                Assert.True (addedPropToken > 0);
+
+                // we don't know exactly what token Roslyn will assign to the added property, but
+                // since the AddInstanceField.dll assembly is relatively small, assume that the
+                // total number of properties in the updated generation is less than 64 and the
+                // token is in that range.  If more code is added, revise this test.
+
+                Assert.True ((addedPropToken & 0x00ffffff) < 64);
+
+
+                var accumResult = x2.FireEvents();
+
+                Assert.Equal (246.0, accumResult);
+
+                var eventInfo = x2.GetType().GetEvent("AddedEvent", BindingFlags.Public | BindingFlags.Instance);
+
+                Assert.NotNull (eventInfo);
+
+                var addedEventToken = eventInfo.MetadataToken;
+
+                Assert.True (addedEventToken > 0);
+
+                // we don't know exactly what token Roslyn will assign to the added event, but
+                // since the AddInstanceField.dll assembly is relatively small, assume that the
+                // total number of events in the updated generation is less than 4 and the
+                // token is in that range.  If more code is added, revise this test.
+
+                Assert.True ((addedEventToken & 0x00ffffff) < 4);
+
+
+            });
+        }
+
         [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
         public static void TestAddNestedClass()
         {

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

@@ -58,6 +58,7 @@
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate\System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate.csproj" />
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis\System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis.csproj" />
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField\System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField.csproj" />
+    <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField\System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField.csproj" />
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass\System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass.csproj" />
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda\System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda.csproj" />
     <ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression\System.Reflection.Metadata.ApplyUpdate.Test.StaticLambdaRegression.csproj" />

src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml

@@ -639,5 +639,9 @@
 
 		<!-- FIXME: only if  feature="System.Reflection.Metadata.MetadataUpdater.IsSupported" featurevalue="true" -->
 		<type fullname="Mono.HotReload.FieldStore" preserve="fields" />
+                <type fullname="Mono.HotReload.InstanceFieldTable">
+                    <!-- hot_reload.c -->
+                    <method name="GetInstanceFieldFieldStore" />
+                </type>
 	</assembly>
 </linker>

src/mono/System.Private.CoreLib/src/Mono/HotReload.cs

@@ -3,15 +3,14 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
 using System.Reflection;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 
 namespace Mono.HotReload;
 
-// TODO: this is just a sketch, instance field additions aren't supported by Mono yet until  https://github.com/dotnet/runtime/issues/63643 is fixed
-#if false
-internal class InstanceFieldTable
+internal sealed class InstanceFieldTable
 {
     // Q: Does CoreCLR EnC allow adding fields to a valuetype?
     // A: No, see EEClass::AddField - if the type has layout or is a valuetype, you can't add fields to it.
@@ -34,9 +33,10 @@ internal class InstanceFieldTable
     // This should behave somewhat like EditAndContinueModule::ResolveOrAddField (and EnCAddedField::Allocate)
     //   we want to create some storage space that has the same lifetime as the instance object.
 
-    // // TODO: should the linker keep this if Hot Reload stuff is enabled?  Hot Reload is predicated on the linker not rewriting user modules, but maybe trimming SPC is ok?
-    internal static FieldStore GetInstanceFieldFieldStore(object inst, RuntimeTypeHandle type, uint fielddef_token)
-        => _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(type, fielddef_token);
+    internal static FieldStore GetInstanceFieldFieldStore(object inst, IntPtr type, uint fielddef_token)
+    {
+        return _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(new RuntimeTypeHandle (type), fielddef_token);
+    }
 
     private static InstanceFieldTable _singleton = new();
 
@@ -50,7 +50,7 @@ private InstanceFieldTable()
     private InstanceFields GetOrCreateInstanceFields(object key)
         => _table.GetOrCreateValue(key);
 
-    private class InstanceFields
+    private sealed class InstanceFields
     {
         private Dictionary<uint, FieldStore> _fields;
         private object _lock;
@@ -61,6 +61,8 @@ public InstanceFields()
             _lock = new();
         }
 
+        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
+            Justification = "Hot reload required untrimmed apps")]
         public FieldStore LookupOrAdd(RuntimeTypeHandle type, uint key)
         {
             if (_fields.TryGetValue(key, out FieldStore? v))
@@ -78,7 +80,6 @@ public FieldStore LookupOrAdd(RuntimeTypeHandle type, uint key)
     }
 
 }
-#endif
 
 // This is similar to System.Diagnostics.EditAndContinueHelper in CoreCLR, except instead of
 // having the allocation logic in native (see EditAndContinueModule::ResolveOrAllocateField,
@@ -96,8 +97,7 @@ private FieldStore (object? loc)
         _loc = loc;
     }
 
-    public object? Location => _loc;
-
+    [RequiresUnreferencedCode("Hot reload required untrimmed apps")]
     public static FieldStore Create (RuntimeTypeHandle type)
     {
         Type t = Type.GetTypeFromHandle(type) ?? throw new ArgumentException(nameof(type), "Type handle was null");