[hot_reload] ignore modified MONO_TABLE_TYPEDEF rows in update (#90166)

* Add test that deletes a custom attribute from a class

* just ignore modified MONO_TABLE_TYPEDEF rows in updates

   We may want to validate that Parent, Interfaces and Attributes columns haven't changed, but it's tricky and might be overly restrictive

Author: lambdageek

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

@@ -5,7 +5,7 @@
 
 namespace System.Reflection.Metadata.ApplyUpdate.Test
 {
-    [AttributeUsage (AttributeTargets.Method, AllowMultiple=true)]
+    [AttributeUsage (AttributeTargets.Method | AttributeTargets.Class, AllowMultiple=true)]
     public class MyDeleteAttribute : Attribute
     {
         public MyDeleteAttribute (string stringValue) { StringValue = stringValue; }
@@ -19,6 +19,7 @@ public class MyDeleteAttribute : Attribute
         public int IntValue {get; set; }
     }
 
+    [MyDeleteAttribute ("xyz")]
     public class ClassWithCustomAttributeDelete
     {
         [MyDeleteAttribute ("abcd")]

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

@@ -5,7 +5,7 @@
 
 namespace System.Reflection.Metadata.ApplyUpdate.Test
 {
-    [AttributeUsage (AttributeTargets.Method, AllowMultiple=true)]
+    [AttributeUsage (AttributeTargets.Method | AttributeTargets.Class, AllowMultiple=true)]
     public class MyDeleteAttribute : Attribute
     {
         public MyDeleteAttribute (string stringValue) { StringValue = stringValue; }

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

@@ -198,18 +198,33 @@ public void CustomAttributeDelete()
             {
                 var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributeDelete).Assembly;
 
+                Type attrType = typeof(System.Reflection.Metadata.ApplyUpdate.Test.MyDeleteAttribute);
+
+                Type preUpdateTy = assm.GetType("System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributeDelete");
+                Assert.NotNull(preUpdateTy);
+                
+                // before the update the type has a MyDeleteAttribute on it
+                Attribute[] cattrs = Attribute.GetCustomAttributes(preUpdateTy, attrType);
+                Assert.NotNull(cattrs);
+                Assert.Equal(1, cattrs.Length);
+
                 ApplyUpdateUtil.ApplyUpdate(assm);
                 ApplyUpdateUtil.ClearAllReflectionCaches();
 
-                // Just check the updated value on one method
 
-                Type attrType = typeof(System.Reflection.Metadata.ApplyUpdate.Test.MyDeleteAttribute);
                 Type ty = assm.GetType("System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributeDelete");
                 Assert.NotNull(ty);
 
+                // After the update, the type has no MyDeleteAttribute on it anymore
+                cattrs = Attribute.GetCustomAttributes(ty, attrType);
+                Assert.NotNull(cattrs);
+                Assert.Equal(0, cattrs.Length);
+
+                // Just check the updated value on one method
+
                 MethodInfo mi1 = ty.GetMethod(nameof(System.Reflection.Metadata.ApplyUpdate.Test.ClassWithCustomAttributeDelete.Method1), BindingFlags.Public | BindingFlags.Static);
                 Assert.NotNull(mi1);
-                Attribute[] cattrs = Attribute.GetCustomAttributes(mi1, attrType);
+                cattrs = Attribute.GetCustomAttributes(mi1, attrType);
                 Assert.NotNull(cattrs);
                 Assert.Equal(0, cattrs.Length);
 

src/mono/mono/component/hot_reload.c

@@ -1668,7 +1668,13 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de
 				i++; /* skip the next record */
 				continue;
 			}
-			/* fallthru */
+			// don't expect to see any other func codes with this table
+			g_assert (func_code == ENC_FUNC_DEFAULT);
+			// If it's an addition, it's an added type definition, continue.
+
+			// If it's a modification, Roslyn sometimes sends this when a custom
+			// attribute is deleted from a type definition.
+			break;
 		}
 		default:
 			if (!is_addition) {
@@ -2279,16 +2285,42 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base
 					 * especially since from the next generation's point of view
 					 * that's what adding a field/method will be. */
 					break;
-				case ENC_FUNC_ADD_EVENT:
-					g_assert_not_reached (); /* FIXME: implement me */
 				default:
 					g_assert_not_reached (); /* unknown func_code */
 				}
 				break;
+			} else {
+				switch (func_code) {
+				case ENC_FUNC_DEFAULT:
+					// Roslyn sends this sometimes when it deletes a custom
+					// attribute.  In this case no rows of the table def have
+					// should have changed from the previous generation.
+
+					// Note: we may want to check that Parent and Interfaces
+					// haven't changed.  But note that's tricky to do: we can't
+					// just compare tokens: the parent could be a TypeRef token,
+					// and roslyn does send a new typeref row (that ends up
+					// referencing the same type definition).  But we also don't
+					// want to convert to a `MonoClass*` too eagerly - if the
+					// class hasn't been used yet we don't want to kick off
+					// class initialization (which could mention the current
+					// class thanks to generic arguments - see class-init.c)
+					// Same with Interfaces.  Fields and Methods in an EnC
+					// updated row are zero.  So that only really leaves
+					// Attributes as the only column we can compare, which
+					// wouldn't tell us much (and perhaps in the future Roslyn
+					// could allow changing visibility, so we wouldn't want to
+					// compare for equality, anyway) So... we're done.
+					break;
+				case ENC_FUNC_ADD_METHOD:
+				case ENC_FUNC_ADD_FIELD:
+					/* modifying an existing class by adding a method or field, etc. */
+					break;
+				default:
+					/* not expecting anything else */
+					g_assert_not_reached ();
+				}
 			}
-			/* modifying an existing class by adding a method or field, etc. */
-			g_assert (!is_addition);
-			g_assert (func_code != ENC_FUNC_DEFAULT);
 			break;
 		}
 		case MONO_TABLE_NESTEDCLASS: {