From 702faa1276b06dcf041adf5e37f12b15663d59b2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 9 Mar 2022 16:30:36 -0500 Subject: [PATCH 01/37] Checkpoint. Infrastructure for added types --- .../mono/component/hot_reload-internals.h | 25 +- src/mono/mono/component/hot_reload-stub.c | 11 + src/mono/mono/component/hot_reload.c | 247 ++++++++++++++---- src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/class-init.c | 10 + src/mono/mono/metadata/metadata-update.c | 6 + src/mono/mono/metadata/metadata-update.h | 3 + 7 files changed, 257 insertions(+), 46 deletions(-) diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index 36d3606e17e4a2..e1f31c84b7d137 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -17,7 +17,7 @@ typedef struct _MonoClassRuntimeMetadataUpdateInfo { gboolean inited; } MonoClassRuntimeMetadataUpdateInfo; -/* Class-specific metadata update info. See +/* Class-specific metadata update info for an existing class. See * mono_class_get_metadata_update_info() Note that this info is associated with * class _definitions_ that can be edited, so primitives, generic instances, * arrays, pointers, etc do not have this info. @@ -32,6 +32,29 @@ struct _MonoClassMetadataUpdateInfo { MonoClassRuntimeMetadataUpdateInfo runtime; }; +/* + * Added type skeleton. + * + * When a hot reload delta is adding brand new class, the runtime allows a lot more leeway than when + * new members are added to existing classes. Anything that is possible to write in a baseline + * assembly is possible with an added class. One complication is that the EnCLog first contains a + * row with a new TypeDef table token, but that table row has zeros for the field and method token + * ids. Instead, each method and field is added by an EnCLog entry with a ENC_FUNC_ADD_METHOD or + * ENC_FUNC_ADD_FIELD function code. We don't want to materialzie the MonoClass for the new type + * definition until we've see all the added methods and fields. Instead when we process the log we + * collect a skeleton for the new class and then use it to create the MonoClass. + * + * We assume that the new methods and fields for a given class form a contiguous run (ie first and + * count are sufficient to identify all the rows belonging to the new class). + */ +typedef struct _MonoAddedDefSkeleton { + uint32_t typedef_token; /* which type is it */ + uint32_t first_method_idx, first_field_idx; + uint32_t method_count; + uint32_t field_count; + /* TODO: metadata-update: property and event adds. */ +} MonoAddedDefSkeleton; + /* Keep in sync with Mono.HotReload.FieldStore in managed */ typedef struct _MonoHotReloadFieldStoreObject { diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 2377b948a7b0ad..99ba72233b6bea 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -86,6 +86,10 @@ hot_reload_stub_get_static_field_addr (MonoClassField *field); static MonoMethod * hot_reload_stub_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); +static gboolean +hot_reload_stub_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); + + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -111,6 +115,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_get_field, &hot_reload_stub_get_static_field_addr, &hot_reload_stub_find_method_by_name, + &hot_reload_stub_get_typedef_skeleton, }; static bool @@ -263,6 +268,12 @@ hot_reload_stub_find_method_by_name (MonoClass *klass, const char *name, int par return NULL; } +static gboolean +hot_reload_stub_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count) +{ + return FALSE; +} + MONO_COMPONENT_EXPORT_ENTRYPOINT MonoComponentHotReload * diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 584b98892fe9cd..61d8f5b9d1a834 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -121,6 +121,9 @@ hot_reload_get_static_field_addr (MonoClassField *field); static MonoMethod * hot_reload_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); +static gboolean +hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); + 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); @@ -149,6 +152,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_field, &hot_reload_get_static_field_addr, &hot_reload_find_method_by_name, + &hot_reload_get_typedef_skeleton, }; MonoComponentHotReload * @@ -240,6 +244,9 @@ struct _BaselineInfo { /* Parents for added methods, fields, etc */ GHashTable *member_parent; /* maps added methoddef or fielddef tokens to typedef tokens */ + + /* Skeletons for all newly-added types from every generation. Accessing the array requires the image lock. */ + GArray *skeletons; }; @@ -376,6 +383,10 @@ baseline_info_destroy (BaselineInfo *info) g_slist_foreach (info->klass_info, klass_info_destroy, NULL); g_slist_free (info->klass_info); } + + if (info->skeletons) + g_array_free (info->skeletons, TRUE); + g_free (info); } @@ -1662,10 +1673,136 @@ dump_assembly_ref_names (MonoImage *image) } } +static void +skeleton_add_member (MonoAddedDefSkeleton *sk, uint32_t member_token) +{ + uint32_t table = mono_metadata_token_table (member_token); + uint32_t idx = mono_metadata_token_index (member_token); + + switch (table) { + case MONO_TABLE_METHOD: + if (!sk->first_method_idx) { + sk->first_method_idx = idx; + sk->method_count = 1; + } else { + /* current token is contiguous with previously seen ones */ + g_assert (sk->first_method_idx + sk->method_count == idx); + sk->method_count++; + } + break; + case MONO_TABLE_FIELD: + if (!sk->first_field_idx) { + sk->first_field_idx = idx; + sk->field_count = 1; + } else { + /* current token is contiguous with previously seen ones */ + g_assert (sk->first_field_idx + sk->field_count == idx); + sk->field_count++; + } + break; + default: + g_error ("Expected method or field def token, got 0x%08x", member_token); + g_assert_not_reached(); + } +} + +typedef struct Pass2Context { + GArray *skeletons; /* Skeleton for each new added type definition */ +} Pass2Context; + +static void +pass2_context_init (Pass2Context *ctx) +{ + ctx->skeletons = g_array_new (FALSE, TRUE, sizeof(MonoAddedDefSkeleton)); +} + +static void +pass2_context_destroy (Pass2Context *ctx) +{ + if (ctx->skeletons) + g_array_free (ctx->skeletons, TRUE); +} + +static void +pass2_context_add_skeleton (Pass2Context *ctx, uint32_t typedef_token) +{ + MonoAddedDefSkeleton sk = {0, }; + g_assert (mono_metadata_token_table (typedef_token) == MONO_TABLE_TYPEDEF); + sk.typedef_token = typedef_token; + g_array_append_val (ctx->skeletons, sk); +} + +static MonoAddedDefSkeleton* +pass2_context_get_skeleton (Pass2Context *ctx, uint32_t typedef_token) +{ + for (int i = 0 ; i < ctx->skeletons->len; ++i) { + MonoAddedDefSkeleton *sk = &((MonoAddedDefSkeleton*)ctx->skeletons->data)[i]; + if (sk->typedef_token == typedef_token) + return sk; + } + return NULL; +} + +static void +pass2_context_add_skeleton_member (Pass2Context *ctx, uint32_t typedef_token, uint32_t member_token) +{ + MonoAddedDefSkeleton* sk = pass2_context_get_skeleton (ctx, typedef_token); + g_assert (sk); + skeleton_add_member (sk, member_token); +} + +static gboolean +pass2_context_is_skeleton (Pass2Context *ctx, uint32_t typedef_token) +{ + return pass2_context_get_skeleton (ctx, typedef_token) != NULL; +} + +/* + * After processing all the EnCLog entries, transfer the skeletons from the Pass2Context to the BaselineInfo + * and make them available for mono_class_create_from_typedef to consume. + */ +static gboolean +baseline_info_consume_skeletons (Pass2Context *ctx, MonoImage *base_image, BaselineInfo *base_info, MonoError *error) +{ + mono_image_lock (base_image); + if (!base_info->skeletons) + base_info->skeletons = g_array_new (FALSE, TRUE, sizeof(MonoAddedDefSkeleton)); + g_array_append_vals (base_info->skeletons, ctx->skeletons->data, ctx->skeletons->len); + mono_image_unlock (base_image); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "pass2: Added %d type definition skeletons. Total now %d.", ctx->skeletons->len, base_info->skeletons->len); + return TRUE; +} + +static gboolean +hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count) +{ + BaselineInfo *info = baseline_info_lookup (base_image); + if (!info || !info->skeletons) + return FALSE; + gboolean found = FALSE; + mono_image_lock (base_image); + for (int i = 0; i < info->skeletons->len; ++i) { + MonoAddedDefSkeleton *sk = &((MonoAddedDefSkeleton*)info->skeletons->data)[i]; + if (sk->typedef_token == typedef_token) { + found = TRUE; + g_assert (first_method_idx); + *first_method_idx = sk->first_method_idx; + g_assert (method_count); + *method_count = sk->method_count; + g_assert (first_field_idx); + *first_field_idx = sk->first_field_idx; + g_assert (field_count); + *field_count = sk->field_count; + break; + } + } + mono_image_unlock (base_image); + return found; +} /* do actuall enclog application */ static gboolean -apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, MonoImage *image_dmeta, DeltaInfo *delta_info, gconstpointer dil_data, uint32_t dil_length, MonoError *error) +apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, MonoImage *image_dmeta, DeltaInfo *delta_info, gconstpointer dil_data, uint32_t dil_length, MonoError *error) { MonoTableInfo *table_enclog = &image_dmeta->tables [MONO_TABLE_ENCLOG]; int rows = table_info_get_rows (table_enclog); @@ -1696,7 +1833,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Pass 2 begin: base '%s' delta image=%p", image_base->name, image_dmeta); #if defined(ALLOW_METHOD_ADD) || defined(ALLOW_FIELD_ADD) - MonoClass *add_member_klass = NULL; + uint32_t add_member_typedef = 0; #endif gboolean assemblyref_updated = FALSE; @@ -1723,12 +1860,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen #ifdef ALLOW_METHOD_ADD case ENC_FUNC_ADD_METHOD: { g_assert (token_table == MONO_TABLE_TYPEDEF); - MonoClass *klass = mono_class_get_checked (image_base, log_token, error); - if (!is_ok (error)) { - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Can't get class with token 0x%08x due to: %s", log_token, mono_error_get_message (error)); - return FALSE; - } - add_member_klass = klass; + add_member_typedef = log_token; break; } @@ -1740,12 +1872,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen #ifdef ALLOW_FIELD_ADD case ENC_FUNC_ADD_FIELD: { g_assert (token_table == MONO_TABLE_TYPEDEF); - MonoClass *klass = mono_class_get_checked (image_base, log_token, error); - if (!is_ok (error)) { - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Can't get class with token 0x%08x due to: %s", log_token, mono_error_get_message (error)); - return FALSE; - } - add_member_klass = klass; + add_member_typedef = log_token; break; } #endif @@ -1800,14 +1927,23 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen break; if (is_addition) { - if (!add_member_klass) - g_error ("EnC: new method added but I don't know the class, should be caught by pass1"); - g_assert (add_member_klass); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); - MonoDebugInformationEnc *method_debug_information = hot_reload_get_method_debug_information (delta_info->ppdb_file, token_index); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Debug info for method 0x%08x has ppdb idx 0x%08x", log_token, method_debug_information ? method_debug_information->idx : 0); - add_method_to_baseline (base_info, delta_info, add_member_klass, log_token, method_debug_information); - add_member_klass = NULL; + g_assertf (add_member_typedef, "EnC: new method added but I don't know the class, should be caught by pass1"); + if (pass2_context_is_skeleton (ctx, add_member_typedef)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to new class 0x%08x", log_token, add_member_typedef); + pass2_context_add_skeleton_member (ctx, add_member_typedef, log_token); + } else { + MonoClass *add_member_klass = mono_class_get_checked (image_base, add_member_typedef, error); + if (!is_ok (error)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Can't get class with token 0x%08x due to: %s", add_member_typedef, mono_error_get_message (error)); + return FALSE; + } + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); + MonoDebugInformationEnc *method_debug_information = hot_reload_get_method_debug_information (delta_info->ppdb_file, token_index); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Debug info for method 0x%08x has ppdb idx 0x%08x", log_token, method_debug_information ? method_debug_information->idx : 0); + add_method_to_baseline (base_info, delta_info, add_member_klass, log_token, method_debug_information); + } + add_member_typedef = 0; } #endif @@ -1830,41 +1966,52 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen g_print ("TODO: this case is still a bit contrived. token=0x%08x with rva=0x%04x\n", log_token, rva); } #if defined(ALLOW_METHOD_ADD) || defined(ALLOW_FIELD_ADD) - add_member_klass = NULL; + add_member_typedef = 0; #endif break; } case MONO_TABLE_FIELD: { #ifdef ALLOW_FIELD_ADD g_assert (is_addition); - g_assert (add_member_klass); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new field 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); + g_assert (add_member_typedef); + if (pass2_context_is_skeleton (ctx, add_member_typedef)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new field 0x%08x to new class 0x%08x", log_token, add_member_typedef); + pass2_context_add_skeleton_member (ctx, add_member_typedef, log_token); + } else { + MonoClass *add_member_klass = mono_class_get_checked (image_base, add_member_typedef, error); + if (!is_ok (error)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Can't get class with token 0x%08x due to: %s", add_member_typedef, mono_error_get_message (error)); + return FALSE; + } + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new field 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); - uint32_t mapped_token = hot_reload_relative_delta_index (image_dmeta, delta_info, log_token); - uint32_t field_flags = mono_metadata_decode_row_col (&image_dmeta->tables [MONO_TABLE_FIELD], mapped_token - 1, MONO_FIELD_FLAGS); + uint32_t mapped_token = hot_reload_relative_delta_index (image_dmeta, delta_info, log_token); + uint32_t field_flags = mono_metadata_decode_row_col (&image_dmeta->tables [MONO_TABLE_FIELD], mapped_token - 1, MONO_FIELD_FLAGS); #ifndef ALLOW_INSTANCE_FIELD_ADD - if ((field_flags & FIELD_ATTRIBUTE_STATIC) == 0) { - /* TODO: implement instance (and literal?) fields */ - mono_trace (G_LOG_LEVEL_WARNING, MONO_TRACE_METADATA_UPDATE, "Adding non-static fields isn't implemented yet (token 0x%08x, class %s.%s)", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); - mono_error_set_not_implemented (error, "Adding non-static fields isn't implemented yet (token 0x%08x, class %s.%s)", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); - return FALSE; - } + if ((field_flags & FIELD_ATTRIBUTE_STATIC) == 0) { + /* TODO: implement instance (and literal?) fields */ + mono_trace (G_LOG_LEVEL_WARNING, MONO_TRACE_METADATA_UPDATE, "Adding non-static fields isn't implemented yet (token 0x%08x, class %s.%s)", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); + mono_error_set_not_implemented (error, "Adding non-static fields isn't implemented yet (token 0x%08x, class %s.%s)", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); + return FALSE; + } #endif - add_field_to_baseline (base_info, delta_info, add_member_klass, log_token); + 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. - */ - metadata_update_field_setup_basic_info_and_resolve (image_base, base_info, generation, delta_info, add_member_klass, log_token, field_flags, error); - 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; + /* 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. + */ + metadata_update_field_setup_basic_info_and_resolve (image_base, base_info, generation, delta_info, add_member_klass, log_token, field_flags, error); + 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; + } } - - add_member_klass = NULL; + + add_member_typedef = 0; #else g_assert_not_reached (); #endif @@ -1878,6 +2025,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen case ENC_FUNC_DEFAULT: /* ok, added a new class */ /* TODO: do things here */ + pass2_context_add_skeleton (ctx, log_token); break; case ENC_FUNC_ADD_METHOD: case ENC_FUNC_ADD_FIELD: @@ -1947,6 +2095,11 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen } } } + + + if (!baseline_info_consume_skeletons (ctx, image_base, base_info, error)) + return FALSE; + return TRUE; } @@ -2093,12 +2246,16 @@ hot_reload_apply_changes (int origin, MonoImage *image_base, gconstpointer dmeta if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE)) dump_update_summary (image_base, image_dmeta); - if (!apply_enclog_pass2 (image_base, base_info, generation, image_dmeta, delta_info, dil_bytes, dil_length, error)) { + Pass2Context pass2ctx = {0,}; + pass2_context_init (&pass2ctx); + if (!apply_enclog_pass2 (&pass2ctx, image_base, base_info, generation, image_dmeta, delta_info, dil_bytes, dil_length, error)) { + pass2_context_destroy (&pass2ctx); mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "Error applying delta image to base=%s, due to: %s", basename, mono_error_get_message (error)); hot_reload_update_cancel (generation); return; } mono_error_assert_ok (error); + pass2_context_destroy (&pass2ctx); MonoAssemblyLoadContext *alc = mono_image_get_alc (image_base); hot_reload_update_publish (alc, generation); diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 790234e5577183..c3aa33cbd01a6f 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -40,6 +40,7 @@ typedef struct _MonoComponentHotReload { MonoClassField* (*get_field) (MonoClass *klass, uint32_t fielddef_token); gpointer (*get_static_field_addr) (MonoClassField *field); MonoMethod* (*find_method_by_name) (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); + gboolean (*get_typedef_skeleton) (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 12e4d336f7e33c..e3548d00250133 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -652,6 +653,15 @@ mono_class_create_from_typedef (MonoImage *image, guint32 type_token, MonoError mono_class_set_field_count (klass, field_last - first_field_idx); if (cols [MONO_TYPEDEF_METHOD_LIST] <= table_info_get_rows (&image->tables [MONO_TABLE_METHOD])) mono_class_set_method_count (klass, method_last - first_method_idx); + } else if (G_UNLIKELY (cols [MONO_TYPEDEF_FIELD_LIST] == 0 && cols [MONO_TYPEDEF_METHOD_LIST] == 0 && image->has_updates)) { + uint32_t first_field_idx, first_method_idx, field_count, method_count; + if (mono_metadata_update_get_typedef_skeleton (image, type_token, &first_method_idx, &method_count, &first_field_idx, &field_count)) { + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "Creating class '%s.%s' from skeleton (first_method_idx = 0x%08x, count = 0x%08x, first_field_idx = 0x%08x, count=0x%08x)", nspace, name, first_method_idx, method_count, first_field_idx, field_count); + mono_class_set_first_field_idx (klass, first_field_idx); + mono_class_set_first_method_idx (klass, first_method_idx); + mono_class_set_field_count (klass, field_count); + mono_class_set_method_count (klass, method_count); + } } /* reserve space to store vector pointer in arrays */ diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index ac31818c5e4992..6e743b10a1baa4 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -182,3 +182,9 @@ mono_metadata_update_find_method_by_name (MonoClass *klass, const char *name, in { return mono_component_hot_reload()->find_method_by_name (klass, name, param_count, flags, error); } + +gboolean +mono_metadata_update_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count) +{ + return mono_component_hot_reload()->get_typedef_skeleton (base_image, typedef_token, first_method_idx, method_count, first_field_idx, field_count); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index ee999a4f0a10f7..367030126dff2c 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -70,6 +70,9 @@ mono_metadata_update_get_field_idx (MonoClassField *field); MonoClassField * mono_metadata_update_get_field (MonoClass *klass, uint32_t fielddef_token); +gboolean +mono_metadata_update_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); + gpointer mono_metadata_update_get_static_field_addr (MonoClassField *field); From 6dc3e90019b8bab95d545e4db4ec24ef0c63349c Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 9 Mar 2022 19:41:43 -0500 Subject: [PATCH 02/37] extra tracing --- src/mono/mono/component/hot_reload.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 61d8f5b9d1a834..5396947077c06e 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1777,12 +1777,14 @@ static gboolean hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count) { BaselineInfo *info = baseline_info_lookup (base_image); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "skeleton lookup begin - wanted 0x%08x", typedef_token); if (!info || !info->skeletons) return FALSE; gboolean found = FALSE; mono_image_lock (base_image); for (int i = 0; i < info->skeletons->len; ++i) { MonoAddedDefSkeleton *sk = &((MonoAddedDefSkeleton*)info->skeletons->data)[i]; + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "skeleton lookup[%d] - wanted 0x%08x got 0x%08x", i, typedef_token, sk->typedef_token); if (sk->typedef_token == typedef_token) { found = TRUE; g_assert (first_method_idx); From 6c08cfc915dea5666ce3bb7572e4c8e1a7c36fa7 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 9 Mar 2022 20:52:38 -0500 Subject: [PATCH 03/37] register member->parent lookups for newly-added classes and members --- src/mono/mono/component/hot_reload.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 5396947077c06e..c6a0abe65ad48a 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -585,13 +585,20 @@ image_open_dmeta_from_data (MonoImage *base_image, uint32_t generation, gconstpo static DeltaInfo* image_append_delta (MonoImage *base, BaselineInfo *base_info, MonoImage *delta, DeltaInfo *delta_info); + +/* Add member->parent reverse lookup for newly-added classes */ +static void +add_member_parent (BaselineInfo *base_info, uint32_t typedef_token, uint32_t member_token); + /* common method, don't use directly, use add_method_to_baseline, add_field_to_baseline, etc */ static void add_member_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t member_token); +/* Add method->parent reverse lookup for existing classes */ static void add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t method_token, MonoDebugInformationEnc* pdb_address); +/* Add field->parent reverse lookup for existing classes */ static void add_field_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t field_token); @@ -1933,6 +1940,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base if (pass2_context_is_skeleton (ctx, add_member_typedef)) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to new class 0x%08x", log_token, add_member_typedef); pass2_context_add_skeleton_member (ctx, add_member_typedef, log_token); + add_member_parent (base_info, add_member_typedef, log_token); } else { MonoClass *add_member_klass = mono_class_get_checked (image_base, add_member_typedef, error); if (!is_ok (error)) { @@ -1979,6 +1987,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base if (pass2_context_is_skeleton (ctx, add_member_typedef)) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new field 0x%08x to new class 0x%08x", log_token, add_member_typedef); pass2_context_add_skeleton_member (ctx, add_member_typedef, log_token); + add_member_parent (base_info, add_member_typedef, log_token); } else { MonoClass *add_member_klass = mono_class_get_checked (image_base, add_member_typedef, error); if (!is_ok (error)) { @@ -2442,21 +2451,29 @@ hot_reload_table_num_rows_slow (MonoImage *base, int table_index) return rows; } +static void +add_member_parent (BaselineInfo *base_info, uint32_t typedef_token, uint32_t member_token) +{ + if (!base_info->member_parent) { + base_info->member_parent = g_hash_table_new (g_direct_hash, g_direct_equal); + } + g_hash_table_insert (base_info->member_parent, GUINT_TO_POINTER (member_token), GUINT_TO_POINTER (typedef_token)); +} + + static void add_member_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t member_token) { /* Check they really passed a table token, not just a table row index */ g_assert (mono_metadata_token_table (member_token) != 0); - if (!base_info->member_parent) { - base_info->member_parent = g_hash_table_new (g_direct_hash, g_direct_equal); - } MonoClassMetadataUpdateInfo *klass_info = mono_class_get_or_add_metadata_update_info (klass); GSList *members = klass_info->added_members; klass_info->added_members = g_slist_prepend_mem_manager (m_class_get_mem_manager (klass), members, GUINT_TO_POINTER (member_token)); - g_hash_table_insert (base_info->member_parent, GUINT_TO_POINTER (member_token), GUINT_TO_POINTER (m_class_get_type_token (klass))); + add_member_parent (base_info, m_class_get_type_token (klass), member_token); } + static void add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t method_token, MonoDebugInformationEnc* pdb_address) { From e2c5f43489e8b895ce796d982017e5af8e7dd919 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 9 Mar 2022 20:53:11 -0500 Subject: [PATCH 04/37] fix off by one error creating class from skeleton --- src/mono/mono/metadata/class-init.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index e3548d00250133..1bb2f0211b7087 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -657,8 +657,8 @@ mono_class_create_from_typedef (MonoImage *image, guint32 type_token, MonoError uint32_t first_field_idx, first_method_idx, field_count, method_count; if (mono_metadata_update_get_typedef_skeleton (image, type_token, &first_method_idx, &method_count, &first_field_idx, &field_count)) { mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "Creating class '%s.%s' from skeleton (first_method_idx = 0x%08x, count = 0x%08x, first_field_idx = 0x%08x, count=0x%08x)", nspace, name, first_method_idx, method_count, first_field_idx, field_count); - mono_class_set_first_field_idx (klass, first_field_idx); - mono_class_set_first_method_idx (klass, first_method_idx); + mono_class_set_first_field_idx (klass, first_field_idx - 1); + mono_class_set_first_method_idx (klass, first_method_idx - 1); mono_class_set_field_count (klass, field_count); mono_class_set_method_count (klass, method_count); } From 5c870a0b266ee15fd44aa6973e3403a0110a5c4b Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 9 Mar 2022 20:53:43 -0500 Subject: [PATCH 05/37] Revert "extra tracing" This reverts commit 51ebd8d77d3d8e8a0e04bfba641134448e3c1813. --- src/mono/mono/component/hot_reload.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index c6a0abe65ad48a..8c2feed81aa60e 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1784,14 +1784,12 @@ static gboolean hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count) { BaselineInfo *info = baseline_info_lookup (base_image); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "skeleton lookup begin - wanted 0x%08x", typedef_token); if (!info || !info->skeletons) return FALSE; gboolean found = FALSE; mono_image_lock (base_image); for (int i = 0; i < info->skeletons->len; ++i) { MonoAddedDefSkeleton *sk = &((MonoAddedDefSkeleton*)info->skeletons->data)[i]; - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "skeleton lookup[%d] - wanted 0x%08x got 0x%08x", i, typedef_token, sk->typedef_token); if (sk->typedef_token == typedef_token) { found = TRUE; g_assert (first_method_idx); From 8d80630a10ac48ed9eb4417acc31524963252b78 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 10 Mar 2022 11:37:09 -0500 Subject: [PATCH 06/37] AddNestedClass test works with mono now; also make it generic --- .../AddNestedClass_v1.cs | 10 ++++++---- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 1 - src/mono/mono/component/hot_reload.c | 2 +- src/mono/mono/metadata/metadata.c | 10 ++++++++-- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs index 04fc6992631d9a..de9ef9e57fee0c 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs @@ -13,16 +13,18 @@ public AddNestedClass() public string TestMethod() { - var n = new Nested(); + var n = new Nested(); n.f = "123"; + n.g = 456; return n.M(); } - private class Nested { + private class Nested { public Nested() { } - internal string f; + internal T f; + internal U g; public string M () { - return f + "456"; + return f.ToString() + g.ToString(); } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 527073494eebb2..f597057f02104c 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -306,7 +306,6 @@ public static void TestAddStaticField() }); } - [ActiveIssue("https://github.com/dotnet/runtime/issues/63643", TestRuntimes.Mono)] [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddNestedClass() { diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 8c2feed81aa60e..9461e0710c4541 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -2433,7 +2433,7 @@ hot_reload_table_num_rows_slow (MonoImage *base, int table_index) { BaselineInfo *base_info = baseline_info_lookup (base); if (!base_info) - return FALSE; + return 0; uint32_t current_gen = hot_reload_get_thread_generation (); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index ded8a8447f26f5..9efd1510c5b0a2 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -7104,7 +7104,7 @@ mono_metadata_get_generic_param_row (MonoImage *image, guint32 token, guint32 *o locator_t loc; g_assert (owner); - if (!tdef->base) + if (!tdef->base && !image->has_updates) return 0; if (mono_metadata_token_table (token) == MONO_TABLE_TYPEDEF) @@ -7123,9 +7123,15 @@ mono_metadata_get_generic_param_row (MonoImage *image, guint32 token, guint32 *o /* FIXME: metadata-update */ - if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) + gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) != NULL; + if (!found && !image->has_updates) return 0; + if (G_UNLIKELY (image->has_updates)) { + if (!found && !mono_metadata_update_metadata_linear_search (image, tdef, &loc, table_locator)) + return 0; + } + /* Find the first entry by searching backwards */ while ((loc.result > 0) && (mono_metadata_decode_row_col (tdef, loc.result - 1, MONO_GENERICPARAM_OWNER) == loc.idx)) loc.result --; From d9fd8563a07bbe38c3cd01f45744ce1b47169e1d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 10 Mar 2022 16:37:59 -0500 Subject: [PATCH 07/37] checkpoint: adding properties it "works" but only because we don't look very hard for the new properties anywhere. reflection is probably pretty broken. --- src/mono/mono/component/hot_reload.c | 112 ++++++++++++++++++++------- 1 file changed, 83 insertions(+), 29 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 9461e0710c4541..738fefbe7cdafe 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -35,7 +35,8 @@ #define ALLOW_CLASS_ADD #define ALLOW_METHOD_ADD #define ALLOW_FIELD_ADD -#undef ALLOW_INSTANCE_FIELD_ADD +#define ALLOW_PROPERTY_ADD +#define ALLOW_INSTANCE_FIELD_ADD typedef struct _BaselineInfo BaselineInfo; typedef struct _DeltaInfo DeltaInfo; @@ -1434,10 +1435,31 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de unsupported_edits = TRUE; break; #endif + case MONO_TABLE_PROPERTYMAP: { +#ifdef ALLOW_PROPERTY_ADD + if (func_code == ENC_FUNC_ADD_PROPERTY) { + g_assert (i + 1 < rows); + i++; /* skip to the next record */ + continue; + } +#endif + if (!is_addition) { + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing table cols.", i, log_token); + mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing table cols. token=0x%08x", log_token); + unsupported_edits = TRUE; + continue; + } + /* new rows, ok */ + break; + } case MONO_TABLE_PROPERTY: { /* modifying a property, ok */ if (!is_addition) break; +#ifdef ALLOW_PROPERTY_ADD + if (func_code == ENC_FUNC_DEFAULT) + continue; /* ok, allowed */ +#endif /* adding a property */ mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support adding new properties.", i, log_token); mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding new properties. token=0x%08x", log_token); @@ -1446,31 +1468,8 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de } case MONO_TABLE_METHODSEMANTICS: { if (is_addition) { - /* new rows are fine, as long as they point at existing methods */ - guint32 sema_cols [MONO_METHOD_SEMA_SIZE]; - int mapped_token = hot_reload_relative_delta_index (image_dmeta, delta_info, mono_metadata_make_token (token_table, token_index)); - g_assert (mapped_token != -1); - mono_metadata_decode_row (&image_dmeta->tables [MONO_TABLE_METHODSEMANTICS], mapped_token - 1, sema_cols, MONO_METHOD_SEMA_SIZE); - - switch (sema_cols [MONO_METHOD_SEMA_SEMANTICS]) { - case METHOD_SEMANTIC_GETTER: - case METHOD_SEMANTIC_SETTER: { - int prop_method_index = sema_cols [MONO_METHOD_SEMA_METHOD]; - /* ok, if it's pointing to an existing getter/setter */ - gboolean is_prop_method_add = prop_method_index-1 >= delta_info->count[MONO_TABLE_METHOD].prev_gen_rows; - if (!is_prop_method_add) - break; - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x adding new getter/setter method 0x%08x to a property is not supported", i, log_token, prop_method_index); - mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding a new getter or setter to a property, token=0x%08x", log_token); - unsupported_edits = TRUE; - continue; - } - default: - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x adding new non-getter/setter property or event methods is not supported.", i, log_token); - mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding new non-getter/setter property or event methods. token=0x%08x", log_token); - unsupported_edits = TRUE; - continue; - } + /* new rows are fine */ + break; } else { mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing table cols.", i, log_token); mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing table cols. token=0x%08x", log_token); @@ -1842,6 +1841,9 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base #if defined(ALLOW_METHOD_ADD) || defined(ALLOW_FIELD_ADD) uint32_t add_member_typedef = 0; #endif +#if defined(ALLOW_PROPERTY_ADD) + uint32_t add_property_propertymap = 0; +#endif gboolean assemblyref_updated = FALSE; for (int i = 0; i < rows ; ++i) { @@ -1883,6 +1885,13 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base break; } #endif + + case ENC_FUNC_ADD_PROPERTY: { + g_assert (token_table == MONO_TABLE_PROPERTYMAP); + add_property_propertymap = log_token; + break; + } + default: g_error ("EnC: unsupported FuncCode, should be caught by pass1"); break; @@ -2044,7 +2053,6 @@ 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_PROPERTY: case ENC_FUNC_ADD_EVENT: g_assert_not_reached (); /* FIXME: implement me */ default: @@ -2062,10 +2070,56 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base #endif break; } + case MONO_TABLE_PROPERTYMAP: { + switch (func_code) { + case ENC_FUNC_DEFAULT: + /* adding a new property map - parent could be new or existing class, but it didn't have a propertymap before */ + g_assert (is_addition); + break; + case ENC_FUNC_ADD_PROPERTY: + /* adding a new property to a propertymap. could be new or existing propertymap. */ + break; + default: + g_assert_not_reached (); /* unexpected func code */ + } + uint32_t cols[MONO_PROPERTY_MAP_SIZE]; + mono_metadata_decode_row (&image_base->tables [MONO_TABLE_PROPERTYMAP], token_index - 1, cols, MONO_PROPERTY_MAP_SIZE); + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "EnC: propertymap parent = 0x%08x, props = 0x%08x\n", cols[MONO_PROPERTY_MAP_PARENT], cols [MONO_PROPERTY_MAP_PROPERTY_LIST]); + break; + } case MONO_TABLE_PROPERTY: { /* allow updates to existing properties. */ - /* FIXME: use DeltaInfo:prev_gen_rows instead of image_base */ - g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table])); + if (!is_addition) + /* FIXME: use DeltaInfo:prev_gen_rows instead of image_base */ + g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table])); + else { + /* TODO: adding a property. */ + g_assert (add_property_propertymap != 0); + + uint32_t parent_type_token = mono_metadata_decode_row_col (&image_base->tables [MONO_TABLE_PROPERTYMAP], mono_metadata_token_index (add_property_propertymap) - 1, MONO_PROPERTY_MAP_PARENT); + parent_type_token = mono_metadata_make_token (MONO_TABLE_TYPEDEF, parent_type_token); + + g_assert (parent_type_token != 0); + + if (pass2_context_is_skeleton (ctx, parent_type_token)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new property 0x%08x to new class 0x%08x", log_token, parent_type_token); + /* nothing to do, actually. the metadata lookup machinery will find it by doing a linear scan of the table. */ + break; + } else { + MonoClass *add_property_klass = mono_class_get_checked (image_base, parent_type_token, error); + if (!is_ok (error)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Can't get class with token 0x%08x due to: %s", parent_type_token, mono_error_get_message (error)); + return FALSE; + } + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new property 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_property_klass), m_class_get_name (add_property_klass)); + + /* TODO: metadata-update: add a new MonoPropertyInfo to the bag on the class? */ + break; + } + + add_property_propertymap = 0; + } /* assuming that property attributes and type haven't changed. */ break; } From d133363a9879a979e918f83b28f6e01adb096158 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 11 Mar 2022 11:10:31 -0500 Subject: [PATCH 08/37] Add a property to AddNestedClass test --- .../AddNestedClass_v1.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs index de9ef9e57fee0c..88d7334be9798c 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs @@ -14,7 +14,7 @@ public AddNestedClass() public string TestMethod() { var n = new Nested(); - n.f = "123"; + n.Eff = "123"; n.g = 456; return n.M(); } @@ -23,8 +23,12 @@ private class Nested { public Nested() { } internal T f; internal U g; + public T Eff { + get => f; + set { f = value; } + } public string M () { - return f.ToString() + g.ToString(); + return Eff.ToString() + g.ToString(); } } } From 3a7f6f5811cbc6b45817dc6fa932d108d482bb01 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 11 Mar 2022 15:09:38 -0500 Subject: [PATCH 09/37] remove allow class/field/method/property ifdefs keep the instance field ifdef for now --- src/mono/mono/component/hot_reload.c | 88 +++------------------------- 1 file changed, 8 insertions(+), 80 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 738fefbe7cdafe..a7686946f2182c 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -32,10 +32,6 @@ #include -#define ALLOW_CLASS_ADD -#define ALLOW_METHOD_ADD -#define ALLOW_FIELD_ADD -#define ALLOW_PROPERTY_ADD #define ALLOW_INSTANCE_FIELD_ADD typedef struct _BaselineInfo BaselineInfo; @@ -1382,21 +1378,9 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de if (token_table != MONO_TABLE_METHOD) continue; -#ifndef ALLOW_METHOD_ADD - - if (is_addition) { - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "\tcannot add new method with token 0x%08x", log_token); - mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: cannot add new method with token 0x%08x", log_token); - unsupported_edits = TRUE; - } - -#endif - -#ifdef ALLOW_METHOD_ADD /* adding a new parameter to a new method is ok */ if (func_code == ENC_FUNC_ADD_PARAM && is_addition) continue; -#endif g_assert (func_code == 0); /* anything else doesn't make sense here */ } @@ -1418,31 +1402,20 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de /* okay, supported */ break; case MONO_TABLE_METHOD: -#ifdef ALLOW_METHOD_ADD if (func_code == ENC_FUNC_ADD_PARAM) continue; /* ok, allowed */ -#endif /* handled above */ break; case MONO_TABLE_FIELD: -#ifdef ALLOW_FIELD_ADD - if (func_code == ENC_FUNC_DEFAULT) - continue; /* ok, allowed */ -#else - /* adding or modifying a field */ - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support adding or modifying fields.", i, log_token); - mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding or modifying fields. token=0x%08x", log_token); - unsupported_edits = TRUE; + /* ok */ + g_assert (func_code == ENC_FUNC_DEFAULT); break; -#endif case MONO_TABLE_PROPERTYMAP: { -#ifdef ALLOW_PROPERTY_ADD if (func_code == ENC_FUNC_ADD_PROPERTY) { g_assert (i + 1 < rows); i++; /* skip to the next record */ continue; } -#endif if (!is_addition) { mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing table cols.", i, log_token); mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing table cols. token=0x%08x", log_token); @@ -1453,18 +1426,9 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de break; } case MONO_TABLE_PROPERTY: { - /* modifying a property, ok */ - if (!is_addition) - break; -#ifdef ALLOW_PROPERTY_ADD - if (func_code == ENC_FUNC_DEFAULT) - continue; /* ok, allowed */ -#endif - /* adding a property */ - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support adding new properties.", i, log_token); - mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding new properties. token=0x%08x", log_token); - unsupported_edits = TRUE; - continue; + /* ok */ + g_assert (func_code == ENC_FUNC_DEFAULT); + break; } case MONO_TABLE_METHODSEMANTICS: { if (is_addition) { @@ -1543,14 +1507,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de break; /* added a row. ok */ } case MONO_TABLE_TYPEDEF: { - gboolean new_class G_GNUC_UNUSED = is_addition; -#ifdef ALLOW_METHOD_ADD - /* only allow adding methods to existing classes for now */ - if ( -#ifndef ALLOW_CLASS_ADD - !new_class && -#endif - func_code == ENC_FUNC_ADD_METHOD) { + if (func_code == ENC_FUNC_ADD_METHOD) { /* next record should be a MONO_TABLE_METHOD addition (func == default) */ g_assert (i + 1 < rows); guint32 next_cols [MONO_ENCLOG_SIZE]; @@ -1565,13 +1522,8 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de i++; /* skip the next record */ continue; } -#endif -#ifdef ALLOW_FIELD_ADD - if ( -#ifndef ALLOW_CLASS_ADD - !new_class && -#endif - func_code == ENC_FUNC_ADD_FIELD) { + + if (func_code == ENC_FUNC_ADD_FIELD) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x AddField to klass 0x%08x, skipping next EnClog record", i, log_token, token_index); g_assert (i + 1 < rows); guint32 next_cols [MONO_ENCLOG_SIZE]; @@ -1586,7 +1538,6 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de i++; /* skip the next record */ continue; } -#endif /* fallthru */ } default: @@ -1838,12 +1789,8 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Pass 2 begin: base '%s' delta image=%p", image_base->name, image_dmeta); -#if defined(ALLOW_METHOD_ADD) || defined(ALLOW_FIELD_ADD) uint32_t add_member_typedef = 0; -#endif -#if defined(ALLOW_PROPERTY_ADD) uint32_t add_property_propertymap = 0; -#endif gboolean assemblyref_updated = FALSE; for (int i = 0; i < rows ; ++i) { @@ -1866,7 +1813,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base switch (func_code) { case ENC_FUNC_DEFAULT: /* default */ break; -#ifdef ALLOW_METHOD_ADD case ENC_FUNC_ADD_METHOD: { g_assert (token_table == MONO_TABLE_TYPEDEF); add_member_typedef = log_token; @@ -1877,14 +1823,11 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base g_assert (token_table == MONO_TABLE_METHOD); break; } -#endif -#ifdef ALLOW_FIELD_ADD case ENC_FUNC_ADD_FIELD: { g_assert (token_table == MONO_TABLE_TYPEDEF); add_member_typedef = log_token; break; } -#endif case ENC_FUNC_ADD_PROPERTY: { g_assert (token_table == MONO_TABLE_PROPERTYMAP); @@ -1937,7 +1880,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base break; } case MONO_TABLE_METHOD: { -#ifdef ALLOW_METHOD_ADD /* if adding a param, handle it with the next record */ if (func_code == ENC_FUNC_ADD_PARAM) break; @@ -1962,7 +1904,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base } add_member_typedef = 0; } -#endif if (!base_info->method_table_update) base_info->method_table_update = g_hash_table_new (g_direct_hash, g_direct_equal); @@ -1982,13 +1923,10 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base /* rva points probably into image_base IL stream. can this ever happen? */ g_print ("TODO: this case is still a bit contrived. token=0x%08x with rva=0x%04x\n", log_token, rva); } -#if defined(ALLOW_METHOD_ADD) || defined(ALLOW_FIELD_ADD) add_member_typedef = 0; -#endif break; } case MONO_TABLE_FIELD: { -#ifdef ALLOW_FIELD_ADD g_assert (is_addition); g_assert (add_member_typedef); if (pass2_context_is_skeleton (ctx, add_member_typedef)) { @@ -2030,13 +1968,9 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base } add_member_typedef = 0; -#else - g_assert_not_reached (); -#endif break; } case MONO_TABLE_TYPEDEF: { -#ifdef ALLOW_CLASS_ADD if (is_addition) { /* Adding a new class. ok */ switch (func_code) { @@ -2060,14 +1994,9 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base } break; } -#endif /* modifying an existing class by adding a method or field, etc. */ g_assert (!is_addition); -#if !defined(ALLOW_METHOD_ADD) && !defined(ALLOW_FIELD_ADD) - g_assert_not_reached (); -#else g_assert (func_code != ENC_FUNC_DEFAULT); -#endif break; } case MONO_TABLE_PROPERTYMAP: { @@ -2129,7 +2058,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base } case MONO_TABLE_PARAM: { /* ok, pass1 checked for disallowed modifications */ - /* ALLOW_METHOD_ADD: FIXME: here we would really like to update the method's paramlist column to point to the new params. */ /* if there were multiple added methods, this comes in as several method * additions, followed by the parameter additions. * From 9131764aa0c5c583d3c4455621187bce8ba63ed1 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 11 Mar 2022 15:44:46 -0500 Subject: [PATCH 10/37] add event to AddNestedClass --- .../AddNestedClass_v1.cs | 16 +++++++++++++++- .../tests/ApplyUpdateTest.cs | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs index 88d7334be9798c..7e57caa71e7a83 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs @@ -16,7 +16,9 @@ public string TestMethod() var n = new Nested(); n.Eff = "123"; n.g = 456; - return n.M(); + n.Evt += new Action (n.DefaultHandler); + n.RaiseEvt(); + return n.M() + n.buf; } private class Nested { @@ -30,6 +32,18 @@ public T Eff { public string M () { return Eff.ToString() + g.ToString(); } + + public event Action Evt; + + public void RaiseEvt () { + Evt ("789"); + } + + public string buf; + + public void DefaultHandler (string s) { + this.buf = s; + } } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index f597057f02104c..b7f36c2dde698d 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -324,7 +324,7 @@ public static void TestAddNestedClass() r = x.TestMethod(); - Assert.Equal("123456", r); + Assert.Equal("123456789", r); }); } From 37481f2eb4fb6f7ed94f640b257094dd52360a3a Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 11 Mar 2022 16:47:44 -0500 Subject: [PATCH 11/37] add DISALLOW_BROKEN_PARENT ifdef allow CustomAttribute row modifications to change Parent, while https://github.com/dotnet/roslyn/issues/60125 is being sorted out --- src/mono/mono/component/hot_reload.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index a7686946f2182c..7979c5489fc151 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1470,11 +1470,16 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de * (its parent is set to 0). Once we support custom attribute * changes, we will support this kind of deletion for real. */ + /* FIXME: https://github.com/dotnet/roslyn/issues/60125 + * Roslyn seems to emit CA rows out of order which puts rows with unexpected Parent values in the wrong place. + */ +#ifdef DISALLOW_BROKEN_PARENT if (ca_base_cols [MONO_CUSTOM_ATTR_PARENT] != ca_upd_cols [MONO_CUSTOM_ATTR_PARENT] && ca_upd_cols [MONO_CUSTOM_ATTR_PARENT] != 0) { mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing CA table cols with a different Parent. token=0x%08x", log_token); unsupported_edits = TRUE; continue; } +#endif break; } else { /* Added a row. ok */ From b5fe05378700c08a42b3b64d05c5c372a9e3d7d3 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 11 Mar 2022 16:48:46 -0500 Subject: [PATCH 12/37] checkpoint: adding events --- src/mono/mono/component/hot_reload.c | 77 +++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 7979c5489fc151..14e308583a098d 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1413,7 +1413,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de case MONO_TABLE_PROPERTYMAP: { if (func_code == ENC_FUNC_ADD_PROPERTY) { g_assert (i + 1 < rows); - i++; /* skip to the next record */ + i++; /* skip the next record */ continue; } if (!is_addition) { @@ -1430,6 +1430,21 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de g_assert (func_code == ENC_FUNC_DEFAULT); break; } + case MONO_TABLE_EVENTMAP: { + if (func_code == ENC_FUNC_ADD_EVENT) { + g_assert (i + 1 < rows); + i++; /* skip the next record */ + continue; + } + if (!is_addition) { + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing table cols.", i, log_token); + mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing table cols. token=0x%08x", log_token); + unsupported_edits = TRUE; + continue; + } + /* new rows, ok */ + break; + } case MONO_TABLE_METHODSEMANTICS: { if (is_addition) { /* new rows are fine */ @@ -1796,6 +1811,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base uint32_t add_member_typedef = 0; uint32_t add_property_propertymap = 0; + uint32_t add_event_eventmap = 0; gboolean assemblyref_updated = FALSE; for (int i = 0; i < rows ; ++i) { @@ -1840,6 +1856,12 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base break; } + case ENC_FUNC_ADD_EVENT: { + g_assert (token_table = MONO_TABLE_EVENTMAP); + add_event_eventmap = log_token; + break; + } + default: g_error ("EnC: unsupported FuncCode, should be caught by pass1"); break; @@ -2057,6 +2079,59 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base /* assuming that property attributes and type haven't changed. */ break; } + case MONO_TABLE_EVENTMAP: { + switch (func_code) { + case ENC_FUNC_DEFAULT: + /* adding a new eventmap - parent could be new or existing class, but it didn't have an eventmap before */ + g_assert (is_addition); + break; + case ENC_FUNC_ADD_EVENT: + /* adding a new event to an eventmap. could be new or existing eventmap. */ + break; + default: + g_assert_not_reached (); /* unexpected func code */ + } + uint32_t cols[MONO_EVENT_MAP_SIZE]; + mono_metadata_decode_row (&image_base->tables [MONO_TABLE_EVENTMAP], token_index - 1, cols, MONO_EVENT_MAP_SIZE); + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "EnC: eventmap parent = 0x%08x, evts = 0x%08x\n", cols[MONO_EVENT_MAP_PARENT], cols [MONO_EVENT_MAP_EVENTLIST]); + break; + + } + case MONO_TABLE_EVENT: { + if (!is_addition) + /* FIXME: use DeltaInfo:prev_gen_rows instead of image_base */ + g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table])); + else { + /* TODO: adding a event. */ + g_assert (add_event_eventmap != 0); + + uint32_t parent_type_token = mono_metadata_decode_row_col (&image_base->tables [MONO_TABLE_EVENTMAP], mono_metadata_token_index (add_event_eventmap) - 1, MONO_EVENT_MAP_PARENT); + parent_type_token = mono_metadata_make_token (MONO_TABLE_TYPEDEF, parent_type_token); + + g_assert (parent_type_token != 0); + + if (pass2_context_is_skeleton (ctx, parent_type_token)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new event 0x%08x to new class 0x%08x", log_token, parent_type_token); + /* nothing to do, actually. the metadata lookup machinery will find it by doing a linear scan of the table. */ + break; + } else { + MonoClass *add_event_klass = mono_class_get_checked (image_base, parent_type_token, error); + if (!is_ok (error)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Can't get class with token 0x%08x due to: %s", parent_type_token, mono_error_get_message (error)); + return FALSE; + } + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new event 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_event_klass), m_class_get_name (add_event_klass)); + + /* TODO: metadata-update: add a new MonoEventInfo to the bag on the class? */ + break; + } + + add_event_eventmap = 0; + } + /* assuming that event attributes and type haven't changed. */ + break; + } case MONO_TABLE_CUSTOMATTRIBUTE: { /* ok, pass1 checked for disallowed modifications */ break; From f0528b8aa6523698a6279382ba684ab888ad30f0 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Mar 2022 12:54:43 -0400 Subject: [PATCH 13/37] Add new test ReflectionAddNewType --- .../ReflectionAddNewType.cs | 14 ++++++++ .../ReflectionAddNewType_v1.cs | 32 +++++++++++++++++ ...plyUpdate.Test.ReflectionAddNewType.csproj | 11 ++++++ .../deltascript.json | 6 ++++ .../tests/ApplyUpdateTest.cs | 34 +++++++++++++++++++ .../tests/System.Runtime.Loader.Tests.csproj | 1 + 6 files changed, 98 insertions(+) create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.csproj create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/deltascript.json diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs new file mode 100644 index 00000000000000..cf90c35975dca8 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs @@ -0,0 +1,14 @@ +// 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.ReflectionAddNewType; + +public interface IExistingInterface { + public string ItfMethod(int i); +} + +public class ZExistingClass +{ +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs new file mode 100644 index 00000000000000..ab3d38b87102ad --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -0,0 +1,32 @@ +// 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.ReflectionAddNewType; + +public interface IExistingInterface { + public string ItfMethod(int i); +} + +public class ZExistingClass +{ + public class NewNestedClass {}; +} + +public class NewToplevelClass : IExistingInterface, ICloneable { + public string ItfMethod(int i) { + return i.ToString(); + } + + public object Clone() { + return new NewToplevelClass(); + } +} + +public struct NewToplevelStruct { +} + +public interface INewInterface : IExistingInterface { + public int AddedItfMethod (string s); +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.csproj new file mode 100644 index 00000000000000..9eb6b1380827ab --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.csproj @@ -0,0 +1,11 @@ + + + System.Runtime.Loader.Tests + $(NetCoreAppCurrent) + true + deltascript.json + + + + + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/deltascript.json new file mode 100644 index 00000000000000..212d612c5dea63 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/deltascript.json @@ -0,0 +1,6 @@ +{ + "changes": [ + {"document": "ReflectionAddNewType.cs", "update": "ReflectionAddNewType_v1.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index b7f36c2dde698d..c448491e8e6d91 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Reflection; using System.Runtime.CompilerServices; using Xunit; @@ -426,5 +427,38 @@ public static void TestStaticLambdaRegression() }); } + + internal static Type CheckReflectedType(Assembly assm, string nameSpace, string typeName, Action moreChecks = null, [CallerMemberName] string callerMemberName = "", [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0) + { + var ty = assm.GetType($"{nameSpace}.{typeName}"); + Assert.True(ty != null, $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetType for '{typeName}' to return non-null in {callerMemberName}"); + Assert.True(typeName == ty.Name, $"{callerFilePath}:{callerLineNumber}: returned type has unexpected name '{ty.Name}' (expected: '{typeName}') in {callerMemberName}"); + if (moreChecks != null) + moreChecks(ty); + return ty; + } + + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestReflectionAddNewType() + { + ApplyUpdateUtil.TestCase(static () => + { + const string ns = "System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType"; + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.ZExistingClass).Assembly; + + CheckReflectedType(assm, ns, "ZExistingClass"); + + ApplyUpdateUtil.ApplyUpdate(assm); + + CheckReflectedType(assm, ns, "ZExistingClass"); + CheckReflectedType(assm, ns, "IExistingInterface"); + + CheckReflectedType(assm, ns, "NewToplevelClass"); + CheckReflectedType(assm, ns, "NewToplevelStruct"); + CheckReflectedType(assm, ns, "INewInterface"); + + + }); + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj index 6c5468b82c1479..50941fe7b3ca73 100644 --- a/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj +++ b/src/libraries/System.Runtime.Loader/tests/System.Runtime.Loader.Tests.csproj @@ -61,6 +61,7 @@ + From 5829e72f4ec7cb89f2d8f594023c6f3346ea5c3c Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Mar 2022 13:27:53 -0400 Subject: [PATCH 14/37] Add new types to the image name cache --- src/mono/mono/component/hot_reload.c | 25 ++++++++++++++++++++++++- src/mono/mono/metadata/class.c | 2 +- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 14e308583a098d..d206c68541a751 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -11,6 +11,7 @@ #include "mono/component/hot_reload-internals.h" #include +#include #include "mono/metadata/assembly-internals.h" #include "mono/metadata/mono-hash-internals.h" #include "mono/metadata/metadata-internals.h" @@ -1777,6 +1778,26 @@ hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, return found; } +/** + * Adds the given newly-added class to the image name cache. Except if the + * class is nested (has one of the nested visibility flags), since those are + * looked up differently. + */ +static void +add_typedef_to_image_name_cache (MonoImage *image_base, uint32_t log_token) +{ + uint32_t cols[MONO_TYPEDEF_SIZE]; + uint32_t row = mono_metadata_token_index (log_token); + mono_metadata_decode_row (&image_base->tables[MONO_TABLE_TYPEDEF], row - 1, cols, MONO_TYPEDEF_SIZE); + uint32_t visib = cols [MONO_TYPEDEF_FLAGS] & TYPE_ATTRIBUTE_VISIBILITY_MASK; + + if (visib >= TYPE_ATTRIBUTE_NESTED_PUBLIC && visib <= TYPE_ATTRIBUTE_NESTED_FAM_OR_ASSEM) + return; + const char *name_space = mono_metadata_string_heap (image_base, cols[MONO_TYPEDEF_NAMESPACE]); + const char *name = mono_metadata_string_heap (image_base, cols[MONO_TYPEDEF_NAME]); + mono_image_add_to_name_cache (image_base, name_space, name, row); +} + /* do actuall enclog application */ static gboolean apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, MonoImage *image_dmeta, DeltaInfo *delta_info, gconstpointer dil_data, uint32_t dil_length, MonoError *error) @@ -2001,11 +2022,13 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base if (is_addition) { /* Adding a new class. ok */ switch (func_code) { - case ENC_FUNC_DEFAULT: + case ENC_FUNC_DEFAULT: { /* ok, added a new class */ /* TODO: do things here */ pass2_context_add_skeleton (ctx, log_token); + add_typedef_to_image_name_cache (image_base, log_token); break; + } case ENC_FUNC_ADD_METHOD: case ENC_FUNC_ADD_FIELD: /* ok, adding a new field or method to a new class */ diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 9d400cb90b0aa0..de7cde1362b914 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -3088,7 +3088,7 @@ mono_image_init_name_cache (MonoImage *image) mono_image_unlock (image); } -/*FIXME Only dynamic assemblies should allow this operation.*/ +/*FIXME Only dynamic assemblies or metadata-update should allow this operation.*/ /** * mono_image_add_to_name_cache: */ From b1e3c04c53c4dea2898c34e2ef2bf244af31bcbe Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Mar 2022 13:39:37 -0400 Subject: [PATCH 15/37] Assembly.GetTypes and additional tests --- .../tests/ApplyUpdateTest.cs | 31 ++++++++++++++----- src/mono/mono/metadata/icall.c | 3 +- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index c448491e8e6d91..0f2619630c0acf 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -428,11 +428,22 @@ public static void TestStaticLambdaRegression() }); } - internal static Type CheckReflectedType(Assembly assm, string nameSpace, string typeName, Action moreChecks = null, [CallerMemberName] string callerMemberName = "", [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0) + private static bool ContainsTypeWithName(Type[] types, string fullName) { - var ty = assm.GetType($"{nameSpace}.{typeName}"); + foreach (var ty in types) { + if (ty.FullName == fullName) + return true; + } + return false; + } + + internal static Type CheckReflectedType(Assembly assm, Type[] allTypes, string nameSpace, string typeName, Action moreChecks = null, [CallerMemberName] string callerMemberName = "", [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0) + { + var fullName = $"{nameSpace}.{typeName}"; + var ty = assm.GetType(fullName); Assert.True(ty != null, $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetType for '{typeName}' to return non-null in {callerMemberName}"); Assert.True(typeName == ty.Name, $"{callerFilePath}:{callerLineNumber}: returned type has unexpected name '{ty.Name}' (expected: '{typeName}') in {callerMemberName}"); + Assert.True(ContainsTypeWithName (allTypes, fullName), $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetTypes to contain '{fullName}', but it didn't in {callerMemberName}"); if (moreChecks != null) moreChecks(ty); return ty; @@ -446,16 +457,20 @@ public static void TestReflectionAddNewType() const string ns = "System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType"; var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.ZExistingClass).Assembly; - CheckReflectedType(assm, ns, "ZExistingClass"); + var allTypes = assm.GetTypes(); + + CheckReflectedType(assm, allTypes, ns, "ZExistingClass"); ApplyUpdateUtil.ApplyUpdate(assm); - CheckReflectedType(assm, ns, "ZExistingClass"); - CheckReflectedType(assm, ns, "IExistingInterface"); + allTypes = assm.GetTypes(); + + CheckReflectedType(assm, allTypes, ns, "ZExistingClass"); + CheckReflectedType(assm, allTypes, ns, "IExistingInterface"); - CheckReflectedType(assm, ns, "NewToplevelClass"); - CheckReflectedType(assm, ns, "NewToplevelStruct"); - CheckReflectedType(assm, ns, "INewInterface"); + CheckReflectedType(assm, allTypes, ns, "NewToplevelClass"); + CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); + CheckReflectedType(assm, allTypes, ns, "INewInterface"); }); diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index bdb83dad31a541..ccdcf5bca12d1c 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -5048,9 +5048,8 @@ image_get_type (MonoImage *image, MonoTableInfo *tdef, int table_idx, int count, static MonoArrayHandle mono_module_get_types (MonoImage *image, MonoArrayHandleOut exceptions, MonoBoolean exportedOnly, MonoError *error) { - /* FIXME: metadata-update */ MonoTableInfo *tdef = &image->tables [MONO_TABLE_TYPEDEF]; - int rows = table_info_get_rows (tdef); + int rows = mono_metadata_table_num_rows (image, MONO_TABLE_TYPEDEF); int i, count; /* we start the count from 1 because we skip the special type */ From 615d2a5b580e486fbdb37b352d95f65bd2999b41 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Mar 2022 15:40:33 -0400 Subject: [PATCH 16/37] Make nested types work --- .../ReflectionAddNewType.cs | 1 + .../ReflectionAddNewType_v1.cs | 15 ++- .../tests/ApplyUpdateTest.cs | 23 +++- src/mono/mono/component/hot_reload.c | 101 ++++++++++++++++-- src/mono/mono/metadata/metadata-internals.h | 1 + src/mono/mono/metadata/metadata.c | 4 +- 6 files changed, 130 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs index cf90c35975dca8..43a636d1f65fdc 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs @@ -11,4 +11,5 @@ public interface IExistingInterface { public class ZExistingClass { + public class PreviousNestedClass { } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index ab3d38b87102ad..a65d6df0fcccc6 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -11,6 +11,7 @@ public interface IExistingInterface { public class ZExistingClass { + public class PreviousNestedClass { } public class NewNestedClass {}; } @@ -19,9 +20,17 @@ public string ItfMethod(int i) { return i.ToString(); } - public object Clone() { + public virtual object Clone() { return new NewToplevelClass(); } + + public class AlsoNested { } +} + +public class NewGenericClass : NewToplevelClass { + public override object Clone() { + return new NewGenericClass(); + } } public struct NewToplevelStruct { @@ -30,3 +39,7 @@ public struct NewToplevelStruct { public interface INewInterface : IExistingInterface { public int AddedItfMethod (string s); } + +public enum NewEnum { + Red, Yellow, Green +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 0f2619630c0acf..46f75c5e389a85 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -442,7 +442,11 @@ internal static Type CheckReflectedType(Assembly assm, Type[] allTypes, string n var fullName = $"{nameSpace}.{typeName}"; var ty = assm.GetType(fullName); Assert.True(ty != null, $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetType for '{typeName}' to return non-null in {callerMemberName}"); - Assert.True(typeName == ty.Name, $"{callerFilePath}:{callerLineNumber}: returned type has unexpected name '{ty.Name}' (expected: '{typeName}') in {callerMemberName}"); + int nestedIdx = typeName.LastIndexOf('+'); + string comparisonName = typeName; + if (nestedIdx != -1) + comparisonName = typeName.Substring(nestedIdx+1); + Assert.True(comparisonName == ty.Name, $"{callerFilePath}:{callerLineNumber}: returned type has unexpected name '{ty.Name}' (expected: '{comparisonName}') in {callerMemberName}"); Assert.True(ContainsTypeWithName (allTypes, fullName), $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetTypes to contain '{fullName}', but it didn't in {callerMemberName}"); if (moreChecks != null) moreChecks(ty); @@ -460,17 +464,32 @@ public static void TestReflectionAddNewType() var allTypes = assm.GetTypes(); CheckReflectedType(assm, allTypes, ns, "ZExistingClass"); + CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); ApplyUpdateUtil.ApplyUpdate(assm); allTypes = assm.GetTypes(); CheckReflectedType(assm, allTypes, ns, "ZExistingClass"); + CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); CheckReflectedType(assm, allTypes, ns, "IExistingInterface"); - CheckReflectedType(assm, allTypes, ns, "NewToplevelClass"); + CheckReflectedType(assm, allTypes, ns, "ZExistingClass+NewNestedClass"); + + CheckReflectedType(assm, allTypes, ns, "NewToplevelClass", static (ty) => { + var nested = ty.GetNestedType("AlsoNested"); + var allNested = ty.GetNestedTypes(); + + Assert.Equal("AlsoNested", nested.Name); + Assert.Same(ty, nested.DeclaringType); + + Assert.Equal(1, allNested.Length); + Assert.Same(nested, allNested[0]); + }); + CheckReflectedType(assm, allTypes, ns, "NewGenericClass`1"); CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); CheckReflectedType(assm, allTypes, ns, "INewInterface"); + CheckReflectedType(assm, allTypes, ns, "NewEnum"); }); diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index d206c68541a751..f7e3f8f0fc5258 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1686,12 +1686,14 @@ skeleton_add_member (MonoAddedDefSkeleton *sk, uint32_t member_token) typedef struct Pass2Context { GArray *skeletons; /* Skeleton for each new added type definition */ + GArray *nested_class_worklist; /* List of tokens in the NestedClass table to re-visit at the end of the pass */ } Pass2Context; static void pass2_context_init (Pass2Context *ctx) { ctx->skeletons = g_array_new (FALSE, TRUE, sizeof(MonoAddedDefSkeleton)); + ctx->nested_class_worklist = g_array_new (FALSE, TRUE, sizeof(uint32_t)); } static void @@ -1699,6 +1701,8 @@ pass2_context_destroy (Pass2Context *ctx) { if (ctx->skeletons) g_array_free (ctx->skeletons, TRUE); + if (ctx->nested_class_worklist) + g_array_free (ctx->nested_class_worklist, TRUE); } static void @@ -1779,23 +1783,94 @@ hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, } /** - * Adds the given newly-added class to the image name cache. Except if the - * class is nested (has one of the nested visibility flags), since those are - * looked up differently. + * Adds the given newly-added class to the image, updating various data structures (e.g. name cache). */ static void -add_typedef_to_image_name_cache (MonoImage *image_base, uint32_t log_token) +add_typedef_to_image_metadata (MonoImage *image_base, uint32_t log_token) { uint32_t cols[MONO_TYPEDEF_SIZE]; uint32_t row = mono_metadata_token_index (log_token); mono_metadata_decode_row (&image_base->tables[MONO_TABLE_TYPEDEF], row - 1, cols, MONO_TYPEDEF_SIZE); uint32_t visib = cols [MONO_TYPEDEF_FLAGS] & TYPE_ATTRIBUTE_VISIBILITY_MASK; - if (visib >= TYPE_ATTRIBUTE_NESTED_PUBLIC && visib <= TYPE_ATTRIBUTE_NESTED_FAM_OR_ASSEM) + /* + * Add the class name to the name chache. Except if the + * class is nested (has one of the nested visibility flags), since those are + * looked up differently. + */ + gboolean isNested = (visib >= TYPE_ATTRIBUTE_NESTED_PUBLIC && visib <= TYPE_ATTRIBUTE_NESTED_FAM_OR_ASSEM); + if (!isNested) { + const char *name_space = mono_metadata_string_heap (image_base, cols[MONO_TYPEDEF_NAMESPACE]); + const char *name = mono_metadata_string_heap (image_base, cols[MONO_TYPEDEF_NAME]); + mono_image_add_to_name_cache (image_base, name_space, name, row); + } +} + +/** + * Given a row in the NestedClass table, add it to the worklist if the enclosing type isn't a skeleton. + */ +static void +add_nested_class_to_worklist (Pass2Context *ctx, MonoImage *image_base, uint32_t log_token) +{ + uint32_t cols[MONO_TABLE_NESTEDCLASS]; + mono_metadata_decode_row (&image_base->tables[MONO_TABLE_NESTEDCLASS], mono_metadata_token_index (log_token) - 1, cols, MONO_NESTED_CLASS_SIZE); + /* if the enclosing clas hasn't been created yet, don't do anything. mono_class_setup_nested_types() will pick up the new row */ + if (pass2_context_is_skeleton (ctx, cols[MONO_NESTED_CLASS_ENCLOSING])) return; - const char *name_space = mono_metadata_string_heap (image_base, cols[MONO_TYPEDEF_NAMESPACE]); - const char *name = mono_metadata_string_heap (image_base, cols[MONO_TYPEDEF_NAME]); - mono_image_add_to_name_cache (image_base, name_space, name, row); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "EnC: adding nested class row 0x%08x (enclosing: 0x%08x, nested: 0x%08x) to worklist", log_token, cols[MONO_NESTED_CLASS_ENCLOSING], cols[MONO_NESTED_CLASS_NESTED]); + + g_array_append_val (ctx->nested_class_worklist, log_token); +} + +/** + * For any enclosing classes that already had their nested classes property initialized, + * add any newly-added classes the the nested classes list. + * + * This has to be called late because we construct the nested class - which + * could have been a skeleton. + */ +static gboolean +pass2_update_nested_classes (Pass2Context *ctx, MonoImage *image_base, MonoError *error) +{ + if (!ctx->nested_class_worklist) + return TRUE; + GArray *arr = ctx->nested_class_worklist; + for (int i = 0; i < arr->len; ++i) { + uint32_t log_token = g_array_index (arr, uint32_t, i); + + uint32_t cols[MONO_TABLE_NESTEDCLASS]; + mono_metadata_decode_row (&image_base->tables[MONO_TABLE_NESTEDCLASS], mono_metadata_token_index (log_token) - 1, cols, MONO_NESTED_CLASS_SIZE); + /* if the enclosing clas hasn't been created yet, don't do anything. mono_class_setup_nested_types() will pick up the new row */ + uint32_t enclosing_typedef_row = cols[MONO_NESTED_CLASS_ENCLOSING]; + if (pass2_context_is_skeleton (ctx, enclosing_typedef_row)) + continue; + + MonoClass *enclosing_klass = mono_class_get_checked (image_base, mono_metadata_make_token (MONO_TABLE_TYPEDEF, enclosing_typedef_row), error); + return_val_if_nok (error, FALSE); + g_assert (enclosing_klass); + /* enclosing class is set up, but noone asked for its nested classes yet. Nothing to do here. */ + if (!m_class_is_nested_classes_inited (enclosing_klass)) + continue; + MonoClass *klass = mono_class_get_checked (image_base, mono_metadata_make_token (MONO_TABLE_TYPEDEF, cols[MONO_NESTED_CLASS_NESTED]), error); + return_val_if_nok (error, FALSE); + g_assert (klass); + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "EnC: worklist row 0x%08x adding to enclosing 0x%08x, the nested class 0x%08x", log_token, cols[MONO_NESTED_CLASS_ENCLOSING], cols[MONO_NESTED_CLASS_NESTED]); + GList *nested_classes = mono_class_get_nested_classes_property (enclosing_klass); + + /* make a list node for the new class */ + GList *new_list = mono_g_list_prepend_image (image_base, NULL, klass); + + /* either set the property to the new node, or append the new node to the existing list */ + if (!nested_classes) { + mono_class_set_nested_classes_property (enclosing_klass, new_list); + } else { + g_list_concat (nested_classes, new_list); + } + } + + return TRUE; + } /* do actuall enclog application */ @@ -2026,7 +2101,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base /* ok, added a new class */ /* TODO: do things here */ pass2_context_add_skeleton (ctx, log_token); - add_typedef_to_image_name_cache (image_base, log_token); + add_typedef_to_image_metadata (image_base, log_token); break; } case ENC_FUNC_ADD_METHOD: @@ -2049,6 +2124,11 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base g_assert (func_code != ENC_FUNC_DEFAULT); break; } + case MONO_TABLE_NESTEDCLASS: { + g_assert (is_addition); + add_nested_class_to_worklist (ctx, image_base, log_token); + break; + } case MONO_TABLE_PROPERTYMAP: { switch (func_code) { case ENC_FUNC_DEFAULT: @@ -2194,6 +2274,9 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base if (!baseline_info_consume_skeletons (ctx, image_base, base_info, error)) return FALSE; + if (!pass2_update_nested_classes (ctx, image_base, error)) + return FALSE; + return TRUE; } diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h index bf94ff432dfd7c..56a59d9c233be1 100644 --- a/src/mono/mono/metadata/metadata-internals.h +++ b/src/mono/mono/metadata/metadata-internals.h @@ -723,6 +723,7 @@ mono_image_strdup_vprintf (MonoImage *image, const char *format, va_list args); char* mono_image_strdup_printf (MonoImage *image, const char *format, ...) MONO_ATTR_FORMAT_PRINTF(2,3); +MONO_COMPONENT_API GList* mono_g_list_prepend_image (MonoImage *image, GList *list, gpointer data); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 9efd1510c5b0a2..d8631c02c84724 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -5022,9 +5022,7 @@ mono_metadata_nesting_typedef (MonoImage *meta, guint32 index, guint32 start_ind start = start_index; - /* FIXME: metadata-udpate */ - - int rows = table_info_get_rows (tdef); + int rows = mono_metadata_table_num_rows (meta, MONO_TABLE_NESTEDCLASS); while (start <= rows) { if (class_index == mono_metadata_decode_row_col (tdef, start - 1, MONO_NESTED_CLASS_ENCLOSING)) break; From 36fbca044db01c7078b871fe572bda278b7c7418 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Mar 2022 16:35:33 -0400 Subject: [PATCH 17/37] GetInterfaces on new types; also Activator.CreateInstance --- .../tests/ApplyUpdateTest.cs | 21 ++++++++++++++++++- src/mono/mono/component/hot_reload.c | 5 +++++ src/mono/mono/metadata/metadata.c | 17 +++++++++++---- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 46f75c5e389a85..76f40f0869e03e 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -476,7 +476,7 @@ public static void TestReflectionAddNewType() CheckReflectedType(assm, allTypes, ns, "ZExistingClass+NewNestedClass"); - CheckReflectedType(assm, allTypes, ns, "NewToplevelClass", static (ty) => { + var newTy = CheckReflectedType(assm, allTypes, ns, "NewToplevelClass", static (ty) => { var nested = ty.GetNestedType("AlsoNested"); var allNested = ty.GetNestedTypes(); @@ -485,13 +485,32 @@ public static void TestReflectionAddNewType() Assert.Equal(1, allNested.Length); Assert.Same(nested, allNested[0]); + + var allInterfaces = ty.GetInterfaces(); + + Assert.Equal (2, allInterfaces.Length); + bool hasICloneable = false, hasINewInterface = false; + for (int i = 0; i < allInterfaces.Length; ++i) { + var itf = allInterfaces[i]; + if (itf.Name == "ICloneable") + hasICloneable = true; + if (itf.Name == "IExistingInterface") + hasINewInterface = true; + } + Assert.True (hasICloneable); + Assert.True (hasINewInterface); }); CheckReflectedType(assm, allTypes, ns, "NewGenericClass`1"); CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); CheckReflectedType(assm, allTypes, ns, "INewInterface"); CheckReflectedType(assm, allTypes, ns, "NewEnum"); + // make some instances using reflection and use them through known interfaces + var o = Activator.CreateInstance (newTy); + + var i = (System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.IExistingInterface)o; + Assert.Equal ("123", i.ItfMethod (123)); }); } } diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index f7e3f8f0fc5258..1c65d4522d811d 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -2262,6 +2262,11 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base */ break; } + case MONO_TABLE_INTERFACEIMPL: { + g_assert (is_addition); + /* added rows ok (for added classes). will be processed when the MonoClass is created. */ + break; + } default: { g_assert (token_index > table_info_get_rows (&image_base->tables [token_table])); if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE)) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index d8631c02c84724..45f18db1a28a85 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -39,6 +39,7 @@ #include #include #include +#include /* Auxiliary structure used for caching inflated signatures */ typedef struct { @@ -4884,18 +4885,26 @@ mono_metadata_interfaces_from_typedef_full (MonoImage *meta, guint32 index, Mono error_init (error); - if (!tdef->base) + if (!tdef->base && !meta->has_updates) return TRUE; loc.idx = mono_metadata_token_index (index); loc.col_idx = MONO_INTERFACEIMPL_CLASS; loc.t = tdef; - /* FIXME: metadata-update */ + gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) != NULL; - if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) + if (!found && !meta->has_updates) return TRUE; + if (G_UNLIKELY (meta->has_updates)) { + if (!found && !mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) { + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "NO Found interfaces for class 0x%08x", index); + return TRUE; + } + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "Found interfaces for class 0x%08x starting at 0x%08x", index, loc.result); + } + start = loc.result; /* * We may end up in the middle of the rows... @@ -4907,7 +4916,7 @@ mono_metadata_interfaces_from_typedef_full (MonoImage *meta, guint32 index, Mono break; } pos = start; - int rows = table_info_get_rows (tdef); + int rows = mono_metadata_table_num_rows (meta, MONO_TABLE_INTERFACEIMPL); while (pos < rows) { mono_metadata_decode_row (tdef, pos, cols, MONO_INTERFACEIMPL_SIZE); if (cols [MONO_INTERFACEIMPL_CLASS] != loc.idx) From 68e2ebad24889966eec7caa301ee69832e296368 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Mar 2022 10:35:02 -0400 Subject: [PATCH 18/37] Mark mono_class_get_nested_classes_property as a component API also mono_class_set_nested_classes_property --- src/mono/mono/metadata/class-internals.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index ccaf81e98ea178..cec6192b1e0bf4 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1401,9 +1401,11 @@ mono_class_get_exception_data (MonoClass *klass); void mono_class_set_exception_data (MonoClass *klass, MonoErrorBoxed *value); +MONO_COMPONENT_API GList* mono_class_get_nested_classes_property (MonoClass *klass); +MONO_COMPONENT_API void mono_class_set_nested_classes_property (MonoClass *klass, GList *value); From 63fb67a5d954d6554d698f58293d708f99c08e50 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Mar 2022 12:46:41 -0400 Subject: [PATCH 19/37] Add GetMethods, GetFields, GetMethod, GetField test --- .../ReflectionAddNewType_v1.cs | 7 ++++ .../tests/ApplyUpdateTest.cs | 39 ++++++++++++++++--- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index a65d6df0fcccc6..58c10bb7a6e8bc 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -13,6 +13,13 @@ public class ZExistingClass { public class PreviousNestedClass { } public class NewNestedClass {}; + + + public string NewMethod (string s, int i) => s + i.ToString(); + + public int NewField; + + public double NewProp { get; set; } } public class NewToplevelClass : IExistingInterface, ICloneable { diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 76f40f0869e03e..ae28ba9aadbab4 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -470,13 +470,40 @@ public static void TestReflectionAddNewType() allTypes = assm.GetTypes(); - CheckReflectedType(assm, allTypes, ns, "ZExistingClass"); + CheckReflectedType(assm, allTypes, ns, "ZExistingClass", static (ty) => + { + var allMethods = ty.GetMethods(); + + MethodInfo newMethod = null; + foreach (var meth in allMethods) + { + if (meth.Name == "NewMethod") + newMethod = meth; + } + Assert.NotNull (newMethod); + + Assert.Equal (newMethod, ty.GetMethod ("NewMethod")); + + var allFields = ty.GetFields(); + + FieldInfo newField = null; + foreach (var fld in allFields) + { + if (fld.Name == "NewField") + newField = fld; + } + Assert.NotNull(newField); + + Assert.Equal(newField, ty.GetField("NewField")); + + }); CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); CheckReflectedType(assm, allTypes, ns, "IExistingInterface"); CheckReflectedType(assm, allTypes, ns, "ZExistingClass+NewNestedClass"); - var newTy = CheckReflectedType(assm, allTypes, ns, "NewToplevelClass", static (ty) => { + var newTy = CheckReflectedType(assm, allTypes, ns, "NewToplevelClass", static (ty) => + { var nested = ty.GetNestedType("AlsoNested"); var allNested = ty.GetNestedTypes(); @@ -497,8 +524,8 @@ public static void TestReflectionAddNewType() if (itf.Name == "IExistingInterface") hasINewInterface = true; } - Assert.True (hasICloneable); - Assert.True (hasINewInterface); + Assert.True(hasICloneable); + Assert.True(hasINewInterface); }); CheckReflectedType(assm, allTypes, ns, "NewGenericClass`1"); CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); @@ -506,11 +533,11 @@ public static void TestReflectionAddNewType() CheckReflectedType(assm, allTypes, ns, "NewEnum"); // make some instances using reflection and use them through known interfaces - var o = Activator.CreateInstance (newTy); + var o = Activator.CreateInstance(newTy); var i = (System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.IExistingInterface)o; - Assert.Equal ("123", i.ItfMethod (123)); + Assert.Equal("123", i.ItfMethod(123)); }); } } From 7861d05f9356c96a340c4bda9c2b2a92a77e05b9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Mar 2022 12:47:17 -0400 Subject: [PATCH 20/37] [class] Implement added method iteration Change the iterator from storing MonoMethod** values to storing an iteration count. For added methods, when the iteration count is more than mono_class_get_method_count, run through the hot reload added_members list and iterate over any relevant methods --- src/mono/mono/component/hot_reload-stub.c | 9 +++++ src/mono/mono/component/hot_reload.c | 43 +++++++++++++++++++++++ src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/class.c | 28 ++++++++++----- src/mono/mono/metadata/metadata-update.c | 6 ++++ src/mono/mono/metadata/metadata-update.h | 3 ++ 6 files changed, 81 insertions(+), 9 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 99ba72233b6bea..591af8ba910770 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -89,6 +89,8 @@ hot_reload_stub_find_method_by_name (MonoClass *klass, const char *name, int par static gboolean hot_reload_stub_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); +static MonoMethod * +hot_reload_stub_added_methods_iter (MonoClass *klass, gpointer *iter); static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, @@ -116,6 +118,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_get_static_field_addr, &hot_reload_stub_find_method_by_name, &hot_reload_stub_get_typedef_skeleton, + &hot_reload_stub_added_methods_iter, }; static bool @@ -274,6 +277,12 @@ hot_reload_stub_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_to return FALSE; } +static MonoMethod * +hot_reload_stub_added_methods_iter (MonoClass *klass, gpointer *iter) +{ + return NULL; +} + MONO_COMPONENT_EXPORT_ENTRYPOINT MonoComponentHotReload * diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 1c65d4522d811d..7e8402d848c92e 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -122,6 +122,9 @@ hot_reload_find_method_by_name (MonoClass *klass, const char *name, int param_co static gboolean hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); +static MonoMethod * +hot_reload_added_methods_iter (MonoClass *klass, gpointer *iter); + 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); @@ -151,6 +154,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_static_field_addr, &hot_reload_find_method_by_name, &hot_reload_get_typedef_skeleton, + &hot_reload_added_methods_iter, }; MonoComponentHotReload * @@ -2952,3 +2956,42 @@ hot_reload_find_method_by_name (MonoClass *klass, const char *name, int param_co return res; } + +static MonoMethod * +hot_reload_added_methods_iter (MonoClass *klass, gpointer *iter) +{ + g_assert (iter); + // invariant: idx is one past the method we previously returned. + uint32_t idx = GPOINTER_TO_UINT (*iter); + g_assert (idx >= mono_class_get_method_count (klass)); + + GSList *members = hot_reload_get_added_members (klass); + if (!members) + return NULL; + // expect to only see class defs here. Rationale: adding methods to generic classes is not + // allowed (if a generation adds a new generic class, it won't be here - those methods will + // be in the normal iteration code, not here. + g_assert (m_class_get_class_kind (klass) == MONO_CLASS_DEF); + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Iterating added methods of 0x%08x idx = %u", m_class_get_type_token (klass), idx); + + // go through the added members incrementing cur_count until it's equal to idx or we run out + // of added members. + int cur_count = mono_class_get_method_count (klass); + for (GSList *ptr = members; ptr; ptr = ptr->next) { + uint32_t token = GPOINTER_TO_UINT(ptr->data); + if (mono_metadata_token_table (token) != MONO_TABLE_METHOD) + continue; + if (cur_count == idx) { + // found a method, advance iter and return the method. + *iter = GUINT_TO_POINTER (1+idx); + ERROR_DECL (local_error); + MonoMethod *method = mono_get_method_checked (m_class_get_image (klass), token, klass, NULL, local_error); + mono_error_cleanup (local_error); + return method; + } + cur_count++; + } + // ran out of added methods, iteration is finished. + return NULL; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index c3aa33cbd01a6f..a2db5139952c9c 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -41,6 +41,7 @@ typedef struct _MonoComponentHotReload { gpointer (*get_static_field_addr) (MonoClassField *field); MonoMethod* (*find_method_by_name) (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); gboolean (*get_typedef_skeleton) (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); + MonoMethod* (*added_methods_iter) (MonoClass *klass, gpointer *iter); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index de7cde1362b914..dd9eae226dafc2 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -5079,9 +5079,9 @@ mono_class_get_fields_internal (MonoClass *klass, gpointer *iter) MonoMethod* mono_class_get_methods (MonoClass* klass, gpointer *iter) { - MonoMethod** method; if (!iter) return NULL; + MonoImage *image = m_class_get_image (klass); if (!*iter) { mono_class_setup_methods (klass); @@ -5090,23 +5090,33 @@ mono_class_get_methods (MonoClass* klass, gpointer *iter) * We can't fail lookup of methods otherwise the runtime will burst in flames on all sort of places. * FIXME we should better report this error to the caller */ - if (!klass_methods) + if (!klass_methods && !image->has_updates) return NULL; + uint32_t idx = 0; /* start from the first */ if (mono_class_get_method_count (klass)) { - *iter = &klass_methods [0]; + // idx is 1 more than the method we just returned + *iter = GUINT_TO_POINTER (idx + 1); return klass_methods [0]; } else { /* no method */ - return NULL; + if (G_LIKELY (!image->has_updates)) + return NULL; + else + *iter = 0; } } - method = (MonoMethod **)*iter; - method++; - if (method < &m_class_get_methods (klass) [mono_class_get_method_count (klass)]) { - *iter = method; - return *method; + // idx is 1 more than the method we returned on the previous iteration + uint32_t idx = GPOINTER_TO_UINT (*iter); + if (idx < mono_class_get_method_count (klass)) { + // if we're still in range, return the next method and advance iter 1 past it. + MonoMethod *method = m_class_get_methods (klass) [idx]; + idx++; + *iter = GUINT_TO_POINTER (idx); + return method; } + if (G_UNLIKELY (image->has_updates)) + return mono_metadata_update_added_methods_iter (klass, iter); return NULL; } diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 6e743b10a1baa4..372f8fc7e93275 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -188,3 +188,9 @@ mono_metadata_update_get_typedef_skeleton (MonoImage *base_image, uint32_t typed { return mono_component_hot_reload()->get_typedef_skeleton (base_image, typedef_token, first_method_idx, method_count, first_field_idx, field_count); } + +MonoMethod * +mono_metadata_update_added_methods_iter (MonoClass *klass, gpointer *iter) +{ + return mono_component_hot_reload()->added_methods_iter (klass, iter); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 367030126dff2c..f4fa04473416b4 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -76,4 +76,7 @@ mono_metadata_update_get_typedef_skeleton (MonoImage *base_image, uint32_t typed gpointer mono_metadata_update_get_static_field_addr (MonoClassField *field); +MonoMethod * +mono_metadata_update_added_methods_iter (MonoClass *klass, gpointer *iter); + #endif /*__MONO_METADATA_UPDATE_H__*/ From 7c18fb9284c0794c92b4f76c5d2053316e98411c Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Mar 2022 15:34:37 -0400 Subject: [PATCH 21/37] Implement added field iteration --- src/mono/mono/component/hot_reload-stub.c | 10 +++++ src/mono/mono/component/hot_reload.c | 37 +++++++++++++++- src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/class-accessors.c | 1 + src/mono/mono/metadata/class.c | 54 +++++++++++++++-------- src/mono/mono/metadata/metadata-update.c | 6 +++ src/mono/mono/metadata/metadata-update.h | 3 ++ 7 files changed, 92 insertions(+), 20 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 591af8ba910770..94df6239b5540e 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -92,6 +92,9 @@ hot_reload_stub_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_to static MonoMethod * hot_reload_stub_added_methods_iter (MonoClass *klass, gpointer *iter); +static MonoClassField * +hot_reload_stub_added_fields_iter (MonoClass *klass, gboolean lazy, gpointer *iter); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -119,6 +122,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_find_method_by_name, &hot_reload_stub_get_typedef_skeleton, &hot_reload_stub_added_methods_iter, + &hot_reload_stub_added_fields_iter, }; static bool @@ -283,6 +287,12 @@ hot_reload_stub_added_methods_iter (MonoClass *klass, gpointer *iter) return NULL; } +static MonoClassField * +hot_reload_stub_added_fields_iter (MonoClass *klass, gboolean lazy, gpointer *iter) +{ + return NULL; +} + MONO_COMPONENT_EXPORT_ENTRYPOINT MonoComponentHotReload * diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 7e8402d848c92e..98a1cddd005150 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -125,6 +125,9 @@ hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, static MonoMethod * hot_reload_added_methods_iter (MonoClass *klass, gpointer *iter); +static MonoClassField * +hot_reload_added_fields_iter (MonoClass *klass, gboolean lazy, gpointer *iter); + 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); @@ -155,6 +158,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_find_method_by_name, &hot_reload_get_typedef_skeleton, &hot_reload_added_methods_iter, + &hot_reload_added_fields_iter, }; MonoComponentHotReload * @@ -2977,7 +2981,7 @@ hot_reload_added_methods_iter (MonoClass *klass, gpointer *iter) // go through the added members incrementing cur_count until it's equal to idx or we run out // of added members. - int cur_count = mono_class_get_method_count (klass); + uint32_t cur_count = mono_class_get_method_count (klass); for (GSList *ptr = members; ptr; ptr = ptr->next) { uint32_t token = GPOINTER_TO_UINT(ptr->data); if (mono_metadata_token_table (token) != MONO_TABLE_METHOD) @@ -2995,3 +2999,34 @@ hot_reload_added_methods_iter (MonoClass *klass, gpointer *iter) // ran out of added methods, iteration is finished. return NULL; } + +static MonoClassField * +hot_reload_added_fields_iter (MonoClass *klass, gboolean lazy G_GNUC_UNUSED, gpointer *iter) +{ + MonoClassMetadataUpdateInfo *info = mono_class_get_metadata_update_info (klass); + if (!info) + return NULL; + + /* FIXME: this needs locking in the multi-threaded case. There could be an update happening that resizes the array. */ + GPtrArray *added_fields = info->added_fields; + uint32_t count = added_fields->len; + + // invariant: idx is one past the field we previously returned. + uint32_t idx = GPOINTER_TO_UINT(*iter); + + g_assert (idx >= mono_class_get_field_count (klass)); + + uint32_t field_idx = idx - mono_class_get_field_count (klass); + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Iterating added fields of 0x%08x idx = %u", m_class_get_type_token (klass), field_idx); + + // if the field index is one past the last added field, we're done + if (field_idx == count) + return NULL; + g_assert (field_idx < count); + MonoClassMetadataUpdateField *field = g_ptr_array_index (added_fields, field_idx); + + idx++; + *iter = GUINT_TO_POINTER (idx); + return &field->field; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index a2db5139952c9c..7b3baaffa81393 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -42,6 +42,7 @@ typedef struct _MonoComponentHotReload { MonoMethod* (*find_method_by_name) (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); gboolean (*get_typedef_skeleton) (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); MonoMethod* (*added_methods_iter) (MonoClass *klass, gpointer *iter); + MonoClassField* (*added_fields_iter) (MonoClass *klass, gboolean lazy, gpointer *iter); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class-accessors.c b/src/mono/mono/metadata/class-accessors.c index 457292de55074c..d161a74c395754 100644 --- a/src/mono/mono/metadata/class-accessors.c +++ b/src/mono/mono/metadata/class-accessors.c @@ -602,6 +602,7 @@ mono_class_get_metadata_update_info (MonoClass *klass) return (MonoClassMetadataUpdateInfo *)get_pointer_property (klass, PROP_METADATA_UPDATE_INFO); case MONO_CLASS_GINST: case MONO_CLASS_GPARAM: + case MONO_CLASS_ARRAY: case MONO_CLASS_POINTER: case MONO_CLASS_GC_FILLER: return NULL; diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index dd9eae226dafc2..8a5e27cdd3bdc9 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -5037,10 +5037,9 @@ mono_class_get_fields (MonoClass* klass, gpointer *iter) MonoClassField* mono_class_get_fields_internal (MonoClass *klass, gpointer *iter) { - MonoClassField* field; if (!iter) return NULL; - /* TODO: metadata-update - also iterate over the added fields */ + MonoImage *image = m_class_get_image (klass); if (!*iter) { mono_class_setup_fields (klass); if (mono_class_has_failure (klass)) @@ -5048,19 +5047,28 @@ mono_class_get_fields_internal (MonoClass *klass, gpointer *iter) /* start from the first */ if (mono_class_get_field_count (klass)) { MonoClassField *klass_fields = m_class_get_fields (klass); - *iter = &klass_fields [0]; + uint32_t idx = 0; + *iter = GUINT_TO_POINTER (idx + 1); return &klass_fields [0]; } else { /* no fields */ - return NULL; + if (G_LIKELY (!image->has_updates)) + return NULL; + else + *iter = 0; } } - field = (MonoClassField *)*iter; - field++; - if (field < &m_class_get_fields (klass) [mono_class_get_field_count (klass)]) { - *iter = field; + // invariant: idx is one past the field we previously returned + uint32_t idx = GPOINTER_TO_UINT(*iter); + if (idx < mono_class_get_field_count (klass)) { + MonoClassField *field = &m_class_get_fields (klass) [idx]; + ++idx; + *iter = GUINT_TO_POINTER (idx); return field; } + if (G_UNLIKELY (image->has_updates)) { + return mono_metadata_update_added_fields_iter (klass, FALSE, iter); + } return NULL; } @@ -6571,11 +6579,9 @@ mono_field_resolve_flags (MonoClassField *field) MonoClassField* mono_class_get_fields_lazy (MonoClass* klass, gpointer *iter) { - MonoClassField* field; if (!iter) return NULL; - /* TODO: metadata-update: iterate over the added fields, too */ - g_assert (!m_class_get_image (klass)->has_updates); + MonoImage *image = m_class_get_image (klass); if (!*iter) { mono_class_setup_basic_field_info (klass); MonoClassField *klass_fields = m_class_get_fields (klass); @@ -6583,18 +6589,28 @@ mono_class_get_fields_lazy (MonoClass* klass, gpointer *iter) return NULL; /* start from the first */ if (mono_class_get_field_count (klass)) { - *iter = &klass_fields [0]; - return (MonoClassField *)*iter; + uint32_t idx = 0; + *iter = GUINT_TO_POINTER (idx+1); + return &klass_fields [0]; } else { /* no fields */ - return NULL; + if (G_LIKELY(!image->has_updates)) + return NULL; + else + *iter = 0; } } - field = (MonoClassField *)*iter; - field++; - if (field < &m_class_get_fields (klass) [mono_class_get_field_count (klass)]) { - *iter = field; - return (MonoClassField *)*iter; + // invariant: idx is one past the field we previously returned + uint32_t idx = GPOINTER_TO_UINT(*iter); + if (idx < mono_class_get_field_count (klass)) { + MonoClassField *field = &m_class_get_fields (klass) [idx]; + ++idx; + *iter = GUINT_TO_POINTER (idx); + return field; + } + if (G_UNLIKELY (image->has_updates)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Lazy iterating added fields %s", m_class_get_name(klass)); + return mono_metadata_update_added_fields_iter (klass, TRUE, iter); } return NULL; } diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 372f8fc7e93275..d5128b43ae285d 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -194,3 +194,9 @@ mono_metadata_update_added_methods_iter (MonoClass *klass, gpointer *iter) { return mono_component_hot_reload()->added_methods_iter (klass, iter); } + +MonoClassField * +mono_metadata_update_added_fields_iter (MonoClass *klass, gboolean lazy, gpointer *iter) +{ + return mono_component_hot_reload()->added_fields_iter (klass, lazy, iter); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index f4fa04473416b4..b7141b2b0ad104 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -79,4 +79,7 @@ mono_metadata_update_get_static_field_addr (MonoClassField *field); MonoMethod * mono_metadata_update_added_methods_iter (MonoClass *klass, gpointer *iter); +MonoClassField * +mono_metadata_update_added_fields_iter (MonoClass *klass, gboolean lazy, gpointer *iter); + #endif /*__MONO_METADATA_UPDATE_H__*/ From 13c32cadbd3ba70422160b3f5d4a69a7cd27af8d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Mar 2022 15:35:02 -0400 Subject: [PATCH 22/37] Add a GetField(BindingFlags.Static) test case --- .../ReflectionAddNewType_v1.cs | 2 ++ .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index 58c10bb7a6e8bc..b5267615fa0b1e 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -19,6 +19,8 @@ public class NewNestedClass {}; public int NewField; + public static DateTime NewStaticField; + public double NewProp { get; set; } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index ae28ba9aadbab4..463c68ca1802c5 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -484,17 +484,22 @@ public static void TestReflectionAddNewType() Assert.Equal (newMethod, ty.GetMethod ("NewMethod")); - var allFields = ty.GetFields(); + var allFields = ty.GetFields(BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public); FieldInfo newField = null; + FieldInfo newStaticField = null; foreach (var fld in allFields) { if (fld.Name == "NewField") newField = fld; + if (fld.Name == "NewStaticField") + newStaticField = fld; } Assert.NotNull(newField); + Assert.NotNull(newStaticField); Assert.Equal(newField, ty.GetField("NewField")); + Assert.Equal(newStaticField, ty.GetField("NewStaticField", BindingFlags.Static | BindingFlags.Public)); }); CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); From d2bcbfc4203c8e10536a05a3f0924bb5d04db30f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Mar 2022 16:59:33 -0400 Subject: [PATCH 23/37] Add Enum.GetNames and GetProperties tests - not working yet --- .../tests/ApplyUpdateTest.cs | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 463c68ca1802c5..1d1339e8816edb 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -501,6 +501,28 @@ public static void TestReflectionAddNewType() Assert.Equal(newField, ty.GetField("NewField")); Assert.Equal(newStaticField, ty.GetField("NewStaticField", BindingFlags.Static | BindingFlags.Public)); + // FIXME: finish this +#if false + var allProperties = ty.GetProperties(); + + PropertyInfo newProp = null; + foreach (var prop in allProperties) + { + if (prop.Name == "NewProp") + newProp = prop; + } + Assert.NotNull(newProp); + + Assert.Equal(newProp, ty.GetProperty("NewProp")); + + MethodInfo newPropGet = newProp.GetGetMethod(); + Assert.NotNull(newPropGet); + MethodInfo newPropSet = newProp.GetSetMethod(); + Assert.NotNull(newPropSet); + + Assert.Equal("get_NewProp", newPropGet.Name); +#endif + }); CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); CheckReflectedType(assm, allTypes, ns, "IExistingInterface"); @@ -535,7 +557,17 @@ public static void TestReflectionAddNewType() CheckReflectedType(assm, allTypes, ns, "NewGenericClass`1"); CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); CheckReflectedType(assm, allTypes, ns, "INewInterface"); - CheckReflectedType(assm, allTypes, ns, "NewEnum"); + CheckReflectedType(assm, allTypes, ns, "NewEnum", static (ty) => { +#if false + var names = Enum.GetNames (ty); + Assert.Equal(3, names.Length); + var vals = Enum.GetValues (ty); + Assert.Equal(3, vals.Length); + + Assert.NotNull(Enum.Parse (ty, "Red")); + Assert.NotNull(Enum.Parse (ty, "Yellow")); +#endif + }); // make some instances using reflection and use them through known interfaces var o = Activator.CreateInstance(newTy); From 64c03ebb368758c720e927703489416e81da5fd4 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Mar 2022 20:05:54 -0400 Subject: [PATCH 24/37] Mark mono_class_get_method_count as a component API also mono_class_get_field_count --- src/mono/mono/metadata/class-internals.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index cec6192b1e0bf4..68b655ce812efc 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1371,12 +1371,14 @@ mono_class_get_first_field_idx (MonoClass *klass); void mono_class_set_first_field_idx (MonoClass *klass, guint32 idx); +MONO_COMPONENT_API guint32 mono_class_get_method_count (MonoClass *klass); void mono_class_set_method_count (MonoClass *klass, guint32 count); +MONO_COMPONENT_API guint32 mono_class_get_field_count (MonoClass *klass); From 51531959313f8bcfb351f3d4504e9e2b5a2b17db Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Mar 2022 20:24:55 -0400 Subject: [PATCH 25/37] Enum values work --- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 2 -- src/mono/mono/metadata/class.c | 3 +-- src/mono/mono/metadata/metadata.c | 7 +++++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 1d1339e8816edb..b72012ed27482b 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -558,7 +558,6 @@ public static void TestReflectionAddNewType() CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); CheckReflectedType(assm, allTypes, ns, "INewInterface"); CheckReflectedType(assm, allTypes, ns, "NewEnum", static (ty) => { -#if false var names = Enum.GetNames (ty); Assert.Equal(3, names.Length); var vals = Enum.GetValues (ty); @@ -566,7 +565,6 @@ public static void TestReflectionAddNewType() Assert.NotNull(Enum.Parse (ty, "Red")); Assert.NotNull(Enum.Parse (ty, "Yellow")); -#endif }); // make some instances using reflection and use them through known interfaces diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 8a5e27cdd3bdc9..bfba08ecfb64bb 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2541,10 +2541,9 @@ mono_class_get_field_token (MonoClassField *field) static int mono_field_get_index (MonoClassField *field) { + g_assert (!m_field_is_from_update (field)); int index = field - m_class_get_fields (m_field_get_parent (field)); g_assert (index >= 0 && index < mono_class_get_field_count (m_field_get_parent (field))); - /* TODO: metadata-update: check if the field was added */ - g_assert (!m_class_get_image (m_field_get_parent (field))->has_updates); return index; } diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 45f18db1a28a85..48a902b19b5a2c 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6266,8 +6266,6 @@ mono_metadata_get_constant_index (MonoImage *meta, guint32 token, guint32 hint) loc.col_idx = MONO_CONSTANT_PARENT; loc.t = tdef; - /* FIXME: metadata-update */ - /* FIXME: Index translation */ if ((hint > 0) && (hint < table_info_get_rows (tdef)) && (mono_metadata_decode_row_col (tdef, hint - 1, MONO_CONSTANT_PARENT) == index)) @@ -6276,6 +6274,11 @@ mono_metadata_get_constant_index (MonoImage *meta, guint32 token, guint32 hint) if (tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) { return loc.result + 1; } + + if (G_UNLIKELY (meta->has_updates)) { + if (mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) + return loc.result + 1; + } return 0; } From e759238ce819ccb158a6d2b2e8d440b3d9536e22 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 16:29:38 -0400 Subject: [PATCH 26/37] Use GSList for added_fields (and props, events); add from_update bit Use GSList to simplify the concurrency story for accessing added_fields (and added_props and added_events, eventually): unlike a GPtrArray it won't resize, so we don't need to lock readers. Add a from_update bit to MonoProperty and MonoEvent - when props or events are added to existing classes, they will have the bit set. That means that pointer arithmetic to figure out the prop (or event) tokens won't be usable (since they're not allocated in one big block). --- .../mono/component/hot_reload-internals.h | 20 +++++++--- src/mono/mono/component/hot_reload.c | 38 ++++++------------- src/mono/mono/metadata/class-internals.h | 18 +++++++++ src/mono/mono/metadata/class.c | 4 ++ 4 files changed, 49 insertions(+), 31 deletions(-) diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index e1f31c84b7d137..e90513fd6c5855 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -27,7 +27,9 @@ struct _MonoClassMetadataUpdateInfo { * to the BaselineInfo for the image and cleanup from there. */ GSList *added_members; /* a set of Method or Field table tokens of any methods or fields added to this class, allocated from the MonoClass mempool */ - GPtrArray *added_fields; /* a set of MonoClassMetadataUpdateField* values for every added field. */ + GSList *added_fields; /* a set of MonoClassMetadataUpdateField* values for every added field. */ + GSList *added_props; /* a set of MonoClassMetadataUpdateProperty* values */ + GSList *added_events; /* a set of MonoClassMetadataUpdateEvent* values */ MonoClassRuntimeMetadataUpdateInfo runtime; }; @@ -67,10 +69,18 @@ typedef struct _MonoClassMetadataUpdateField { uint32_t generation; /* when this field was added */ uint32_t token; /* the Field table token where this field was defined. (this won't make * sense for generic instances, once EnC is supported there) */ - /* if non-zero the EnC update came before the parent class was initialized. The field is - * stored in the instance at this offset. MonoClassField:offset is -1. Not used for static - * fields. */ - int before_init_instance_offset; } MonoClassMetadataUpdateField; +typedef struct _MonoClassMetadataUpdateProperty { + MonoProperty prop; + uint32_t generation; /* when this prop was added */ + uint32_t token; /* the Property table token where this prop was defined. */ +} MonoClassMetadataUpdateProperty; + +typedef struct _MonoClassMetadataUpdateEvent { + MonoEvent evt; + uint32_t generatino; /* when this event was added */ + uint32_t token; /* the Event table token where this event was defined. */ +} MonoClassMetadataUpdateEvent; + #endif/*_MONO_COMPONENT_HOT_RELOAD_INTERNALS_H*/ diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 98a1cddd005150..c00ee5241b9b88 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -365,9 +365,7 @@ static void klass_info_destroy (gpointer value, gpointer user_data G_GNUC_UNUSED) { MonoClassMetadataUpdateInfo *info = (MonoClassMetadataUpdateInfo *)value; - /* added_members is allocated from the class mempool, don't free it here */ - /* The MonoClassMetadataUpdateField is allocated from the class mempool, don't free it here */ - g_ptr_array_free (info->added_fields, TRUE); + /* added_members, added_fields, added_props, added_events list nodes are allocated from the class mempool, don't free them here */ if (info->runtime.static_fields) { mono_g_hash_table_destroy (info->runtime.static_fields); @@ -2771,11 +2769,10 @@ static MonoClassField * hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token) { MonoClassMetadataUpdateInfo *info = mono_class_get_or_add_metadata_update_info (klass); g_assert (mono_metadata_token_table (fielddef_token) == MONO_TABLE_FIELD); - /* FIXME: this needs locking in the multi-threaded case. There could be an update happening that resizes the array. */ - GPtrArray *added_fields = info->added_fields; - uint32_t count = added_fields->len; - for (uint32_t i = 0; i < count; ++i) { - MonoClassMetadataUpdateField *field = (MonoClassMetadataUpdateField *)g_ptr_array_index (added_fields, i); + GSList *added_fields = info->added_fields; + + for (GSList *p = added_fields; p; p = p->next) { + MonoClassMetadataUpdateField *field = (MonoClassMetadataUpdateField *)p->data; if (field->token == fielddef_token) return &field->field; } @@ -2786,11 +2783,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) { - // TODO: hang a "pending field" struct off the parent_klass if !parent_klass->fields - // In that case we can do things simpler, maybe by just creating the MonoClassField array as usual, and just relying on the normal layout algorithm to make space for the instance. - - if (!m_class_is_inited (parent_klass)) - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding fielddef 0x%08x to uninited class 0x%08x", fielddef_token, m_class_get_type_token (parent_klass)); + g_assert (m_class_is_inited (parent_klass)); MonoClassMetadataUpdateInfo *parent_info = mono_class_get_or_add_metadata_update_info (parent_klass); @@ -2810,11 +2803,7 @@ metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, Basel if (!is_ok (error)) return NULL; - if (!parent_info->added_fields) { - parent_info->added_fields = g_ptr_array_new (); - } - - g_ptr_array_add (parent_info->added_fields, field); + parent_info->added_fields = g_slist_prepend_mem_manager (m_class_get_mem_manager (parent_klass), parent_info->added_fields, field); return field; } @@ -3007,9 +2996,7 @@ hot_reload_added_fields_iter (MonoClass *klass, gboolean lazy G_GNUC_UNUSED, gpo if (!info) return NULL; - /* FIXME: this needs locking in the multi-threaded case. There could be an update happening that resizes the array. */ - GPtrArray *added_fields = info->added_fields; - uint32_t count = added_fields->len; + GSList *added_fields = info->added_fields; // invariant: idx is one past the field we previously returned. uint32_t idx = GPOINTER_TO_UINT(*iter); @@ -3020,12 +3007,11 @@ hot_reload_added_fields_iter (MonoClass *klass, gboolean lazy G_GNUC_UNUSED, gpo mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Iterating added fields of 0x%08x idx = %u", m_class_get_type_token (klass), field_idx); - // if the field index is one past the last added field, we're done - if (field_idx == count) + GSList *field_node = g_slist_nth (added_fields, field_idx); + /* we reached the end, we're done */ + if (!field_node) return NULL; - g_assert (field_idx < count); - MonoClassMetadataUpdateField *field = g_ptr_array_index (added_fields, field_idx); - + MonoClassMetadataUpdateField *field = (MonoClassMetadataUpdateField *)field_node->data; idx++; *iter = GUINT_TO_POINTER (idx); return &field->field; diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index 68b655ce812efc..59dd6e6139dd1d 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -193,6 +193,9 @@ struct _MonoProperty { MonoMethod *get; MonoMethod *set; guint32 attrs; + /* added by metadata-update after class was created; + * not in MonoClassPropertyInfo array - don't do ptr arithmetic */ + guint32 from_update : 1; }; struct _MonoEvent { @@ -205,6 +208,9 @@ struct _MonoEvent { MonoMethod **other; #endif guint32 attrs; + /* added by metadata-update after class was created; + * not in MonoClassEventInfo array - don't do ptr arithmetic */ + guint32 from_update : 1; }; /* type of exception being "on hold" for later processing (see exception_type) */ @@ -1604,6 +1610,18 @@ m_field_is_from_update (MonoClassField *field) return (m_field_get_meta_flags (field) & MONO_CLASS_FIELD_META_FLAG_FROM_UPDATE) != 0; } +static inline gboolean +m_property_is_from_update (MonoProperty *prop) +{ + return prop->from_update != 0; +} + +static inline gboolean +m_event_is_from_update (MonoEvent *evt) +{ + return evt->from_update != 0; +} + /* * Memory allocation for images/classes/methods * diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index bfba08ecfb64bb..ce55091028fac6 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2653,6 +2653,8 @@ mono_class_get_event_token (MonoEvent *event) MonoClassEventInfo *info = mono_class_get_event_info (klass); if (info) { for (i = 0; i < info->count; ++i) { + /* TODO: metadata-update: get tokens for added props, too */ + g_assert (!m_event_is_from_update (&info->events[i])); if (&info->events [i] == event) return mono_metadata_make_token (MONO_TABLE_EVENT, info->first + i + 1); } @@ -2696,6 +2698,8 @@ mono_class_get_property_token (MonoProperty *prop) gpointer iter = NULL; MonoClassPropertyInfo *info = mono_class_get_property_info (klass); while ((p = mono_class_get_properties (klass, &iter))) { + /* TODO: metadata-update: get tokens for added props, too */ + g_assert (!m_property_is_from_update (p)); if (&info->properties [i] == prop) return mono_metadata_make_token (MONO_TABLE_PROPERTY, info->first + i + 1); From ccd590f927ea42a63180e86f791e0991184262fa Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 16:56:45 -0400 Subject: [PATCH 27/37] Reflection on props in new classes works --- .../ReflectionAddNewType_v1.cs | 5 ++ .../tests/ApplyUpdateTest.cs | 18 +++++++ .../mono/component/hot_reload-internals.h | 5 +- src/mono/mono/component/hot_reload-stub.c | 10 ++++ src/mono/mono/component/hot_reload.c | 50 ++++++++++++++++++- src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/metadata-update.c | 6 +++ src/mono/mono/metadata/metadata-update.h | 3 ++ src/mono/mono/metadata/metadata.c | 36 +++++++++---- 9 files changed, 122 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index b5267615fa0b1e..efc9cc0213ad2e 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -34,6 +34,11 @@ public virtual object Clone() { } public class AlsoNested { } + + public float NewProp {get; set;} + + public byte[] OtherNewProp {get; set;} + } public class NewGenericClass : NewToplevelClass { diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index b72012ed27482b..224bf9271b6308 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -553,6 +553,24 @@ public static void TestReflectionAddNewType() } Assert.True(hasICloneable); Assert.True(hasINewInterface); + + var allProperties = ty.GetProperties(); + + PropertyInfo newProp = null; + foreach (var prop in allProperties) + { + if (prop.Name == "NewProp") + newProp = prop; + } + Assert.NotNull(newProp); + + Assert.Equal(newProp, ty.GetProperty("NewProp")); + MethodInfo newPropGet = newProp.GetGetMethod(); + Assert.NotNull(newPropGet); + MethodInfo newPropSet = newProp.GetSetMethod(); + Assert.NotNull(newPropSet); + + Assert.Equal("get_NewProp", newPropGet.Name); }); CheckReflectedType(assm, allTypes, ns, "NewGenericClass`1"); CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index e90513fd6c5855..a668eb295c4388 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -54,7 +54,10 @@ typedef struct _MonoAddedDefSkeleton { uint32_t first_method_idx, first_field_idx; uint32_t method_count; uint32_t field_count; - /* TODO: metadata-update: property and event adds. */ + uint32_t first_prop_idx; + uint32_t prop_count; + uint32_t first_event_idx; + uint32_t event_count; } MonoAddedDefSkeleton; diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 94df6239b5540e..87fe960d7eb9c1 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -89,6 +89,9 @@ hot_reload_stub_find_method_by_name (MonoClass *klass, const char *name, int par static gboolean hot_reload_stub_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); +static gboolean +hot_reload_stub_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count); + static MonoMethod * hot_reload_stub_added_methods_iter (MonoClass *klass, gpointer *iter); @@ -121,6 +124,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_get_static_field_addr, &hot_reload_stub_find_method_by_name, &hot_reload_stub_get_typedef_skeleton, + &hot_reload_stub_get_typedef_skeleton_properties, &hot_reload_stub_added_methods_iter, &hot_reload_stub_added_fields_iter, }; @@ -281,6 +285,12 @@ hot_reload_stub_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_to return FALSE; } +static gboolean +hot_reload_stub_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count) +{ + return FALSE; +} + static MonoMethod * hot_reload_stub_added_methods_iter (MonoClass *klass, gpointer *iter) { diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index c00ee5241b9b88..b9182bad71dbbe 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -122,6 +122,9 @@ hot_reload_find_method_by_name (MonoClass *klass, const char *name, int param_co static gboolean hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); +static gboolean +hot_reload_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count); + static MonoMethod * hot_reload_added_methods_iter (MonoClass *klass, gpointer *iter); @@ -157,6 +160,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_static_field_addr, &hot_reload_find_method_by_name, &hot_reload_get_typedef_skeleton, + &hot_reload_get_typedef_skeleton_properties, &hot_reload_added_methods_iter, &hot_reload_added_fields_iter, }; @@ -1684,6 +1688,24 @@ skeleton_add_member (MonoAddedDefSkeleton *sk, uint32_t member_token) sk->field_count++; } break; + case MONO_TABLE_PROPERTY: + if (!sk->first_prop_idx) { + sk->first_prop_idx = idx; + sk->prop_count = 1; + } else { + g_assert (sk->first_prop_idx + sk->prop_count == idx); + sk->prop_count++; + } + break; + case MONO_TABLE_EVENT: + if (!sk->first_event_idx) { + sk->first_event_idx = idx; + sk->event_count = 1; + } else { + g_assert (sk->first_event_idx + sk->event_count == idx); + sk->event_count++; + } + break; default: g_error ("Expected method or field def token, got 0x%08x", member_token); g_assert_not_reached(); @@ -1788,6 +1810,29 @@ hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, return found; } +static gboolean +hot_reload_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count) +{ + BaselineInfo *info = baseline_info_lookup (base_image); + if (!info || !info->skeletons) + return FALSE; + gboolean found = FALSE; + mono_image_lock (base_image); + for (int i = 0; i < info->skeletons->len; ++i) { + MonoAddedDefSkeleton *sk = &((MonoAddedDefSkeleton*)info->skeletons->data)[i]; + if (sk->typedef_token == typedef_token) { + found = TRUE; + g_assert (first_prop_idx); + *first_prop_idx = sk->first_prop_idx; + g_assert (prop_count); + *prop_count = sk->prop_count; + break; + } + } + mono_image_unlock (base_image); + return found; +} + /** * Adds the given newly-added class to the image, updating various data structures (e.g. name cache). */ @@ -2169,7 +2214,8 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base if (pass2_context_is_skeleton (ctx, parent_type_token)) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new property 0x%08x to new class 0x%08x", log_token, parent_type_token); - /* nothing to do, actually. the metadata lookup machinery will find it by doing a linear scan of the table. */ + pass2_context_add_skeleton_member (ctx, parent_type_token, log_token); + add_member_parent (base_info, parent_type_token, log_token); break; } else { MonoClass *add_property_klass = mono_class_get_checked (image_base, parent_type_token, error); @@ -2179,7 +2225,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base } mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new property 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_property_klass), m_class_get_name (add_property_klass)); - /* TODO: metadata-update: add a new MonoPropertyInfo to the bag on the class? */ + /* TODO: metadata-update: add a new MonoClassMetadataUpdatePropertyInfo to added_props */ break; } diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 7b3baaffa81393..fca388c5d667c4 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -41,6 +41,7 @@ typedef struct _MonoComponentHotReload { gpointer (*get_static_field_addr) (MonoClassField *field); MonoMethod* (*find_method_by_name) (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); gboolean (*get_typedef_skeleton) (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); + gboolean (*get_typedef_skeleton_properties) (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count); MonoMethod* (*added_methods_iter) (MonoClass *klass, gpointer *iter); MonoClassField* (*added_fields_iter) (MonoClass *klass, gboolean lazy, gpointer *iter); } MonoComponentHotReload; diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index d5128b43ae285d..35c6a603cafeeb 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -189,6 +189,12 @@ mono_metadata_update_get_typedef_skeleton (MonoImage *base_image, uint32_t typed return mono_component_hot_reload()->get_typedef_skeleton (base_image, typedef_token, first_method_idx, method_count, first_field_idx, field_count); } +gboolean +metadata_update_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count) +{ + return mono_component_hot_reload()->get_typedef_skeleton_properties (base_image, typedef_token, first_prop_idx, prop_count); +} + MonoMethod * mono_metadata_update_added_methods_iter (MonoClass *klass, gpointer *iter) { diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index b7141b2b0ad104..9637050f5c1a3e 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -73,6 +73,9 @@ mono_metadata_update_get_field (MonoClass *klass, uint32_t fielddef_token); gboolean mono_metadata_update_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); +gboolean +metadata_update_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count); + gpointer mono_metadata_update_get_static_field_addr (MonoClassField *field); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 48a902b19b5a2c..80af3e707bc997 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6393,23 +6393,36 @@ mono_metadata_properties_from_typedef (MonoImage *meta, guint32 index, guint *en *end_idx = 0; - if (!tdef->base) + if (!tdef->base && !meta->has_updates) return 0; loc.t = tdef; loc.col_idx = MONO_PROPERTY_MAP_PARENT; loc.idx = index + 1; - /* FIXME: metadata-update */ + gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) != NULL; - if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) + if (!found && !meta->has_updates) return 0; + gboolean found_update = FALSE; + if (G_UNLIKELY (meta->has_updates)) { + if (!found) { + uint32_t count; + if (metadata_update_get_typedef_skeleton_properties (meta, mono_metadata_make_token (MONO_TABLE_TYPEDEF, index + 1), &start, &count)) { + *end_idx = start + count - 1; + return start - 1; + } else { + return 0; + } + } + } + start = mono_metadata_decode_row_col (tdef, loc.result, MONO_PROPERTY_MAP_PROPERTY_LIST); - if (loc.result + 1 < table_info_get_rows (tdef)) { + if (loc.result + 1 < mono_metadata_table_num_rows (meta, MONO_TABLE_PROPERTYMAP)) { end = mono_metadata_decode_row_col (tdef, loc.result + 1, MONO_PROPERTY_MAP_PROPERTY_LIST) - 1; } else { - end = table_info_get_rows (&meta->tables [MONO_TABLE_PROPERTY]); + end = mono_metadata_table_num_rows (meta, MONO_TABLE_PROPERTY); } *end_idx = end; @@ -6433,7 +6446,7 @@ mono_metadata_methods_from_property (MonoImage *meta, guint32 index, guint *en MonoTableInfo *msemt = &meta->tables [MONO_TABLE_METHODSEMANTICS]; *end_idx = 0; - if (!msemt->base) + if (!msemt->base && !meta->has_updates) return 0; if (meta->uncompressed_metadata) @@ -6443,11 +6456,16 @@ mono_metadata_methods_from_property (MonoImage *meta, guint32 index, guint *en loc.col_idx = MONO_METHOD_SEMA_ASSOCIATION; loc.idx = ((index + 1) << MONO_HAS_SEMANTICS_BITS) | MONO_HAS_SEMANTICS_PROPERTY; /* Method association coded index */ - /* FIXME: metadata-update */ + gboolean found = msemt->base && mono_binary_search (&loc, msemt->base, table_info_get_rows (msemt), msemt->row_size, table_locator) != NULL; - if (!mono_binary_search (&loc, msemt->base, table_info_get_rows (msemt), msemt->row_size, table_locator)) + if (!found && !meta->has_updates) return 0; + if (G_UNLIKELY (meta->has_updates)) { + if (!found && !mono_metadata_update_metadata_linear_search (meta, msemt, &loc, table_locator)) + return 0; + } + start = loc.result; /* * We may end up in the middle of the rows... @@ -6459,7 +6477,7 @@ mono_metadata_methods_from_property (MonoImage *meta, guint32 index, guint *en break; } end = start + 1; - int rows = table_info_get_rows (msemt); + int rows = mono_metadata_table_num_rows (meta, MONO_TABLE_METHODSEMANTICS); while (end < rows) { mono_metadata_decode_row (msemt, end, cols, MONO_METHOD_SEMA_SIZE); if (cols [MONO_METHOD_SEMA_ASSOCIATION] != loc.idx) From 4c8e0f1aabe529f027c2c0f750afcf9e19aa96ff Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 20:37:39 -0400 Subject: [PATCH 28/37] nit: unused variable --- src/mono/mono/metadata/metadata.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 80af3e707bc997..ad5d5577b4cdb9 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6405,7 +6405,6 @@ mono_metadata_properties_from_typedef (MonoImage *meta, guint32 index, guint *en if (!found && !meta->has_updates) return 0; - gboolean found_update = FALSE; if (G_UNLIKELY (meta->has_updates)) { if (!found) { uint32_t count; From a9d12935fbf44615c57283b8d3f40ddde8c58e2e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 20:37:50 -0400 Subject: [PATCH 29/37] events on new classes work --- .../ReflectionAddNewType_v1.cs | 2 ++ .../tests/ApplyUpdateTest.cs | 18 +++++++++++ src/mono/mono/component/hot_reload-stub.c | 10 ++++++ src/mono/mono/component/hot_reload.c | 30 ++++++++++++++++- src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/metadata-update.c | 6 ++++ src/mono/mono/metadata/metadata-update.h | 3 ++ src/mono/mono/metadata/metadata.c | 32 ++++++++++++++----- 8 files changed, 93 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index efc9cc0213ad2e..fc564153ca84aa 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -39,6 +39,8 @@ public class AlsoNested { } public byte[] OtherNewProp {get; set;} + public event EventHandler NewEvent; + public event EventHandler OtherNewEvent; } public class NewGenericClass : NewToplevelClass { diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 224bf9271b6308..0e8651a4ef6418 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -571,6 +571,24 @@ public static void TestReflectionAddNewType() Assert.NotNull(newPropSet); Assert.Equal("get_NewProp", newPropGet.Name); + + var allEvents = ty.GetEvents(); + + EventInfo newEvt = null; + foreach (var evt in allEvents) + { + if (evt.Name == "NewEvent") + newEvt = evt; + } + Assert.NotNull(newEvt); + + Assert.Equal(newEvt, ty.GetEvent("NewEvent")); + MethodInfo newEvtAdd = newEvt.GetAddMethod(); + Assert.NotNull(newEvtAdd); + MethodInfo newEvtRemove = newEvt.GetRemoveMethod(); + Assert.NotNull(newEvtRemove); + + Assert.Equal("add_NewEvent", newEvtAdd.Name); }); CheckReflectedType(assm, allTypes, ns, "NewGenericClass`1"); CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 87fe960d7eb9c1..5043423c203b0f 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -92,6 +92,9 @@ hot_reload_stub_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_to static gboolean hot_reload_stub_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count); +static gboolean +hot_reload_stub_get_typedef_skeleton_events (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_event_idx, uint32_t *event_count); + static MonoMethod * hot_reload_stub_added_methods_iter (MonoClass *klass, gpointer *iter); @@ -125,6 +128,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_find_method_by_name, &hot_reload_stub_get_typedef_skeleton, &hot_reload_stub_get_typedef_skeleton_properties, + &hot_reload_stub_get_typedef_skeleton_events, &hot_reload_stub_added_methods_iter, &hot_reload_stub_added_fields_iter, }; @@ -291,6 +295,12 @@ hot_reload_stub_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t return FALSE; } +static gboolean +hot_reload_stub_get_typedef_skeleton_events (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_event_idx, uint32_t *event_count) +{ + return FALSE; +} + static MonoMethod * hot_reload_stub_added_methods_iter (MonoClass *klass, gpointer *iter) { diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index b9182bad71dbbe..2053e7d39fdce2 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -125,6 +125,9 @@ hot_reload_get_typedef_skeleton (MonoImage *base_image, uint32_t typedef_token, static gboolean hot_reload_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count); +static gboolean +hot_reload_get_typedef_skeleton_events (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_event_idx, uint32_t *event_count); + static MonoMethod * hot_reload_added_methods_iter (MonoClass *klass, gpointer *iter); @@ -161,6 +164,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_find_method_by_name, &hot_reload_get_typedef_skeleton, &hot_reload_get_typedef_skeleton_properties, + &hot_reload_get_typedef_skeleton_events, &hot_reload_added_methods_iter, &hot_reload_added_fields_iter, }; @@ -1833,6 +1837,29 @@ hot_reload_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t type return found; } +static gboolean +hot_reload_get_typedef_skeleton_events (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_event_idx, uint32_t *event_count) +{ + BaselineInfo *info = baseline_info_lookup (base_image); + if (!info || !info->skeletons) + return FALSE; + gboolean found = FALSE; + mono_image_lock (base_image); + for (int i = 0; i < info->skeletons->len; ++i) { + MonoAddedDefSkeleton *sk = &((MonoAddedDefSkeleton*)info->skeletons->data)[i]; + if (sk->typedef_token == typedef_token) { + found = TRUE; + g_assert (first_event_idx); + *first_event_idx = sk->first_event_idx; + g_assert (event_count); + *event_count = sk->event_count; + break; + } + } + mono_image_unlock (base_image); + return found; +} + /** * Adds the given newly-added class to the image, updating various data structures (e.g. name cache). */ @@ -2268,7 +2295,8 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base if (pass2_context_is_skeleton (ctx, parent_type_token)) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new event 0x%08x to new class 0x%08x", log_token, parent_type_token); - /* nothing to do, actually. the metadata lookup machinery will find it by doing a linear scan of the table. */ + pass2_context_add_skeleton_member (ctx, parent_type_token, log_token); + add_member_parent (base_info, parent_type_token, log_token); break; } else { MonoClass *add_event_klass = mono_class_get_checked (image_base, parent_type_token, error); diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index fca388c5d667c4..62c7c305c5ed7d 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -42,6 +42,7 @@ typedef struct _MonoComponentHotReload { MonoMethod* (*find_method_by_name) (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); gboolean (*get_typedef_skeleton) (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_method_idx, uint32_t *method_count, uint32_t *first_field_idx, uint32_t *field_count); gboolean (*get_typedef_skeleton_properties) (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count); + gboolean (*get_typedef_skeleton_events) (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_event_idx, uint32_t *event_count); MonoMethod* (*added_methods_iter) (MonoClass *klass, gpointer *iter); MonoClassField* (*added_fields_iter) (MonoClass *klass, gboolean lazy, gpointer *iter); } MonoComponentHotReload; diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 35c6a603cafeeb..bddbfa129e8f5d 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -195,6 +195,12 @@ metadata_update_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t return mono_component_hot_reload()->get_typedef_skeleton_properties (base_image, typedef_token, first_prop_idx, prop_count); } +gboolean +metadata_update_get_typedef_skeleton_events (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_event_idx, uint32_t *event_count) +{ + return mono_component_hot_reload()->get_typedef_skeleton_events (base_image, typedef_token, first_event_idx, event_count); +} + MonoMethod * mono_metadata_update_added_methods_iter (MonoClass *klass, gpointer *iter) { diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 9637050f5c1a3e..1cf2200e6d511d 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -76,6 +76,9 @@ mono_metadata_update_get_typedef_skeleton (MonoImage *base_image, uint32_t typed gboolean metadata_update_get_typedef_skeleton_properties (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_prop_idx, uint32_t *prop_count); +gboolean +metadata_update_get_typedef_skeleton_events (MonoImage *base_image, uint32_t typedef_token, uint32_t *first_event_idx, uint32_t *event_count); + gpointer mono_metadata_update_get_static_field_addr (MonoClassField *field); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index ad5d5577b4cdb9..d89ff962633ff7 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6299,18 +6299,29 @@ mono_metadata_events_from_typedef (MonoImage *meta, guint32 index, guint *end_id *end_idx = 0; - if (!tdef->base) + if (!tdef->base && !meta->has_updates) return 0; loc.t = tdef; loc.col_idx = MONO_EVENT_MAP_PARENT; loc.idx = index + 1; - /* FIXME: metadata-update */ - - if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) + gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) != NULL; + if (!found && !meta->has_updates) return 0; + if (G_UNLIKELY (meta->has_updates)) { + if (!found) { + uint32_t count; + if (metadata_update_get_typedef_skeleton_events (meta, mono_metadata_make_token (MONO_TABLE_TYPEDEF, index + 1), &start, &count)) { + *end_idx = start + count - 1; + return start - 1; + } else { + return 0; + } + } + } + start = mono_metadata_decode_row_col (tdef, loc.result, MONO_EVENT_MAP_EVENTLIST); if (loc.result + 1 < table_info_get_rows (tdef)) { end = mono_metadata_decode_row_col (tdef, loc.result + 1, MONO_EVENT_MAP_EVENTLIST) - 1; @@ -6339,7 +6350,7 @@ mono_metadata_methods_from_event (MonoImage *meta, guint32 index, guint *end_i MonoTableInfo *msemt = &meta->tables [MONO_TABLE_METHODSEMANTICS]; *end_idx = 0; - if (!msemt->base) + if (!msemt->base && !meta->has_updates) return 0; if (meta->uncompressed_metadata) @@ -6349,11 +6360,16 @@ mono_metadata_methods_from_event (MonoImage *meta, guint32 index, guint *end_i loc.col_idx = MONO_METHOD_SEMA_ASSOCIATION; loc.idx = ((index + 1) << MONO_HAS_SEMANTICS_BITS) | MONO_HAS_SEMANTICS_EVENT; /* Method association coded index */ - /* FIXME: metadata-update */ + gboolean found = msemt->base && mono_binary_search (&loc, msemt->base, table_info_get_rows (msemt), msemt->row_size, table_locator) != NULL; - if (!mono_binary_search (&loc, msemt->base, table_info_get_rows (msemt), msemt->row_size, table_locator)) + if (!found && !meta->has_updates) return 0; + if (G_UNLIKELY (meta->has_updates)) { + if (!found && !mono_metadata_update_metadata_linear_search (meta, msemt, &loc, table_locator)) + return 0; + } + start = loc.result; /* * We may end up in the middle of the rows... @@ -6365,7 +6381,7 @@ mono_metadata_methods_from_event (MonoImage *meta, guint32 index, guint *end_i break; } end = start + 1; - int rows = table_info_get_rows (msemt); + int rows = mono_metadata_table_num_rows (meta, MONO_TABLE_METHODSEMANTICS); while (end < rows) { mono_metadata_decode_row (msemt, end, cols, MONO_METHOD_SEMA_SIZE); if (cols [MONO_METHOD_SEMA_ASSOCIATION] != loc.idx) From fa0ec655b18ccf985ac83d3a7f2511895280837f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 21:50:54 -0400 Subject: [PATCH 30/37] Add CustomAttribute reflection test --- .../ReflectionAddNewType.cs | 7 +++++ .../ReflectionAddNewType_v1.cs | 12 +++++++ .../tests/ApplyUpdateTest.cs | 31 +++++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs index 43a636d1f65fdc..23c961bd9697e0 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs @@ -13,3 +13,10 @@ public class ZExistingClass { public class PreviousNestedClass { } } + +[AttributeUsage(AttributeTargets.All, AllowMultiple=true, Inherited=false)] +public class CustomNoteAttribute : Attribute { + public CustomNoteAttribute(string note) {Note = note;} + + public string Note; +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index fc564153ca84aa..4da606755c4af3 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -24,17 +24,29 @@ public class NewNestedClass {}; public double NewProp { get; set; } } +[AttributeUsage(AttributeTargets.All, AllowMultiple=true, Inherited=false)] +public class CustomNoteAttribute : Attribute { + public CustomNoteAttribute(string note) {Note = note;} + + public string Note; +} + +[CustomNote("123")] public class NewToplevelClass : IExistingInterface, ICloneable { public string ItfMethod(int i) { return i.ToString(); } + [CustomNote("abcd")] + public void SomeMethod(int x) {} + public virtual object Clone() { return new NewToplevelClass(); } public class AlsoNested { } + [CustomNote("hijkl")] public float NewProp {get; set;} public byte[] OtherNewProp {get; set;} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 0e8651a4ef6418..39186e9ca5d385 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -453,6 +453,33 @@ internal static Type CheckReflectedType(Assembly assm, Type[] allTypes, string n return ty; } + + internal static void CheckCustomNoteAttribute(MemberInfo subject, string expectedAttributeValue, [CallerMemberName] string callerMemberName = "", [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0) + { + var attrData = subject.GetCustomAttributesData(); + CustomAttributeData noteData = null; + foreach (var cad in attrData) + { + if (cad.AttributeType.FullName.Contains("CustomNoteAttribute")) + noteData = cad; + } + Assert.True(noteData != null, $"{callerFilePath}:{callerLineNumber}: expected a CustomNoteAttribute attributes on '{subject.Name}', but got null, in {callerMemberName}"); + Assert.True(1 == noteData.ConstructorArguments.Count, $"{callerFilePath}:{callerLineNumber}: expected exactly 1 constructor argument on CustomNoteAttribute, got {noteData.ConstructorArguments.Count}, in {callerMemberName}"); + object argVal = noteData.ConstructorArguments[0].Value; + Assert.True(expectedAttributeValue.Equals(argVal), $"{callerFilePath}:{callerLineNumber}: expected '{expectedAttributeValue}' as CustomNoteAttribute argument, got '{argVal}', in {callerMemberName}"); + + var attrs = subject.GetCustomAttributes(false); + object note = null; + foreach (var attr in attrs) + { + if (attr.GetType().FullName.Contains("CustomNoteAttribute")) + note = attr; + } + Assert.True(note != null, $"{callerFilePath}:{callerLineNumber}: expected a CustomNoteAttribute object on '{subject.Name}', but got null, in {callerMemberName}"); + object v = note.GetType().GetField("Note").GetValue(note); + Assert.True(expectedAttributeValue.Equals(v), $"{callerFilePath}:{callerLineNumber}: expected '{expectedAttributeValue}' in CustomNoteAttribute Note field, but got '{v}', in {callerMemberName}"); + } + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestReflectionAddNewType() { @@ -531,6 +558,8 @@ public static void TestReflectionAddNewType() var newTy = CheckReflectedType(assm, allTypes, ns, "NewToplevelClass", static (ty) => { + CheckCustomNoteAttribute(ty, "123"); + var nested = ty.GetNestedType("AlsoNested"); var allNested = ty.GetNestedTypes(); @@ -572,6 +601,8 @@ public static void TestReflectionAddNewType() Assert.Equal("get_NewProp", newPropGet.Name); + CheckCustomNoteAttribute (newProp, "hijkl"); + var allEvents = ty.GetEvents(); EventInfo newEvt = null; From 0b61b588b4f9159b72a5112617d4519dad57c486 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 22:22:09 -0400 Subject: [PATCH 31/37] fix whitespace --- .../AddNestedClass_v1.cs | 32 +- .../ReflectionAddNewType_v1.cs | 2 +- .../tests/ApplyUpdateTest.cs | 414 +++++++++--------- 3 files changed, 224 insertions(+), 224 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs index 7e57caa71e7a83..fa8f5ed30b564f 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs @@ -15,35 +15,35 @@ public string TestMethod() { var n = new Nested(); n.Eff = "123"; - n.g = 456; - n.Evt += new Action (n.DefaultHandler); - n.RaiseEvt(); + n.g = 456; + n.Evt += new Action (n.DefaultHandler); + n.RaiseEvt(); return n.M() + n.buf; } private class Nested { public Nested() { } internal T f; - internal U g; - public T Eff { - get => f; - set { f = value; } - } + internal U g; + public T Eff { + get => f; + set { f = value; } + } public string M () { return Eff.ToString() + g.ToString(); } - public event Action Evt; + public event Action Evt; - public void RaiseEvt () { - Evt ("789"); - } + public void RaiseEvt () { + Evt ("789"); + } - public string buf; + public string buf; - public void DefaultHandler (string s) { - this.buf = s; - } + public void DefaultHandler (string s) { + this.buf = s; + } } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index 4da606755c4af3..d3a535da84b4df 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -57,7 +57,7 @@ public class AlsoNested { } public class NewGenericClass : NewToplevelClass { public override object Clone() { - return new NewGenericClass(); + return new NewGenericClass(); } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 39186e9ca5d385..687812e5c3a8aa 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -428,219 +428,219 @@ public static void TestStaticLambdaRegression() }); } - private static bool ContainsTypeWithName(Type[] types, string fullName) - { - foreach (var ty in types) { - if (ty.FullName == fullName) - return true; - } - return false; - } - - internal static Type CheckReflectedType(Assembly assm, Type[] allTypes, string nameSpace, string typeName, Action moreChecks = null, [CallerMemberName] string callerMemberName = "", [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0) - { - var fullName = $"{nameSpace}.{typeName}"; - var ty = assm.GetType(fullName); - Assert.True(ty != null, $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetType for '{typeName}' to return non-null in {callerMemberName}"); - int nestedIdx = typeName.LastIndexOf('+'); - string comparisonName = typeName; - if (nestedIdx != -1) - comparisonName = typeName.Substring(nestedIdx+1); - Assert.True(comparisonName == ty.Name, $"{callerFilePath}:{callerLineNumber}: returned type has unexpected name '{ty.Name}' (expected: '{comparisonName}') in {callerMemberName}"); - Assert.True(ContainsTypeWithName (allTypes, fullName), $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetTypes to contain '{fullName}', but it didn't in {callerMemberName}"); - if (moreChecks != null) - moreChecks(ty); - return ty; - } - - - internal static void CheckCustomNoteAttribute(MemberInfo subject, string expectedAttributeValue, [CallerMemberName] string callerMemberName = "", [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0) - { - var attrData = subject.GetCustomAttributesData(); - CustomAttributeData noteData = null; - foreach (var cad in attrData) - { - if (cad.AttributeType.FullName.Contains("CustomNoteAttribute")) - noteData = cad; - } - Assert.True(noteData != null, $"{callerFilePath}:{callerLineNumber}: expected a CustomNoteAttribute attributes on '{subject.Name}', but got null, in {callerMemberName}"); - Assert.True(1 == noteData.ConstructorArguments.Count, $"{callerFilePath}:{callerLineNumber}: expected exactly 1 constructor argument on CustomNoteAttribute, got {noteData.ConstructorArguments.Count}, in {callerMemberName}"); - object argVal = noteData.ConstructorArguments[0].Value; - Assert.True(expectedAttributeValue.Equals(argVal), $"{callerFilePath}:{callerLineNumber}: expected '{expectedAttributeValue}' as CustomNoteAttribute argument, got '{argVal}', in {callerMemberName}"); - - var attrs = subject.GetCustomAttributes(false); - object note = null; - foreach (var attr in attrs) - { - if (attr.GetType().FullName.Contains("CustomNoteAttribute")) - note = attr; - } - Assert.True(note != null, $"{callerFilePath}:{callerLineNumber}: expected a CustomNoteAttribute object on '{subject.Name}', but got null, in {callerMemberName}"); - object v = note.GetType().GetField("Note").GetValue(note); - Assert.True(expectedAttributeValue.Equals(v), $"{callerFilePath}:{callerLineNumber}: expected '{expectedAttributeValue}' in CustomNoteAttribute Note field, but got '{v}', in {callerMemberName}"); - } - - [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] - public static void TestReflectionAddNewType() - { - ApplyUpdateUtil.TestCase(static () => - { - const string ns = "System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType"; - var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.ZExistingClass).Assembly; - - var allTypes = assm.GetTypes(); - - CheckReflectedType(assm, allTypes, ns, "ZExistingClass"); - CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); - - ApplyUpdateUtil.ApplyUpdate(assm); - - allTypes = assm.GetTypes(); - - CheckReflectedType(assm, allTypes, ns, "ZExistingClass", static (ty) => - { - var allMethods = ty.GetMethods(); - - MethodInfo newMethod = null; - foreach (var meth in allMethods) - { - if (meth.Name == "NewMethod") - newMethod = meth; - } - Assert.NotNull (newMethod); - - Assert.Equal (newMethod, ty.GetMethod ("NewMethod")); - - var allFields = ty.GetFields(BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public); - - FieldInfo newField = null; - FieldInfo newStaticField = null; - foreach (var fld in allFields) - { - if (fld.Name == "NewField") - newField = fld; - if (fld.Name == "NewStaticField") - newStaticField = fld; - } - Assert.NotNull(newField); - Assert.NotNull(newStaticField); - - Assert.Equal(newField, ty.GetField("NewField")); - Assert.Equal(newStaticField, ty.GetField("NewStaticField", BindingFlags.Static | BindingFlags.Public)); - - // FIXME: finish this + private static bool ContainsTypeWithName(Type[] types, string fullName) + { + foreach (var ty in types) { + if (ty.FullName == fullName) + return true; + } + return false; + } + + internal static Type CheckReflectedType(Assembly assm, Type[] allTypes, string nameSpace, string typeName, Action moreChecks = null, [CallerMemberName] string callerMemberName = "", [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0) + { + var fullName = $"{nameSpace}.{typeName}"; + var ty = assm.GetType(fullName); + Assert.True(ty != null, $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetType for '{typeName}' to return non-null in {callerMemberName}"); + int nestedIdx = typeName.LastIndexOf('+'); + string comparisonName = typeName; + if (nestedIdx != -1) + comparisonName = typeName.Substring(nestedIdx+1); + Assert.True(comparisonName == ty.Name, $"{callerFilePath}:{callerLineNumber}: returned type has unexpected name '{ty.Name}' (expected: '{comparisonName}') in {callerMemberName}"); + Assert.True(ContainsTypeWithName (allTypes, fullName), $"{callerFilePath}:{callerLineNumber}: expected Assembly.GetTypes to contain '{fullName}', but it didn't in {callerMemberName}"); + if (moreChecks != null) + moreChecks(ty); + return ty; + } + + + internal static void CheckCustomNoteAttribute(MemberInfo subject, string expectedAttributeValue, [CallerMemberName] string callerMemberName = "", [CallerFilePath] string callerFilePath = "", [CallerLineNumber] int callerLineNumber = 0) + { + var attrData = subject.GetCustomAttributesData(); + CustomAttributeData noteData = null; + foreach (var cad in attrData) + { + if (cad.AttributeType.FullName.Contains("CustomNoteAttribute")) + noteData = cad; + } + Assert.True(noteData != null, $"{callerFilePath}:{callerLineNumber}: expected a CustomNoteAttribute attributes on '{subject.Name}', but got null, in {callerMemberName}"); + Assert.True(1 == noteData.ConstructorArguments.Count, $"{callerFilePath}:{callerLineNumber}: expected exactly 1 constructor argument on CustomNoteAttribute, got {noteData.ConstructorArguments.Count}, in {callerMemberName}"); + object argVal = noteData.ConstructorArguments[0].Value; + Assert.True(expectedAttributeValue.Equals(argVal), $"{callerFilePath}:{callerLineNumber}: expected '{expectedAttributeValue}' as CustomNoteAttribute argument, got '{argVal}', in {callerMemberName}"); + + var attrs = subject.GetCustomAttributes(false); + object note = null; + foreach (var attr in attrs) + { + if (attr.GetType().FullName.Contains("CustomNoteAttribute")) + note = attr; + } + Assert.True(note != null, $"{callerFilePath}:{callerLineNumber}: expected a CustomNoteAttribute object on '{subject.Name}', but got null, in {callerMemberName}"); + object v = note.GetType().GetField("Note").GetValue(note); + Assert.True(expectedAttributeValue.Equals(v), $"{callerFilePath}:{callerLineNumber}: expected '{expectedAttributeValue}' in CustomNoteAttribute Note field, but got '{v}', in {callerMemberName}"); + } + + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestReflectionAddNewType() + { + ApplyUpdateUtil.TestCase(static () => + { + const string ns = "System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType"; + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.ZExistingClass).Assembly; + + var allTypes = assm.GetTypes(); + + CheckReflectedType(assm, allTypes, ns, "ZExistingClass"); + CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); + + ApplyUpdateUtil.ApplyUpdate(assm); + + allTypes = assm.GetTypes(); + + CheckReflectedType(assm, allTypes, ns, "ZExistingClass", static (ty) => + { + var allMethods = ty.GetMethods(); + + MethodInfo newMethod = null; + foreach (var meth in allMethods) + { + if (meth.Name == "NewMethod") + newMethod = meth; + } + Assert.NotNull (newMethod); + + Assert.Equal (newMethod, ty.GetMethod ("NewMethod")); + + var allFields = ty.GetFields(BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public); + + FieldInfo newField = null; + FieldInfo newStaticField = null; + foreach (var fld in allFields) + { + if (fld.Name == "NewField") + newField = fld; + if (fld.Name == "NewStaticField") + newStaticField = fld; + } + Assert.NotNull(newField); + Assert.NotNull(newStaticField); + + Assert.Equal(newField, ty.GetField("NewField")); + Assert.Equal(newStaticField, ty.GetField("NewStaticField", BindingFlags.Static | BindingFlags.Public)); + + // FIXME: finish this #if false - var allProperties = ty.GetProperties(); + var allProperties = ty.GetProperties(); - PropertyInfo newProp = null; - foreach (var prop in allProperties) - { - if (prop.Name == "NewProp") - newProp = prop; - } - Assert.NotNull(newProp); + PropertyInfo newProp = null; + foreach (var prop in allProperties) + { + if (prop.Name == "NewProp") + newProp = prop; + } + Assert.NotNull(newProp); - Assert.Equal(newProp, ty.GetProperty("NewProp")); + Assert.Equal(newProp, ty.GetProperty("NewProp")); - MethodInfo newPropGet = newProp.GetGetMethod(); - Assert.NotNull(newPropGet); - MethodInfo newPropSet = newProp.GetSetMethod(); - Assert.NotNull(newPropSet); + MethodInfo newPropGet = newProp.GetGetMethod(); + Assert.NotNull(newPropGet); + MethodInfo newPropSet = newProp.GetSetMethod(); + Assert.NotNull(newPropSet); - Assert.Equal("get_NewProp", newPropGet.Name); + Assert.Equal("get_NewProp", newPropGet.Name); #endif - }); - CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); - CheckReflectedType(assm, allTypes, ns, "IExistingInterface"); - - CheckReflectedType(assm, allTypes, ns, "ZExistingClass+NewNestedClass"); - - var newTy = CheckReflectedType(assm, allTypes, ns, "NewToplevelClass", static (ty) => - { - CheckCustomNoteAttribute(ty, "123"); - - var nested = ty.GetNestedType("AlsoNested"); - var allNested = ty.GetNestedTypes(); - - Assert.Equal("AlsoNested", nested.Name); - Assert.Same(ty, nested.DeclaringType); - - Assert.Equal(1, allNested.Length); - Assert.Same(nested, allNested[0]); - - var allInterfaces = ty.GetInterfaces(); - - Assert.Equal (2, allInterfaces.Length); - bool hasICloneable = false, hasINewInterface = false; - for (int i = 0; i < allInterfaces.Length; ++i) { - var itf = allInterfaces[i]; - if (itf.Name == "ICloneable") - hasICloneable = true; - if (itf.Name == "IExistingInterface") - hasINewInterface = true; - } - Assert.True(hasICloneable); - Assert.True(hasINewInterface); - - var allProperties = ty.GetProperties(); - - PropertyInfo newProp = null; - foreach (var prop in allProperties) - { - if (prop.Name == "NewProp") - newProp = prop; - } - Assert.NotNull(newProp); - - Assert.Equal(newProp, ty.GetProperty("NewProp")); - MethodInfo newPropGet = newProp.GetGetMethod(); - Assert.NotNull(newPropGet); - MethodInfo newPropSet = newProp.GetSetMethod(); - Assert.NotNull(newPropSet); - - Assert.Equal("get_NewProp", newPropGet.Name); - - CheckCustomNoteAttribute (newProp, "hijkl"); - - var allEvents = ty.GetEvents(); - - EventInfo newEvt = null; - foreach (var evt in allEvents) - { - if (evt.Name == "NewEvent") - newEvt = evt; - } - Assert.NotNull(newEvt); - - Assert.Equal(newEvt, ty.GetEvent("NewEvent")); - MethodInfo newEvtAdd = newEvt.GetAddMethod(); - Assert.NotNull(newEvtAdd); - MethodInfo newEvtRemove = newEvt.GetRemoveMethod(); - Assert.NotNull(newEvtRemove); - - Assert.Equal("add_NewEvent", newEvtAdd.Name); - }); - CheckReflectedType(assm, allTypes, ns, "NewGenericClass`1"); - CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); - CheckReflectedType(assm, allTypes, ns, "INewInterface"); - CheckReflectedType(assm, allTypes, ns, "NewEnum", static (ty) => { - var names = Enum.GetNames (ty); - Assert.Equal(3, names.Length); - var vals = Enum.GetValues (ty); - Assert.Equal(3, vals.Length); - - Assert.NotNull(Enum.Parse (ty, "Red")); - Assert.NotNull(Enum.Parse (ty, "Yellow")); - }); - - // make some instances using reflection and use them through known interfaces - var o = Activator.CreateInstance(newTy); - - var i = (System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.IExistingInterface)o; - - Assert.Equal("123", i.ItfMethod(123)); - }); - } + }); + CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); + CheckReflectedType(assm, allTypes, ns, "IExistingInterface"); + + CheckReflectedType(assm, allTypes, ns, "ZExistingClass+NewNestedClass"); + + var newTy = CheckReflectedType(assm, allTypes, ns, "NewToplevelClass", static (ty) => + { + CheckCustomNoteAttribute(ty, "123"); + + var nested = ty.GetNestedType("AlsoNested"); + var allNested = ty.GetNestedTypes(); + + Assert.Equal("AlsoNested", nested.Name); + Assert.Same(ty, nested.DeclaringType); + + Assert.Equal(1, allNested.Length); + Assert.Same(nested, allNested[0]); + + var allInterfaces = ty.GetInterfaces(); + + Assert.Equal (2, allInterfaces.Length); + bool hasICloneable = false, hasINewInterface = false; + for (int i = 0; i < allInterfaces.Length; ++i) { + var itf = allInterfaces[i]; + if (itf.Name == "ICloneable") + hasICloneable = true; + if (itf.Name == "IExistingInterface") + hasINewInterface = true; + } + Assert.True(hasICloneable); + Assert.True(hasINewInterface); + + var allProperties = ty.GetProperties(); + + PropertyInfo newProp = null; + foreach (var prop in allProperties) + { + if (prop.Name == "NewProp") + newProp = prop; + } + Assert.NotNull(newProp); + + Assert.Equal(newProp, ty.GetProperty("NewProp")); + MethodInfo newPropGet = newProp.GetGetMethod(); + Assert.NotNull(newPropGet); + MethodInfo newPropSet = newProp.GetSetMethod(); + Assert.NotNull(newPropSet); + + Assert.Equal("get_NewProp", newPropGet.Name); + + CheckCustomNoteAttribute (newProp, "hijkl"); + + var allEvents = ty.GetEvents(); + + EventInfo newEvt = null; + foreach (var evt in allEvents) + { + if (evt.Name == "NewEvent") + newEvt = evt; + } + Assert.NotNull(newEvt); + + Assert.Equal(newEvt, ty.GetEvent("NewEvent")); + MethodInfo newEvtAdd = newEvt.GetAddMethod(); + Assert.NotNull(newEvtAdd); + MethodInfo newEvtRemove = newEvt.GetRemoveMethod(); + Assert.NotNull(newEvtRemove); + + Assert.Equal("add_NewEvent", newEvtAdd.Name); + }); + CheckReflectedType(assm, allTypes, ns, "NewGenericClass`1"); + CheckReflectedType(assm, allTypes, ns, "NewToplevelStruct"); + CheckReflectedType(assm, allTypes, ns, "INewInterface"); + CheckReflectedType(assm, allTypes, ns, "NewEnum", static (ty) => { + var names = Enum.GetNames (ty); + Assert.Equal(3, names.Length); + var vals = Enum.GetValues (ty); + Assert.Equal(3, vals.Length); + + Assert.NotNull(Enum.Parse (ty, "Red")); + Assert.NotNull(Enum.Parse (ty, "Yellow")); + }); + + // make some instances using reflection and use them through known interfaces + var o = Activator.CreateInstance(newTy); + + var i = (System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.IExistingInterface)o; + + Assert.Equal("123", i.ItfMethod(123)); + }); + } } } From 6b59577d5fc22e3712ca28d5c42193f004bc8eab Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 22:24:23 -0400 Subject: [PATCH 32/37] remove test for props on existing type - it's not part of this PR --- .../tests/ApplyUpdateTest.cs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 687812e5c3a8aa..7050cc028d3bed 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -528,28 +528,6 @@ public static void TestReflectionAddNewType() Assert.Equal(newField, ty.GetField("NewField")); Assert.Equal(newStaticField, ty.GetField("NewStaticField", BindingFlags.Static | BindingFlags.Public)); - // FIXME: finish this -#if false - var allProperties = ty.GetProperties(); - - PropertyInfo newProp = null; - foreach (var prop in allProperties) - { - if (prop.Name == "NewProp") - newProp = prop; - } - Assert.NotNull(newProp); - - Assert.Equal(newProp, ty.GetProperty("NewProp")); - - MethodInfo newPropGet = newProp.GetGetMethod(); - Assert.NotNull(newPropGet); - MethodInfo newPropSet = newProp.GetSetMethod(); - Assert.NotNull(newPropSet); - - Assert.Equal("get_NewProp", newPropGet.Name); -#endif - }); CheckReflectedType(assm, allTypes, ns, "ZExistingClass+PreviousNestedClass"); CheckReflectedType(assm, allTypes, ns, "IExistingInterface"); From a393f3bf22961470ab7fc93339192fa57ad507d7 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 22:28:48 -0400 Subject: [PATCH 33/37] instance field additions to existing types aren't supported yet --- src/mono/mono/component/hot_reload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 2053e7d39fdce2..155ccd3990e551 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -33,7 +33,7 @@ #include -#define ALLOW_INSTANCE_FIELD_ADD +#undef ALLOW_INSTANCE_FIELD_ADD typedef struct _BaselineInfo BaselineInfo; typedef struct _DeltaInfo DeltaInfo; From 8d129c107986a6e3153e743418f7ce1618497c2c Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 22:37:07 -0400 Subject: [PATCH 34/37] rm TODO and FIXME --- src/mono/mono/component/hot_reload.c | 3 --- src/mono/mono/metadata/metadata.c | 2 -- 2 files changed, 5 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 155ccd3990e551..69e21daaceb9ab 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -2177,7 +2177,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base switch (func_code) { case ENC_FUNC_DEFAULT: { /* ok, added a new class */ - /* TODO: do things here */ pass2_context_add_skeleton (ctx, log_token); add_typedef_to_image_metadata (image_base, log_token); break; @@ -2231,7 +2230,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base /* FIXME: use DeltaInfo:prev_gen_rows instead of image_base */ g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table])); else { - /* TODO: adding a property. */ g_assert (add_property_propertymap != 0); uint32_t parent_type_token = mono_metadata_decode_row_col (&image_base->tables [MONO_TABLE_PROPERTYMAP], mono_metadata_token_index (add_property_propertymap) - 1, MONO_PROPERTY_MAP_PARENT); @@ -2285,7 +2283,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base /* FIXME: use DeltaInfo:prev_gen_rows instead of image_base */ g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table])); else { - /* TODO: adding a event. */ g_assert (add_event_eventmap != 0); uint32_t parent_type_token = mono_metadata_decode_row_col (&image_base->tables [MONO_TABLE_EVENTMAP], mono_metadata_token_index (add_event_eventmap) - 1, MONO_EVENT_MAP_PARENT); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index d89ff962633ff7..80b55fa08ceeab 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -7164,8 +7164,6 @@ mono_metadata_get_generic_param_row (MonoImage *image, guint32 token, guint32 *o loc.col_idx = MONO_GENERICPARAM_OWNER; loc.t = tdef; - /* FIXME: metadata-update */ - gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) != NULL; if (!found && !image->has_updates) return 0; From 9917ef3c704104da0c6f2e95710e6f7853f7331d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Mar 2022 23:14:52 -0400 Subject: [PATCH 35/37] store prop and event from_update bit in attrs The upper 16 bits aren't used for ECMA flags, so grab a bit for metadata-update --- src/mono/mono/component/debugger-agent.c | 2 +- src/mono/mono/metadata/class-internals.h | 25 ++++++++++++++++++------ src/mono/mono/metadata/class.c | 4 ++-- src/mono/mono/metadata/icall.c | 4 ++-- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index d53619b8bc8f19..7451304a4b51d1 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -8012,7 +8012,7 @@ type_commands_internal (int command, MonoClass *klass, MonoDomain *domain, guint buffer_add_string (buf, p->name); buffer_add_methodid (buf, domain, p->get); buffer_add_methodid (buf, domain, p->set); - buffer_add_int (buf, p->attrs); + buffer_add_int (buf, p->attrs & ~MONO_PROPERTY_META_FLAG_MASK); i ++; } g_assert (i == nprops); diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index 59dd6e6139dd1d..0d0ae1389d8520 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -192,12 +192,18 @@ struct _MonoProperty { const char *name; MonoMethod *get; MonoMethod *set; - guint32 attrs; + guint32 attrs; /* upper bits store non-ECMA flags */ +}; + +/* non-ECMA flags for the MonoProperty attrs field */ +enum { /* added by metadata-update after class was created; * not in MonoClassPropertyInfo array - don't do ptr arithmetic */ - guint32 from_update : 1; + MONO_PROPERTY_META_FLAG_FROM_UPDATE = 0x00010000, + MONO_PROPERTY_META_FLAG_MASK = 0x00010000, }; + struct _MonoEvent { MonoClass *parent; const char *name; @@ -207,12 +213,19 @@ struct _MonoEvent { #ifndef MONO_SMALL_CONFIG MonoMethod **other; #endif - guint32 attrs; + guint32 attrs; /* upper bits store non-ECMA flags */ +}; + +/* non-ECMA flags for the MonoEvent attrs field */ +enum { /* added by metadata-update after class was created; * not in MonoClassEventInfo array - don't do ptr arithmetic */ - guint32 from_update : 1; + MONO_EVENT_META_FLAG_FROM_UPDATE = 0x00010000, + + MONO_EVENT_META_FLAG_MASK = 0x00010000, }; + /* type of exception being "on hold" for later processing (see exception_type) */ typedef enum { MONO_EXCEPTION_NONE = 0, @@ -1613,13 +1626,13 @@ m_field_is_from_update (MonoClassField *field) static inline gboolean m_property_is_from_update (MonoProperty *prop) { - return prop->from_update != 0; + return (prop->attrs & MONO_PROPERTY_META_FLAG_FROM_UPDATE) != 0; } static inline gboolean m_event_is_from_update (MonoEvent *evt) { - return evt->from_update != 0; + return (evt->attrs & MONO_EVENT_META_FLAG_FROM_UPDATE) != 0; } /* diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index ce55091028fac6..dca80a49c89b72 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -5624,7 +5624,7 @@ mono_property_get_parent (MonoProperty *prop) guint32 mono_property_get_flags (MonoProperty *prop) { - return prop->attrs; + return prop->attrs & ~MONO_PROPERTY_META_FLAG_MASK; } /** @@ -5694,7 +5694,7 @@ mono_event_get_parent (MonoEvent *event) guint32 mono_event_get_flags (MonoEvent *event) { - return event->attrs; + return event->attrs & ~MONO_EVENT_META_FLAG_MASK; } /** diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index ccdcf5bca12d1c..dd8524bf08ed9b 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -2349,7 +2349,7 @@ ves_icall_RuntimePropertyInfo_get_property_info (MonoReflectionPropertyHandle pr } if ((req_info & PInfo_Attributes) != 0) - info->attrs = pproperty->attrs; + info->attrs = (pproperty->attrs & ~MONO_PROPERTY_META_FLAG_MASK); if ((req_info & PInfo_GetMethod) != 0) { MonoClass *property_klass = MONO_HANDLE_GETVAL (property, klass); @@ -2414,7 +2414,7 @@ ves_icall_RuntimeEventInfo_get_event_info (MonoReflectionMonoEventHandle ref_eve return_if_nok (error); MONO_STRUCT_SETREF_INTERNAL (info, name, MONO_HANDLE_RAW (ev_name)); - info->attrs = event->attrs; + info->attrs = event->attrs & ~MONO_EVENT_META_FLAG_MASK; MonoReflectionMethodHandle rm; if (event->add) { From 7a4c892ac3fc415dbea4f849fc5c335d4772f8f2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 17 Mar 2022 08:46:47 -0400 Subject: [PATCH 36/37] remove instance fields from reflection test since Mono doesn't support them yet --- .../ReflectionAddNewType_v1.cs | 5 ++++- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index d3a535da84b4df..107f3b72c68bc6 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -17,11 +17,14 @@ public class NewNestedClass {}; public string NewMethod (string s, int i) => s + i.ToString(); + // Mono doesn't support instance fields yet +#if false public int NewField; +#endif public static DateTime NewStaticField; - public double NewProp { get; set; } + public static double NewProp { get; set; } } [AttributeUsage(AttributeTargets.All, AllowMultiple=true, Inherited=false)] diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 7050cc028d3bed..ce69168955a0e7 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -513,19 +513,26 @@ public static void TestReflectionAddNewType() var allFields = ty.GetFields(BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public); + // Mono doesn't do instance fields yet +#if false FieldInfo newField = null; +#endif FieldInfo newStaticField = null; foreach (var fld in allFields) { +#if false if (fld.Name == "NewField") newField = fld; +#endif if (fld.Name == "NewStaticField") newStaticField = fld; } +#if false Assert.NotNull(newField); - Assert.NotNull(newStaticField); - Assert.Equal(newField, ty.GetField("NewField")); +#endif + + Assert.NotNull(newStaticField); Assert.Equal(newStaticField, ty.GetField("NewStaticField", BindingFlags.Static | BindingFlags.Public)); }); From 2c56d90d3fefb052f1126eef86742a7b598744d7 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 17 Mar 2022 15:25:25 -0400 Subject: [PATCH 37/37] make the Browser EAT lanes happy Add some fields to the baseline versions of some test assemblies to keep some types that are used in the deltas from getting trimmed away --- .../AddNestedClass.cs | 4 ++++ .../AddNestedClass_v1.cs | 4 ++++ .../ReflectionAddNewType.cs | 15 ++++++++++++++- .../ReflectionAddNewType_v1.cs | 15 ++++++++++++++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass.cs index d5bd6ce27fd0eb..277098ffc77ff8 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass.cs @@ -7,6 +7,10 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test { public class AddNestedClass { + public static Action X; // make the linker happy + public static Delegate Y; + public event Action Evt; + public void R () { Evt ("123"); } public AddNestedClass() { } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs index fa8f5ed30b564f..fcca03192d58a4 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs @@ -7,6 +7,10 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test { public class AddNestedClass { + public static Action X; // make the linker happy + public static Delegate Y; + public event Action Evt; + public void R () { Evt ("123"); } public AddNestedClass() { } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs index 23c961bd9697e0..7d720e44585f3a 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType.cs @@ -9,9 +9,22 @@ public interface IExistingInterface { public string ItfMethod(int i); } +public struct QExistingStruct +{ +} + +public enum FExistingEnum { + One, Two +} + public class ZExistingClass { - public class PreviousNestedClass { } + public class PreviousNestedClass { + public static DateTime Now; // make the linker happy + public static ICloneable C; + public event EventHandler E; + public void R() { E(this,"123"); } + } } [AttributeUsage(AttributeTargets.All, AllowMultiple=true, Inherited=false)] diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs index 107f3b72c68bc6..02762f11f05128 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType/ReflectionAddNewType_v1.cs @@ -9,9 +9,22 @@ public interface IExistingInterface { public string ItfMethod(int i); } +public struct QExistingStruct +{ +} + +public enum FExistingEnum { + One, Two +} + public class ZExistingClass { - public class PreviousNestedClass { } + public class PreviousNestedClass { + public static DateTime Now; // make the linker happy + public static ICloneable C; + public event EventHandler E; + public void R() { E(this,"123"); } + } public class NewNestedClass {};