Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Mono] Add mono hot reload support for updating parameter name #85796

Merged
merged 15 commits into from
Jun 16, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 2 additions & 24 deletions src/mono/mono/component/hot_reload.c
Original file line number Diff line number Diff line change
@@ -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);
15 changes: 10 additions & 5 deletions src/mono/mono/metadata/metadata.c
Original file line number Diff line number Diff line change
@@ -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 */
58 changes: 48 additions & 10 deletions src/mono/mono/metadata/reflection-cache.h
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@
#include <mono/metadata/mono-hash.h>
#include <mono/metadata/mempool.h>
#include <mono/utils/mono-error-internals.h>
#include <mono/metadata/metadata-update.h>

/*
* 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,22 +169,22 @@ 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));
return_val_if_nok (error, NULL_HANDLE);
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__*/
16 changes: 8 additions & 8 deletions src/mono/mono/metadata/reflection.c
Original file line number Diff line number Diff line change
@@ -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);
}

/**
4 changes: 2 additions & 2 deletions src/mono/mono/metadata/sre.c
Original file line number Diff line number Diff line change
@@ -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);
}

/*
38 changes: 38 additions & 0 deletions src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs
Original file line number Diff line number Diff line change
@@ -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<string>(), 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;
// }
// );
// }
}
}
Original file line number Diff line number Diff line change
@@ -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);
// }
// }
}
Original file line number Diff line number Diff line change
@@ -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);
// }
// }
}