Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/7.0-staging] [hot_reload] ignore modified MONO_TABLE_TYPEDEF rows in update #90172

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -19,6 +19,7 @@ public class MyDeleteAttribute : Attribute
public int IntValue {get; set; }
}

[MyDeleteAttribute ("xyz")]
public class ClassWithCustomAttributeDelete
{
[MyDeleteAttribute ("abcd")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
21 changes: 18 additions & 3 deletions src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -200,18 +200,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);

Expand Down
46 changes: 40 additions & 6 deletions src/mono/mono/component/hot_reload.c
Original file line number Diff line number Diff line change
Expand Up @@ -1624,7 +1624,15 @@ 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 (!is_addition) {
fanyang-mono marked this conversation as resolved.
Show resolved Hide resolved
// Roslyn sometimes sends this when a custom attribute is deleted from
// a type definition.
break;
}
/* otherwise it's an added type definition, continue */
break;
}
default:
if (!is_addition) {
Expand Down Expand Up @@ -2244,16 +2252,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: {
Expand Down