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

[hot_reload] Fix unresolved token when new nested structs are used #76618

Merged
merged 2 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -25,6 +25,8 @@ public class PreviousNestedClass {
public event EventHandler<string> E;
public void R() { E(this,"123"); }
}

public static void ExistingMethod () {}
}

[AttributeUsage(AttributeTargets.All, AllowMultiple=true, Inherited=false)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ public class NewNestedClass {};
public static DateTime NewStaticField;

public static double NewProp { get; set; }

public static void ExistingMethod ()
{
// modified
NewStaticField2 = new AnotherAddedClass();
}

public static AnotherAddedClass NewStaticField2;
}

[AttributeUsage(AttributeTargets.All, AllowMultiple=true, Inherited=false)]
Expand Down Expand Up @@ -87,3 +95,22 @@ public interface INewInterface : IExistingInterface {
public enum NewEnum {
Red, Yellow, Green
}

public class AnotherAddedClass
{
public struct NewNestedStruct
{
public double D;
public object O;
}

public NewNestedStruct S;

public AnotherAddedClass()
{
S = new NewNestedStruct {
D = 1234.0,
O = "1234",
};
}
}
2 changes: 2 additions & 0 deletions src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,8 @@ public static void TestReflectionAddNewType()
var i = (System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.IExistingInterface)o;

Assert.Equal("123", i.ItfMethod(123));

System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.ZExistingClass.ExistingMethod ();
});
}
}
Expand Down
22 changes: 13 additions & 9 deletions src/mono/mono/component/hot_reload.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ static const char *
hot_reload_get_capabilities (void);

static MonoClassMetadataUpdateField *
metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, uint32_t field_flags, MonoError *error);
metadata_update_field_setup_basic_info (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, uint32_t field_flags);

static MonoComponentHotReload fn_table = {
{ MONO_COMPONENT_ITF_VERSION, &hot_reload_available },
Expand Down Expand Up @@ -2189,11 +2189,11 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base

add_field_to_baseline (base_info, delta_info, add_member_klass, log_token);

/* This actually does more than mono_class_setup_basic_field_info and
* resolves MonoClassField:type and sets MonoClassField:offset to -1 to make
* it easier to spot that the field is special.
/* This actually does slightly more than
* mono_class_setup_basic_field_info and sets MonoClassField:offset
* to -1 to make it easier to spot that the field is special.
*/
metadata_update_field_setup_basic_info_and_resolve (image_base, base_info, generation, delta_info, add_member_klass, log_token, field_flags, error);
metadata_update_field_setup_basic_info (image_base, base_info, generation, delta_info, add_member_klass, log_token, field_flags);
if (!is_ok (error)) {
mono_trace (G_LOG_LEVEL_WARNING, MONO_TRACE_METADATA_UPDATE, "Could not setup field (token 0x%08x) due to: %s", log_token, mono_error_get_message (error));
return FALSE;
Expand Down Expand Up @@ -2884,7 +2884,7 @@ hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token) {


static MonoClassMetadataUpdateField *
metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, uint32_t field_flags, MonoError *error)
metadata_update_field_setup_basic_info (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, uint32_t field_flags)
{
if (!m_class_is_inited (parent_klass))
mono_class_init_internal (parent_klass);
Expand All @@ -2903,9 +2903,13 @@ metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, Basel
uint32_t name_idx = mono_metadata_decode_table_row_col (image_base, MONO_TABLE_FIELD, mono_metadata_token_index (fielddef_token) - 1, MONO_FIELD_NAME);
field->field.name = mono_metadata_string_heap (image_base, name_idx);

mono_field_resolve_type (&field->field, error);
if (!is_ok (error))
return NULL;
/* It's important not to try and resolve field->type at this point. If the field's type is a
* newly-added struct, we don't want to resolve it early here if we're going to add fields
* and methods to it. It seems that for nested structs, the field additions come after the
* field addition to the enclosing struct. So if the enclosing struct has a field of the
* nested type, resolving the field type here will make it look like the nested struct has
* no fields.
*/

parent_info->added_fields = g_slist_prepend_mem_manager (m_class_get_mem_manager (parent_klass), parent_info->added_fields, field);

Expand Down
10 changes: 8 additions & 2 deletions src/mono/mono/metadata/class.c
Original file line number Diff line number Diff line change
Expand Up @@ -6604,8 +6604,14 @@ mono_field_resolve_type (MonoClassField *field, MonoError *error)
static guint32
mono_field_resolve_flags (MonoClassField *field)
{
/* Fields in metadata updates are pre-resolved, so this method should not be called. */
g_assert (!m_field_is_from_update (field));
if (G_UNLIKELY (m_field_is_from_update (field))) {
/* metadata-update: Just resolve the whole field, for simplicity. */
ERROR_DECL (error);
mono_field_resolve_type (field, error);
mono_error_assert_ok (error);
g_assert (field->type);
return field->type->attrs;
}

MonoClass *klass = m_field_get_parent (field);
MonoImage *image = m_class_get_image (klass);
Expand Down