From 9e3c042efeb6c11487a978ed16f448ab7b409bd2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 26 Oct 2022 23:24:47 -0400 Subject: [PATCH 01/15] Add new test ReflectionAddNewMethod --- .../ReflectionAddNewMethod.cs | 16 +++++++ .../ReflectionAddNewMethod_v1.cs | 21 +++++++++ ...yUpdate.Test.ReflectionAddNewMethod.csproj | 11 +++++ .../deltascript.json | 6 +++ .../tests/ApplyUpdateTest.cs | 47 +++++++++++++++++++ .../tests/System.Runtime.Loader.Tests.csproj | 1 + 6 files changed, 102 insertions(+) create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/ReflectionAddNewMethod.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/ReflectionAddNewMethod_v1.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod.csproj create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/deltascript.json diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/ReflectionAddNewMethod.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/ReflectionAddNewMethod.cs new file mode 100644 index 00000000000000..c483ce17129def --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/ReflectionAddNewMethod.cs @@ -0,0 +1,16 @@ +// 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 +{ + public class ReflectionAddNewMethod + { + public string ExistingMethod(string u, double f) + { + return u + f.ToString();; + } + + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/ReflectionAddNewMethod_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/ReflectionAddNewMethod_v1.cs new file mode 100644 index 00000000000000..bd4804ab16aa70 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/ReflectionAddNewMethod_v1.cs @@ -0,0 +1,21 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System; +using System.Runtime.CompilerServices; +using CancellationToken = System.Threading.CancellationToken; + +namespace System.Reflection.Metadata.ApplyUpdate.Test +{ + public class ReflectionAddNewMethod + { + public string ExistingMethod(string u, double f) + { + return u + f.ToString();; + } + + public double AddedNewMethod(char c, float h, string w, CancellationToken ct = default, [CallerMemberName] string callerName = "") + { + return ((double)Convert.ToInt32(c)) + h; + } + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod.csproj new file mode 100644 index 00000000000000..f2b921019f7b33 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod.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.ReflectionAddNewMethod/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/deltascript.json new file mode 100644 index 00000000000000..a9e2fde9d00786 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod/deltascript.json @@ -0,0 +1,6 @@ +{ + "changes": [ + {"document": "ReflectionAddNewMethod.cs", "update": "ReflectionAddNewMethod_v1.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index f66b0d3fefe079..7bf4beb9b1169b 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -629,5 +629,52 @@ public static void TestReflectionAddNewType() System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewType.ZExistingClass.ExistingMethod (); }); } + + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestReflectionAddNewMethod() + { + ApplyUpdateUtil.TestCase(static () => + { + var ty = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod); + var assm = ty.Assembly; + + var allMethods = ty.GetMethods(); + + foreach (var m in allMethods) { + Console.WriteLine ($"method: {m.Name}"); + } + const int objectMethods = 4; + Assert.Equal (objectMethods + 1, allMethods.Length); + + ApplyUpdateUtil.ApplyUpdate(assm); + + allMethods = ty.GetMethods(); + Assert.Equal (objectMethods + 2, allMethods.Length); + + var mi = ty.GetMethod ("AddedNewMethod"); + + Assert.NotNull (mi); + + var retParm = mi.ReturnParameter; + Assert.NotNull (retParm); + Console.WriteLine ($"return parm: {retParm}"); + var retCas = retParm.GetCustomAttributes(); + foreach (var rca in retCas) { + Console.WriteLine ($" - ca: {rca}"); + } + + var parms = mi.GetParameters(); + + foreach (var parm in parms) { + Console.WriteLine ($"parm: {parm}"); + var cas = parm.GetCustomAttributes(); + foreach (var ca in cas) { + Console.WriteLine ($" - ca: {ca}"); + } + } + + Assert.Equal (5, parms.Length); + }); + } } } 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 50941fe7b3ca73..6c5a8469726179 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 @@ -62,6 +62,7 @@ + From a50e6a576cd028521f655d5eff3a9c4e148f9291 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 26 Oct 2022 23:25:24 -0400 Subject: [PATCH 02/15] FIXME: get_param_names, get_marshal_info and custom_attrs need work --- src/mono/mono/metadata/custom-attrs.c | 2 +- src/mono/mono/metadata/loader.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 98dbd70d6b9334..4019fd451fb0e7 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2250,7 +2250,7 @@ mono_custom_attrs_from_param_checked (MonoMethod *method, guint32 param, MonoErr return NULL; ca = &image->tables [MONO_TABLE_METHOD]; - /* FIXME: metadata-update */ + /* FIXME: metadata-update - lookup added params */ param_list = mono_metadata_decode_row_col (ca, method_index - 1, MONO_METHOD_PARAMLIST); if (method_index == table_info_get_rows (ca)) { param_last = table_info_get_rows (&image->tables [MONO_TABLE_PARAM]) + 1; diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 86b60c0a6e3236..8f03b922ba6bd5 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -1472,6 +1472,11 @@ mono_method_get_param_names (MonoMethod *method, const char **names) param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); + if (G_UNLIKELY (param_index == 0)) { + /* TODO: metadata-dupate: get the method param rows */ + return; + } + if (idx < table_info_get_rows (methodt)) lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); else @@ -1567,6 +1572,11 @@ mono_method_get_marshal_info (MonoMethod *method, MonoMarshalSpec **mspecs) guint32 cols [MONO_PARAM_SIZE]; guint param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); + if (G_UNLIKELY (param_index == 0)) { + /* TODO metadata-update: can we have marshaling info on added methods? */ + return; + } + if (idx < table_info_get_rows (methodt)) lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); else From 130b191db09e38032d0b65799d27ea6f4021b5c3 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 26 Oct 2022 23:27:46 -0400 Subject: [PATCH 03/15] WIP - add a method param reverse lookup --- .../mono/component/hot_reload-internals.h | 6 ++ src/mono/mono/component/hot_reload.c | 59 ++++++++++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index a668eb295c4388..9abf1d4c8db18f 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -86,4 +86,10 @@ typedef struct _MonoClassMetadataUpdateEvent { uint32_t token; /* the Event table token where this event was defined. */ } MonoClassMetadataUpdateEvent; +typedef struct _MonoMethodMetadataUpdateParamInfo { + uint32_t method_token; /* which method is this about */ + uint32_t first_param_token; /* a Param token */ + uint32_t param_count; +} MonoMethodMetadataUpdateParamInfo; + #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 166fe683e56d21..4e93ce7c96ddba 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -272,6 +272,9 @@ struct _BaselineInfo { /* Parents for added methods, fields, etc */ GHashTable *member_parent; /* maps added methoddef or fielddef tokens to typedef tokens */ + /* Params for added methods */ + GHashTable *method_params; /* maps methoddef tokens to a MonoClassMetadataUpdateMethodParamInfo* */ + /* Skeletons for all newly-added types from every generation. Accessing the array requires the image lock. */ GArray *skeletons; }; @@ -412,6 +415,12 @@ baseline_info_destroy (BaselineInfo *info) if (info->skeletons) g_array_free (info->skeletons, TRUE); + if (info->method_parent) + g_hash_table_destroy (info->method_parent); + + if (info->method_params) + g_hash_table_destroy (info->method_params); + g_free (info); } @@ -2019,6 +2028,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; + uint32_t add_field_method = 0; gboolean assemblyref_updated = FALSE; for (guint32 i = 0; i < rows ; ++i) { @@ -2049,6 +2059,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base case ENC_FUNC_ADD_PARAM: { g_assert (token_table == MONO_TABLE_METHOD); + add_field_method = log_token; break; } case ENC_FUNC_ADD_FIELD: { @@ -2115,8 +2126,11 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base } case MONO_TABLE_METHOD: { /* if adding a param, handle it with the next record */ - if (func_code == ENC_FUNC_ADD_PARAM) + if (func_code == ENC_FUNC_ADD_PARAM) { + g_assert (is_addition); break; + } + g_assert (func_code = ENC_FUNC_DEFAULT); if (!base_info->method_table_update) base_info->method_table_update = g_hash_table_new (g_direct_hash, g_direct_equal); @@ -2366,9 +2380,26 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base * * So by the time we see the param additions, the methods are already in. * - * FIXME: we need a lookaside table (like member_parent) for every place - * that looks at MONO_METHOD_PARAMLIST */ + if (is_addition) { + g_assert (add_field_method != 0); + /* + * FIXME: we need a lookaside table (like member_parent) for every place + * that looks at MONO_METHOD_PARAMLIST + */ + + uint32_t parent_type_token = hot_reload_member_parent (image_base, add_field_method); + g_assert (parent_type_token != 0); // we added a parameter to a method that was added + if (pass2_context_is_skeleton (ctx, parent_type_token)) { + // it's a parameter on a new method in a brand new class + // FIXME: need to do something here? + } else { + // it's a parameter on a new method in an existing class + add_param_info_for_method (base_info, log_token, add_field_method); + } + add_field_method = 0; + break; + } break; } case MONO_TABLE_INTERFACEIMPL: { @@ -2819,6 +2850,28 @@ hot_reload_field_parent (MonoImage *base_image, uint32_t field_token) } +static void +add_param_info_for_method (BaseInfo *base_info, uint32_t param_token, uint32_t method_token) +{ + if (!base_info->method_params) { + base_info->method_params = g_hash_table_new (g_direct_hash, g_direct_equal, NULL, g_free); + } + MonoMethodMetadataUpdateParamInfo* info = NULL; + info = g_hash_table_lookup (base_info->method_params, GUINT_TO_POINTER (method_token)); + if (!info) { + // FIXME locking + info = g_new0 (MonoMethodMetadataUpdateParamInfo, 1); + g_hash_table_insert (base_info->method_params, GUINT_TO_POINTER (method_token), info); + info->first_param = first_param_token; + info->param_count = 1; + } else { + uint32_t param_index = mono_metadata_token_index (param_token); + // expect params for a single method to be a contiguous sequence of rows + g_assert (mono_metadata_token_index (info->first_param) + param_count == param_index); + info->param_count++; + } +} + /* HACK - keep in sync with locator_t in metadata/metadata.c */ typedef struct { guint32 idx; /* The index that we are trying to locate */ From ddb5e1fa2c79dd7b1ed481f2b538ec70ada34303 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 27 Oct 2022 15:22:17 -0400 Subject: [PATCH 04/15] look up params from added methods --- src/mono/mono/component/hot_reload-stub.c | 12 ++++- src/mono/mono/component/hot_reload.c | 49 ++++++++++++++---- src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/custom-attrs.c | 14 +++-- src/mono/mono/metadata/loader.c | 63 ++++++++++++++--------- src/mono/mono/metadata/metadata-update.c | 8 ++- src/mono/mono/metadata/metadata-update.h | 3 ++ 7 files changed, 112 insertions(+), 38 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index e97e90499e3534..94730bbe26b164 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -110,6 +110,9 @@ hot_reload_get_num_methods_added (MonoClass *klass); static const char * hot_reload_get_capabilities (void); +static uint32_t +hot_reload_stub_get_method_params (MonoImage *base_image, uint32_t methoddef_token, uint32_t *out_param_count_opt); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -142,7 +145,8 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_added_fields_iter, &hot_reload_get_num_fields_added, &hot_reload_get_num_methods_added, - &hot_reload_get_capabilities + &hot_reload_get_capabilities, + &hot_reload_stub_get_method_params, }; static bool @@ -343,6 +347,12 @@ hot_reload_get_capabilities (void) return ""; } +static uint32_t +hot_reload_stub_get_method_params (MonoImage *base_image, uint32_t methoddef_token, uint32_t *out_param_count_opt) +{ + return 0; +} + MONO_COMPONENT_EXPORT_ENTRYPOINT MonoComponentHotReload * mono_component_hot_reload_init (void) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 4e93ce7c96ddba..873f3e0f2a3f14 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -144,6 +144,9 @@ hot_reload_get_num_methods_added (MonoClass *klass); static const char * hot_reload_get_capabilities (void); +static uint32_t +hot_reload_get_method_params (MonoImage *base_image, uint32_t methoddef_token, uint32_t *out_param_count_opt); + static MonoClassMetadataUpdateField * metadata_update_field_setup_basic_info (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, uint32_t field_flags); @@ -179,7 +182,8 @@ static MonoComponentHotReload fn_table = { &hot_reload_added_fields_iter, &hot_reload_get_num_fields_added, &hot_reload_get_num_methods_added, - &hot_reload_get_capabilities + &hot_reload_get_capabilities, + &hot_reload_get_method_params, }; MonoComponentHotReload * @@ -415,8 +419,8 @@ baseline_info_destroy (BaselineInfo *info) if (info->skeletons) g_array_free (info->skeletons, TRUE); - if (info->method_parent) - g_hash_table_destroy (info->method_parent); + if (info->member_parent) + g_hash_table_destroy (info->member_parent); if (info->method_params) g_hash_table_destroy (info->method_params); @@ -636,6 +640,11 @@ add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClas static void add_field_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t field_token); +/* Add a method->params lookup for new methods in existing classes */ +static void +add_param_info_for_method (BaselineInfo *base_info, uint32_t param_token, uint32_t method_token); + + void hot_reload_init (void) { @@ -2130,7 +2139,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base g_assert (is_addition); break; } - g_assert (func_code = ENC_FUNC_DEFAULT); + g_assert (func_code == ENC_FUNC_DEFAULT); if (!base_info->method_table_update) base_info->method_table_update = g_hash_table_new (g_direct_hash, g_direct_equal); @@ -2388,7 +2397,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base * that looks at MONO_METHOD_PARAMLIST */ - uint32_t parent_type_token = hot_reload_member_parent (image_base, add_field_method); + uint32_t parent_type_token = hot_reload_method_parent (image_base, add_field_method); g_assert (parent_type_token != 0); // we added a parameter to a method that was added if (pass2_context_is_skeleton (ctx, parent_type_token)) { // it's a parameter on a new method in a brand new class @@ -2851,10 +2860,10 @@ hot_reload_field_parent (MonoImage *base_image, uint32_t field_token) static void -add_param_info_for_method (BaseInfo *base_info, uint32_t param_token, uint32_t method_token) +add_param_info_for_method (BaselineInfo *base_info, uint32_t param_token, uint32_t method_token) { if (!base_info->method_params) { - base_info->method_params = g_hash_table_new (g_direct_hash, g_direct_equal, NULL, g_free); + base_info->method_params = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_free); } MonoMethodMetadataUpdateParamInfo* info = NULL; info = g_hash_table_lookup (base_info->method_params, GUINT_TO_POINTER (method_token)); @@ -2862,12 +2871,12 @@ add_param_info_for_method (BaseInfo *base_info, uint32_t param_token, uint32_t m // FIXME locking info = g_new0 (MonoMethodMetadataUpdateParamInfo, 1); g_hash_table_insert (base_info->method_params, GUINT_TO_POINTER (method_token), info); - info->first_param = first_param_token; + info->first_param_token = param_token; info->param_count = 1; } else { uint32_t param_index = mono_metadata_token_index (param_token); // expect params for a single method to be a contiguous sequence of rows - g_assert (mono_metadata_token_index (info->first_param) + param_count == param_index); + g_assert (mono_metadata_token_index (info->first_param_token) + info->param_count == param_index); info->param_count++; } } @@ -3201,6 +3210,28 @@ hot_reload_get_num_methods_added (MonoClass *klass) return count; } +static uint32_t +hot_reload_get_method_params (MonoImage *base_image, uint32_t methoddef_token, uint32_t *out_param_count_opt) +{ + BaselineInfo *base_info = baseline_info_lookup (base_image); + g_assert (base_info); + + /* FIXME: locking in case the hash table grows */ + MonoMethodMetadataUpdateParamInfo* info = NULL; + info = g_hash_table_lookup (base_info->method_params, GUINT_TO_POINTER (methoddef_token)); + if (!info) { + if (out_param_count_opt) + *out_param_count_opt = 0; + return 0; + } + + if (out_param_count_opt) + *out_param_count_opt = info->param_count; + + return mono_metadata_token_index (info->first_param_token); +} + + static const char * hot_reload_get_capabilities (void) { diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index f7627245583af5..40c6a0f263f417 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -48,6 +48,7 @@ typedef struct _MonoComponentHotReload { uint32_t (*get_num_fields_added) (MonoClass *klass); uint32_t (*get_num_methods_added) (MonoClass *klass); const char* (*get_capabilities) (void); + uint32_t (*get_method_params) (MonoImage *base_image, uint32_t methoddef_token, uint32_t *out_param_count_opt); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 4019fd451fb0e7..9a1d364625f368 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2252,10 +2252,18 @@ mono_custom_attrs_from_param_checked (MonoMethod *method, guint32 param, MonoErr /* FIXME: metadata-update - lookup added params */ param_list = mono_metadata_decode_row_col (ca, method_index - 1, MONO_METHOD_PARAMLIST); - if (method_index == table_info_get_rows (ca)) { - param_last = table_info_get_rows (&image->tables [MONO_TABLE_PARAM]) + 1; + if (G_UNLIKELY (param_list == 0 && image->has_updates)) { + uint32_t count; + param_list = mono_metadata_update_get_method_params (image, mono_metadata_make_token (MONO_TABLE_METHOD, method_index), &count); + if (!param_list) + return NULL; + param_last = param_list + count; } else { - param_last = mono_metadata_decode_row_col (ca, method_index, MONO_METHOD_PARAMLIST); + if (method_index == table_info_get_rows (ca)) { + param_last = table_info_get_rows (&image->tables [MONO_TABLE_PARAM]) + 1; + } else { + param_last = mono_metadata_decode_row_col (ca, method_index, MONO_METHOD_PARAMLIST); + } } ca = &image->tables [MONO_TABLE_PARAM]; diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index 8f03b922ba6bd5..f6ee5b2b10957e 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -1472,15 +1472,18 @@ mono_method_get_param_names (MonoMethod *method, const char **names) param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); - if (G_UNLIKELY (param_index == 0)) { - /* TODO: metadata-dupate: get the method param rows */ - return; + if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { + uint32_t count; + param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), &count); + if (!param_index) + return; + lastp = param_index + count; + } else { + if (idx < table_info_get_rows (methodt)) + lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); + else + lastp = table_info_get_rows (paramt) + 1; } - - if (idx < table_info_get_rows (methodt)) - lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; for (i = param_index; i < lastp; ++i) { mono_metadata_decode_row (paramt, i -1, cols, MONO_PARAM_SIZE); if (cols [MONO_PARAM_SEQUENCE] && cols [MONO_PARAM_SEQUENCE] <= signature->param_count) /* skip return param spec and bounds check*/ @@ -1572,16 +1575,19 @@ mono_method_get_marshal_info (MonoMethod *method, MonoMarshalSpec **mspecs) guint32 cols [MONO_PARAM_SIZE]; guint param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); - if (G_UNLIKELY (param_index == 0)) { - /* TODO metadata-update: can we have marshaling info on added methods? */ - return; + if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { + uint32_t count; + param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), &count); + if (!param_index) + return; + lastp = param_index + count; + } else { + if (idx < table_info_get_rows (methodt)) + lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); + else + lastp = table_info_get_rows (paramt) + 1; } - - if (idx < table_info_get_rows (methodt)) - lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; - + for (i = param_index; i < lastp; ++i) { mono_metadata_decode_row (paramt, i -1, cols, MONO_PARAM_SIZE); @@ -1624,18 +1630,27 @@ mono_method_has_marshal_info (MonoMethod *method) mono_class_init_internal (klass); - methodt = &m_class_get_image (klass)->tables [MONO_TABLE_METHOD]; - paramt = &m_class_get_image (klass)->tables [MONO_TABLE_PARAM]; + MonoImage *klass_image = m_class_get_image (klass); + methodt = &klass_image->tables [MONO_TABLE_METHOD]; + paramt = &klass_image->tables [MONO_TABLE_PARAM]; idx = mono_method_get_index (method); if (idx > 0) { guint32 cols [MONO_PARAM_SIZE]; guint param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); - if (idx + 1 < table_info_get_rows (methodt)) - lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; - + if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { + uint32_t count; + param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), &count); + if (!param_index) + return FALSE; + lastp = param_index + count; + } else { + if (idx + 1 < table_info_get_rows (methodt)) + lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); + else + lastp = table_info_get_rows (paramt) + 1; + } + for (i = param_index; i < lastp; ++i) { mono_metadata_decode_row (paramt, i -1, cols, MONO_PARAM_SIZE); diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 0bfb211c6f0caa..9541a25384c00a 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -229,4 +229,10 @@ uint32_t mono_metadata_update_get_num_methods_added (MonoClass *klass) { return mono_component_hot_reload()->get_num_methods_added (klass); -} \ No newline at end of file +} + +uint32_t +mono_metadata_update_get_method_params (MonoImage *image, uint32_t methoddef_token, uint32_t *out_param_count_opt) +{ + return mono_component_hot_reload()->get_method_params (image, methoddef_token, out_param_count_opt); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 05cc4e5c80316c..e68ca1a8ad71ae 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -93,4 +93,7 @@ mono_metadata_update_get_num_fields_added (MonoClass *klass); uint32_t mono_metadata_update_get_num_methods_added (MonoClass *klass); + +uint32_t +mono_metadata_update_get_method_params (MonoImage *image, uint32_t methoddef_token, uint32_t *out_param_count_opt); #endif /*__MONO_METADATA_UPDATE_H__*/ From 1e94e62b69ef7a69e85ae07caf024912247d6b20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Thu, 27 Oct 2022 16:14:58 -0400 Subject: [PATCH 05/15] Remove FIXMEs and unused field --- src/mono/mono/component/hot_reload-internals.h | 1 - src/mono/mono/component/hot_reload.c | 5 ----- src/mono/mono/metadata/custom-attrs.c | 1 - 3 files changed, 7 deletions(-) diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index 9abf1d4c8db18f..cc97538749d056 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -87,7 +87,6 @@ typedef struct _MonoClassMetadataUpdateEvent { } MonoClassMetadataUpdateEvent; typedef struct _MonoMethodMetadataUpdateParamInfo { - uint32_t method_token; /* which method is this about */ uint32_t first_param_token; /* a Param token */ uint32_t param_count; } MonoMethodMetadataUpdateParamInfo; diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 873f3e0f2a3f14..9680af31b73a49 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -2392,11 +2392,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base */ if (is_addition) { g_assert (add_field_method != 0); - /* - * FIXME: we need a lookaside table (like member_parent) for every place - * that looks at MONO_METHOD_PARAMLIST - */ - uint32_t parent_type_token = hot_reload_method_parent (image_base, add_field_method); g_assert (parent_type_token != 0); // we added a parameter to a method that was added if (pass2_context_is_skeleton (ctx, parent_type_token)) { diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 9a1d364625f368..6e975de0d631d9 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2250,7 +2250,6 @@ mono_custom_attrs_from_param_checked (MonoMethod *method, guint32 param, MonoErr return NULL; ca = &image->tables [MONO_TABLE_METHOD]; - /* FIXME: metadata-update - lookup added params */ param_list = mono_metadata_decode_row_col (ca, method_index - 1, MONO_METHOD_PARAMLIST); if (G_UNLIKELY (param_list == 0 && image->has_updates)) { uint32_t count; From 01a653a05dc1ba0cf336bf3133bd3836872472fb Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 27 Oct 2022 16:29:45 -0400 Subject: [PATCH 06/15] remove writelines from test --- .../tests/ApplyUpdateTest.cs | 64 +++++++++++-------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 7bf4beb9b1169b..a8c4d7f82c4d71 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -635,46 +635,56 @@ public static void TestReflectionAddNewMethod() { ApplyUpdateUtil.TestCase(static () => { - var ty = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod); + var ty = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod); var assm = ty.Assembly; var allMethods = ty.GetMethods(); - foreach (var m in allMethods) { - Console.WriteLine ($"method: {m.Name}"); - } - const int objectMethods = 4; - Assert.Equal (objectMethods + 1, allMethods.Length); + const int objectMethods = 4; + Assert.Equal (objectMethods + 1, allMethods.Length); ApplyUpdateUtil.ApplyUpdate(assm); - allMethods = ty.GetMethods(); - Assert.Equal (objectMethods + 2, allMethods.Length); + allMethods = ty.GetMethods(); + Assert.Equal (objectMethods + 2, allMethods.Length); - var mi = ty.GetMethod ("AddedNewMethod"); + var mi = ty.GetMethod ("AddedNewMethod"); - Assert.NotNull (mi); + Assert.NotNull (mi); + + var retParm = mi.ReturnParameter; + Assert.NotNull (retParm); + Assert.NotNull (retParm.ParameterType); + Assert.Equal (-1, retParm.Position); + + var retCas = retParm.GetCustomAttributes(false); + Assert.NotNull(retCas); + Assert.Equal(0, retCas.Length); + + var parms = mi.GetParameters(); + Assert.Equal (5, parms.Length); - var retParm = mi.ReturnParameter; - Assert.NotNull (retParm); - Console.WriteLine ($"return parm: {retParm}"); - var retCas = retParm.GetCustomAttributes(); - foreach (var rca in retCas) { - Console.WriteLine ($" - ca: {rca}"); - } + int parmPos = 0; + foreach (var parm in parms) + { + Assert.NotNull(parm); + Assert.NotNull(parm.ParameterType); + Assert.Equal(parmPos, parm.Position); + Assert.NotNull(parm.Name); + + var cas = parm.GetCustomAttributes(false); + foreach (var ca in cas) { + Assert.NotNull (ca); + } - var parms = mi.GetParameters(); + parmPos++; + } - foreach (var parm in parms) { - Console.WriteLine ($"parm: {parm}"); - var cas = parm.GetCustomAttributes(); - foreach (var ca in cas) { - Console.WriteLine ($" - ca: {ca}"); - } - } + Assert.Equal (1, parms[4].GetCustomAttributes(false).Length); + Assert.Equal (typeof (CallerMemberNameAttribute), parms[4].GetCustomAttributes(false)[0].GetType()); - Assert.Equal (5, parms.Length); - }); + // TODO: check the default values of the last two params + }); } } } From c935dfaf9605eb2e4d63243e10b3938ab811419e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 27 Oct 2022 18:53:37 -0400 Subject: [PATCH 07/15] fix test on coreclr --- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index a8c4d7f82c4d71..da7935fc03dbe9 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -638,14 +638,18 @@ public static void TestReflectionAddNewMethod() var ty = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod); var assm = ty.Assembly; - var allMethods = ty.GetMethods(); + var bindingFlags = BindingFlags.Instance | BindingFlags.Public; + var allMethods = ty.GetMethods(bindingFlags); - const int objectMethods = 4; + int objectMethods = typeof(object).GetMethods(bindingFlags).Length; Assert.Equal (objectMethods + 1, allMethods.Length); ApplyUpdateUtil.ApplyUpdate(assm); + ApplyUpdateUtil.ClearAllReflectionCaches(); + + ty = typeof(System.Reflection.Metadata.ApplyUpdate.Test.ReflectionAddNewMethod); - allMethods = ty.GetMethods(); + allMethods = ty.GetMethods(bindingFlags); Assert.Equal (objectMethods + 2, allMethods.Length); var mi = ty.GetMethod ("AddedNewMethod"); From f4a58e18e1c2a5bb55a9f4df868ebbd397d20107 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 28 Oct 2022 16:54:26 -0400 Subject: [PATCH 08/15] why does coreclr have 2 attributes here?? --- src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index da7935fc03dbe9..28622ce89014bb 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -684,6 +684,9 @@ public static void TestReflectionAddNewMethod() parmPos++; } + foreach (var ca in parms[4].GetCustomAttributes(false)) { + Console.WriteLine (" - parm[4] has {0}", ca); + } Assert.Equal (1, parms[4].GetCustomAttributes(false).Length); Assert.Equal (typeof (CallerMemberNameAttribute), parms[4].GetCustomAttributes(false)[0].GetType()); From 50aa8f93a4123c1a3769cd6424d60949f9c31ddf Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 28 Oct 2022 18:05:16 -0400 Subject: [PATCH 09/15] There should be 2 attributes on the 4th param --- .../tests/ApplyUpdateTest.cs | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 28622ce89014bb..0a5cffc4056c1e 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -3,6 +3,7 @@ using System.Reflection; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using Xunit; namespace System.Reflection.Metadata @@ -684,11 +685,22 @@ public static void TestReflectionAddNewMethod() parmPos++; } - foreach (var ca in parms[4].GetCustomAttributes(false)) { - Console.WriteLine (" - parm[4] has {0}", ca); - } - Assert.Equal (1, parms[4].GetCustomAttributes(false).Length); - Assert.Equal (typeof (CallerMemberNameAttribute), parms[4].GetCustomAttributes(false)[0].GetType()); + var parmAttrs = parms[4].GetCustomAttributes(false); + Assert.Equal (2, parmAttrs.Length); + bool foundCallerMemberName = false; + bool foundOptional = false; + foreach (var pa in parmAttrs) { + if (typeof (CallerMemberNameAttribute).Equals(pa.GetType())) + { + foundCallerMemberName = true; + } + if (typeof (OptionalAttribute).Equals(pa.GetType())) + { + foundOptional = true; + } + } + Assert.True(foundCallerMemberName); + Assert.True(foundOptional); // TODO: check the default values of the last two params }); From b3f903fb95ba5023e02af3f04affd52e05d523d2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 28 Oct 2022 20:47:50 -0400 Subject: [PATCH 10/15] one more place that looks at params --- src/mono/mono/metadata/metadata.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 690df1e8157398..c7154dc1cc29d9 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -2280,14 +2280,18 @@ mono_metadata_method_has_param_attrs (MonoImage *m, int def) MonoTableInfo *methodt = &m->tables [MONO_TABLE_METHOD]; guint lastp, i, param_index = mono_metadata_decode_row_col (methodt, def - 1, MONO_METHOD_PARAMLIST); - if (param_index == 0) - return FALSE; - - /* FIXME: metadata-update */ - if (GINT_TO_UINT32(def) < table_info_get_rows (methodt)) - lastp = mono_metadata_decode_row_col (methodt, def, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (&m->tables [MONO_TABLE_PARAM]) + 1; + if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { + uint32_t count; + param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, def), &count); + if (!param_index) + return FALSE; + lastp = param_index + count; + } else { + if (GINT_TO_UINT32(def) < table_info_get_rows (methodt)) + lastp = mono_metadata_decode_row_col (methodt, def, MONO_METHOD_PARAMLIST); + else + lastp = table_info_get_rows (&m->tables [MONO_TABLE_PARAM]) + 1; + } for (i = param_index; i < lastp; ++i) { guint32 flags = mono_metadata_decode_row_col (paramt, i - 1, MONO_PARAM_FLAGS); From 8b3c7e9c0ed6167b7f1c70e013f73cb32fd35be2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Sat, 29 Oct 2022 00:06:08 -0400 Subject: [PATCH 11/15] A couple more places where we look at the Params table --- src/mono/mono/metadata/loader.c | 3 +++ src/mono/mono/metadata/metadata.c | 25 ++++++++++++++----------- src/mono/mono/metadata/reflection.c | 19 ++++++++++++++----- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index f6ee5b2b10957e..f68eaf09a94626 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -1512,6 +1512,9 @@ mono_method_get_param_token (MonoMethod *method, int index) if (idx > 0) { guint param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); + if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { + param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), NULL); + } if (index == -1) /* Return value */ return mono_metadata_make_token (MONO_TABLE_PARAM, 0); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index c7154dc1cc29d9..f63ec38ee465dc 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -2280,9 +2280,9 @@ mono_metadata_method_has_param_attrs (MonoImage *m, int def) MonoTableInfo *methodt = &m->tables [MONO_TABLE_METHOD]; guint lastp, i, param_index = mono_metadata_decode_row_col (methodt, def - 1, MONO_METHOD_PARAMLIST); - if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { + if (G_UNLIKELY (param_index == 0 && m->has_updates)) { uint32_t count; - param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, def), &count); + param_index = mono_metadata_update_get_method_params (m, mono_metadata_make_token (MONO_TABLE_METHOD, def), &count); if (!param_index) return FALSE; lastp = param_index + count; @@ -2322,15 +2322,18 @@ mono_metadata_get_param_attrs (MonoImage *m, int def, guint32 param_count) guint lastp, i, param_index = mono_metadata_decode_row_col (methodt, def - 1, MONO_METHOD_PARAMLIST); int *pattrs = NULL; - /* hot reload deltas may specify 0 for the param table index */ - if (param_index == 0) - return NULL; - - /* FIXME: metadata-update */ - if (GINT_TO_UINT32(def) < mono_metadata_table_num_rows (m, MONO_TABLE_METHOD)) - lastp = mono_metadata_decode_row_col (methodt, def, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; + if (G_UNLIKELY (param_index == 0 && m->has_updates)) { + uint32_t count; + param_index = mono_metadata_update_get_method_params (m, mono_metadata_make_token (MONO_TABLE_METHOD, def), &count); + if (!param_index) + return NULL; + lastp = param_index + count; + } else { + if (GINT_TO_UINT32(def) < mono_metadata_table_num_rows (m, MONO_TABLE_METHOD)) + lastp = mono_metadata_decode_row_col (methodt, def, MONO_METHOD_PARAMLIST); + else + lastp = table_info_get_rows (paramt) + 1; + } for (i = param_index; i < lastp; ++i) { mono_metadata_decode_row (paramt, i - 1, cols, MONO_PARAM_SIZE); diff --git a/src/mono/mono/metadata/reflection.c b/src/mono/mono/metadata/reflection.c index 0149b9d5c6a6da..098dd3860b38bc 100644 --- a/src/mono/mono/metadata/reflection.c +++ b/src/mono/mono/metadata/reflection.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -1421,12 +1422,20 @@ get_default_param_value_blobs (MonoMethod *method, char **blobs, guint32 *types) /* lastp is the starting param index for the next method in the table, or * one past the last row if this is the last method */ - /* FIXME: metadata-update : will this work with added methods ? */ + /* FIXME: metadata-update : will this work with added methods - no, needs a change */ param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); - if (!mono_metadata_table_bounds_check (image, MONO_TABLE_METHOD, idx + 1)) - lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; + if (G_UNLIKELY (param_index == 0 && image->has_updates)) { + uint32_t count; + param_index = mono_metadata_update_get_method_params (image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), &count); + if (!param_index) + return; + lastp = param_index + count; + } else { + if (!mono_metadata_table_bounds_check (image, MONO_TABLE_METHOD, idx + 1)) + lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); + else + lastp = table_info_get_rows (paramt) + 1; + } for (i = param_index; i < lastp; ++i) { guint32 paramseq; From 81d4e6d7c0d77b345cdc432e41b8cce91273dc6e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Sat, 29 Oct 2022 01:13:58 -0400 Subject: [PATCH 12/15] Check default values on params on added method --- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 0a5cffc4056c1e..44076340a1b655 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -702,7 +702,11 @@ public static void TestReflectionAddNewMethod() Assert.True(foundCallerMemberName); Assert.True(foundOptional); - // TODO: check the default values of the last two params + Assert.True(parms[3].HasDefaultValue); + Assert.True(parms[4].HasDefaultValue); + + Assert.Null(parms[3].DefaultValue); + Assert.Equal(string.Empty, parms[4].DefaultValue); }); } } From 9fbb7bd97323c755855a40ee78e5122d7e83ca06 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Sat, 29 Oct 2022 15:32:36 -0400 Subject: [PATCH 13/15] fix lookup if table is empty --- src/mono/mono/component/hot_reload.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 9680af31b73a49..c412aa3efe6662 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -3212,6 +3212,10 @@ hot_reload_get_method_params (MonoImage *base_image, uint32_t methoddef_token, u g_assert (base_info); /* FIXME: locking in case the hash table grows */ + + if (!base_info->method_params) + return 0; + MonoMethodMetadataUpdateParamInfo* info = NULL; info = g_hash_table_lookup (base_info->method_params, GUINT_TO_POINTER (methoddef_token)); if (!info) { From 2d191c213f0e29539b9b521800a7e100a9650155 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Sat, 29 Oct 2022 18:51:08 -0400 Subject: [PATCH 14/15] add a gratuitious typeof assert otherwise the CancellationToken type is trimmed on wasm --- src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 44076340a1b655..92f19a6b0c023e 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -702,6 +702,9 @@ public static void TestReflectionAddNewMethod() Assert.True(foundCallerMemberName); Assert.True(foundOptional); + // n.b. this typeof() also makes the rest of the test work on Wasm with aggressive trimming. + Assert.Equal (typeof(System.Threading.CancellationToken), parms[3].ParameterType); + Assert.True(parms[3].HasDefaultValue); Assert.True(parms[4].HasDefaultValue); From a29fdb2a6c2a49cd4041016886607ad66d717165 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 31 Oct 2022 12:03:20 -0400 Subject: [PATCH 15/15] Add a single mono_metadata_get_method_params function remove duplicated code --- src/mono/mono/metadata/custom-attrs.c | 20 ++---- src/mono/mono/metadata/loader.c | 73 +++++--------------- src/mono/mono/metadata/metadata-internals.h | 3 + src/mono/mono/metadata/metadata.c | 75 +++++++++++++-------- src/mono/mono/metadata/reflection.c | 23 ++----- 5 files changed, 76 insertions(+), 118 deletions(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 6e975de0d631d9..2bce3b3e79781c 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2248,22 +2248,12 @@ mono_custom_attrs_from_param_checked (MonoMethod *method, guint32 param, MonoErr method_index = mono_method_get_index (method); if (!method_index) return NULL; - ca = &image->tables [MONO_TABLE_METHOD]; - param_list = mono_metadata_decode_row_col (ca, method_index - 1, MONO_METHOD_PARAMLIST); - if (G_UNLIKELY (param_list == 0 && image->has_updates)) { - uint32_t count; - param_list = mono_metadata_update_get_method_params (image, mono_metadata_make_token (MONO_TABLE_METHOD, method_index), &count); - if (!param_list) - return NULL; - param_last = param_list + count; - } else { - if (method_index == table_info_get_rows (ca)) { - param_last = table_info_get_rows (&image->tables [MONO_TABLE_PARAM]) + 1; - } else { - param_last = mono_metadata_decode_row_col (ca, method_index, MONO_METHOD_PARAMLIST); - } - } + param_list = mono_metadata_get_method_params (image, method_index, ¶m_last); + + if (!param_list) + return NULL; + ca = &image->tables [MONO_TABLE_PARAM]; found = FALSE; diff --git a/src/mono/mono/metadata/loader.c b/src/mono/mono/metadata/loader.c index f68eaf09a94626..51803fa5808f72 100644 --- a/src/mono/mono/metadata/loader.c +++ b/src/mono/mono/metadata/loader.c @@ -1409,7 +1409,6 @@ mono_method_get_param_names (MonoMethod *method, const char **names) { int i, lastp; MonoClass *klass; - MonoTableInfo *methodt; MonoTableInfo *paramt; MonoMethodSignature *signature; guint32 idx; @@ -1463,27 +1462,18 @@ mono_method_get_param_names (MonoMethod *method, const char **names) return; } - methodt = &klass_image->tables [MONO_TABLE_METHOD]; paramt = &klass_image->tables [MONO_TABLE_PARAM]; idx = mono_method_get_index (method); if (idx > 0) { + guint32 cols [MONO_PARAM_SIZE]; guint param_index; - param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); + param_index = mono_metadata_get_method_params (klass_image, idx, (uint32_t*)&lastp); + + if (!param_index) + return; - if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { - uint32_t count; - param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), &count); - if (!param_index) - return; - lastp = param_index + count; - } else { - if (idx < table_info_get_rows (methodt)) - lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; - } for (i = param_index; i < lastp; ++i) { mono_metadata_decode_row (paramt, i -1, cols, MONO_PARAM_SIZE); if (cols [MONO_PARAM_SEQUENCE] && cols [MONO_PARAM_SEQUENCE] <= signature->param_count) /* skip return param spec and bounds check*/ @@ -1499,7 +1489,6 @@ guint32 mono_method_get_param_token (MonoMethod *method, int index) { MonoClass *klass = method->klass; - MonoTableInfo *methodt; guint32 idx; mono_class_init_internal (klass); @@ -1507,14 +1496,10 @@ mono_method_get_param_token (MonoMethod *method, int index) MonoImage *klass_image = m_class_get_image (klass); g_assert (!image_is_dynamic (klass_image)); - methodt = &klass_image->tables [MONO_TABLE_METHOD]; idx = mono_method_get_index (method); if (idx > 0) { - guint param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); - - if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { - param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), NULL); - } + guint param_index = mono_metadata_get_method_params (klass_image, idx, NULL); + if (index == -1) /* Return value */ return mono_metadata_make_token (MONO_TABLE_PARAM, 0); @@ -1533,7 +1518,6 @@ mono_method_get_marshal_info (MonoMethod *method, MonoMarshalSpec **mspecs) { int i, lastp; MonoClass *klass = method->klass; - MonoTableInfo *methodt; MonoTableInfo *paramt; MonoMethodSignature *signature; guint32 idx; @@ -1571,26 +1555,15 @@ mono_method_get_marshal_info (MonoMethod *method, MonoMarshalSpec **mspecs) mono_class_init_internal (klass); MonoImage *klass_image = m_class_get_image (klass); - methodt = &klass_image->tables [MONO_TABLE_METHOD]; paramt = &klass_image->tables [MONO_TABLE_PARAM]; idx = mono_method_get_index (method); if (idx > 0) { guint32 cols [MONO_PARAM_SIZE]; - guint param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); - - if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { - uint32_t count; - param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), &count); - if (!param_index) - return; - lastp = param_index + count; - } else { - if (idx < table_info_get_rows (methodt)) - lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; - } - + guint param_index = mono_metadata_get_method_params (klass_image, idx, (uint32_t*)&lastp); + + if (!param_index) + return; + for (i = param_index; i < lastp; ++i) { mono_metadata_decode_row (paramt, i -1, cols, MONO_PARAM_SIZE); @@ -1614,7 +1587,6 @@ mono_method_has_marshal_info (MonoMethod *method) { int i, lastp; MonoClass *klass = method->klass; - MonoTableInfo *methodt; MonoTableInfo *paramt; guint32 idx; @@ -1634,26 +1606,15 @@ mono_method_has_marshal_info (MonoMethod *method) mono_class_init_internal (klass); MonoImage *klass_image = m_class_get_image (klass); - methodt = &klass_image->tables [MONO_TABLE_METHOD]; paramt = &klass_image->tables [MONO_TABLE_PARAM]; idx = mono_method_get_index (method); if (idx > 0) { guint32 cols [MONO_PARAM_SIZE]; - guint param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); - - if (G_UNLIKELY (param_index == 0 && klass_image->has_updates)) { - uint32_t count; - param_index = mono_metadata_update_get_method_params (klass_image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), &count); - if (!param_index) - return FALSE; - lastp = param_index + count; - } else { - if (idx + 1 < table_info_get_rows (methodt)) - lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; - } - + guint param_index = mono_metadata_get_method_params (klass_image, idx, (uint32_t*)&lastp); + + if (!param_index) + return FALSE; + for (i = param_index; i < lastp; ++i) { mono_metadata_decode_row (paramt, i -1, cols, MONO_PARAM_SIZE); diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h index 5c6ea98fca365c..91845152d3ecce 100644 --- a/src/mono/mono/metadata/metadata-internals.h +++ b/src/mono/mono/metadata/metadata-internals.h @@ -1248,4 +1248,7 @@ mono_metadata_table_to_ptr_table (int table_num) } } +uint32_t +mono_metadata_get_method_params (MonoImage *image, uint32_t method_idx, uint32_t *last_param_out); + #endif /* __MONO_METADATA_INTERNALS_H__ */ diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index f63ec38ee465dc..0c63c068929173 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -2277,21 +2277,12 @@ gboolean mono_metadata_method_has_param_attrs (MonoImage *m, int def) { MonoTableInfo *paramt = &m->tables [MONO_TABLE_PARAM]; - MonoTableInfo *methodt = &m->tables [MONO_TABLE_METHOD]; - guint lastp, i, param_index = mono_metadata_decode_row_col (methodt, def - 1, MONO_METHOD_PARAMLIST); + guint lastp, i, param_index; - if (G_UNLIKELY (param_index == 0 && m->has_updates)) { - uint32_t count; - param_index = mono_metadata_update_get_method_params (m, mono_metadata_make_token (MONO_TABLE_METHOD, def), &count); - if (!param_index) - return FALSE; - lastp = param_index + count; - } else { - if (GINT_TO_UINT32(def) < table_info_get_rows (methodt)) - lastp = mono_metadata_decode_row_col (methodt, def, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (&m->tables [MONO_TABLE_PARAM]) + 1; - } + param_index = mono_metadata_get_method_params (m, def, (uint32_t*)&lastp); + + if (!param_index) + return FALSE; for (i = param_index; i < lastp; ++i) { guint32 flags = mono_metadata_decode_row_col (paramt, i - 1, MONO_PARAM_FLAGS); @@ -2317,23 +2308,14 @@ int* mono_metadata_get_param_attrs (MonoImage *m, int def, guint32 param_count) { MonoTableInfo *paramt = &m->tables [MONO_TABLE_PARAM]; - MonoTableInfo *methodt = &m->tables [MONO_TABLE_METHOD]; guint32 cols [MONO_PARAM_SIZE]; - guint lastp, i, param_index = mono_metadata_decode_row_col (methodt, def - 1, MONO_METHOD_PARAMLIST); + guint lastp, i, param_index; int *pattrs = NULL; - if (G_UNLIKELY (param_index == 0 && m->has_updates)) { - uint32_t count; - param_index = mono_metadata_update_get_method_params (m, mono_metadata_make_token (MONO_TABLE_METHOD, def), &count); - if (!param_index) - return NULL; - lastp = param_index + count; - } else { - if (GINT_TO_UINT32(def) < mono_metadata_table_num_rows (m, MONO_TABLE_METHOD)) - lastp = mono_metadata_decode_row_col (methodt, def, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; - } + param_index = mono_metadata_get_method_params (m, def, (uint32_t*)&lastp); + + if (!param_index) + return NULL; for (i = param_index; i < lastp; ++i) { mono_metadata_decode_row (paramt, i - 1, cols, MONO_PARAM_SIZE); @@ -8131,3 +8113,40 @@ mono_metadata_get_class_guid (MonoClass* klass, guint8* guid, MonoError *error) g_warning ("Generated GUIDs only implemented for interfaces!"); #endif } + +uint32_t +mono_metadata_get_method_params (MonoImage *image, uint32_t method_idx, uint32_t *last_param_out) +{ + if (last_param_out) + *last_param_out = 0; + if (!method_idx) + return 0; + + MonoTableInfo *methodt = &image->tables [MONO_TABLE_METHOD]; + + uint32_t param_index, lastp; + + param_index = mono_metadata_decode_row_col (methodt, method_idx - 1, MONO_METHOD_PARAMLIST); + + if (G_UNLIKELY (param_index == 0 && image->has_updates)) { + uint32_t count; + param_index = mono_metadata_update_get_method_params (image, mono_metadata_make_token (MONO_TABLE_METHOD, method_idx), &count); + if (!param_index) + return 0; + lastp = param_index + count; + } else { + /* lastp is the starting param index for the next method in the table, or + * one past the last row if this is the last method + */ + + if (method_idx < table_info_get_rows (methodt)) + lastp = mono_metadata_decode_row_col (methodt, method_idx, MONO_METHOD_PARAMLIST); + else + lastp = table_info_get_rows (&image->tables [MONO_TABLE_PARAM]) + 1; + } + + if (last_param_out) + *last_param_out = lastp; + + return param_index; +} diff --git a/src/mono/mono/metadata/reflection.c b/src/mono/mono/metadata/reflection.c index 098dd3860b38bc..52107a74225441 100644 --- a/src/mono/mono/metadata/reflection.c +++ b/src/mono/mono/metadata/reflection.c @@ -1392,7 +1392,6 @@ get_default_param_value_blobs (MonoMethod *method, char **blobs, guint32 *types) MonoMethodSignature *methodsig = mono_method_signature_internal (method); MonoTableInfo *constt; - MonoTableInfo *methodt; MonoTableInfo *paramt; if (!methodsig->param_count) @@ -1412,30 +1411,16 @@ get_default_param_value_blobs (MonoMethod *method, char **blobs, guint32 *types) return; } - methodt = &image->tables [MONO_TABLE_METHOD]; paramt = &image->tables [MONO_TABLE_PARAM]; constt = &image->tables [MONO_TABLE_CONSTANT]; idx = mono_method_get_index (method); g_assert (idx != 0); - /* lastp is the starting param index for the next method in the table, or - * one past the last row if this is the last method - */ - /* FIXME: metadata-update : will this work with added methods - no, needs a change */ - param_index = mono_metadata_decode_row_col (methodt, idx - 1, MONO_METHOD_PARAMLIST); - if (G_UNLIKELY (param_index == 0 && image->has_updates)) { - uint32_t count; - param_index = mono_metadata_update_get_method_params (image, mono_metadata_make_token (MONO_TABLE_METHOD, idx), &count); - if (!param_index) - return; - lastp = param_index + count; - } else { - if (!mono_metadata_table_bounds_check (image, MONO_TABLE_METHOD, idx + 1)) - lastp = mono_metadata_decode_row_col (methodt, idx, MONO_METHOD_PARAMLIST); - else - lastp = table_info_get_rows (paramt) + 1; - } + param_index = mono_metadata_get_method_params (image, idx, &lastp); + + if (!param_index) + return; for (i = param_index; i < lastp; ++i) { guint32 paramseq;