diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index ac8236f78775ad..150330769a34c6 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1600,29 +1600,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de } case MONO_TABLE_PARAM: { *should_invalidate_transformed_code = true; - if (!is_addition) { - /* We only allow modifications where the parameter name doesn't change. */ - uint32_t base_param [MONO_PARAM_SIZE]; - uint32_t upd_param [MONO_PARAM_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_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x PARAM update. mapped index = 0x%08x\n", i, log_token, mapped_token); - - mono_metadata_decode_row (&image_dmeta->tables [MONO_TABLE_PARAM], mapped_token - 1, upd_param, MONO_PARAM_SIZE); - mono_metadata_decode_row (&image_base->tables [MONO_TABLE_PARAM], token_index - 1, base_param, MONO_PARAM_SIZE); - - const char *base_name = mono_metadata_string_heap (image_base, base_param [MONO_PARAM_NAME]); - const char *upd_name = mono_metadata_string_heap (image_base, upd_param [MONO_PARAM_NAME]); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x: 0x%08x PARAM update: seq = %d (base = %d), name = '%s' (base = '%s')\n", i, log_token, upd_param [MONO_PARAM_SEQUENCE], base_param [MONO_PARAM_SEQUENCE], upd_name, base_name); - if (strcmp (base_name, upd_name) != 0 || base_param [MONO_PARAM_SEQUENCE] != upd_param [MONO_PARAM_SEQUENCE]) { - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing PARAM table cols.", i, log_token); - mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing PARAM table cols. token=0x%08x", log_token); - unsupported_edits = TRUE; - continue; - } - break; - } else - break; /* added a row. ok */ + break; } case MONO_TABLE_TYPEDEF: { *should_invalidate_transformed_code = true; @@ -3493,7 +3471,7 @@ hot_reload_get_method_params (MonoImage *base_image, uint32_t methoddef_token, u static const char * hot_reload_get_capabilities (void) { - return "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition ChangeCustomAttributes AddInstanceFieldToExistingType GenericAddMethodToExistingType GenericUpdateMethod"; + return "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition ChangeCustomAttributes AddInstanceFieldToExistingType GenericAddMethodToExistingType GenericUpdateMethod UpdateParameters"; } static GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_instance_field_table); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index b9b30d622349f7..33289280b816af 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -5126,12 +5126,17 @@ mono_metadata_custom_attrs_from_index (MonoImage *meta, guint32 index) /* FIXME: Index translation */ 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 && !mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) + if (!found) { + if (G_LIKELY (!meta->has_updates)) { return 0; + } else { + if ((mono_metadata_table_num_rows (meta, MONO_TABLE_CUSTOMATTRIBUTE) > table_info_get_rows (tdef))) { + if (!mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) + return 0; + } else { + return 0; + } + } } /* Find the first entry by searching backwards */ diff --git a/src/mono/mono/metadata/reflection-cache.h b/src/mono/mono/metadata/reflection-cache.h index 2d91d4d74bd6e3..de92ac2dd673e4 100644 --- a/src/mono/mono/metadata/reflection-cache.h +++ b/src/mono/mono/metadata/reflection-cache.h @@ -12,6 +12,7 @@ #include #include #include +#include /* * We need to return always the same object for MethodInfo, FieldInfo etc.. @@ -22,8 +23,14 @@ typedef struct { gpointer item; MonoClass *refclass; + uint32_t generation; /* 0 is normal; hot reload may change it */ } ReflectedEntry; +enum { + MONO_REFL_CACHE_DEFAULT = 0, + MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE = 1, +}; + gboolean mono_reflected_equal (gconstpointer a, gconstpointer b); @@ -46,7 +53,7 @@ free_reflected_entry (ReflectedEntry *entry) } static inline MonoObject* -cache_object (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer item, MonoObject* o) +cache_object (MonoMemoryManager *mem_manager, int flags, MonoClass *klass, gpointer item, MonoObject* o) { MonoObject *obj; ReflectedEntry pe; @@ -59,6 +66,10 @@ cache_object (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer item, M ReflectedEntry *e = alloc_reflected_entry (mem_manager); e->item = item; e->refclass = klass; + if (G_UNLIKELY(mono_metadata_has_updates()) && ((flags & MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE) == 0)) + e->generation = mono_metadata_update_get_thread_generation(); + else + e->generation = 0; mono_conc_g_hash_table_insert (mem_manager->refobject_hash, e, o); obj = o; } @@ -67,7 +78,7 @@ cache_object (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer item, M } static inline MonoObjectHandle -cache_object_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer item, MonoObjectHandle o) +cache_object_handle (MonoMemoryManager *mem_manager, int flags, MonoClass *klass, gpointer item, MonoObjectHandle o) { MonoObjectHandle obj; ReflectedEntry pe; @@ -83,6 +94,10 @@ cache_object_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer ReflectedEntry *e = alloc_reflected_entry (mem_manager); e->item = item; e->refclass = klass; + if (G_UNLIKELY(mono_metadata_has_updates()) && ((flags & MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE) == 0)) + e->generation = mono_metadata_update_get_thread_generation(); + else + e->generation = 0; mono_conc_g_hash_table_insert (mem_manager->refobject_hash, e, MONO_HANDLE_RAW (o)); MONO_HANDLE_ASSIGN (obj, o); } @@ -92,6 +107,10 @@ cache_object_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer ReflectedEntry *e = alloc_reflected_entry (mem_manager); e->item = item; e->refclass = klass; + if (G_UNLIKELY(mono_metadata_has_updates()) && ((flags & MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE) == 0)) + e->generation = mono_metadata_update_get_thread_generation(); + else + e->generation = 0; mono_weak_hash_table_insert (mem_manager->weak_refobject_hash, e, MONO_HANDLE_RAW (o)); MONO_HANDLE_ASSIGN (obj, o); } @@ -100,13 +119,14 @@ cache_object_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer return obj; } -#define CACHE_OBJECT(t,mem_manager,p,o,k) ((t) (cache_object ((mem_manager), (k), (p), (o)))) -#define CACHE_OBJECT_HANDLE(t,mem_manager,p,o,k) (MONO_HANDLE_CAST (t, cache_object_handle ((mem_manager), (k), (p), (o)))) +#define CACHE_OBJECT(t,mem_manager,flags,p,o,k) ((t) (cache_object ((mem_manager), (flags), (k), (p), (o)))) +#define CACHE_OBJECT_HANDLE(t,mem_manager,flags,p,o,k) (MONO_HANDLE_CAST (t, cache_object_handle ((mem_manager), (flags), (k), (p), (o)))) static inline MonoObjectHandle -check_object_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer item) +check_object_handle (MonoMemoryManager *mem_manager, int flags, MonoClass *klass, gpointer item) { MonoObjectHandle obj_handle; + gpointer orig_e, orig_value; ReflectedEntry e; e.item = item; e.refclass = klass; @@ -123,6 +143,24 @@ check_object_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer MonoWeakHashTable *hash = mem_manager->weak_refobject_hash; obj_handle = MONO_HANDLE_NEW (MonoObject, (MonoObject *)mono_weak_hash_table_lookup (hash, &e)); } + + if (!mem_manager->collectible) { + MonoConcGHashTable *hash = mem_manager->refobject_hash; + if (mono_conc_g_hash_table_lookup_extended (hash, &e, &orig_e, &orig_value)) + if (mono_metadata_has_updates() && ((flags & MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE) == 0) && ((ReflectedEntry *)orig_e)->generation < mono_metadata_update_get_thread_generation()) { + mono_conc_g_hash_table_remove (hash, &e); + free_reflected_entry ((ReflectedEntry *)orig_e); + obj_handle = MONO_HANDLE_NEW (MonoObject, NULL); + } else { + obj_handle = MONO_HANDLE_NEW (MonoObject, (MonoObject *)orig_value); + } + else { + obj_handle = MONO_HANDLE_NEW (MonoObject, NULL); + } + } else { + MonoWeakHashTable *hash = mem_manager->weak_refobject_hash; + obj_handle = MONO_HANDLE_NEW (MonoObject, (MonoObject *)mono_weak_hash_table_lookup (hash, &e)); + } mono_mem_manager_unlock (mem_manager); return obj_handle; @@ -131,10 +169,10 @@ check_object_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer typedef MonoObjectHandle (*ReflectionCacheConstructFunc_handle) (MonoClass*, gpointer, gpointer, MonoError *); static inline MonoObjectHandle -check_or_construct_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpointer item, gpointer user_data, MonoError *error, ReflectionCacheConstructFunc_handle construct) +check_or_construct_handle (MonoMemoryManager *mem_manager, int flags, MonoClass *klass, gpointer item, gpointer user_data, MonoError *error, ReflectionCacheConstructFunc_handle construct) { error_init (error); - MonoObjectHandle obj = check_object_handle (mem_manager, klass, item); + MonoObjectHandle obj = check_object_handle (mem_manager, flags, klass, item); if (!MONO_HANDLE_IS_NULL (obj)) return obj; MONO_HANDLE_ASSIGN (obj, construct (klass, item, user_data, error)); @@ -142,11 +180,11 @@ check_or_construct_handle (MonoMemoryManager *mem_manager, MonoClass *klass, gpo if (MONO_HANDLE_IS_NULL (obj)) return obj; /* note no caching if there was an error in construction */ - return cache_object_handle (mem_manager, klass, item, obj); + return cache_object_handle (mem_manager, flags, klass, item, obj); } -#define CHECK_OR_CONSTRUCT_HANDLE(type,mem_manager, item,klass,construct,user_data) \ +#define CHECK_OR_CONSTRUCT_HANDLE(type,mem_manager,flags,item,klass,construct,user_data) \ (MONO_HANDLE_CAST (type, check_or_construct_handle ( \ - (mem_manager), (klass), (item), (user_data), error, (ReflectionCacheConstructFunc_handle) (construct)))) + (mem_manager), (flags), (klass), (item), (user_data), error, (ReflectionCacheConstructFunc_handle) (construct)))) #endif /*__MONO_METADATA_REFLECTION_CACHE_H__*/ diff --git a/src/mono/mono/metadata/reflection.c b/src/mono/mono/metadata/reflection.c index 37898f954ff348..dd4f2c63f7c628 100644 --- a/src/mono/mono/metadata/reflection.c +++ b/src/mono/mono/metadata/reflection.c @@ -243,7 +243,7 @@ MonoReflectionAssemblyHandle mono_assembly_get_object_handle (MonoAssembly *assembly, MonoError *error) { error_init (error); - return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionAssembly, m_image_get_mem_manager (assembly->image), assembly, NULL, assembly_object_construct, NULL); + return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionAssembly, m_image_get_mem_manager (assembly->image), MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE, assembly, NULL, assembly_object_construct, NULL); } /** @@ -311,7 +311,7 @@ MonoReflectionModuleHandle mono_module_get_object_handle (MonoImage *image, MonoError *error) { error_init (error); - return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionModule, m_image_get_mem_manager (image), image, NULL, module_object_construct, NULL); + return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionModule, m_image_get_mem_manager (image), MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE, image, NULL, module_object_construct, NULL); } /** @@ -670,7 +670,7 @@ mono_method_get_object_handle (MonoMethod *method, MonoClass *refclass, MonoErro refclass = method->klass; // FIXME: For methods/params etc., use the mem manager for refclass or a merged one ? - return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionMethod, m_method_get_mem_manager (method), method, refclass, method_object_construct, NULL); + return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionMethod, m_method_get_mem_manager (method), MONO_REFL_CACHE_DEFAULT, method, refclass, method_object_construct, NULL); } /* * mono_method_get_object_checked: @@ -776,7 +776,7 @@ MonoReflectionFieldHandle mono_field_get_object_handle (MonoClass *klass, MonoClassField *field, MonoError *error) { error_init (error); - return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionField, m_class_get_mem_manager (m_field_get_parent (field)), field, klass, field_object_construct, NULL); + return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionField, m_class_get_mem_manager (m_field_get_parent (field)), MONO_REFL_CACHE_DEFAULT, field, klass, field_object_construct, NULL); } /* @@ -844,7 +844,7 @@ property_object_construct (MonoClass *klass, MonoProperty *property, gpointer us MonoReflectionPropertyHandle mono_property_get_object_handle (MonoClass *klass, MonoProperty *property, MonoError *error) { - return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionProperty, m_class_get_mem_manager (property->parent), property, klass, property_object_construct, NULL); + return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionProperty, m_class_get_mem_manager (property->parent), MONO_REFL_CACHE_DEFAULT, property, klass, property_object_construct, NULL); } /** @@ -909,7 +909,7 @@ MonoReflectionEventHandle mono_event_get_object_handle (MonoClass *klass, MonoEvent *event, MonoError *error) { error_init (error); - return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionEvent, m_class_get_mem_manager (event->parent), event, klass, event_object_construct, NULL); + return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionEvent, m_class_get_mem_manager (event->parent), MONO_REFL_CACHE_DEFAULT, event, klass, event_object_construct, NULL); } @@ -1167,7 +1167,7 @@ mono_param_get_objects_internal (MonoMethod *method, MonoClass *refclass, MonoEr /* Note: the cache is based on the address of the signature into the method * since we already cache MethodInfos with the method as keys. */ - return CHECK_OR_CONSTRUCT_HANDLE (MonoArray, m_method_get_mem_manager (method), &method->signature, refclass, param_objects_construct, method); + return CHECK_OR_CONSTRUCT_HANDLE (MonoArray, m_method_get_mem_manager (method), MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE, &method->signature, refclass, param_objects_construct, method); fail: return MONO_HANDLE_NEW (MonoArray, NULL); } @@ -1392,7 +1392,7 @@ MonoReflectionMethodBodyHandle mono_method_body_get_object_handle (MonoMethod *method, MonoError *error) { error_init (error); - return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionMethodBody, m_method_get_mem_manager (method), method, NULL, method_body_object_construct, NULL); + return CHECK_OR_CONSTRUCT_HANDLE (MonoReflectionMethodBody, m_method_get_mem_manager (method), MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE, method, NULL, method_body_object_construct, NULL); } /** diff --git a/src/mono/mono/metadata/sre.c b/src/mono/mono/metadata/sre.c index b582c5cf4219bc..d7c1b63b3917c8 100644 --- a/src/mono/mono/metadata/sre.c +++ b/src/mono/mono/metadata/sre.c @@ -1188,13 +1188,13 @@ mono_image_create_token (MonoDynamicImage *assembly, MonoObjectHandle obj, static gpointer register_assembly (MonoReflectionAssembly *res, MonoAssembly *assembly) { - return CACHE_OBJECT (MonoReflectionAssembly *, mono_mem_manager_get_ambient (), assembly, &res->object, NULL); + return CACHE_OBJECT (MonoReflectionAssembly *, mono_mem_manager_get_ambient (), MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE, assembly, &res->object, NULL); } static MonoReflectionModuleBuilderHandle register_module (MonoReflectionModuleBuilderHandle res, MonoDynamicImage *module) { - return CACHE_OBJECT_HANDLE (MonoReflectionModuleBuilder, mono_mem_manager_get_ambient (), module, MONO_HANDLE_CAST (MonoObject, res), NULL); + return CACHE_OBJECT_HANDLE (MonoReflectionModuleBuilder, mono_mem_manager_get_ambient (), MONO_REFL_CACHE_NO_HOT_RELOAD_INVALIDATE, module, MONO_HANDLE_CAST (MonoObject, res), NULL); } /* diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs index 3a1953183dfdb5..1ce00cb2bca501 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs @@ -599,5 +599,43 @@ await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", scrip }, "c", num_fields: 2); }); } + + // Enable this test when https://github.com/dotnet/hotreload-utils/pull/264 flows into dotnet/runtime repo + // [ConditionalFact(nameof(RunningOnChrome))] + // public async Task DebugHotReloadMethod_ChangeParameterName() + // { + // string asm_file = Path.Combine(DebuggerTestAppPath, "ApplyUpdateReferencedAssembly.dll"); + // string pdb_file = Path.Combine(DebuggerTestAppPath, "ApplyUpdateReferencedAssembly.pdb"); + // string asm_file_hot_reload = Path.Combine(DebuggerTestAppPath, "../wasm/ApplyUpdateReferencedAssembly.dll"); + + // var bp_notchanged = await SetBreakpoint(".*/MethodBody1.cs$", 89, 12, use_regex: true); + // // var bp_invalid = await SetBreakpoint(".*/MethodBody1.cs$", 59, 12, use_regex: true); + + // var pause_location = await LoadAssemblyAndTestHotReloadUsingSDBWithoutChanges( + // asm_file, pdb_file, "MethodBody9", "test", expectBpResolvedEvent: true, sourcesToWait: new string [] { "MethodBody0.cs", "MethodBody1.cs" }); + + // CheckLocation("dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 89, 12, scripts, pause_location["callFrames"]?[0]["location"]); + // await StepAndCheck(StepKind.Over, "dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 90, 12, "ApplyUpdateReferencedAssembly.MethodBody9.M1", + // locals_fn: async (locals) => + // { + // CheckNumber(locals, "a", 1); + // await Task.CompletedTask; + // } + // ); + // //apply first update + // pause_location = await LoadAssemblyAndTestHotReloadUsingSDB( + // asm_file_hot_reload, "MethodBody9", "test", 1); + + // JToken top_frame = pause_location["callFrames"]?[0]; + // AssertEqual("ApplyUpdateReferencedAssembly.MethodBody9.M1", top_frame?["functionName"]?.Value(), top_frame?.ToString()); + // CheckLocation("dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 89, 12, scripts, top_frame["location"]); + // await StepAndCheck(StepKind.Over, "dotnet://ApplyUpdateReferencedAssembly.dll/MethodBody1.cs", 90, 12, "ApplyUpdateReferencedAssembly.MethodBody9.M1", + // locals_fn: async (locals) => + // { + // CheckNumber(locals, "x", 1); + // await Task.CompletedTask; + // } + // ); + // } } } diff --git a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1.cs b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1.cs index 3abc1d4b538bb7..14f8e5316f8a19 100644 --- a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1.cs +++ b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1.cs @@ -57,4 +57,41 @@ public static void StaticMethod1 () { Console.WriteLine("original"); } } + + + + + + + + + + + + + + + + + + + + + + + + + + + + + // public class MethodBody9 { + // public static int M1(int a, int b) { + // return a + b; + // } + + // public static int test() { + // return M1(1, 2); + // } + // } } diff --git a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1_v1.cs b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1_v1.cs index cef3214d9c89e9..7c69a0ca7b0fa3 100644 --- a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1_v1.cs +++ b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly/MethodBody1_v1.cs @@ -84,4 +84,14 @@ public void InstanceMethod () { Console.WriteLine($"add a breakpoint the instance method of the new class"); } } + + // public class MethodBody9 { + // public static int M1(int x, int y) { + // return x + y; + // } + + // public static int test() { + // return M1(1, 2); + // } + // } }