From 74ac7d4d065f5f29a0811b7ce9cc5a76a1cb6024 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 27 Sep 2022 12:11:58 -0400 Subject: [PATCH 01/55] [hot_reload] Add new AddInstanceField test --- .../AddInstanceField.cs | 25 +++++++++++++++ .../AddInstanceField_v1.cs | 31 +++++++++++++++++++ ...a.ApplyUpdate.Test.AddInstanceField.csproj | 11 +++++++ .../deltascript.json | 6 ++++ .../tests/ApplyUpdateTest.cs | 30 ++++++++++++++++++ .../tests/System.Runtime.Loader.Tests.csproj | 1 + 6 files changed, 104 insertions(+) create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField.csproj create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/deltascript.json diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs new file mode 100644 index 00000000000000..9b9d388fd7f270 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs @@ -0,0 +1,25 @@ +// 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 AddInstanceField + { + public AddInstanceField () { + } + + public string GetStringField => _stringField; + public double GetDoubleField => _doubleField; + + private string _stringField; + private double _doubleField; + + public void TestMethod () { + _stringField = "abcd"; + _doubleField = 3.14159; + } + + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs new file mode 100644 index 00000000000000..6c02ca72994d54 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs @@ -0,0 +1,31 @@ +// 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 AddInstanceField + { + public AddInstanceField () { + _doubleField2 = -5.5e12; + _stringField2 = "New Initial Value"; + } + + public string GetStringField => _stringField2; + public double GetDoubleField => _doubleField2; + + private string _stringField; + private string _stringField2; + private double _doubleField; + private double _doubleField2; + + public void TestMethod () { + _stringField = "spqr"; + _stringField2 = "4567"; + _doubleField = 2.71828; + _doubleField2 = 0.707106; + } + + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField.csproj new file mode 100644 index 00000000000000..712e5b36fcce23 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField.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.AddInstanceField/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/deltascript.json new file mode 100644 index 00000000000000..3c81223a4fc7d9 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/deltascript.json @@ -0,0 +1,6 @@ +{ + "changes": [ + {"document": "AddInstanceField.cs", "update": "AddInstanceField_v1.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 92f19a6b0c023e..07025c938825b1 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -308,6 +308,36 @@ public static void TestAddStaticField() }); } + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestAddInstanceField() + { + // Test that adding a new instance field to an existing class is supported + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField).Assembly; + + var x1 = new System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField(); + + x1.TestMethod(); + + Assert.Equal ("abcd", x1.GetStringField); + Assert.Equal (3.14159, x1.GetDoubleField); + + ApplyUpdateUtil.ApplyUpdate(assm); + + x1.TestMethod(); + + Assert.Equal ("4567", x1.GetStringField); + Assert.Equal (0.707106, x1.GetDoubleField); + + var x2 = new System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField(); + + Assert.Equal ("New Initial Value", x2.GetStringField); + Assert.Equal (-5.5e12, x2.GetDoubleField); + + }); + } + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddNestedClass() { 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 6c5a8469726179..041e21c9d9af8b 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 @@ -58,6 +58,7 @@ + From 0841c4088254038cbbc454dad537c6cbab52361e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 29 Sep 2022 11:16:22 -0400 Subject: [PATCH 02/55] WIP builds doesn't link; need impl for mono_metadata_update_ldflda --- .../src/Mono/HotReload.cs | 10 ++-- src/mono/mono/component/hot_reload.c | 2 +- src/mono/mono/metadata/metadata-update.h | 4 ++ src/mono/mono/mini/interp/interp.c | 13 ++++- src/mono/mono/mini/interp/mintops.def | 4 ++ src/mono/mono/mini/interp/transform.c | 53 ++++++++++++++++++- 6 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs index f9096a68869bf0..be5eeec8e1544b 100644 --- a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs +++ b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs @@ -10,8 +10,8 @@ namespace Mono.HotReload; // TODO: this is just a sketch, instance field additions aren't supported by Mono yet until https://github.com/dotnet/runtime/issues/63643 is fixed -#if false -internal class InstanceFieldTable +#if true +internal sealed class InstanceFieldTable { // Q: Does CoreCLR EnC allow adding fields to a valuetype? // A: No, see EEClass::AddField - if the type has layout or is a valuetype, you can't add fields to it. @@ -35,8 +35,8 @@ internal class InstanceFieldTable // we want to create some storage space that has the same lifetime as the instance object. // // TODO: should the linker keep this if Hot Reload stuff is enabled? Hot Reload is predicated on the linker not rewriting user modules, but maybe trimming SPC is ok? - internal static FieldStore GetInstanceFieldFieldStore(object inst, RuntimeTypeHandle type, uint fielddef_token) - => _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(type, fielddef_token); + internal static FieldStore GetInstanceFieldFieldStore(object inst, IntPtr type, uint fielddef_token) + => _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(new RuntimeTypeHandle (type), fielddef_token); private static InstanceFieldTable _singleton = new(); @@ -50,7 +50,7 @@ private InstanceFieldTable() private InstanceFields GetOrCreateInstanceFields(object key) => _table.GetOrCreateValue(key); - private class InstanceFields + private sealed class InstanceFields { private Dictionary _fields; private object _lock; diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index c412aa3efe6662..cd7c43ab511189 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -34,7 +34,7 @@ #include -#undef ALLOW_INSTANCE_FIELD_ADD +#define ALLOW_INSTANCE_FIELD_ADD typedef struct _BaselineInfo BaselineInfo; typedef struct _DeltaInfo DeltaInfo; diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index e68ca1a8ad71ae..b0a2b8f6c77f3e 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -96,4 +96,8 @@ 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); + +gpointer +mono_metadata_update_ldflda (MonoObject *instance, MonoType *field_type, guint32 fielddef_token, MonoError *error); + #endif /*__MONO_METADATA_UPDATE_H__*/ diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 062b9f290c577c..3bc0480fb26218 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -7504,7 +7504,17 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; ip += 3; MINT_IN_BREAK; } - + MINT_IN_CASE(MINT_METADATA_UPDATE_LDFLDA) { + gpointer *dest = LOCAL_VAR (ip [1], gpointer); + MonoObject *inst = LOCAL_VAR (ip [2], MonoObject*); + MonoType *field_type = frame->imethod->data_items [ip [3]]; // correct offset? + MonoType *fielddef_token = frame->imethod->data_items [ip [4]]; // offset? + // FIXME: can we emit a call directly instead of a runtime-invoke? + *dest = mono_metadata_update_ldflda (inst, field_type, fielddef_token, error); // FIXME write barrier + mono_interp_error_cleanup (error); + ip += 5; + MINT_IN_BREAK; + } #ifdef HOST_BROWSER MINT_IN_CASE(MINT_TIER_NOP_JITERPRETER) { ip += 3; @@ -7611,7 +7621,6 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; MINT_IN_BREAK; } #endif - #if !USE_COMPUTED_GOTO default: interp_error_xsx ("Unimplemented opcode: %04x %s at 0x%x\n", *ip, mono_interp_opname (*ip), GPTRDIFF_TO_INT (ip - frame->imethod->code)); diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index 8fa0c024a41d4e..a19a6e1aec506a 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -800,7 +800,11 @@ OPDEF(MINT_INTRINS_64ORDINAL_IGNORE_CASE_ASCII, "intrins_64ordinal_ignore_case_a OPDEF(MINT_INTRINS_U32_TO_DECSTR, "intrins_u32_to_decstr", 5, 1, 1, MintOpTwoShorts) OPDEF(MINT_INTRINS_WIDEN_ASCII_TO_UTF16, "intrins_widen_ascii_to_utf16", 5, 1, 3, MintOpNoArgs) +OPDEF(MINT_METADATA_UPDATE_LDFLDA, "metadata_update.ldflda", 5, 1, 1, MintOpTwoShorts) + // TODO: Make this wasm only OPDEF(MINT_TIER_PREPARE_JITERPRETER, "tier_prepare_jiterpreter", 3, 0, 0, MintOpInt) OPDEF(MINT_TIER_NOP_JITERPRETER, "tier_nop_jiterpreter", 3, 0, 0, MintOpInt) OPDEF(MINT_TIER_ENTER_JITERPRETER, "tier_enter_jiterpreter", 3, 0, 0, MintOpInt) + + diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 89a611b1713764..c42cddcfd81301 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -1919,6 +1919,42 @@ interp_emit_ldelema (TransformData *td, MonoClass *array_class, MonoClass *check interp_ins_set_dreg (td->last_ins, td->sp [-1].local); } +static GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_instance_field_table); + +static GENERATE_GET_CLASS_WITH_CACHE(hot_reload_instance_field_table, "Mono.HotReload", "InstanceFieldTable"); + + +static void +interp_emit_metadata_update_ldflda (TransformData *td, MonoClassField *field, MonoError *error) +{ + g_assert (m_field_is_from_update (field)); + MonoType *field_type = field->type; + g_assert (!m_type_is_byref (field_type)); + MonoClass *field_klass = mono_class_from_mono_type_internal (field_type); + /* get a heap-allocated version of the field type */ + field_type = m_class_get_byval_arg (field_klass); + guint32 field_token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); + +#if 0 + // FIXME: this should go in interp.c + static MonoMethod *get_instance_store = NULL; + if (G_UNLIKELY (get_instance_store == NULL)) { + MonoClass *table_class = mono_class_get_hot_reload_instance_field_table_class (); + get_instance_store = mono_class_get_method_from_name_checked (table_class, "GetInstanceFieldStore", 3, 0, error); + mono_error_assert_ok (error); + } + g_assert (get_instance_store); +#endif + interp_add_ins (td, MINT_METADATA_UPDATE_LDFLDA); + td->sp--; + interp_ins_set_sreg (td->last_ins, td->sp [0].local); + push_simple_type (td, STACK_TYPE_MP); + interp_ins_set_dreg (td->last_ins, td->sp [-1].local); + td->last_ins->data [0] = get_data_item_index (td, field_type); + td->last_ins->data [1] = get_data_item_index (td, GUINT_TO_POINTER (field_token)); +} + + /* Return TRUE if call transformation is finished */ static gboolean interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClass *constrained_class, MonoMethodSignature *csignature, gboolean readonly, int *op) @@ -6177,8 +6213,21 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, mono_class_vtable_checked (field_klass, error); goto_if_nok (error, exit); } else { - /* TODO: metadata-update: implement me */ - g_assert (!m_field_is_from_update (field)); + if (G_UNLIKELY (m_field_is_from_update (field))) { + // FIXME: if field is byref, we probably just want mono_defaults.int_class + g_assert (!m_type_is_byref (field->type)); + MonoClass *field_class = mono_class_from_mono_type_internal (field->type); + MonoType *local_type = m_class_get_byval_arg (field_class); + int local = create_interp_local (td, local_type); + store_local (td, local); + interp_emit_metadata_update_ldflda (td, field, error); + goto_if_nok (error, exit); + load_local (td, local); + interp_emit_stobj (td, field_class); + td->ip += 5; + break; + } + int opcode = MINT_STFLD_I1 + mt - MINT_TYPE_I1; #ifdef NO_UNALIGNED_ACCESS if ((mt == MINT_TYPE_I8 || mt == MINT_TYPE_R8) && field->offset % SIZEOF_VOID_P != 0) From f24421834783baea2298d2a51cf4d164a77d1d6b Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 29 Sep 2022 13:24:53 -0400 Subject: [PATCH 03/55] WIP builds --- src/mono/mono/component/hot_reload-stub.c | 11 ++++++++ src/mono/mono/component/hot_reload.c | 32 +++++++++++++++++++++++ src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/metadata-update.c | 6 +++++ src/mono/mono/metadata/metadata-update.h | 2 +- src/mono/mono/mini/interp/interp.c | 9 ++++--- src/mono/mono/mini/interp/transform.c | 10 ------- 7 files changed, 57 insertions(+), 14 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 94730bbe26b164..852a79a2cedea0 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -113,6 +113,9 @@ 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 gpointer +hot_reload_stub_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -147,6 +150,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_num_methods_added, &hot_reload_get_capabilities, &hot_reload_stub_get_method_params, + &hot_reload_stub_added_field_ldflda, }; static bool @@ -353,9 +357,16 @@ hot_reload_stub_get_method_params (MonoImage *base_image, uint32_t methoddef_tok return 0; } +static gpointer +hot_reload_stub_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error) +{ + return NULL; +} + MONO_COMPONENT_EXPORT_ENTRYPOINT MonoComponentHotReload * mono_component_hot_reload_init (void) { return component_hot_reload_stub_init (); } + diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index cd7c43ab511189..eadd1ace834ac8 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -147,6 +147,9 @@ 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 gpointer +hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); + 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); @@ -184,6 +187,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_num_methods_added, &hot_reload_get_capabilities, &hot_reload_get_method_params, + &hot_reload_added_field_ldflda, }; MonoComponentHotReload * @@ -3236,3 +3240,31 @@ hot_reload_get_capabilities (void) { return "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition ChangeCustomAttributes"; } + +static GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_instance_field_table); + +static GENERATE_GET_CLASS_WITH_CACHE(hot_reload_instance_field_table, "Mono.HotReload", "InstanceFieldTable"); + + +static gpointer +hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error) +{ + // FIXME: this should go in interp.c + static MonoMethod *get_instance_store = NULL; + if (G_UNLIKELY (get_instance_store == NULL)) { + MonoClass *table_class = mono_class_get_hot_reload_instance_field_table_class (); + get_instance_store = mono_class_get_method_from_name_checked (table_class, "GetInstanceFieldStore", 3, 0, error); + mono_error_assert_ok (error); + } + g_assert (get_instance_store); + + gpointer args[3]; + + args[0] = instance; + args[1] = &field_type; + args[2] = &fielddef_token; + + gpointer result; + result = mono_runtime_invoke_checked (get_instance_store, NULL, args, error); + return result; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 40c6a0f263f417..046e52a9f0c3d4 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -49,6 +49,7 @@ typedef struct _MonoComponentHotReload { 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); + gpointer (*added_field_ldflda) (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 9541a25384c00a..6e912bfc194447 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -236,3 +236,9 @@ mono_metadata_update_get_method_params (MonoImage *image, uint32_t methoddef_tok { return mono_component_hot_reload()->get_method_params (image, methoddef_token, out_param_count_opt); } + +gpointer +mono_metadata_update_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error) +{ + return mono_component_hot_reload()->added_field_ldflda (instance, field_type, fielddef_token, error); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index b0a2b8f6c77f3e..ab6dd60ac39c33 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -98,6 +98,6 @@ uint32_t mono_metadata_update_get_method_params (MonoImage *image, uint32_t methoddef_token, uint32_t *out_param_count_opt); gpointer -mono_metadata_update_ldflda (MonoObject *instance, MonoType *field_type, guint32 fielddef_token, MonoError *error); +mono_metadata_update_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); #endif /*__MONO_METADATA_UPDATE_H__*/ diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 3bc0480fb26218..a4a8c1a9a5dde1 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -7507,10 +7507,13 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; MINT_IN_CASE(MINT_METADATA_UPDATE_LDFLDA) { gpointer *dest = LOCAL_VAR (ip [1], gpointer); MonoObject *inst = LOCAL_VAR (ip [2], MonoObject*); - MonoType *field_type = frame->imethod->data_items [ip [3]]; // correct offset? - MonoType *fielddef_token = frame->imethod->data_items [ip [4]]; // offset? + MonoType *field_type = frame->imethod->data_items [ip [3]]; + uint32_t fielddef_token = GPOINTER_TO_UINT32 (frame->imethod->data_items [ip [4]]); // FIXME: can we emit a call directly instead of a runtime-invoke? - *dest = mono_metadata_update_ldflda (inst, field_type, fielddef_token, error); // FIXME write barrier + gpointer field_addr = mono_metadata_update_added_field_ldflda (inst, field_type, fielddef_token, error); + mono_gc_wbarrier_generic_store_internal (dest, field_addr); + /* FIXME: think about pinning the FieldStore and adding a second opcode to + * unpin it */ mono_interp_error_cleanup (error); ip += 5; MINT_IN_BREAK; diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index c42cddcfd81301..a0a037e6e7514e 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -1935,16 +1935,6 @@ interp_emit_metadata_update_ldflda (TransformData *td, MonoClassField *field, Mo field_type = m_class_get_byval_arg (field_klass); guint32 field_token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); -#if 0 - // FIXME: this should go in interp.c - static MonoMethod *get_instance_store = NULL; - if (G_UNLIKELY (get_instance_store == NULL)) { - MonoClass *table_class = mono_class_get_hot_reload_instance_field_table_class (); - get_instance_store = mono_class_get_method_from_name_checked (table_class, "GetInstanceFieldStore", 3, 0, error); - mono_error_assert_ok (error); - } - g_assert (get_instance_store); -#endif interp_add_ins (td, MINT_METADATA_UPDATE_LDFLDA); td->sp--; interp_ins_set_sreg (td->last_ins, td->sp [0].local); From b13e6ffc9e1127d8c863e72f67cd79f65f3ef289 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 29 Sep 2022 16:31:15 -0400 Subject: [PATCH 04/55] WIP test runs, fails on string load/stores --- .../src/ILLink/ILLink.Descriptors.xml | 4 ++++ .../src/Mono/HotReload.cs | 18 +++++++++++------- src/mono/mono/component/hot_reload.c | 2 +- src/mono/mono/mini/interp/transform.c | 16 ++++++++++++++-- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml b/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml index ee6d7863ebd6b1..34793a744e799a 100644 --- a/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml +++ b/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml @@ -644,5 +644,9 @@ + + + + diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs index be5eeec8e1544b..ae7b20a341a68b 100644 --- a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs +++ b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -35,8 +36,8 @@ internal sealed class InstanceFieldTable // we want to create some storage space that has the same lifetime as the instance object. // // TODO: should the linker keep this if Hot Reload stuff is enabled? Hot Reload is predicated on the linker not rewriting user modules, but maybe trimming SPC is ok? - internal static FieldStore GetInstanceFieldFieldStore(object inst, IntPtr type, uint fielddef_token) - => _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(new RuntimeTypeHandle (type), fielddef_token); + internal static ref object? GetInstanceFieldFieldStore(object inst, IntPtr type, uint fielddef_token) + => ref _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(new RuntimeTypeHandle (type), fielddef_token); private static InstanceFieldTable _singleton = new(); @@ -61,18 +62,20 @@ public InstanceFields() _lock = new(); } - public FieldStore LookupOrAdd(RuntimeTypeHandle type, uint key) + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", + Justification = "Hot reload required untrimmed apps")] + public ref object? LookupOrAdd(RuntimeTypeHandle type, uint key) { if (_fields.TryGetValue(key, out FieldStore? v)) - return v; + return ref v.Location; lock (_lock) { if (_fields.TryGetValue (key, out FieldStore? v2)) - return v2; + return ref v2.Location; FieldStore s = FieldStore.Create(type); _fields.Add(key, s); - return s; + return ref s.Location; } } } @@ -96,8 +99,9 @@ private FieldStore (object? loc) _loc = loc; } - public object? Location => _loc; + public ref object? Location => ref _loc; + [RequiresUnreferencedCode("Hot reload required untrimmed apps")] public static FieldStore Create (RuntimeTypeHandle type) { Type t = Type.GetTypeFromHandle(type) ?? throw new ArgumentException(nameof(type), "Type handle was null"); diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index eadd1ace834ac8..b7fc9035355d3c 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -3253,7 +3253,7 @@ hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint3 static MonoMethod *get_instance_store = NULL; if (G_UNLIKELY (get_instance_store == NULL)) { MonoClass *table_class = mono_class_get_hot_reload_instance_field_table_class (); - get_instance_store = mono_class_get_method_from_name_checked (table_class, "GetInstanceFieldStore", 3, 0, error); + get_instance_store = mono_class_get_method_from_name_checked (table_class, "GetInstanceFieldFieldStore", 3, 0, error); mono_error_assert_ok (error); } g_assert (get_instance_store); diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index a0a037e6e7514e..43d75fb161d95f 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -6150,8 +6150,20 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, push_type (td, stack_type [mt], field_klass); interp_ins_set_dreg (td->last_ins, td->sp [-1].local); } else { - /* TODO: metadata-update: implement me */ - g_assert (!m_field_is_from_update (field)); + if (G_UNLIKELY (m_field_is_from_update (field))) { + g_assert (!m_type_is_byref (field->type)); + MonoClass *field_class = mono_class_from_mono_type_internal (field->type); + MonoType *local_type = m_class_get_byval_arg (field_class); + interp_emit_metadata_update_ldflda (td, field, error); + goto_if_nok (error, exit); + interp_add_ins (td, interp_get_ldind_for_mt (mt)); + interp_ins_set_sreg (td->last_ins, td->sp [-1].local); + td->sp--; + push_type (td, stack_type [mt], field_class); + interp_ins_set_dreg (td->last_ins, td->sp[-1].local); + td->ip += 5; + break; + } int opcode = MINT_LDFLD_I1 + mt - MINT_TYPE_I1; #ifdef NO_UNALIGNED_ACCESS if ((mt == MINT_TYPE_I8 || mt == MINT_TYPE_R8) && field->offset % SIZEOF_VOID_P != 0) From a404ee3102a327b11105da2c2dae2b8998d89cef Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 30 Sep 2022 14:57:43 -0400 Subject: [PATCH 05/55] Simple test works --- .../src/Mono/HotReload.cs | 17 ++++++++--------- src/mono/mono/component/hot_reload.c | 13 ++++++++++--- src/mono/mono/mini/interp/interp.c | 4 ++-- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs index ae7b20a341a68b..51694041c62f93 100644 --- a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs +++ b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs @@ -35,9 +35,10 @@ internal sealed class InstanceFieldTable // This should behave somewhat like EditAndContinueModule::ResolveOrAddField (and EnCAddedField::Allocate) // we want to create some storage space that has the same lifetime as the instance object. - // // TODO: should the linker keep this if Hot Reload stuff is enabled? Hot Reload is predicated on the linker not rewriting user modules, but maybe trimming SPC is ok? - internal static ref object? GetInstanceFieldFieldStore(object inst, IntPtr type, uint fielddef_token) - => ref _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(new RuntimeTypeHandle (type), fielddef_token); + internal static FieldStore GetInstanceFieldFieldStore(object inst, IntPtr type, uint fielddef_token) + { + return _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(new RuntimeTypeHandle (type), fielddef_token); + } private static InstanceFieldTable _singleton = new(); @@ -64,18 +65,18 @@ public InstanceFields() [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode", Justification = "Hot reload required untrimmed apps")] - public ref object? LookupOrAdd(RuntimeTypeHandle type, uint key) + public FieldStore LookupOrAdd(RuntimeTypeHandle type, uint key) { if (_fields.TryGetValue(key, out FieldStore? v)) - return ref v.Location; + return v; lock (_lock) { if (_fields.TryGetValue (key, out FieldStore? v2)) - return ref v2.Location; + return v2; FieldStore s = FieldStore.Create(type); _fields.Add(key, s); - return ref s.Location; + return s; } } } @@ -99,8 +100,6 @@ private FieldStore (object? loc) _loc = loc; } - public ref object? Location => ref _loc; - [RequiresUnreferencedCode("Hot reload required untrimmed apps")] public static FieldStore Create (RuntimeTypeHandle type) { diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index b7fc9035355d3c..c2eac496384f21 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -3249,7 +3249,6 @@ static GENERATE_GET_CLASS_WITH_CACHE(hot_reload_instance_field_table, "Mono.HotR static gpointer hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error) { - // FIXME: this should go in interp.c static MonoMethod *get_instance_store = NULL; if (G_UNLIKELY (get_instance_store == NULL)) { MonoClass *table_class = mono_class_get_hot_reload_instance_field_table_class (); @@ -3264,7 +3263,15 @@ hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint3 args[1] = &field_type; args[2] = &fielddef_token; - gpointer result; - result = mono_runtime_invoke_checked (get_instance_store, NULL, args, error); + MonoHotReloadFieldStoreObject *field_store; + field_store = (MonoHotReloadFieldStoreObject*) mono_runtime_invoke_checked (get_instance_store, NULL, args, error); + gpointer result = NULL; + /* If it's a value type, return a ptr to the beginning of the + * boxed data in FieldStore:_loc. If it's a reference type, + * return the address of FieldStore:_loc itself. */ + if (!mono_type_is_reference (field_type)) + result = mono_object_unbox_internal (field_store->_loc); + else + result = (gpointer)&field_store->_loc; return result; } diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index a4a8c1a9a5dde1..d7f847d5986f3c 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -7505,15 +7505,15 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; MINT_IN_BREAK; } MINT_IN_CASE(MINT_METADATA_UPDATE_LDFLDA) { - gpointer *dest = LOCAL_VAR (ip [1], gpointer); + gpointer *dest = (gpointer*)(locals + ip [1]); MonoObject *inst = LOCAL_VAR (ip [2], MonoObject*); MonoType *field_type = frame->imethod->data_items [ip [3]]; uint32_t fielddef_token = GPOINTER_TO_UINT32 (frame->imethod->data_items [ip [4]]); // FIXME: can we emit a call directly instead of a runtime-invoke? gpointer field_addr = mono_metadata_update_added_field_ldflda (inst, field_type, fielddef_token, error); - mono_gc_wbarrier_generic_store_internal (dest, field_addr); /* FIXME: think about pinning the FieldStore and adding a second opcode to * unpin it */ + mono_gc_wbarrier_generic_store_internal (dest, field_addr); mono_interp_error_cleanup (error); ip += 5; MINT_IN_BREAK; From c40cb120010ae86f34da18e98bf5ed5aa04781f1 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 30 Sep 2022 15:01:15 -0400 Subject: [PATCH 06/55] Add AddInstanceFieldToExistingType capability to runtime --- src/mono/mono/component/hot_reload.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index c2eac496384f21..f2cdba561f9a8c 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -3238,7 +3238,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"; + return "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition ChangeCustomAttributes AddInstanceFieldToExistingType"; } static GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_instance_field_table); From 0c7ab0be2d8e2841c5f88b0a0ae7abd924ed1f8e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 30 Sep 2022 15:34:03 -0400 Subject: [PATCH 07/55] fix comment; remove unused var --- src/mono/mono/mini/interp/transform.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 43d75fb161d95f..59748ba140e0e6 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -6124,7 +6124,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, interp_emit_sfld_access (td, field, field_klass, mt, TRUE, error); goto_if_nok (error, exit); } else if (td->sp [-1].type == STACK_TYPE_VT) { - /* TODO: metadata-update: implement me */ + /* metadata-update: can't add fields to structs */ g_assert (!m_field_is_from_update (field)); int size = 0; /* First we pop the vt object from the stack. Then we push the field */ @@ -6153,7 +6153,6 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, if (G_UNLIKELY (m_field_is_from_update (field))) { g_assert (!m_type_is_byref (field->type)); MonoClass *field_class = mono_class_from_mono_type_internal (field->type); - MonoType *local_type = m_class_get_byval_arg (field_class); interp_emit_metadata_update_ldflda (td, field, error); goto_if_nok (error, exit); interp_add_ins (td, interp_get_ldind_for_mt (mt)); From ceaee7cacac8ed25e8d1424fc9e717ab09596b83 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 3 Oct 2022 15:00:46 -0400 Subject: [PATCH 08/55] Add instance field: reflection and nested struct --- .../AddInstanceField_v1.cs | 11 ++++++++++ .../tests/ApplyUpdateTest.cs | 22 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs index 6c02ca72994d54..babb363031649b 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs @@ -10,6 +10,10 @@ public class AddInstanceField public AddInstanceField () { _doubleField2 = -5.5e12; _stringField2 = "New Initial Value"; + NewStructField = new NewStruct { + D = -1984.0, + O = new int[2] { 15, 17 }, + }; } public string GetStringField => _stringField2; @@ -27,5 +31,12 @@ public void TestMethod () { _doubleField2 = 0.707106; } + public struct NewStruct + { + public double D; + public object O; + } + + public NewStruct NewStructField; } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 07025c938825b1..61c036cbd2771e 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -335,6 +335,28 @@ public static void TestAddInstanceField() Assert.Equal ("New Initial Value", x2.GetStringField); Assert.Equal (-5.5e12, x2.GetDoubleField); + // now check that reflection can get/set the new fields + var fi = x2.GetType().GetField("NewStructField"); + + Assert.NotNull(fi); + + var s = fi.GetValue (x2); + + Assert.NotNull(x2); + + var fid = fi.GetType().GetField("D"); + + Assert.NotNull(fid); + + + Assert.Equal(-1984.0, fid.GetValue(s)); + + var tr = TypedReference.MakeTypedReference (x2, new FieldInfo[] {fi}); + + fid.SetValueDirect(tr, (object)34567.0); + + Assert.Equal (34567.0, fid.GetValueDirect (tr)); + }); } From d9d8827f9168ebcb3f532e456b416f8fd091da67 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 4 Oct 2022 16:10:16 -0400 Subject: [PATCH 09/55] make TypedReference work with added fields --- src/mono/mono/metadata/icall.c | 32 ++++++++++++++++++++++++-------- src/mono/mono/metadata/object.c | 12 ++++++++---- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index dc3a22f72c7cd8..cc60b52fc54432 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -6301,27 +6301,43 @@ ves_icall_System_TypedReference_InternalMakeTypedReference (MonoTypedRef *res, M (void)mono_handle_class (target); - int offset = 0; + /* if relative, offset is from the start of target. Otherwise offset is actually an address */ + gboolean relative = TRUE; + intptr_t offset = 0; for (guint i = 0; i < mono_array_handle_length (fields); ++i) { MonoClassField *f; MONO_HANDLE_ARRAY_GETVAL (f, fields, MonoClassField*, i); g_assert (f); - /* TODO: metadata-update: the first field might be added, right? the rest are inside structs */ - g_assert (!m_field_is_from_update (f)); - - if (i == 0) - offset = m_field_get_offset (f); - else + if (i == 0) { + if (G_LIKELY (!m_field_is_from_update (f))) + offset = m_field_get_offset (f); + else { + /* The first field was added by a metadata-update to an exsiting type. + * Since it's store outside the object, offset is an absolute address + */ + relative = FALSE; + ERROR_DECL (error); + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (f)); + offset = (intptr_t) mono_metadata_update_added_field_ldflda (MONO_HANDLE_RAW (target), f->type, token, error); + mono_error_assert_ok (error); + } + } else { + /* metadata-update: the first field might be added, the rest are inside structs */ + g_assert (!m_field_is_from_update (f)); offset += m_field_get_offset (f) - sizeof (MonoObject); + } (void)mono_class_from_mono_type_internal (f->type); ftype = f->type; } res->type = ftype; res->klass = mono_class_from_mono_type_internal (ftype); - res->value = (guint8*)MONO_HANDLE_RAW (target) + offset; + if (G_LIKELY (relative)) + res->value = (guint8*)MONO_HANDLE_RAW (target) + offset; + else + res->value = (guint8*)offset; } void diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index fde5f8ed8476fc..ac7824273c9282 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -2932,10 +2932,14 @@ mono_field_get_value_internal (MonoObject *obj, MonoClassField *field, void *val g_return_if_fail (!(field->type->attrs & FIELD_ATTRIBUTE_STATIC)); - /* TODO: metadata-update: implement me */ - g_assert (!m_field_is_from_update (field)); - - src = (char*)obj + m_field_get_offset (field); + if (G_UNLIKELY (m_field_is_from_update (field))) { + ERROR_DECL (error); + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); + src = mono_metadata_update_added_field_ldflda (obj, field->type, token, error); + mono_error_assert_ok (error); + } else { + src = (char*)obj + m_field_get_offset (field); + } mono_copy_value (field->type, value, src, TRUE); } From cfc91d37142df50e0f3d39761afa9fe804f306e1 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 4 Oct 2022 16:10:35 -0400 Subject: [PATCH 10/55] Add FieldInfo.SetValue testcase --- .../tests/ApplyUpdateTest.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 61c036cbd2771e..9044d6f2e2091b 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -344,19 +344,20 @@ public static void TestAddInstanceField() Assert.NotNull(x2); - var fid = fi.GetType().GetField("D"); - + var fid = fi.FieldType.GetField("D"); Assert.NotNull(fid); - - Assert.Equal(-1984.0, fid.GetValue(s)); - var tr = TypedReference.MakeTypedReference (x2, new FieldInfo[] {fi}); - fid.SetValueDirect(tr, (object)34567.0); - Assert.Equal (34567.0, fid.GetValueDirect (tr)); + fi = x2.GetType().GetField("_doubleField2", BindingFlags.NonPublic | BindingFlags.Instance); + + Assert.NotNull(fi); + + fi.SetValue(x2, 65535.01); + Assert.Equal(65535.01, x2.GetDoubleField); + }); } From d423ae87bb7606281e904db1f5049493b6933bcb Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 4 Oct 2022 16:13:57 -0400 Subject: [PATCH 11/55] make FieldInfo.SetValue work --- src/mono/mono/metadata/object.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index ac7824273c9282..473a815e488b53 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -2773,10 +2773,14 @@ mono_field_set_value_internal (MonoObject *obj, MonoClassField *field, void *val if ((field->type->attrs & FIELD_ATTRIBUTE_STATIC)) return; - /* TODO: metadata-update: implement support for added fields */ - g_assert (!m_field_is_from_update (field)); + if (G_UNLIKELY (m_field_is_from_update (field))) { + ERROR_DECL (error); + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); + dest = mono_metadata_update_added_field_ldflda (obj, field->type, token, error); + mono_error_assert_ok (error); + } else + dest = (char*)obj + m_field_get_offset (field); - dest = (char*)obj + m_field_get_offset (field); mono_copy_value (field->type, dest, value, value && field->type->type == MONO_TYPE_PTR); } From 95318ff490286d315354ecaa76a239c96dc32336 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 4 Oct 2022 16:15:39 -0400 Subject: [PATCH 12/55] fix whitespace --- src/mono/mono/metadata/object.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index 473a815e488b53..16f83a4bcbea9b 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -2773,13 +2773,13 @@ mono_field_set_value_internal (MonoObject *obj, MonoClassField *field, void *val if ((field->type->attrs & FIELD_ATTRIBUTE_STATIC)) return; - if (G_UNLIKELY (m_field_is_from_update (field))) { - ERROR_DECL (error); - uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); - dest = mono_metadata_update_added_field_ldflda (obj, field->type, token, error); - mono_error_assert_ok (error); - } else - dest = (char*)obj + m_field_get_offset (field); + if (G_UNLIKELY (m_field_is_from_update (field))) { + ERROR_DECL (error); + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); + dest = mono_metadata_update_added_field_ldflda (obj, field->type, token, error); + mono_error_assert_ok (error); + } else + dest = (char*)obj + m_field_get_offset (field); mono_copy_value (field->type, dest, value, value && field->type->type == MONO_TYPE_PTR); } @@ -2937,13 +2937,13 @@ mono_field_get_value_internal (MonoObject *obj, MonoClassField *field, void *val g_return_if_fail (!(field->type->attrs & FIELD_ATTRIBUTE_STATIC)); if (G_UNLIKELY (m_field_is_from_update (field))) { - ERROR_DECL (error); - uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); - src = mono_metadata_update_added_field_ldflda (obj, field->type, token, error); - mono_error_assert_ok (error); - } else { - src = (char*)obj + m_field_get_offset (field); - } + ERROR_DECL (error); + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); + src = mono_metadata_update_added_field_ldflda (obj, field->type, token, error); + mono_error_assert_ok (error); + } else { + src = (char*)obj + m_field_get_offset (field); + } mono_copy_value (field->type, value, src, TRUE); } From b05d541627cb79a3d0a0c090206aad751f83672b Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 4 Oct 2022 16:22:41 -0400 Subject: [PATCH 13/55] more whitespace --- src/mono/mono/component/hot_reload.c | 36 ++++++++++++------------ src/mono/mono/component/hot_reload.h | 2 +- src/mono/mono/metadata/class.c | 14 +++++----- src/mono/mono/metadata/icall.c | 42 ++++++++++++++-------------- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index f2cdba561f9a8c..ee28b56704821f 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -187,7 +187,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_num_methods_added, &hot_reload_get_capabilities, &hot_reload_get_method_params, - &hot_reload_added_field_ldflda, + &hot_reload_added_field_ldflda, }; MonoComponentHotReload * @@ -3257,21 +3257,21 @@ hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint3 } g_assert (get_instance_store); - gpointer args[3]; - - args[0] = instance; - args[1] = &field_type; - args[2] = &fielddef_token; - - MonoHotReloadFieldStoreObject *field_store; - field_store = (MonoHotReloadFieldStoreObject*) mono_runtime_invoke_checked (get_instance_store, NULL, args, error); - gpointer result = NULL; - /* If it's a value type, return a ptr to the beginning of the - * boxed data in FieldStore:_loc. If it's a reference type, - * return the address of FieldStore:_loc itself. */ - if (!mono_type_is_reference (field_type)) - result = mono_object_unbox_internal (field_store->_loc); - else - result = (gpointer)&field_store->_loc; - return result; + gpointer args[3]; + + args[0] = instance; + args[1] = &field_type; + args[2] = &fielddef_token; + + MonoHotReloadFieldStoreObject *field_store; + field_store = (MonoHotReloadFieldStoreObject*) mono_runtime_invoke_checked (get_instance_store, NULL, args, error); + gpointer result = NULL; + /* If it's a value type, return a ptr to the beginning of the + * boxed data in FieldStore:_loc. If it's a reference type, + * return the address of FieldStore:_loc itself. */ + if (!mono_type_is_reference (field_type)) + result = mono_object_unbox_internal (field_store->_loc); + else + result = (gpointer)&field_store->_loc; + return result; } diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 046e52a9f0c3d4..abea77e9abaff9 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -49,7 +49,7 @@ typedef struct _MonoComponentHotReload { 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); - gpointer (*added_field_ldflda) (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); + gpointer (*added_field_ldflda) (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 9ac6c35553bbd7..cd4ebb6899cbe5 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -6605,13 +6605,13 @@ static guint32 mono_field_resolve_flags (MonoClassField *field) { if (G_UNLIKELY (m_field_is_from_update (field))) { - /* metadata-update: Just resolve the whole field, for simplicity. */ - ERROR_DECL (error); - mono_field_resolve_type (field, error); - mono_error_assert_ok (error); - g_assert (field->type); - return field->type->attrs; - } + /* metadata-update: Just resolve the whole field, for simplicity. */ + ERROR_DECL (error); + mono_field_resolve_type (field, error); + mono_error_assert_ok (error); + g_assert (field->type); + return field->type->attrs; + } MonoClass *klass = m_field_get_parent (field); MonoImage *image = m_class_get_image (klass); diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index cc60b52fc54432..ca53f2aead759d 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -6301,8 +6301,8 @@ ves_icall_System_TypedReference_InternalMakeTypedReference (MonoTypedRef *res, M (void)mono_handle_class (target); - /* if relative, offset is from the start of target. Otherwise offset is actually an address */ - gboolean relative = TRUE; + /* if relative, offset is from the start of target. Otherwise offset is actually an address */ + gboolean relative = TRUE; intptr_t offset = 0; for (guint i = 0; i < mono_array_handle_length (fields); ++i) { MonoClassField *f; @@ -6311,33 +6311,33 @@ ves_icall_System_TypedReference_InternalMakeTypedReference (MonoTypedRef *res, M g_assert (f); if (i == 0) { - if (G_LIKELY (!m_field_is_from_update (f))) - offset = m_field_get_offset (f); - else { - /* The first field was added by a metadata-update to an exsiting type. - * Since it's store outside the object, offset is an absolute address - */ - relative = FALSE; - ERROR_DECL (error); - uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (f)); - offset = (intptr_t) mono_metadata_update_added_field_ldflda (MONO_HANDLE_RAW (target), f->type, token, error); - mono_error_assert_ok (error); - } + if (G_LIKELY (!m_field_is_from_update (f))) + offset = m_field_get_offset (f); + else { + /* The first field was added by a metadata-update to an exsiting type. + * Since it's store outside the object, offset is an absolute address + */ + relative = FALSE; + ERROR_DECL (error); + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (f)); + offset = (intptr_t) mono_metadata_update_added_field_ldflda (MONO_HANDLE_RAW (target), f->type, token, error); + mono_error_assert_ok (error); + } } else { - /* metadata-update: the first field might be added, the rest are inside structs */ - g_assert (!m_field_is_from_update (f)); + /* metadata-update: the first field might be added, the rest are inside structs */ + g_assert (!m_field_is_from_update (f)); offset += m_field_get_offset (f) - sizeof (MonoObject); - } + } (void)mono_class_from_mono_type_internal (f->type); ftype = f->type; } res->type = ftype; res->klass = mono_class_from_mono_type_internal (ftype); - if (G_LIKELY (relative)) - res->value = (guint8*)MONO_HANDLE_RAW (target) + offset; - else - res->value = (guint8*)offset; + if (G_LIKELY (relative)) + res->value = (guint8*)MONO_HANDLE_RAW (target) + offset; + else + res->value = (guint8*)offset; } void From afdaabf2d001f80ae79c419fbb1d28d5bd0da6a9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 4 Oct 2022 16:23:33 -0400 Subject: [PATCH 14/55] remove TODO --- src/mono/System.Private.CoreLib/src/Mono/HotReload.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs index 51694041c62f93..afdcc34cabbc9c 100644 --- a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs +++ b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs @@ -10,8 +10,6 @@ namespace Mono.HotReload; -// TODO: this is just a sketch, instance field additions aren't supported by Mono yet until https://github.com/dotnet/runtime/issues/63643 is fixed -#if true internal sealed class InstanceFieldTable { // Q: Does CoreCLR EnC allow adding fields to a valuetype? @@ -82,7 +80,6 @@ public FieldStore LookupOrAdd(RuntimeTypeHandle type, uint key) } } -#endif // This is similar to System.Diagnostics.EditAndContinueHelper in CoreCLR, except instead of // having the allocation logic in native (see EditAndContinueModule::ResolveOrAllocateField, From 9582cb38ddcaa74bbd0a88a72da68eedfd517245 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 4 Oct 2022 20:08:23 -0400 Subject: [PATCH 15/55] fix Windows build error --- src/mono/mono/metadata/icall.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index ca53f2aead759d..84147fff3739e2 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -6318,7 +6318,6 @@ ves_icall_System_TypedReference_InternalMakeTypedReference (MonoTypedRef *res, M * Since it's store outside the object, offset is an absolute address */ relative = FALSE; - ERROR_DECL (error); uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (f)); offset = (intptr_t) mono_metadata_update_added_field_ldflda (MONO_HANDLE_RAW (target), f->type, token, error); mono_error_assert_ok (error); From 042b4314a429644345e79c30be93f1619de30824 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 5 Oct 2022 11:33:08 -0400 Subject: [PATCH 16/55] review feedback --- src/mono/mono/mini/interp/interp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index d7f847d5986f3c..3315c6d385d916 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -7505,7 +7505,6 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; MINT_IN_BREAK; } MINT_IN_CASE(MINT_METADATA_UPDATE_LDFLDA) { - gpointer *dest = (gpointer*)(locals + ip [1]); MonoObject *inst = LOCAL_VAR (ip [2], MonoObject*); MonoType *field_type = frame->imethod->data_items [ip [3]]; uint32_t fielddef_token = GPOINTER_TO_UINT32 (frame->imethod->data_items [ip [4]]); @@ -7513,7 +7512,7 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; gpointer field_addr = mono_metadata_update_added_field_ldflda (inst, field_type, fielddef_token, error); /* FIXME: think about pinning the FieldStore and adding a second opcode to * unpin it */ - mono_gc_wbarrier_generic_store_internal (dest, field_addr); + LOCAL_VAR (ip [1], gpointer) = field_addr; mono_interp_error_cleanup (error); ip += 5; MINT_IN_BREAK; From a5a88156d6a22b4d92fea8c172c7985ccb10c635 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 5 Oct 2022 11:33:25 -0400 Subject: [PATCH 17/55] Implement LDFLDA opcode for added fields; add test --- .../AddInstanceField.cs | 5 ++++ .../AddInstanceField_v1.cs | 12 +++++++-- .../tests/ApplyUpdateTest.cs | 2 +- src/mono/mono/mini/interp/transform.c | 27 ++++++++++--------- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs index 9b9d388fd7f270..eacd25e3094bc6 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs @@ -21,5 +21,10 @@ public void TestMethod () { _doubleField = 3.14159; } + public void IncRefDouble (ref double d) + { + d += 1.0; + } + } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs index babb363031649b..aa0ac825b2ab32 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs @@ -8,12 +8,20 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test public class AddInstanceField { public AddInstanceField () { - _doubleField2 = -5.5e12; + _doubleField2 = 5.5; _stringField2 = "New Initial Value"; NewStructField = new NewStruct { - D = -1984.0, + D = -1985.0, O = new int[2] { 15, 17 }, }; + // a little bit ldflda testing + IncRefDouble (ref NewStructField.D); + IncRefDouble (ref _doubleField2); + } + + public void IncRefDouble (ref double d) + { + d += 1.0; } public string GetStringField => _stringField2; diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 9044d6f2e2091b..3fab7e721cadb6 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -333,7 +333,7 @@ public static void TestAddInstanceField() var x2 = new System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField(); Assert.Equal ("New Initial Value", x2.GetStringField); - Assert.Equal (-5.5e12, x2.GetDoubleField); + Assert.Equal (6.5, x2.GetDoubleField); // now check that reflection can get/set the new fields var fi = x2.GetType().GetField("NewStructField"); diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 59748ba140e0e6..9b29bc148935ff 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -1919,11 +1919,6 @@ interp_emit_ldelema (TransformData *td, MonoClass *array_class, MonoClass *check interp_ins_set_dreg (td->last_ins, td->sp [-1].local); } -static GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_instance_field_table); - -static GENERATE_GET_CLASS_WITH_CACHE(hot_reload_instance_field_table, "Mono.HotReload", "InstanceFieldTable"); - - static void interp_emit_metadata_update_ldflda (TransformData *td, MonoClassField *field, MonoError *error) { @@ -6077,9 +6072,17 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, interp_emit_ldsflda (td, field, error); goto_if_nok (error, exit); } else { - td->sp--; /* TODO: metadata-update: implement me. If it's an added field, emit a call to the helper method instead of MINT_LDFLDA_UNSAFE */ - g_assert (!m_field_is_from_update (field)); + if (G_UNLIKELY (m_field_is_from_update (field))) { + /* metadata-update: can't add byref fields */ + g_assert (!m_type_is_byref (ftype)); + MonoClass *field_class = mono_class_from_mono_type_internal (ftype); + interp_emit_metadata_update_ldflda (td, field, error); + goto_if_nok (error, exit); + td->ip += 5; + break; + } + td->sp--; int foffset = m_class_is_valuetype (klass) ? m_field_get_offset (field) - MONO_ABI_SIZEOF (MonoObject) : m_field_get_offset (field); if (td->sp->type == STACK_TYPE_O || td->sp->type == STACK_TYPE_I) { interp_add_ins (td, MINT_LDFLDA); @@ -6151,8 +6154,8 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, interp_ins_set_dreg (td->last_ins, td->sp [-1].local); } else { if (G_UNLIKELY (m_field_is_from_update (field))) { - g_assert (!m_type_is_byref (field->type)); - MonoClass *field_class = mono_class_from_mono_type_internal (field->type); + g_assert (!m_type_is_byref (ftype)); + MonoClass *field_class = mono_class_from_mono_type_internal (ftype); interp_emit_metadata_update_ldflda (td, field, error); goto_if_nok (error, exit); interp_add_ins (td, interp_get_ldind_for_mt (mt)); @@ -6215,9 +6218,9 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, goto_if_nok (error, exit); } else { if (G_UNLIKELY (m_field_is_from_update (field))) { - // FIXME: if field is byref, we probably just want mono_defaults.int_class - g_assert (!m_type_is_byref (field->type)); - MonoClass *field_class = mono_class_from_mono_type_internal (field->type); + // metadata-update: Can't add byref fields + g_assert (!m_type_is_byref (ftype)); + MonoClass *field_class = mono_class_from_mono_type_internal (ftype); MonoType *local_type = m_class_get_byval_arg (field_class); int local = create_interp_local (td, local_type); store_local (td, local); From afe37d6b427d98e954afd4ec3638755be40c8d3e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 5 Oct 2022 14:30:53 -0400 Subject: [PATCH 18/55] fix build --- src/mono/mono/mini/interp/transform.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 9b29bc148935ff..9fb36b7f6197d1 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -6076,7 +6076,6 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, if (G_UNLIKELY (m_field_is_from_update (field))) { /* metadata-update: can't add byref fields */ g_assert (!m_type_is_byref (ftype)); - MonoClass *field_class = mono_class_from_mono_type_internal (ftype); interp_emit_metadata_update_ldflda (td, field, error); goto_if_nok (error, exit); td->ip += 5; From 17a9927001030bad3b7eeeba366859698176f09d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 6 Oct 2022 10:09:11 -0400 Subject: [PATCH 19/55] rm TODO --- src/mono/mono/mini/interp/transform.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 9fb36b7f6197d1..e71d94bb51ba2b 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -6072,7 +6072,6 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, interp_emit_ldsflda (td, field, error); goto_if_nok (error, exit); } else { - /* TODO: metadata-update: implement me. If it's an added field, emit a call to the helper method instead of MINT_LDFLDA_UNSAFE */ if (G_UNLIKELY (m_field_is_from_update (field))) { /* metadata-update: can't add byref fields */ g_assert (!m_type_is_byref (ftype)); From 62ab2e6e99fb19f20fa617de31525330f85d582b Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 6 Oct 2022 10:09:20 -0400 Subject: [PATCH 20/55] Disable test on CoreCLR --- src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 3fab7e721cadb6..4a5d2436649d51 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -308,6 +308,7 @@ public static void TestAddStaticField() }); } + [ActiveIssue("https://github.com/dotnet/runtime/issues/76702", TestRuntimes.CoreCLR)] [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddInstanceField() { From 69de82420fc4238f2d204f51c8b2576d0cb2735d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 6 Oct 2022 16:37:05 -0400 Subject: [PATCH 21/55] WIP start adding a wasm debugger test --- .../DebuggerTestSuite/HotReloadTests.cs | 28 +++++++++++++++ .../ApplyUpdateReferencedAssembly3.csproj | 35 +++++++++++++++++++ .../MethodBody2.cs | 23 ++++++++++++ .../MethodBody2_v1.cs | 25 +++++++++++++ .../MethodBody2_v2.cs | 27 ++++++++++++++ .../deltascript.json | 7 ++++ .../tests/debugger-test/debugger-test.csproj | 1 + 7 files changed, 146 insertions(+) create mode 100644 src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/ApplyUpdateReferencedAssembly3.csproj create mode 100644 src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2.cs create mode 100644 src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v1.cs create mode 100644 src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs create mode 100644 src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/deltascript.json diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs index eec98fcbd1902b..9ec50ee8dd1050 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs @@ -561,5 +561,33 @@ public async Task DebugHotReloadMethod_AddingNewMethodWithoutAnyOtherChange_With CheckLocation("dotnet://ApplyUpdateReferencedAssembly2.dll/MethodBody2.cs", 12, 12, scripts, pause_location["callFrames"]?[0]["location"]); await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", $"dotnet://ApplyUpdateReferencedAssembly2.dll/MethodBody2.cs", 18, 12, "ApplyUpdateReferencedAssembly.AddMethod.StaticMethod2"); } + + [ConditionalFact(nameof(RunningOnChrome))] + public async Task DebugHotReload_NewInstanceFields() + { + string asm_name = "ApplyUpdateReferencedAssembly3"; + string asm_file = Path.Combine (DebuggerTestAppPath, asm_name + ".dll"); + string pdb_file = Path.Combine (DebuggerTestAppPath, asm_name + ".pdb"); + string asm_file_hot_reload = Path.Combine (DebuggerTestAppPath, "..", "wasm", asm_name+ ".dll"); + _testOutput.WriteLine ("aleksey 123"); + var pause_location = await LoadAssemblyAndTestHotReload(asm_file, pdb_file, asm_file_hot_reload, "AddInstanceFields", "StaticMethod1", + expectBpResolvedEvent: false, sourcesToWait: new string [] { "MethodBody2.cs" }); + var frame = pause_location["callFrames"][0]; + var c_props = await GetObjectOnFrame (frame, "c"); + _testOutput.WriteLine ("aleksey 234"); +#if false + await CheckProps (c_props, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c", num_fields: 0); + await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, + locals_fn: async (locals) => { + var c_props1 = await GetObjectOnFrame (locals, "c"); + await CheckProps (c_props1, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c1", num_fields: 1); + }); + await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, + locals_fn: async (locals) => { + var c_props2 = await GetObjectOnFrame (locals, "c"); + await CheckProps (c_props2, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c2", num_fields: 2); + }); +#endif + } } } diff --git a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/ApplyUpdateReferencedAssembly3.csproj b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/ApplyUpdateReferencedAssembly3.csproj new file mode 100644 index 00000000000000..f024e0e58b3f6b --- /dev/null +++ b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/ApplyUpdateReferencedAssembly3.csproj @@ -0,0 +1,35 @@ + + + true + deltascript.json + library + false + true + + false + true + + false + false + false + true + + + true + + + + + + + + + + + diff --git a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2.cs b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2.cs new file mode 100644 index 00000000000000..4c5f28e85e30b2 --- /dev/null +++ b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2.cs @@ -0,0 +1,23 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System; +//keep the same line number for class in the original file and the updates ones +namespace ApplyUpdateReferencedAssembly +{ + public class AddInstanceFields { + public static string StaticMethod1 () { + C c = new(); + Debugger.Break(); + return "OLD STRING"; + } + + public class C { + public C() + { + } + } + + } +} diff --git a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v1.cs b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v1.cs new file mode 100644 index 00000000000000..92a4dde4a1acf0 --- /dev/null +++ b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v1.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System; +//keep the same line number for class in the original file and the updates ones +namespace ApplyUpdateReferencedAssembly +{ + public class AddInstanceFields { + public static string StaticMethod1 () { + C c = new(); + Debugger.Break(); + return "OLD STRING"; + } + + public class C { + public C() + { + Field1 = 123.0; + } + public double Field1; + } + } + +} diff --git a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs new file mode 100644 index 00000000000000..b7a3c6a9e92f23 --- /dev/null +++ b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; +using System; +//keep the same line number for class in the original file and the updates ones +namespace ApplyUpdateReferencedAssembly +{ + public class AddInstanceFields { + public static string StaticMethod1 () { + C c = new(); + Debugger.Break(); + return "OLD STRING"; + } + + public class C { + public C() + { + Field1 = 123.0; + Field2 = "abcd"; + } + public double Field1; + public string Field2; + } + } + +} diff --git a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/deltascript.json b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/deltascript.json new file mode 100644 index 00000000000000..6603dcff23d1f8 --- /dev/null +++ b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/deltascript.json @@ -0,0 +1,7 @@ +{ + "changes": [ + {"document": "MethodBody2.cs", "update": "MethodBody2_v1.cs"}, + {"document": "MethodBody2.cs", "update": "MethodBody2_v2.cs"} + ] +} + diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj b/src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj index a9d8b0e66ecdc0..ea37306876ee73 100644 --- a/src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj +++ b/src/mono/wasm/debugger/tests/debugger-test/debugger-test.csproj @@ -21,6 +21,7 @@ + From 2c7b8e7c8cc372b48e22da729abe683f4298a825 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 18 Oct 2022 11:33:33 -0400 Subject: [PATCH 22/55] WIP XXX - semi-broken debugger test --- .../debugger/DebuggerTestSuite/HotReloadTests.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs index 9ec50ee8dd1050..b1f537a1c1fcfa 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs @@ -573,15 +573,19 @@ public async Task DebugHotReload_NewInstanceFields() var pause_location = await LoadAssemblyAndTestHotReload(asm_file, pdb_file, asm_file_hot_reload, "AddInstanceFields", "StaticMethod1", expectBpResolvedEvent: false, sourcesToWait: new string [] { "MethodBody2.cs" }); var frame = pause_location["callFrames"][0]; - var c_props = await GetObjectOnFrame (frame, "c"); + var locals = await GetProperties(frame["callFrameId"].Value()); + await CheckObject(locals, "c", "ApplyUpdateReferencedAssembly.AddInstanceFields.C"); + //var c_props = await GetObjectOnFrame (frame, "c"); + //_testOutput.WriteLine ("aleksey cprops ---- {0}", c_props.ToString()); + //await CheckProps (c_props, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields.C"), "c", num_fields: 0); _testOutput.WriteLine ("aleksey 234"); -#if false - await CheckProps (c_props, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c", num_fields: 0); await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, locals_fn: async (locals) => { - var c_props1 = await GetObjectOnFrame (locals, "c"); - await CheckProps (c_props1, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c1", num_fields: 1); + //var c_props1 = await GetObjectOnFrame (locals, "c"); + _testOutput.WriteLine ("aleksey cprops1 ---- {0}", locals); + await CheckProps (locals, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c", num_fields: 1); }); +#if false await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, locals_fn: async (locals) => { var c_props2 = await GetObjectOnFrame (locals, "c"); From dadda4e412f531a3fe885b46d260e894214574c0 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 24 Oct 2022 16:06:49 -0400 Subject: [PATCH 23/55] OK - test is failing in the debugger agent (see details) * Assertion at /Users/alklig/work/dotnet-runtime/runtime/src/mono/mono/component/debugger-agent.c:9829, condition `!m_field_is_from_update (f)' not met --- .../wasm/debugger/DebuggerTestSuite/HotReloadTests.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs index b1f537a1c1fcfa..748b3264d5063d 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs @@ -583,7 +583,13 @@ await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", scrip locals_fn: async (locals) => { //var c_props1 = await GetObjectOnFrame (locals, "c"); _testOutput.WriteLine ("aleksey cprops1 ---- {0}", locals); - await CheckProps (locals, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c", num_fields: 1); + //await CheckProps (locals, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c", num_fields: 1); + await CheckObject(locals, "c", "ApplyUpdateReferencedAssembly.AddInstanceFields.C"); + var c = await GetObjectOnLocals(locals, "c"); + // TODO: get the field "Field1" from "c" and check its value + await CheckProps (c, new { + Field1 = TNumber(123), + }, "c", num_fields: 1); }); #if false await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, From b9b0c6ba52394954b06ddf285f150590cbe870c1 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 25 Oct 2022 15:52:41 -0400 Subject: [PATCH 24/55] make the debugger test a bit more interesting --- .../tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs index b7a3c6a9e92f23..379adbe266908e 100644 --- a/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs +++ b/src/mono/wasm/debugger/tests/ApplyUpdateReferencedAssembly3/MethodBody2_v2.cs @@ -9,6 +9,7 @@ namespace ApplyUpdateReferencedAssembly public class AddInstanceFields { public static string StaticMethod1 () { C c = new(); + c.Field2 = "spqr"; Debugger.Break(); return "OLD STRING"; } From 18f440f70f7469c72b2f2188ca01f1a6891ee602 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 25 Oct 2022 15:53:01 -0400 Subject: [PATCH 25/55] implement a debugger test that gets/sets instance fields --- .../DebuggerTestSuite/HotReloadTests.cs | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs index 748b3264d5063d..c1740d6f69e3d9 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs @@ -569,35 +569,39 @@ public async Task DebugHotReload_NewInstanceFields() string asm_file = Path.Combine (DebuggerTestAppPath, asm_name + ".dll"); string pdb_file = Path.Combine (DebuggerTestAppPath, asm_name + ".pdb"); string asm_file_hot_reload = Path.Combine (DebuggerTestAppPath, "..", "wasm", asm_name+ ".dll"); - _testOutput.WriteLine ("aleksey 123"); var pause_location = await LoadAssemblyAndTestHotReload(asm_file, pdb_file, asm_file_hot_reload, "AddInstanceFields", "StaticMethod1", expectBpResolvedEvent: false, sourcesToWait: new string [] { "MethodBody2.cs" }); var frame = pause_location["callFrames"][0]; var locals = await GetProperties(frame["callFrameId"].Value()); await CheckObject(locals, "c", "ApplyUpdateReferencedAssembly.AddInstanceFields.C"); - //var c_props = await GetObjectOnFrame (frame, "c"); - //_testOutput.WriteLine ("aleksey cprops ---- {0}", c_props.ToString()); - //await CheckProps (c_props, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields.C"), "c", num_fields: 0); - _testOutput.WriteLine ("aleksey 234"); await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, locals_fn: async (locals) => { - //var c_props1 = await GetObjectOnFrame (locals, "c"); - _testOutput.WriteLine ("aleksey cprops1 ---- {0}", locals); - //await CheckProps (locals, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c", num_fields: 1); await CheckObject(locals, "c", "ApplyUpdateReferencedAssembly.AddInstanceFields.C"); +#if true var c = await GetObjectOnLocals(locals, "c"); - // TODO: get the field "Field1" from "c" and check its value await CheckProps (c, new { Field1 = TNumber(123), }, "c", num_fields: 1); + var cObj = GetAndAssertObjectWithName (locals, "c"); + await SetValueOnObject (cObj, "Field1", "456.5"); + + c = await GetObjectOnLocals(locals, "c"); + await CheckProps (c, new { + Field1 = TNumber("456.5", isDecimal: true), + }, "c", num_fields: 1); +#endif }); -#if false await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, locals_fn: async (locals) => { - var c_props2 = await GetObjectOnFrame (locals, "c"); - await CheckProps (c_props2, TObject("ApplyUpdateReferencedAssembly.AddInstanceFields+C"), "c2", num_fields: 2); + _testOutput.WriteLine ("aleksey cprops1 ---- {0}", locals); + await CheckObject(locals, "c", "ApplyUpdateReferencedAssembly.AddInstanceFields.C"); + var c = await GetObjectOnLocals(locals, "c"); + _testOutput.WriteLine ("aleksey cprops1 ---- {0}", c); + await CheckProps (c, new { + Field1 = TNumber(123), + Field2 = TString("spqr"), + }, "c", num_fields: 2); }); -#endif } } } From ef7805d1c6ee78840769017e4277e549c5de101d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 25 Oct 2022 15:53:19 -0400 Subject: [PATCH 26/55] Clear the debugger type cache on EnC update we only need to clear the type on which fields or methods were added. but right now we just clear the whole cache --- .../wasm/debugger/BrowserDebugProxy/DebugStore.cs | 12 +++++++----- .../wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 2 +- .../wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 4 ++++ 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs index 32727996e98400..e07fdd718ae9bf 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs @@ -904,14 +904,14 @@ public void SetDebugId(int id) debugId = id; } - public bool EnC(byte[] meta, byte[] pdb) + public bool EnC(MonoSDBHelper sdbAgent, byte[] meta, byte[] pdb) { var asmStream = new MemoryStream(meta); MetadataReader asmMetadataReader = MetadataReaderProvider.FromMetadataStream(asmStream).GetMetadataReader(); var pdbStream = new MemoryStream(pdb); MetadataReader pdbMetadataReader = MetadataReaderProvider.FromPortablePdbStream(pdbStream).GetMetadataReader(); enCMetadataReader.Add(new (asmMetadataReader, pdbMetadataReader)); - PopulateEnC(asmMetadataReader, pdbMetadataReader); + PopulateEnC(sdbAgent, asmMetadataReader, pdbMetadataReader); return true; } private static int GetTypeDefIdx(MetadataReader asmMetadataReaderParm, int number) @@ -958,10 +958,12 @@ public string EnCGetString(StringHandle strHandle) return asmMetadataReaderLocal.GetString(MetadataTokens.StringHandle(strIdx)); } - private void PopulateEnC(MetadataReader asmMetadataReaderParm, MetadataReader pdbMetadataReaderParm) + private void PopulateEnC(MonoSDBHelper sdbAgent, MetadataReader asmMetadataReaderParm, MetadataReader pdbMetadataReaderParm) { TypeInfo typeInfo = null; int methodIdxAsm = 1; + sdbAgent.ResetTypes(); // FIXME: only remove the cache for the affected type if fields or methods are added + foreach (var entry in asmMetadataReaderParm.GetEditAndContinueLogEntries()) { if (entry.Operation == EditAndContinueOperation.AddMethod || @@ -1351,9 +1353,9 @@ private sealed class DebugItem public string Url { get; set; } public Task Data { get; set; } } - public static IEnumerable EnC(AssemblyInfo asm, byte[] meta_data, byte[] pdb_data) + public static IEnumerable EnC(MonoSDBHelper sdbAgent, AssemblyInfo asm, byte[] meta_data, byte[] pdb_data) { - asm.EnC(meta_data, pdb_data); + asm.EnC(sdbAgent, meta_data, pdb_data); return GetEnCMethods(asm); } diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 5b6bf24290f731..45c772a3428129 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -844,7 +844,7 @@ private async Task ProcessEnC(SessionId sessionId, ExecutionContext contex var assemblyName = await context.SdbAgent.GetAssemblyNameFromModule(moduleId, token); DebugStore store = await LoadStore(sessionId, true, token); AssemblyInfo asm = store.GetAssemblyByName(assemblyName); - var methods = DebugStore.EnC(asm, meta_buf, pdb_buf); + var methods = DebugStore.EnC(context.SdbAgent, asm, meta_buf, pdb_buf); foreach (var method in methods) { await ResetBreakpoint(sessionId, store, method, token); diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index de9585dd1691d0..f5d9dc06dbcc0b 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -833,6 +833,10 @@ public void ResetStore(DebugStore store) ClearCache(); } + public void ResetTypes() { + this.types = new (); + } + public async Task GetAssemblyInfo(int assemblyId, CancellationToken token) { if (assemblies.TryGetValue(assemblyId, out AssemblyInfo asm)) From 79f5abc9d490a9454aa8ca11023b28a498d73a7f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 25 Oct 2022 15:54:03 -0400 Subject: [PATCH 27/55] [sdb] implement get/set field for added instance fields --- src/mono/mono/component/debugger-agent.c | 30 ++++++++++++++++++------ src/mono/mono/metadata/metadata-update.h | 4 ++-- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 0d155aed44e2c7..56514b2357823d 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -9825,8 +9825,6 @@ object_commands (int command, guint8 *p, guint8 *end, Buffer *buf) if (!found) goto invalid_fieldid; get_field_value: - /* TODO: metadata-update: implement support for added fields */ - g_assert (!m_field_is_from_update (f)); if (f->type->attrs & FIELD_ATTRIBUTE_STATIC) { guint8 *val; MonoVTable *vtable; @@ -9849,7 +9847,17 @@ object_commands (int command, guint8 *p, guint8 *end, Buffer *buf) buffer_add_value (buf, f->type, val, obj->vtable->domain); g_free (val); } else { - void *field_value = (guint8*)obj + m_field_get_offset (f); + void *field_value = NULL; + if (G_UNLIKELY (m_field_is_from_update (f))) { + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (f)); + field_value = mono_metadata_update_added_field_ldflda (obj, f->type, token, error); + if (!is_ok (error)) { + mono_error_cleanup (error); + goto invalid_object; + } + } else { + field_value = (guint8*)obj + m_field_get_offset (f); + } buffer_add_value (buf, f->type, field_value, obj->vtable->domain); } @@ -9874,9 +9882,6 @@ object_commands (int command, guint8 *p, guint8 *end, Buffer *buf) if (!found) goto invalid_fieldid; - /* TODO: metadata-update: implement support for added fields. */ - g_assert (!m_field_is_from_update (f)); - if (f->type->attrs & FIELD_ATTRIBUTE_STATIC) { guint8 *val; MonoVTable *vtable; @@ -9900,7 +9905,18 @@ object_commands (int command, guint8 *p, guint8 *end, Buffer *buf) mono_field_static_set_value_internal (vtable, f, val); g_free (val); } else { - err = decode_value (f->type, obj->vtable->domain, (guint8*)obj + m_field_get_offset (f), p, &p, end, TRUE); + void *dest = NULL; + if (G_UNLIKELY (m_field_is_from_update (f))) { + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (f)); + dest = mono_metadata_update_added_field_ldflda (obj, f->type, token, error); + if (!is_ok (error)) { + mono_error_cleanup (error); + goto invalid_fieldid; + } + } else { + dest = (guint8*)obj + m_field_get_offset (f); + } + err = decode_value (f->type, obj->vtable->domain, dest, p, &p, end, TRUE); if (err != ERR_NONE) goto exit; } diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index ab6dd60ac39c33..1ac696ab84571f 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -64,7 +64,7 @@ mono_metadata_update_metadata_linear_search (MonoImage *base_image, MonoTableInf MonoMethod* mono_metadata_update_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); -uint32_t +MONO_COMPONENT_API uint32_t mono_metadata_update_get_field_idx (MonoClassField *field); MonoClassField * @@ -97,7 +97,7 @@ 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); -gpointer +MONO_COMPONENT_API gpointer mono_metadata_update_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); #endif /*__MONO_METADATA_UPDATE_H__*/ From 13a4b25f72aaab7257616745d86f7be41abcffe5 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 26 Oct 2022 15:20:27 -0400 Subject: [PATCH 28/55] [handles] remove the field getter/setter macros They had no callers, and now we don't do things this way - we assume the enclosing object had been pinned by a stack reference. --- src/mono/mono/metadata/handle.h | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/src/mono/mono/metadata/handle.h b/src/mono/mono/metadata/handle.h index e18ce15c210d36..2d9b709be1dba6 100644 --- a/src/mono/mono/metadata/handle.h +++ b/src/mono/mono/metadata/handle.h @@ -487,28 +487,6 @@ This is why we evaluate index and value before any call to MONO_HANDLE_RAW or ot #define mono_handle_domain(handle) MONO_HANDLE_DOMAIN ((handle)) -/* Given an object and a MonoClassField, return the value (must be non-object) - * of the field. It's the caller's responsibility to check that the object is - * of the correct class. */ -#define MONO_HANDLE_GET_FIELD_VAL(HANDLE,TYPE,FIELD) (*(TYPE *)(mono_handle_unsafe_field_addr (MONO_HANDLE_CAST (MonoObject, (HANDLE)), (FIELD)))) -#define MONO_HANDLE_GET_FIELD_BOOL(handle, type, field) (MONO_BOOL (MONO_HANDLE_GET_FIELD_VAL ((handle), type, (field)))) - -#define MONO_HANDLE_NEW_GET_FIELD(HANDLE,TYPE,FIELD) MONO_HANDLE_NEW (TYPE, MONO_HANDLE_SUPPRESS (*(TYPE**)(mono_handle_unsafe_field_addr (MONO_HANDLE_CAST (MonoObject, MONO_HANDLE_UNSUPPRESS (HANDLE)), (FIELD))))) - -#define MONO_HANDLE_SET_FIELD_VAL(HANDLE,TYPE,FIELD,VAL) do { \ - MonoObjectHandle __obj = (HANDLE); \ - MonoClassField *__field = (FIELD); \ - TYPE __value = (VAL); \ - *(TYPE*)(mono_handle_unsafe_field_addr (__obj, __field)) = __value; \ - } while (0) - -#define MONO_HANDLE_SET_FIELD_REF(HANDLE,FIELD,VALH) do { \ - MonoObjectHandle __obj = MONO_HANDLE_CAST (MonoObject, (HANDLE)); \ - MonoClassField *__field = (FIELD); \ - MonoObjectHandle __value = MONO_HANDLE_CAST (MonoObject, (VALH)); \ - MONO_HANDLE_SUPPRESS (mono_gc_wbarrier_generic_store_internal (mono_handle_unsafe_field_addr (__obj, __field), MONO_HANDLE_RAW (__value))); \ - } while (0) - #define MONO_HANDLE_GET_CLASS(handle) (MONO_HANDLE_GETVAL (MONO_HANDLE_CAST (MonoObject, (handle)), vtable)->klass) /* Baked typed handles we all want */ @@ -603,15 +581,6 @@ mono_handle_assign_raw (MonoObjectHandleOut dest, void *src) return dest; } -/* It is unsafe to call this function directly - it does not pin the handle! Use MONO_HANDLE_GET_FIELD_VAL(). */ -static inline gpointer -mono_handle_unsafe_field_addr (MonoObjectHandle h, MonoClassField *field) -{ - /* TODO: metadata-update: fix all callers */ - g_assert (!m_field_is_from_update (field)); - return MONO_HANDLE_SUPPRESS (((gchar *)MONO_HANDLE_RAW (h)) + field->offset); -} - /* Matches ObjectHandleOnStack in managed code */ typedef MonoObject **MonoObjectHandleOnStack; From 88a466aebec0f4441df91006285e7bf59b78e3f2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 26 Oct 2022 15:22:29 -0400 Subject: [PATCH 29/55] [metadata-update] comments on asserts --- src/mono/mono/metadata/icall.c | 2 +- src/mono/mono/metadata/object.c | 1 + src/mono/mono/mini/aot-compiler.c | 1 + src/mono/mono/mini/mini-amd64.c | 1 + 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 84147fff3739e2..6d6bb962b9280a 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -2027,7 +2027,7 @@ ves_icall_RuntimeFieldInfo_GetFieldOffset (MonoReflectionFieldHandle field, Mono MonoClassField *class_field = MONO_HANDLE_GETVAL (field, field); mono_class_setup_fields (m_field_get_parent (class_field)); - /* TODO: metadata-update: figure out what CoreCLR does in this situation. */ + /* metadata-update: mono only calls this for ExplicitLayout types */ g_assert (!m_field_is_from_update (class_field)); return m_field_get_offset (class_field) - MONO_ABI_SIZEOF (MonoObject); diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index 16f83a4bcbea9b..c5b28dd564132e 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -7823,6 +7823,7 @@ mono_class_value_size (MonoClass *klass, guint32 *align) gpointer mono_vtype_get_field_addr (gpointer vtype, MonoClassField *field) { + /* metadata-update: no added fields in valuetypes */ g_assert (!m_field_is_from_update (field)); return ((char*)vtype) + m_field_get_offset (field) - MONO_ABI_SIZEOF (MonoObject); } diff --git a/src/mono/mono/mini/aot-compiler.c b/src/mono/mono/mini/aot-compiler.c index f7fdc9e9771d75..75249bea3ad1c6 100644 --- a/src/mono/mono/mini/aot-compiler.c +++ b/src/mono/mono/mini/aot-compiler.c @@ -3011,6 +3011,7 @@ mono_get_field_token (MonoClassField *field) MonoClass *klass = m_field_get_parent (field); int i; + /* metadata-update: there are no updates during AOT */ g_assert (!m_field_is_from_update (field)); int fcount = mono_class_get_field_count (klass); MonoClassField *klass_fields = m_class_get_fields (klass); diff --git a/src/mono/mono/mini/mini-amd64.c b/src/mono/mono/mini/mini-amd64.c index bfede8ad36c7f9..a92ce7cf1a1e43 100644 --- a/src/mono/mono/mini/mini-amd64.c +++ b/src/mono/mono/mini/mini-amd64.c @@ -391,6 +391,7 @@ collect_field_info_nested (MonoClass *klass, GArray *fields_array, int offset, g while ((field = mono_class_get_fields_internal (klass, &iter))) { if (field->type->attrs & FIELD_ATTRIBUTE_STATIC) continue; + /* metadata-update: we're in the JIT here, right? updates aren't supported */ g_assert (!m_field_is_from_update (field)); if (MONO_TYPE_ISSTRUCT (field->type)) { collect_field_info_nested (mono_class_from_mono_type_internal (field->type), fields_array, m_field_get_offset (field) - MONO_ABI_SIZEOF (MonoObject), pinvoke, unicode); From 4d7ee0b968019d3febfc7f593c0437e2d509ee57 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 26 Oct 2022 15:23:38 -0400 Subject: [PATCH 30/55] implement mono_field_get_addr added field support --- src/mono/mono/metadata/object.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index c5b28dd564132e..dd322c5f7516a4 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -2869,9 +2869,15 @@ mono_field_get_addr (MonoObject *obj, MonoVTable *vt, MonoClassField *field) if (field->type->attrs & FIELD_ATTRIBUTE_STATIC) return mono_static_field_get_addr (vt, field); else { - /* TODO: metadata-update: implement support for added fields */ - g_assert (!m_field_is_from_update (field)); - return (guint8*)obj + m_field_get_offset (field); + if (G_LIKELY (!m_field_is_from_update (field))) + return (guint8*)obj + m_field_get_offset (field); + else { + ERROR_DECL (error); + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (field)); + gpointer addr = mono_metadata_update_added_field_ldflda (obj, field->type, token, error); + mono_error_assert_ok (error); + return addr; + } } } From 67e02a012f2059c23fae0f226ed8070a2bc57c3e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 26 Oct 2022 15:24:01 -0400 Subject: [PATCH 31/55] add TODO for initializers for added literals --- src/mono/mono/metadata/class.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index cd4ebb6899cbe5..8f908d64d10053 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2539,6 +2539,7 @@ mono_class_get_field_token (MonoClassField *field) static int mono_field_get_index (MonoClassField *field) { + /* metadata-update: the callers of this method need changes to support updates */ g_assert (!m_field_is_from_update (field)); int index = GPTRDIFF_TO_INT (field - m_class_get_fields (m_field_get_parent (field))); g_assert (index >= 0 && GINT_TO_UINT32(index) < mono_class_get_field_count (m_field_get_parent (field))); @@ -2569,6 +2570,9 @@ mono_class_get_field_default_value (MonoClassField *field, MonoTypeEnum *def_typ mono_class_set_field_def_values (klass, def_values); } + /* TODO: metadata-update - added literal fields */ + g_assert (!m_field_is_from_update (field)); + field_index = mono_field_get_index (field); if (!def_values [field_index].data) { @@ -5547,7 +5551,7 @@ mono_field_get_rva (MonoClassField *field, int swizzle) g_assert (field->type->attrs & FIELD_ATTRIBUTE_HAS_FIELD_RVA); - /* TODO: metadata-update: make this work. */ + /* metadata-update: TODO support added static fields with initializers. */ g_assert (!m_field_is_from_update (field)); def_values = mono_class_get_field_def_values_with_swizzle (klass, swizzle); From 8d1a90e0db8b5f3aa661176a7a89b443ad38dd4e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 10 Nov 2022 15:11:42 -0500 Subject: [PATCH 32/55] remove last use of MONO_HANDLE_SET_FIELD_REF --- src/mono/mono/metadata/icall.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 6d6bb962b9280a..6119005ae4fd65 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -2168,9 +2168,19 @@ ves_icall_RuntimeFieldInfo_SetValueInternal (MonoReflectionFieldHandle field, Mo mono_field_static_set_value_internal (vtable, cf, v); } else { - if (isref) - MONO_HANDLE_SET_FIELD_REF (obj, cf, value); - else + if (isref) { + MonoObject *obj_ptr = MONO_HANDLE_RAW (obj); + MonoObject *value_ptr = MONO_HANDLE_RAW (value); + gpointer *dest; + if (G_LIKELY (!m_field_is_from_update (cf))) { + dest = (gpointer*)(((char *)obj_ptr) + m_field_get_offset (cf)); + } else { + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_update_get_field_idx (cf)); + dest = mono_metadata_update_added_field_ldflda (obj_ptr, cf->type, token, error); + mono_error_assert_ok (error); + } + mono_gc_wbarrier_generic_store_internal (dest, value_ptr); + } else mono_field_set_value_internal (MONO_HANDLE_RAW (obj), cf, v); /* FIXME: make mono_field_set_value take a handle for obj */ } leave: From 2a0107162946306d42ac37056e29a0005e74c5a1 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Nov 2022 11:40:44 -0500 Subject: [PATCH 33/55] WeakAttribute is not picked up from added fields --- src/mono/mono/metadata/custom-attrs.c | 2 ++ src/mono/mono/metadata/metadata.c | 12 ++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 2bce3b3e79781c..f97ea23a2f4a26 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -2890,6 +2890,8 @@ init_weak_fields_inner (MonoImage *image, GHashTable *indexes) } } else { /* FIXME: metadata-update */ + if (G_UNLIKELY (image->has_updates)) + g_warning ("[WeakAttribute] ignored on fields added by hot reload in '%s'", image->name); /* Memberref pointing to a typeref */ tdef = &image->tables [MONO_TABLE_MEMBERREF]; diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 0c63c068929173..2d8cbf60c3f042 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6942,18 +6942,18 @@ mono_metadata_get_marshal_info (MonoImage *meta, guint32 idx, gboolean is_field) locator_t loc; MonoTableInfo *tdef = &meta->tables [MONO_TABLE_FIELDMARSHAL]; - if (!tdef->base) - return NULL; - loc.t = tdef; loc.col_idx = MONO_FIELD_MARSHAL_PARENT; loc.idx = ((idx + 1) << MONO_HAS_FIELD_MARSHAL_BITS) | (is_field? MONO_HAS_FIELD_MARSHAL_FIELDSREF: MONO_HAS_FIELD_MARSHAL_PARAMDEF); - /* FIXME: metadata-update */ /* FIXME: Index translation */ - if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) - return NULL; + gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator); + + if (G_UNLIKELY (meta->has_updates)) { + if (!found && !mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) + return NULL; + } return mono_metadata_blob_heap (meta, mono_metadata_decode_row_col (tdef, loc.result, MONO_FIELD_MARSHAL_NATIVE_TYPE)); } From 8fabc0d81912ffc21e0262659a6f78e18eeefb5f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Nov 2022 15:12:39 -0500 Subject: [PATCH 34/55] Add a test for an array with RVA --- .../AddInstanceField.cs | 5 +++++ .../AddInstanceField_v1.cs | 6 ++++++ .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 5 +++++ 3 files changed, 16 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs index eacd25e3094bc6..f5502cf4ba1bb5 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs @@ -16,11 +16,16 @@ public AddInstanceField () { private string _stringField; private double _doubleField; + private int[] _intArrayFieldWithInit = new[] { 2, 4, 6, 8, 10, 12 }; + public void TestMethod () { _stringField = "abcd"; _doubleField = 3.14159; } + public int GetIntArrayLength() => _intArrayFieldWithInit?.Length ?? -1; + public int GetIntArrayElt(int i) => _intArrayFieldWithInit[i]; + public void IncRefDouble (ref double d) { d += 1.0; diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs index aa0ac825b2ab32..e3bb33a7f74520 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs @@ -32,6 +32,9 @@ public void IncRefDouble (ref double d) private double _doubleField; private double _doubleField2; + private int[] _intArrayFieldWithInit = new[] { 2, 4, 6, 8, 10, 12 }; + private int[] _intArrayFieldWithInit2 = new[] { 1, 3, 5, 7, 9, 11 }; + public void TestMethod () { _stringField = "spqr"; _stringField2 = "4567"; @@ -39,6 +42,9 @@ public void TestMethod () { _doubleField2 = 0.707106; } + public int GetIntArrayLength() => _intArrayFieldWithInit2?.Length ?? -1; + public int GetIntArrayElt(int i) => _intArrayFieldWithInit2[i]; + public struct NewStruct { public double D; diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 4a5d2436649d51..2fa7f6f1871b36 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -331,10 +331,15 @@ public static void TestAddInstanceField() Assert.Equal ("4567", x1.GetStringField); Assert.Equal (0.707106, x1.GetDoubleField); + Assert.Equal (-1, x1.GetIntArrayLength ()); // new field on existing object is initially null + var x2 = new System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField(); Assert.Equal ("New Initial Value", x2.GetStringField); Assert.Equal (6.5, x2.GetDoubleField); + + Assert.Equal (6, x2.GetIntArrayLength()); + Assert.Equal (7, x2.GetIntArrayElt (3)); // now check that reflection can get/set the new fields var fi = x2.GetType().GetField("NewStructField"); From ca9c3c93267125ba0d58cadbfe130a416b29b3e2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Nov 2022 15:14:30 -0500 Subject: [PATCH 35/55] Impl mono_metadata_field_info_full --- src/mono/mono/metadata/class-init.c | 1 + src/mono/mono/metadata/metadata.c | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 32576a730d6ce6..2a917dae03f4a5 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -2273,6 +2273,7 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_ int idx = first_field_idx + i; guint32 offset; mono_metadata_field_info (klass->image, idx, &offset, NULL, NULL); + /* metadata-update: updates to explicit layout classes are not allowed. */ field_offsets [i] = offset + MONO_ABI_SIZEOF (MonoObject); } ftype = mono_type_get_underlying_type (field->type); diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 2d8cbf60c3f042..0dc16d92bfa165 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6192,7 +6192,7 @@ mono_metadata_field_info_full (MonoImage *meta, guint32 index, guint32 *offset, loc.col_idx = MONO_FIELD_LAYOUT_FIELD; loc.t = tdef; - /* FIXME: metadata-update */ + /* metadata-update: explicit layout not supported, just return -1 */ if (tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) { *offset = mono_metadata_decode_row_col (tdef, loc.result, MONO_FIELD_LAYOUT_OFFSET); @@ -6206,7 +6206,14 @@ mono_metadata_field_info_full (MonoImage *meta, guint32 index, guint32 *offset, loc.col_idx = MONO_FIELD_RVA_FIELD; loc.t = tdef; - if (tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) { + gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator); + + if (G_UNLIKELY (meta->has_updates)) { + if (!found) + found = (mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator) != NULL); + } + + if (found) { /* * LAMESPEC: There is no signature, no nothing, just the raw data. */ From 49a907e9597b09c2c3a67f84f66aaa81dbc2eeff Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Nov 2022 15:21:14 -0500 Subject: [PATCH 36/55] mono_class_get_field_token --- src/mono/mono/metadata/class.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 8f908d64d10053..7c27687b0927b4 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2510,6 +2510,13 @@ mono_class_get_field_token (MonoClassField *field) mono_class_setup_fields (klass); + if (G_UNLIKELY (m_class_get_image (klass)->has_updates)) { + if (G_UNLIKELY (m_field_is_from_update (field))) { + uint32_t idx = mono_metadata_update_get_field_idx (field); + return mono_metadata_make_token (MONO_TABLE_FIELD, idx); + } + } + while (klass) { MonoClassField *klass_fields = m_class_get_fields (klass); if (!klass_fields) @@ -2525,10 +2532,6 @@ mono_class_get_field_token (MonoClassField *field) return mono_metadata_make_token (MONO_TABLE_FIELD, idx); } } - if (G_UNLIKELY (m_class_get_image (klass)->has_updates)) { - /* TODO: metadata-update: check if the field was added. */ - g_assert_not_reached (); - } klass = m_class_get_parent (klass); } From 54f4f2a2a8efe2a63c71e6915d6faf30b6f037ff Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Nov 2022 15:30:11 -0500 Subject: [PATCH 37/55] mono_field_get_rva --- src/mono/mono/metadata/class.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 7c27687b0927b4..11e88dc2eb225c 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -5554,7 +5554,7 @@ mono_field_get_rva (MonoClassField *field, int swizzle) g_assert (field->type->attrs & FIELD_ATTRIBUTE_HAS_FIELD_RVA); - /* metadata-update: TODO support added static fields with initializers. */ + /* metadata-update: added static fields with initializers don't seem to get here */ g_assert (!m_field_is_from_update (field)); def_values = mono_class_get_field_def_values_with_swizzle (klass, swizzle); From 78eeee21beec520049c20ed31b89e32b46b5bd84 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Nov 2022 15:43:17 -0500 Subject: [PATCH 38/55] Add SetValueDirect/GetValueDirect test (failing) --- src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 2fa7f6f1871b36..a16504c1986036 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -364,6 +364,11 @@ public static void TestAddInstanceField() fi.SetValue(x2, 65535.01); Assert.Equal(65535.01, x2.GetDoubleField); + tr = __makeref(x2); + fi.SetValueDirect (tr, 32768.2); + Assert.Equal (32768.2, x2.GetDoubleField); + Assert.Equal ((object)32768.2, fi.GetValueDirect (tr)); + }); } From afe88ecfc8733dd9c94ceb42040486d087076283 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Nov 2022 15:55:30 -0500 Subject: [PATCH 39/55] Implement GetValueDirect - hot reload test passes now --- src/mono/mono/metadata/icall.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index 6119005ae4fd65..b4d39021e5b131 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -2211,15 +2211,18 @@ ves_icall_System_RuntimeFieldHandle_GetValueDirect (MonoReflectionFieldHandle fi MonoClassField *field = MONO_HANDLE_GETVAL (field_h, field); MonoClass *klass = mono_class_from_mono_type_internal (field->type); - /* TODO: metadata-update: get the values of added fields */ - g_assert (!m_field_is_from_update (field)); if (!MONO_TYPE_ISSTRUCT (m_class_get_byval_arg (m_field_get_parent (field)))) { - mono_error_set_not_implemented (error, ""); - return MONO_HANDLE_NEW (MonoObject, NULL); + MonoObjectHandle objHandle = typed_reference_to_object (obj, error); + return_val_if_nok (error, MONO_HANDLE_NEW (MonoObject, NULL)); + return ves_icall_RuntimeFieldInfo_GetValueInternal (field_h, objHandle, error); } else if (MONO_TYPE_IS_REFERENCE (field->type)) { + /* metadata-update: can't add fields to structs */ + g_assert (!m_field_is_from_update (field)); return MONO_HANDLE_NEW (MonoObject, *(MonoObject**)((guint8*)obj->value + m_field_get_offset (field) - sizeof (MonoObject))); } else { + /* metadata-update can't add fields to structs */ + g_assert (!m_field_is_from_update (field)); return mono_value_box_handle (klass, (guint8*)obj->value + m_field_get_offset (field) - sizeof (MonoObject), error); } } @@ -2233,16 +2236,17 @@ ves_icall_System_RuntimeFieldHandle_SetValueDirect (MonoReflectionFieldHandle fi mono_class_setup_fields (m_field_get_parent (f)); - /* TODO: metadata-update: set the values of added fields */ - g_assert (!m_field_is_from_update (f)); - if (!MONO_TYPE_ISSTRUCT (m_class_get_byval_arg (m_field_get_parent (f)))) { MonoObjectHandle objHandle = typed_reference_to_object (obj, error); return_if_nok (error); ves_icall_RuntimeFieldInfo_SetValueInternal (field_h, objHandle, value_h, error); } else if (MONO_TYPE_IS_REFERENCE (f->type)) { + /* metadata-update: can't add fields to structs */ + g_assert (!m_field_is_from_update (f)); mono_copy_value (f->type, (guint8*)obj->value + m_field_get_offset (f) - sizeof (MonoObject), MONO_HANDLE_RAW (value_h), FALSE); } else { + /* metadata-update: can't add fields to structs */ + g_assert (!m_field_is_from_update (f)); MonoGCHandle gchandle = NULL; g_assert (MONO_HANDLE_RAW (value_h)); mono_copy_value (f->type, (guint8*)obj->value + m_field_get_offset (f) - sizeof (MonoObject), mono_object_handle_pin_unbox (value_h, &gchandle), FALSE); From 6e9811d59cb33201eeea43acdd9858bedb87bbda Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 14 Nov 2022 17:07:42 -0500 Subject: [PATCH 40/55] fix init --- src/mono/mono/metadata/metadata.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 0dc16d92bfa165..9fc6e541fc9cfc 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6180,7 +6180,7 @@ mono_metadata_field_info_full (MonoImage *meta, guint32 index, guint32 *offset, MonoMarshalSpec **marshal_spec, gboolean alloc_from_image) { MonoTableInfo *tdef; - locator_t loc; + locator_t loc = {0,}; loc.idx = index + 1; if (meta->uncompressed_metadata) @@ -6946,7 +6946,7 @@ mono_type_to_unmanaged (MonoType *type, MonoMarshalSpec *mspec, gboolean as_fiel const char* mono_metadata_get_marshal_info (MonoImage *meta, guint32 idx, gboolean is_field) { - locator_t loc; + locator_t loc = {0,}; MonoTableInfo *tdef = &meta->tables [MONO_TABLE_FIELDMARSHAL]; loc.t = tdef; From d90d17c256e8f84501882fd8bada196270eb3856 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Nov 2022 16:22:03 -0500 Subject: [PATCH 41/55] Added test for auto property; crashes in ves_icall_RuntimePropertyInfo_get_property_info --- .../AddInstanceField.cs | 1 + .../AddInstanceField_v1.cs | 9 +++++++++ .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 12 ++++++++++++ 3 files changed, 22 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs index f5502cf4ba1bb5..ab10f7e49e68a6 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs @@ -31,5 +31,6 @@ public void IncRefDouble (ref double d) d += 1.0; } + public string GetStringProp => string.Empty; } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs index e3bb33a7f74520..c07c96c8faff5b 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs @@ -17,6 +17,8 @@ public AddInstanceField () { // a little bit ldflda testing IncRefDouble (ref NewStructField.D); IncRefDouble (ref _doubleField2); + + AddedStringAutoProp = "abcd"; } public void IncRefDouble (ref double d) @@ -40,6 +42,7 @@ public void TestMethod () { _stringField2 = "4567"; _doubleField = 2.71828; _doubleField2 = 0.707106; + AddedStringAutoProp = AddedStringAutoProp + "Test"; } public int GetIntArrayLength() => _intArrayFieldWithInit2?.Length ?? -1; @@ -52,5 +55,11 @@ public struct NewStruct } public NewStruct NewStructField; + + public string GetStringProp => AddedStringAutoProp; + + public string AddedStringAutoProp { get; set; } + + } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index a16504c1986036..cd6342ee7e115a 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -369,6 +369,18 @@ public static void TestAddInstanceField() Assert.Equal (32768.2, x2.GetDoubleField); Assert.Equal ((object)32768.2, fi.GetValueDirect (tr)); + Assert.Equal("abcd", x2.GetStringProp); + + var propInfo = x2.GetType().GetProperty("AddedStringAutoProp", BindingFlags.Public | BindingFlags.Instance); + + Assert.NotNull(propInfo); + Assert.Equal("abcd", propInfo.GetMethod.Invoke (x2, new object[] {})); + + x2.TestMethod(); + + Assert.Equal("abcdTest", x2.GetStringProp); + + }); } From 517978e97876f4769957e8974d884e51e20eaf3f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 15 Nov 2022 18:46:00 -0500 Subject: [PATCH 42/55] fix rebase whitespace --- src/mono/mono/mini/interp/interp.c | 1 + src/mono/mono/mini/interp/mintops.def | 2 -- src/mono/mono/mini/interp/transform.c | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 3315c6d385d916..2bb33dab673a81 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -7623,6 +7623,7 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; MINT_IN_BREAK; } #endif + #if !USE_COMPUTED_GOTO default: interp_error_xsx ("Unimplemented opcode: %04x %s at 0x%x\n", *ip, mono_interp_opname (*ip), GPTRDIFF_TO_INT (ip - frame->imethod->code)); diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index a19a6e1aec506a..b2a40b413b9f25 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -806,5 +806,3 @@ OPDEF(MINT_METADATA_UPDATE_LDFLDA, "metadata_update.ldflda", 5, 1, 1, MintOpTwoS OPDEF(MINT_TIER_PREPARE_JITERPRETER, "tier_prepare_jiterpreter", 3, 0, 0, MintOpInt) OPDEF(MINT_TIER_NOP_JITERPRETER, "tier_nop_jiterpreter", 3, 0, 0, MintOpInt) OPDEF(MINT_TIER_ENTER_JITERPRETER, "tier_enter_jiterpreter", 3, 0, 0, MintOpInt) - - diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index e71d94bb51ba2b..d7d71ac3b6ea27 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -6229,7 +6229,6 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, td->ip += 5; break; } - int opcode = MINT_STFLD_I1 + mt - MINT_TYPE_I1; #ifdef NO_UNALIGNED_ACCESS if ((mt == MINT_TYPE_I8 || mt == MINT_TYPE_R8) && field->offset % SIZEOF_VOID_P != 0) From 619d956cbd01e26a19c59f32b384e03bb08cac91 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 16 Nov 2022 16:51:49 -0500 Subject: [PATCH 43/55] WIP checkpoint add_props Note: we're not doing anything with the new PropertyMap row Note2: we get MethodSemantics rows for properties that got updated, and we're currently ignoring them. Need to check that this is reasonable. (A test would be to use reflection to grab a getter or setter whose method body was changed and then try and invoke it and verify that we're calling the correct method.) --- src/mono/mono/component/hot_reload.c | 123 ++++++++++++++++++++++++++- src/mono/mono/metadata/class-init.c | 5 ++ src/mono/mono/metadata/metadata.c | 10 ++- 3 files changed, 135 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index ee28b56704821f..8f0896cea93750 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -153,6 +153,15 @@ hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint3 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); +static uint32_t +hot_reload_member_parent (MonoImage *base_image, uint32_t member_token); + +static MonoClassMetadataUpdateProperty * +add_property_to_existing_class (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t property_token, uint32_t property_flags); + +static void +add_semantic_method_to_existing_property (MonoImage *image_base, BaselineInfo *base_info, uint32_t semantics, uint32_t klass_token, uint32_t prop_token, uint32_t method_token); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_available }, &hot_reload_set_fastpath_data, @@ -2281,6 +2290,7 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base mono_metadata_decode_row (&image_base->tables [MONO_TABLE_PROPERTYMAP], token_index - 1, prop_cols, MONO_PROPERTY_MAP_SIZE); mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "EnC: propertymap parent = 0x%08x, props = 0x%08x\n", prop_cols [MONO_PROPERTY_MAP_PARENT], prop_cols [MONO_PROPERTY_MAP_PROPERTY_LIST]); + /* FIXME: is prop_cols [MONO_PROPERTY_MAP_PROPERTY_LIST] an index into the delta? should we store it in the parent? */ break; } case MONO_TABLE_PROPERTY: { @@ -2309,7 +2319,11 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base } mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new property 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_property_klass), m_class_get_name (add_property_klass)); - /* TODO: metadata-update: add a new MonoClassMetadataUpdatePropertyInfo to added_props */ + add_member_parent (base_info, parent_type_token, log_token); + + uint32_t mapped_token = hot_reload_relative_delta_index (image_dmeta, delta_info, log_token); + uint32_t property_flags = mono_metadata_decode_row_col (&image_dmeta->tables [MONO_TABLE_PROPERTY], mapped_token - 1, MONO_PROPERTY_FLAGS); + add_property_to_existing_class (image_base, base_info, generation, delta_info, add_property_klass, log_token, property_flags); break; } @@ -2415,6 +2429,36 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base /* added rows ok (for added classes). will be processed when the MonoClass is created. */ break; } + case MONO_TABLE_METHODSEMANTICS: { + g_assert (is_addition); + /* added rows ok (for new or existing classes). modifications rejected by pass1 */ + uint32_t sema_cols[MONO_METHOD_SEMA_SIZE]; + + uint32_t mapped_token = hot_reload_relative_delta_index (image_dmeta, delta_info, log_token); + + mono_metadata_decode_row (&image_dmeta->tables [MONO_TABLE_METHODSEMANTICS], mapped_token - 1, sema_cols, MONO_METHOD_SEMA_SIZE); + + uint32_t assoc_idx = sema_cols [MONO_METHOD_SEMA_ASSOCIATION] >> MONO_HAS_SEMANTICS_BITS; + /* prop or event */ + gboolean is_prop = (sema_cols [MONO_METHOD_SEMA_ASSOCIATION] & MONO_HAS_SEMANTICS_MASK) == MONO_HAS_SEMANTICS_PROPERTY; + uint32_t method_idx = sema_cols [MONO_METHOD_SEMA_METHOD]; + uint32_t semantics = sema_cols [MONO_METHOD_SEMA_SEMANTICS]; + uint32_t assoc_token = mono_metadata_make_token (is_prop ? MONO_TABLE_PROPERTY : MONO_TABLE_EVENT, assoc_idx); + uint32_t method_token = mono_metadata_make_token (MONO_TABLE_METHOD, method_idx); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "MethodSemantics [0x%08x] = { method_idx = 0x%08x, semantics = 0x%08x, association = 0x%08x (idx = 0x%08x, is_prop = %s)} ", log_token, method_idx, semantics, sema_cols [MONO_METHOD_SEMA_ASSOCIATION], assoc_idx, is_prop ? "true" : "false"); + /* class that owns the property or event */ + uint32_t klass_token = hot_reload_member_parent (image_base, assoc_token); + if (klass_token == 0) + break; /* FIXME: these all seems to be from existing properties. Is this because we're ignoring the PropertyMap update, and we're actually getting new contiguous rows ? */ + if (pass2_context_is_skeleton (ctx, klass_token)) { + /* nothing to do, the semantics rows for the new class are contiguous and will be inited when the class is created */ + } else { + /* attach the new method to the correct place on the property/event */ + g_assert (is_prop); /* TODO: event */ + add_semantic_method_to_existing_property (image_base, base_info, semantics, klass_token, assoc_token, method_token); + } + break; + } default: { g_assert (token_index > table_info_get_rows (&image_base->tables [token_table])); if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE)) @@ -2943,6 +2987,20 @@ hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token) { return NULL; } +static MonoProperty * +hot_reload_get_property (MonoClass *klass, uint32_t property_token) +{ + MonoClassMetadataUpdateInfo *info = mono_class_get_or_add_metadata_update_info (klass); + g_assert (mono_metadata_token_table (property_token) == MONO_TABLE_PROPERTY); + GSList *added_props = info->added_props; + + for (GSList *p = added_props; p; p = p->next) { + MonoClassMetadataUpdateProperty *prop = (MonoClassMetadataUpdateProperty*)p->data; + if (prop->token == property_token) + return &prop->prop; + } + return NULL; +} 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) @@ -2977,6 +3035,69 @@ metadata_update_field_setup_basic_info (MonoImage *image_base, BaselineInfo *bas return field; } +MonoClassMetadataUpdateProperty * +add_property_to_existing_class (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t property_token, uint32_t property_flags) +{ + if (!m_class_is_inited (parent_klass)) + mono_class_init_internal (parent_klass); + + MonoClassMetadataUpdateInfo *parent_info = mono_class_get_or_add_metadata_update_info (parent_klass); + + MonoClassMetadataUpdateProperty *prop = mono_class_alloc0 (parent_klass, sizeof (MonoClassMetadataUpdateProperty)); + + prop->prop.parent = parent_klass; + + uint32_t name_idx = mono_metadata_decode_table_row_col (image_base, MONO_TABLE_PROPERTY, mono_metadata_token_index (property_token) - 1, MONO_PROPERTY_NAME); + prop->prop.name = mono_metadata_string_heap (image_base, name_idx); + + prop->prop.attrs = property_flags | MONO_PROPERTY_META_FLAG_FROM_UPDATE; + /* get and set will be set when we process the added MethodSemantics rows */ + + + prop->token = property_token; + + parent_info->added_props = g_slist_prepend_mem_manager (m_class_get_mem_manager (parent_klass), parent_info->added_props, prop); + + return prop; + +} + +void +add_semantic_method_to_existing_property (MonoImage *image_base, BaselineInfo *base_info, uint32_t semantics, uint32_t klass_token, uint32_t prop_token, uint32_t method_token) +{ + ERROR_DECL (error); + gboolean is_getter; + switch (semantics) { + case METHOD_SEMANTIC_GETTER: + is_getter = TRUE; + break; + case METHOD_SEMANTIC_SETTER: + is_getter = FALSE; + break; + default: + g_error ("EnC: Expected getter or setter semantic but got 0x%08x for method 0x%08x", semantics, method_token); + } + + MonoClass *klass = mono_class_get_checked (image_base, klass_token, error); + mono_error_assert_ok (error); + g_assert (klass); + + MonoProperty *prop = hot_reload_get_property (klass, prop_token); + g_assert (prop != NULL); + + g_assert (m_property_is_from_update (prop)); + + MonoMethod **dest = is_getter ? &prop->get : &prop->set; + + g_assert (*dest == NULL); + + MonoMethod *method = mono_get_method_checked (image_base, method_token, klass, NULL, error); + mono_error_assert_ok (error); + g_assert (method != NULL); + + *dest = method; +} + static void ensure_class_runtime_info_inited (MonoClass *klass, MonoClassRuntimeMetadataUpdateInfo *runtime_info) { diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 2a917dae03f4a5..c396c3a42624cb 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -3587,6 +3587,11 @@ mono_class_setup_properties (MonoClass *klass) first = ginfo->first; count = ginfo->count; } else { + /* + * metadata-update: note this is only adding properties from the base image. new + * properties added to an existing class won't be here since they're not in the + * contiguous rows [first,last]. + */ first = mono_metadata_properties_from_typedef (klass->image, mono_metadata_token_index (klass->type_token) - 1, &last); g_assert ((last - first) >= 0); count = last - first; diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 9fc6e541fc9cfc..07be4a05f8a1f4 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6440,10 +6440,16 @@ mono_metadata_properties_from_typedef (MonoImage *meta, guint32 index, guint *en } start = mono_metadata_decode_row_col (tdef, loc.result, MONO_PROPERTY_MAP_PROPERTY_LIST); - if (loc.result + 1 < mono_metadata_table_num_rows (meta, MONO_TABLE_PROPERTYMAP)) { + /* + * metadata-update: note this next line needs block needs to look at the number of rows in + * PropertyMap and Property of the base image. Updates will add rows for new properties, + * but they won't be contiguous. if we set end to the number of rows in the updated + * Property table, the range will include properties from some other class + */ + if (loc.result + 1 < table_info_get_rows (&meta->tables [MONO_TABLE_PROPERTYMAP])) { end = mono_metadata_decode_row_col (tdef, loc.result + 1, MONO_PROPERTY_MAP_PROPERTY_LIST) - 1; } else { - end = mono_metadata_table_num_rows (meta, MONO_TABLE_PROPERTY); + end = table_info_get_rows (&meta->tables [MONO_TABLE_PROPERTY]); } *end_idx = GUINT32_TO_UINT(end); From 9b6503a7600a8afb730dba48adc3d2324ee8b93b Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 17 Nov 2022 15:17:19 -0500 Subject: [PATCH 44/55] fixup comment - repeated MethodSemantics rows can happen --- src/mono/mono/component/hot_reload.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 8f0896cea93750..a95beaf21be555 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -2448,8 +2448,26 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "MethodSemantics [0x%08x] = { method_idx = 0x%08x, semantics = 0x%08x, association = 0x%08x (idx = 0x%08x, is_prop = %s)} ", log_token, method_idx, semantics, sema_cols [MONO_METHOD_SEMA_ASSOCIATION], assoc_idx, is_prop ? "true" : "false"); /* class that owns the property or event */ uint32_t klass_token = hot_reload_member_parent (image_base, assoc_token); - if (klass_token == 0) - break; /* FIXME: these all seems to be from existing properties. Is this because we're ignoring the PropertyMap update, and we're actually getting new contiguous rows ? */ + if (klass_token == 0) { + /* This can happen because Roslyn emits new MethodSemantics + * rows when a getter/setter method is updated. If you have + * something like: + * + * public string MyProp => string.Empty; + * + * and you change it to + * + * public string MyProp => "abcd"; + * + * Roslyn emits a MethodDef update (with the new method body RVA), a + * Property update (with the same content as the previous + * generation) and a new MethodSemantics row. + * + * In that case the assoc token points to the mutated Property row, + * for which we don't have a member_parent entry. So just ignore it. + */ + break; + } if (pass2_context_is_skeleton (ctx, klass_token)) { /* nothing to do, the semantics rows for the new class are contiguous and will be inited when the class is created */ } else { From 21dad8b028acba492f28d96133355e78ddb00390 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 17 Nov 2022 15:50:56 -0500 Subject: [PATCH 45/55] added properties iteration --- src/mono/mono/component/hot_reload-stub.c | 11 +++++++ src/mono/mono/component/hot_reload.c | 35 +++++++++++++++++++++++ src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/class.c | 26 +++++++++++------ src/mono/mono/metadata/metadata-update.c | 7 +++++ src/mono/mono/metadata/metadata-update.h | 3 ++ 6 files changed, 75 insertions(+), 8 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 852a79a2cedea0..ad5a926037afb3 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -116,6 +116,9 @@ hot_reload_stub_get_method_params (MonoImage *base_image, uint32_t methoddef_tok static gpointer hot_reload_stub_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); +static MonoProperty * +hot_reload_stub_added_properties_iter (MonoClass *klass, gpointer *iter); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -151,6 +154,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_capabilities, &hot_reload_stub_get_method_params, &hot_reload_stub_added_field_ldflda, + &hot_reload_stub_added_properties_iter, }; static bool @@ -363,6 +367,13 @@ hot_reload_stub_added_field_ldflda (MonoObject *instance, MonoType *field_type, return NULL; } +static MonoProperty * +hot_reload_stub_added_properties_iter (MonoClass *klass, gpointer *iter) +{ + return NULL; +} + + 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 a95beaf21be555..33f0c9bcdeffe9 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -150,6 +150,9 @@ hot_reload_get_method_params (MonoImage *base_image, uint32_t methoddef_token, u static gpointer hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); +static MonoProperty * +hot_reload_added_properties_iter (MonoClass *klass, gpointer *iter); + 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); @@ -197,6 +200,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_capabilities, &hot_reload_get_method_params, &hot_reload_added_field_ldflda, + &hot_reload_added_properties_iter, }; MonoComponentHotReload * @@ -3414,3 +3418,34 @@ hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint3 result = (gpointer)&field_store->_loc; return result; } + +static MonoProperty * +hot_reload_added_properties_iter (MonoClass *klass, gpointer *iter) +{ + MonoClassMetadataUpdateInfo *info = mono_class_get_metadata_update_info (klass); + if (!info) + return NULL; + + GSList *added_props = info->added_props; + + // invariant: idx is one past the field we previously returned. + uint32_t idx = GPOINTER_TO_UINT(*iter); + + MonoClassPropertyInfo *prop_info = mono_class_get_property_info (klass); + g_assert (idx >= prop_info->count); + + uint32_t prop_idx = idx - prop_info->count; + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Iterating added properties of 0x%08x idx = %u", m_class_get_type_token (klass), prop_idx); + + GSList *prop_node = g_slist_nth (added_props, prop_idx); + + /* we reached the end, we're done */ + if (!prop_node) + return NULL; + MonoClassMetadataUpdateProperty *prop = (MonoClassMetadataUpdateProperty *)prop_node->data; + + idx++; + *iter = GUINT_TO_POINTER (idx); + return &prop->prop; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index abea77e9abaff9..8b7f1d06edbaef 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -50,6 +50,7 @@ typedef struct _MonoComponentHotReload { const char* (*get_capabilities) (void); uint32_t (*get_method_params) (MonoImage *base_image, uint32_t methoddef_token, uint32_t *out_param_count_opt); gpointer (*added_field_ldflda) (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); + MonoProperty* (*added_properties_iter) (MonoClass *klass, gpointer *iter); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 11e88dc2eb225c..d21b015a976260 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2599,6 +2599,7 @@ mono_class_get_field_default_value (MonoClassField *field, MonoTypeEnum *def_typ static int mono_property_get_index (MonoProperty *prop) { + g_assert (!m_property_is_from_update (prop)); MonoClassPropertyInfo *info = mono_class_get_property_info (prop->parent); int index = GPTRDIFF_TO_INT (prop - info->properties); @@ -5222,19 +5223,28 @@ mono_class_get_properties (MonoClass* klass, gpointer *iter) MonoClassPropertyInfo *info = mono_class_get_property_info (klass); /* start from the first */ if (info->count) { - *iter = &info->properties [0]; - return (MonoProperty *)*iter; + uint32_t idx = 0; + *iter = GUINT_TO_POINTER (idx + 1); + return (MonoProperty *)&info->properties [0]; } else { /* no fields */ - return NULL; + if (G_LIKELY (!m_class_get_image (klass)->has_updates)) + return NULL; + else + *iter = 0; } } - property = (MonoProperty *)*iter; - property++; + // invariant: idx is one past the field we previously returned + uint32_t idx = GPOINTER_TO_UINT (*iter); MonoClassPropertyInfo *info = mono_class_get_property_info (klass); - if (property < &info->properties [info->count]) { - *iter = property; - return (MonoProperty *)*iter; + if (idx < info->count) { + MonoProperty *property = &info->properties [idx]; + ++idx; + *iter = GUINT_TO_POINTER (idx); + return property; + } + if (G_UNLIKELY (m_class_get_image (klass)->has_updates)) { + return mono_metadata_update_added_properties_iter (klass, iter); } return NULL; } diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 6e912bfc194447..51e60ee8ce3f8f 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -242,3 +242,10 @@ mono_metadata_update_added_field_ldflda (MonoObject *instance, MonoType *field_t { return mono_component_hot_reload()->added_field_ldflda (instance, field_type, fielddef_token, error); } + + +MonoProperty * +mono_metadata_update_added_properties_iter (MonoClass *klass, gpointer *iter) +{ + return mono_component_hot_reload()->added_properties_iter (klass, iter); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 1ac696ab84571f..1f3c4ad1be22df 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -100,4 +100,7 @@ mono_metadata_update_get_method_params (MonoImage *image, uint32_t methoddef_tok MONO_COMPONENT_API gpointer mono_metadata_update_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); +MonoProperty * +mono_metadata_update_added_properties_iter (MonoClass *klass, gpointer *iter); + #endif /*__MONO_METADATA_UPDATE_H__*/ From 2bc5e5a247b345d5f960689e2253d7d475da6ab0 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 17 Nov 2022 16:09:16 -0500 Subject: [PATCH 46/55] protect callers of mono_class_get_property_token --- src/mono/mono/metadata/class.c | 2 ++ src/mono/mono/metadata/reflection.c | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index d21b015a976260..12f17fbea5d397 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2637,6 +2637,8 @@ mono_class_get_property_default_value (MonoProperty *property, MonoTypeEnum *def } return NULL; } + /* TODO: metadata-update: if property is from update, get the token directly. */ + g_assert (!m_property_is_from_update (property)); cindex = mono_metadata_get_constant_index (klass_image, mono_class_get_property_token (property), 0); if (!cindex) return NULL; diff --git a/src/mono/mono/metadata/reflection.c b/src/mono/mono/metadata/reflection.c index 52107a74225441..299561c4a97bdf 100644 --- a/src/mono/mono/metadata/reflection.c +++ b/src/mono/mono/metadata/reflection.c @@ -2502,7 +2502,9 @@ mono_reflection_get_token_checked (MonoObjectHandle obj, MonoError *error) } else if (strcmp (klass_name, "RuntimePropertyInfo") == 0) { MonoReflectionPropertyHandle p = MONO_HANDLE_CAST (MonoReflectionProperty, obj); - token = mono_class_get_property_token (MONO_HANDLE_GETVAL (p, property)); + MonoProperty *prop = MONO_HANDLE_GETVAL (p, property); + g_assert (!m_property_is_from_update (prop)); // TODO: metadata-update: get the token directly + token = mono_class_get_property_token (prop); } else if (strcmp (klass_name, "RuntimeEventInfo") == 0) { MonoReflectionMonoEventHandle p = MONO_HANDLE_CAST (MonoReflectionMonoEvent, obj); From ac34462c7e07fd6d603dcb5b0c5c97850f6a40be Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 18 Nov 2022 09:20:29 -0500 Subject: [PATCH 47/55] fix build warnings --- src/mono/mono/metadata/class.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 12f17fbea5d397..7a7416422ef327 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -5217,7 +5217,6 @@ mono_class_get_methods (MonoClass* klass, gpointer *iter) MonoProperty* mono_class_get_properties (MonoClass* klass, gpointer *iter) { - MonoProperty* property; if (!iter) return NULL; if (!*iter) { From fbcd1b25535f54ab9aa5e3be4cc97e04b1ad6377 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 18 Nov 2022 11:43:39 -0500 Subject: [PATCH 48/55] fix dynamic component builds --- src/mono/mono/metadata/class-internals.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index d0158da21a67c6..3a15e146e0c0e4 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1350,7 +1350,7 @@ MONO_COMPONENT_API void mono_class_set_nested_classes_property (MonoClass *klass, GList *value); -MonoClassPropertyInfo* +MONO_COMPONENT_API MonoClassPropertyInfo* mono_class_get_property_info (MonoClass *klass); void From 2934993a70198d7fc52124bed58bc38bfaae049d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 18 Nov 2022 13:51:54 -0500 Subject: [PATCH 49/55] make mono_class_get_property_token work for added props --- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 10 ++++++++++ src/mono/mono/component/hot_reload-stub.c | 10 ++++++++++ src/mono/mono/component/hot_reload.c | 13 +++++++++++++ src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/class.c | 10 +++++++++- src/mono/mono/metadata/custom-attrs.c | 2 ++ src/mono/mono/metadata/icall.c | 3 +++ src/mono/mono/metadata/metadata-update.c | 6 ++++++ src/mono/mono/metadata/metadata-update.h | 3 +++ src/mono/mono/metadata/reflection.c | 1 - 10 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index cd6342ee7e115a..053be1863ebd12 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -380,6 +380,16 @@ public static void TestAddInstanceField() Assert.Equal("abcdTest", x2.GetStringProp); + var addedPropToken = propInfo.MetadataToken; + + Assert.True (addedPropToken > 0); + + // we don't know exactly what token Roslyn will assign to the added property, but + // since the AddInstanceField.dll assembly is relatively small, assume that the + // total number of properties in the updated generation is less than 64 and the + // token is in that range. If more code is added, revise this test. + + Assert.True ((addedPropToken & 0x00ffffff) < 64); }); } diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index ad5a926037afb3..28b612f7f5e4d0 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -119,6 +119,9 @@ hot_reload_stub_added_field_ldflda (MonoObject *instance, MonoType *field_type, static MonoProperty * hot_reload_stub_added_properties_iter (MonoClass *klass, gpointer *iter); +static uint32_t +hot_reload_stub_get_property_idx (MonoProperty *prop); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -155,6 +158,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_get_method_params, &hot_reload_stub_added_field_ldflda, &hot_reload_stub_added_properties_iter, + &hot_reload_stub_get_property_idx, }; static bool @@ -373,6 +377,12 @@ hot_reload_stub_added_properties_iter (MonoClass *klass, gpointer *iter) return NULL; } +static uint32_t +hot_reload_stub_get_property_idx (MonoProperty *prop) +{ + return 0; +} + MONO_COMPONENT_EXPORT_ENTRYPOINT MonoComponentHotReload * diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 33f0c9bcdeffe9..faf7cafb448fff 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -153,6 +153,10 @@ hot_reload_added_field_ldflda (MonoObject *instance, MonoType *field_type, uint3 static MonoProperty * hot_reload_added_properties_iter (MonoClass *klass, gpointer *iter); +static uint32_t +hot_reload_get_property_idx (MonoProperty *prop); + + 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); @@ -201,6 +205,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_method_params, &hot_reload_added_field_ldflda, &hot_reload_added_properties_iter, + &hot_reload_get_property_idx, }; MonoComponentHotReload * @@ -3449,3 +3454,11 @@ hot_reload_added_properties_iter (MonoClass *klass, gpointer *iter) *iter = GUINT_TO_POINTER (idx); return &prop->prop; } + +uint32_t +hot_reload_get_property_idx (MonoProperty *prop) +{ + g_assert (m_property_is_from_update (prop)); + MonoClassMetadataUpdateProperty *prop_info = (MonoClassMetadataUpdateProperty *)prop; + return mono_metadata_token_index (prop_info->token); +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 8b7f1d06edbaef..1a27cd1db72b7d 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -51,6 +51,7 @@ typedef struct _MonoComponentHotReload { uint32_t (*get_method_params) (MonoImage *base_image, uint32_t methoddef_token, uint32_t *out_param_count_opt); gpointer (*added_field_ldflda) (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); MonoProperty* (*added_properties_iter) (MonoClass *klass, gpointer *iter); + uint32_t (*get_property_idx) (MonoProperty *prop); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 7a7416422ef327..a17522f06007d3 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2637,7 +2637,7 @@ mono_class_get_property_default_value (MonoProperty *property, MonoTypeEnum *def } return NULL; } - /* TODO: metadata-update: if property is from update, get the token directly. */ + /* metadata-update: Roslyn doesn't emit HasDefault on added properties. */ g_assert (!m_property_is_from_update (property)); cindex = mono_metadata_get_constant_index (klass_image, mono_class_get_property_token (property), 0); if (!cindex) @@ -2699,6 +2699,14 @@ guint32 mono_class_get_property_token (MonoProperty *prop) { MonoClass *klass = prop->parent; + + if (G_UNLIKELY (m_class_get_image (klass)->has_updates)) { + if (G_UNLIKELY (m_property_is_from_update (prop))) { + uint32_t idx = mono_metadata_update_get_property_idx (prop); + return mono_metadata_make_token (MONO_TABLE_PROPERTY, idx); + } + } + while (klass) { MonoProperty* p; int i = 0; diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index f97ea23a2f4a26..06b77f2ba990f9 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -169,6 +169,8 @@ find_field_index (MonoClass *klass, MonoClassField *field) { static guint32 find_property_index (MonoClass *klass, MonoProperty *property) { + if (G_UNLIKELY (m_property_is_from_update (property))) + return mono_metadata_update_get_property_idx (property); MonoClassPropertyInfo *info = mono_class_get_property_info (klass); for (guint32 i = 0; i < info->count; ++i) { diff --git a/src/mono/mono/metadata/icall.c b/src/mono/mono/metadata/icall.c index b4d39021e5b131..c548f46427ddc1 100644 --- a/src/mono/mono/metadata/icall.c +++ b/src/mono/mono/metadata/icall.c @@ -6519,6 +6519,9 @@ ves_icall_property_info_get_default_value (MonoReflectionPropertyHandle property return NULL_HANDLE; } + /* metadata-update: looks like Roslyn doesn't set the HasDefault attribute for updates */ + g_assert (!m_property_is_from_update (prop)); + def_value = mono_class_get_property_default_value (prop, &def_type); mono_type_from_blob_type (&blob_type, def_type, type); diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 51e60ee8ce3f8f..85ffa58303ad26 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -249,3 +249,9 @@ mono_metadata_update_added_properties_iter (MonoClass *klass, gpointer *iter) { return mono_component_hot_reload()->added_properties_iter (klass, iter); } + +uint32_t +mono_metadata_update_get_property_idx (MonoProperty *prop) +{ + return mono_component_hot_reload()->get_property_idx (prop); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 1f3c4ad1be22df..667660a3307890 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -103,4 +103,7 @@ mono_metadata_update_added_field_ldflda (MonoObject *instance, MonoType *field_t MonoProperty * mono_metadata_update_added_properties_iter (MonoClass *klass, gpointer *iter); +uint32_t +mono_metadata_update_get_property_idx (MonoProperty *prop); + #endif /*__MONO_METADATA_UPDATE_H__*/ diff --git a/src/mono/mono/metadata/reflection.c b/src/mono/mono/metadata/reflection.c index 299561c4a97bdf..57a7c58ccda822 100644 --- a/src/mono/mono/metadata/reflection.c +++ b/src/mono/mono/metadata/reflection.c @@ -2503,7 +2503,6 @@ mono_reflection_get_token_checked (MonoObjectHandle obj, MonoError *error) MonoReflectionPropertyHandle p = MONO_HANDLE_CAST (MonoReflectionProperty, obj); MonoProperty *prop = MONO_HANDLE_GETVAL (p, property); - g_assert (!m_property_is_from_update (prop)); // TODO: metadata-update: get the token directly token = mono_class_get_property_token (prop); } else if (strcmp (klass_name, "RuntimeEventInfo") == 0) { MonoReflectionMonoEventHandle p = MONO_HANDLE_CAST (MonoReflectionMonoEvent, obj); From 9c4c18ab16afbea8f6a3864fff30674a977503a9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 18 Nov 2022 14:02:33 -0500 Subject: [PATCH 50/55] add test for adding instance event --- .../AddInstanceField.cs | 6 ++++++ .../AddInstanceField_v1.cs | 12 ++++++++++++ .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 5 +++++ 3 files changed, 23 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs index ab10f7e49e68a6..cc56e775c09427 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs @@ -32,5 +32,11 @@ public void IncRefDouble (ref double d) } public string GetStringProp => string.Empty; + + public event EventHandler ExistingEvent; + + public void FireEvents() { + ExistingEvent(this, 123.0); + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs index c07c96c8faff5b..5c82ff087bf722 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs @@ -19,6 +19,11 @@ public AddInstanceField () { IncRefDouble (ref _doubleField2); AddedStringAutoProp = "abcd"; + + AddedEvent += MyHandler; + + void MyHandler (object sender, double data) { + } } public void IncRefDouble (ref double d) @@ -60,6 +65,13 @@ public struct NewStruct public string AddedStringAutoProp { get; set; } + public event EventHandler ExistingEvent; + public event EventHandler AddedEvent; + + public void FireEvents() { + ExistingEvent(this, 123.0); + AddedEvent(this, 123.0); + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 053be1863ebd12..87330eaa4351cf 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -391,6 +391,11 @@ public static void TestAddInstanceField() Assert.True ((addedPropToken & 0x00ffffff) < 64); + + var eventInfo = x2.GetType().GetEvent("AddedEvent", BindingFlags.Public | BindingFlags.Instance); + + Assert.NotNull (eventInfo); + }); } From 3216155ed905b8895d850c194b1ded8a8d34b3f6 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 18 Nov 2022 15:51:01 -0500 Subject: [PATCH 51/55] basic event reflection works --- .../mono/component/hot_reload-internals.h | 2 +- src/mono/mono/component/hot_reload-stub.c | 10 ++ src/mono/mono/component/hot_reload.c | 152 ++++++++++++++++-- src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/class.c | 26 +-- src/mono/mono/metadata/metadata-update.c | 6 + src/mono/mono/metadata/metadata-update.h | 3 + src/mono/mono/metadata/metadata.c | 6 + 8 files changed, 185 insertions(+), 21 deletions(-) diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index cc97538749d056..65cdcde8f56bfb 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -82,7 +82,7 @@ typedef struct _MonoClassMetadataUpdateProperty { typedef struct _MonoClassMetadataUpdateEvent { MonoEvent evt; - uint32_t generatino; /* when this event was added */ + uint32_t generation; /* when this event was added */ uint32_t token; /* the Event table token where this event was defined. */ } MonoClassMetadataUpdateEvent; diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 28b612f7f5e4d0..5f078e439a6389 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -122,6 +122,9 @@ hot_reload_stub_added_properties_iter (MonoClass *klass, gpointer *iter); static uint32_t hot_reload_stub_get_property_idx (MonoProperty *prop); +static MonoEvent * +hot_reload_stub_added_events_iter (MonoClass *klass, gpointer *iter); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -159,6 +162,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_added_field_ldflda, &hot_reload_stub_added_properties_iter, &hot_reload_stub_get_property_idx, + &hot_reload_stub_added_events_iter, }; static bool @@ -383,6 +387,11 @@ hot_reload_stub_get_property_idx (MonoProperty *prop) return 0; } +MonoEvent * +hot_reload_stub_added_events_iter (MonoClass *klass, gpointer *iter) +{ + return NULL; +} MONO_COMPONENT_EXPORT_ENTRYPOINT MonoComponentHotReload * @@ -391,3 +400,4 @@ mono_component_hot_reload_init (void) return component_hot_reload_stub_init (); } + diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index faf7cafb448fff..018bdc77c81d6b 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -156,6 +156,8 @@ hot_reload_added_properties_iter (MonoClass *klass, gpointer *iter); static uint32_t hot_reload_get_property_idx (MonoProperty *prop); +MonoEvent * +hot_reload_added_events_iter (MonoClass *klass, gpointer *iter); 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); @@ -169,6 +171,12 @@ add_property_to_existing_class (MonoImage *image_base, BaselineInfo *base_info, static void add_semantic_method_to_existing_property (MonoImage *image_base, BaselineInfo *base_info, uint32_t semantics, uint32_t klass_token, uint32_t prop_token, uint32_t method_token); +MonoClassMetadataUpdateEvent * +add_event_to_existing_class (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t event_token, uint32_t event_flags); + +static void +add_semantic_method_to_existing_event (MonoImage *image_base, BaselineInfo *base_info, uint32_t semantics, uint32_t klass_token, uint32_t event_token, uint32_t method_token); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_available }, &hot_reload_set_fastpath_data, @@ -206,6 +214,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_added_field_ldflda, &hot_reload_added_properties_iter, &hot_reload_get_property_idx, + &hot_reload_added_events_iter, }; MonoComponentHotReload * @@ -2299,7 +2308,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base mono_metadata_decode_row (&image_base->tables [MONO_TABLE_PROPERTYMAP], token_index - 1, prop_cols, MONO_PROPERTY_MAP_SIZE); mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "EnC: propertymap parent = 0x%08x, props = 0x%08x\n", prop_cols [MONO_PROPERTY_MAP_PARENT], prop_cols [MONO_PROPERTY_MAP_PROPERTY_LIST]); - /* FIXME: is prop_cols [MONO_PROPERTY_MAP_PROPERTY_LIST] an index into the delta? should we store it in the parent? */ break; } case MONO_TABLE_PROPERTY: { @@ -2385,7 +2393,12 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base } mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new event 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_event_klass), m_class_get_name (add_event_klass)); - /* TODO: metadata-update: add a new MonoEventInfo to the bag on the class? */ + add_member_parent (base_info, parent_type_token, log_token); + + uint32_t mapped_token = hot_reload_relative_delta_index (image_dmeta, delta_info, log_token); + uint32_t event_flags = mono_metadata_decode_row_col (&image_dmeta->tables [MONO_TABLE_EVENT], mapped_token - 1, MONO_EVENT_FLAGS); + add_event_to_existing_class (image_base, base_info, generation, delta_info, add_event_klass, log_token, event_flags); + break; } @@ -2481,8 +2494,10 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base /* nothing to do, the semantics rows for the new class are contiguous and will be inited when the class is created */ } else { /* attach the new method to the correct place on the property/event */ - g_assert (is_prop); /* TODO: event */ - add_semantic_method_to_existing_property (image_base, base_info, semantics, klass_token, assoc_token, method_token); + if (is_prop) + add_semantic_method_to_existing_property (image_base, base_info, semantics, klass_token, assoc_token, method_token); + else + add_semantic_method_to_existing_event (image_base, base_info, semantics, klass_token, assoc_token, method_token); } break; } @@ -3029,6 +3044,21 @@ hot_reload_get_property (MonoClass *klass, uint32_t property_token) return NULL; } +static MonoEvent * +hot_reload_get_event (MonoClass *klass, uint32_t event_token) +{ + MonoClassMetadataUpdateInfo *info = mono_class_get_or_add_metadata_update_info (klass); + g_assert (mono_metadata_token_table (event_token) == MONO_TABLE_EVENT); + GSList *added_events = info->added_events; + + for (GSList *p = added_events; p; p = p->next) { + MonoClassMetadataUpdateEvent *evt = (MonoClassMetadataUpdateEvent*)p->data; + if (evt->token == event_token) + return &evt->evt; + } + return NULL; +} + 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) { @@ -3089,33 +3119,101 @@ add_property_to_existing_class (MonoImage *image_base, BaselineInfo *base_info, } +MonoClassMetadataUpdateEvent * +add_event_to_existing_class (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t event_token, uint32_t event_flags) +{ + if (!m_class_is_inited (parent_klass)) + mono_class_init_internal (parent_klass); + + MonoClassMetadataUpdateInfo *parent_info = mono_class_get_or_add_metadata_update_info (parent_klass); + + MonoClassMetadataUpdateEvent *evt = mono_class_alloc0 (parent_klass, sizeof (MonoClassMetadataUpdateEvent)); + + evt->evt.parent = parent_klass; + + uint32_t name_idx = mono_metadata_decode_table_row_col (image_base, MONO_TABLE_EVENT, mono_metadata_token_index (event_token) - 1, MONO_EVENT_NAME); + evt->evt.name = mono_metadata_string_heap (image_base, name_idx); + + evt->evt.attrs = event_flags | MONO_EVENT_META_FLAG_FROM_UPDATE; + /* add/remove/raise will be set when we process the added MethodSemantics rows */ + + evt->token = event_token; + + parent_info->added_events = g_slist_prepend_mem_manager (m_class_get_mem_manager (parent_klass), parent_info->added_events, evt); + + return evt; + +} + + void add_semantic_method_to_existing_property (MonoImage *image_base, BaselineInfo *base_info, uint32_t semantics, uint32_t klass_token, uint32_t prop_token, uint32_t method_token) { ERROR_DECL (error); - gboolean is_getter; + + MonoClass *klass = mono_class_get_checked (image_base, klass_token, error); + mono_error_assert_ok (error); + g_assert (klass); + + MonoProperty *prop = hot_reload_get_property (klass, prop_token); + g_assert (prop != NULL); + + g_assert (m_property_is_from_update (prop)); + + MonoMethod **dest = NULL; switch (semantics) { case METHOD_SEMANTIC_GETTER: - is_getter = TRUE; + dest = &prop->get; break; case METHOD_SEMANTIC_SETTER: - is_getter = FALSE; + dest = &prop->set; break; default: g_error ("EnC: Expected getter or setter semantic but got 0x%08x for method 0x%08x", semantics, method_token); } + + g_assert (dest != NULL); + g_assert (*dest == NULL); + + MonoMethod *method = mono_get_method_checked (image_base, method_token, klass, NULL, error); + mono_error_assert_ok (error); + g_assert (method != NULL); + + *dest = method; +} + +void +add_semantic_method_to_existing_event (MonoImage *image_base, BaselineInfo *base_info, uint32_t semantics, uint32_t klass_token, uint32_t event_token, uint32_t method_token) +{ + ERROR_DECL (error); + MonoClass *klass = mono_class_get_checked (image_base, klass_token, error); mono_error_assert_ok (error); g_assert (klass); - MonoProperty *prop = hot_reload_get_property (klass, prop_token); - g_assert (prop != NULL); + MonoEvent *evt = hot_reload_get_event (klass, event_token); + g_assert (evt != NULL); - g_assert (m_property_is_from_update (prop)); + g_assert (m_event_is_from_update (evt)); - MonoMethod **dest = is_getter ? &prop->get : &prop->set; + MonoMethod **dest = NULL; + + switch (semantics) { + case METHOD_SEMANTIC_ADD_ON: + dest = &evt->add; + break; + case METHOD_SEMANTIC_REMOVE_ON: + dest = &evt->remove; + break; + case METHOD_SEMANTIC_FIRE: + dest = &evt->raise; + break; + default: + g_error ("EnC: Expected add/remove/raise semantic but got 0x%08x for method 0x%08x", semantics, method_token); + } + g_assert (dest != NULL); g_assert (*dest == NULL); MonoMethod *method = mono_get_method_checked (image_base, method_token, klass, NULL, error); @@ -3462,3 +3560,35 @@ hot_reload_get_property_idx (MonoProperty *prop) MonoClassMetadataUpdateProperty *prop_info = (MonoClassMetadataUpdateProperty *)prop; return mono_metadata_token_index (prop_info->token); } + + +MonoEvent * +hot_reload_added_events_iter (MonoClass *klass, gpointer *iter) +{ + MonoClassMetadataUpdateInfo *info = mono_class_get_metadata_update_info (klass); + if (!info) + return NULL; + + GSList *added_events = info->added_events; + + // invariant: idx is one past the field we previously returned. + uint32_t idx = GPOINTER_TO_UINT(*iter); + + MonoClassEventInfo *event_info = mono_class_get_event_info (klass); + g_assert (idx >= event_info->count); + + uint32_t event_idx = idx - event_info->count; + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Iterating added events of 0x%08x idx = %u", m_class_get_type_token (klass), event_idx); + + GSList *event_node = g_slist_nth (added_events, event_idx); + + /* we reached the end, we're done */ + if (!event_node) + return NULL; + MonoClassMetadataUpdateEvent *evt = (MonoClassMetadataUpdateEvent *)event_node->data; + + idx++; + *iter = GUINT_TO_POINTER (idx); + return &evt->evt; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 1a27cd1db72b7d..28011489c574c6 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -52,6 +52,7 @@ typedef struct _MonoComponentHotReload { gpointer (*added_field_ldflda) (MonoObject *instance, MonoType *field_type, uint32_t fielddef_token, MonoError *error); MonoProperty* (*added_properties_iter) (MonoClass *klass, gpointer *iter); uint32_t (*get_property_idx) (MonoProperty *prop); + MonoEvent* (*added_events_iter) (MonoClass *klass, gpointer *iter); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index a17522f06007d3..526da08e1dd647 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -5273,7 +5273,6 @@ mono_class_get_properties (MonoClass* klass, gpointer *iter) MonoEvent* mono_class_get_events (MonoClass* klass, gpointer *iter) { - MonoEvent* event; if (!iter) return NULL; if (!*iter) { @@ -5281,19 +5280,28 @@ mono_class_get_events (MonoClass* klass, gpointer *iter) MonoClassEventInfo *info = mono_class_get_event_info (klass); /* start from the first */ if (info->count) { - *iter = &info->events [0]; - return (MonoEvent *)*iter; + uint32_t idx = 0; + *iter = GUINT_TO_POINTER (idx + 1); + return (MonoEvent *)&info->events [0]; } else { /* no fields */ - return NULL; + if (G_LIKELY (!m_class_get_image (klass)->has_updates)) + return NULL; + else + *iter = 0; } } - event = (MonoEvent *)*iter; - event++; + // invariant: idx is one past the event we previously returned + uint32_t idx = GPOINTER_TO_UINT (*iter); MonoClassEventInfo *info = mono_class_get_event_info (klass); - if (event < &info->events [info->count]) { - *iter = event; - return (MonoEvent *)*iter; + if (idx < info->count) { + MonoEvent *event = &info->events[idx]; + ++idx; + *iter = GUINT_TO_POINTER (idx); + return event; + } + if (G_UNLIKELY (m_class_get_image (klass)->has_updates)) { + return mono_metadata_update_added_events_iter (klass, iter); } return NULL; } diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 85ffa58303ad26..8372876fecbb07 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -255,3 +255,9 @@ mono_metadata_update_get_property_idx (MonoProperty *prop) { return mono_component_hot_reload()->get_property_idx (prop); } + +MonoEvent * +mono_metadata_update_added_events_iter (MonoClass *klass, gpointer *iter) +{ + return mono_component_hot_reload()->added_events_iter (klass, iter); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 667660a3307890..9323e91a2dce76 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -106,4 +106,7 @@ mono_metadata_update_added_properties_iter (MonoClass *klass, gpointer *iter); uint32_t mono_metadata_update_get_property_idx (MonoProperty *prop); +MonoEvent * +mono_metadata_update_added_events_iter (MonoClass *klass, gpointer *iter); + #endif /*__MONO_METADATA_UPDATE_H__*/ diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 07be4a05f8a1f4..3683a42488b41e 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6327,6 +6327,12 @@ mono_metadata_events_from_typedef (MonoImage *meta, guint32 index, guint *end_id } start = mono_metadata_decode_row_col (tdef, loc.result, MONO_EVENT_MAP_EVENTLIST); + /* + * metadata-update: note this next line needs block needs to look at the number of rows in + * EventMap and Event of the base image. Updates will add rows for new properties, + * but they won't be contiguous. if we set end to the number of rows in the updated + * Property table, the range will include properties from some other class + */ if (loc.result + 1 < table_info_get_rows (tdef)) { end = mono_metadata_decode_row_col (tdef, loc.result + 1, MONO_EVENT_MAP_EVENTLIST) - 1; } else { From a6dadc1abeaa7b280a6fb158a22c121432dba92f Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 18 Nov 2022 15:55:05 -0500 Subject: [PATCH 52/55] Fire the new event, too --- .../AddInstanceField.cs | 11 ++++++++++- .../AddInstanceField_v1.cs | 14 +++++++++++++- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 4 ++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs index cc56e775c09427..0b10a8b11fc9c5 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField.cs @@ -35,8 +35,17 @@ public void IncRefDouble (ref double d) public event EventHandler ExistingEvent; - public void FireEvents() { + public double Accumulator; + + private void AccumHandler (object sender, double value) => Accumulator += value; + + public double FireEvents() { + Accumulator = 0.0; + ExistingEvent += AccumHandler; ExistingEvent(this, 123.0); + ExistingEvent -= AccumHandler; + + return Accumulator; } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs index 5c82ff087bf722..23626acf5af6f1 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddInstanceField/AddInstanceField_v1.cs @@ -68,9 +68,21 @@ public struct NewStruct public event EventHandler ExistingEvent; public event EventHandler AddedEvent; - public void FireEvents() { + public double Accumulator; + + private void AccumHandler (object sender, double value) => Accumulator += value; + + public double FireEvents() { + Accumulator = 0.0; + ExistingEvent += AccumHandler; ExistingEvent(this, 123.0); + ExistingEvent -= AccumHandler; + + AddedEvent += AccumHandler; AddedEvent(this, 123.0); + AddedEvent -= AccumHandler; + + return Accumulator; } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 87330eaa4351cf..48de847e71b571 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -392,6 +392,10 @@ public static void TestAddInstanceField() Assert.True ((addedPropToken & 0x00ffffff) < 64); + var accumResult = x2.FireEvents(); + + Assert.Equal (246.0, accumResult); + var eventInfo = x2.GetType().GetEvent("AddedEvent", BindingFlags.Public | BindingFlags.Instance); Assert.NotNull (eventInfo); From bd571f91b2b9b91bc1b6b5423b82ebb61ef07ce9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 18 Nov 2022 16:03:58 -0500 Subject: [PATCH 53/55] make reflection MetadataToken work --- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 12 ++++++++++++ src/mono/mono/component/hot_reload-stub.c | 10 ++++++++++ src/mono/mono/component/hot_reload.c | 12 ++++++++++++ src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/class.c | 8 +++++++- src/mono/mono/metadata/custom-attrs.c | 2 ++ src/mono/mono/metadata/metadata-update.c | 6 ++++++ src/mono/mono/metadata/metadata-update.h | 4 ++++ 8 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 48de847e71b571..141bc239084441 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -400,6 +400,18 @@ public static void TestAddInstanceField() Assert.NotNull (eventInfo); + var addedEventToken = eventInfo.MetadataToken; + + Assert.True (addedEventToken > 0); + + // we don't know exactly what token Roslyn will assign to the added event, but + // since the AddInstanceField.dll assembly is relatively small, assume that the + // total number of events in the updated generation is less than 4 and the + // token is in that range. If more code is added, revise this test. + + Assert.True ((addedEventToken & 0x00ffffff) < 4); + + }); } diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 5f078e439a6389..73fc1fba50f381 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -125,6 +125,9 @@ hot_reload_stub_get_property_idx (MonoProperty *prop); static MonoEvent * hot_reload_stub_added_events_iter (MonoClass *klass, gpointer *iter); +static uint32_t +hot_reload_stub_get_event_idx (MonoEvent *evt); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -163,6 +166,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_added_properties_iter, &hot_reload_stub_get_property_idx, &hot_reload_stub_added_events_iter, + &hot_reload_stub_get_event_idx, }; static bool @@ -393,6 +397,12 @@ hot_reload_stub_added_events_iter (MonoClass *klass, gpointer *iter) return NULL; } +static uint32_t +hot_reload_stub_get_event_idx (MonoEvent *evt) +{ + 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 018bdc77c81d6b..3bd349fd701ac0 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -159,6 +159,9 @@ hot_reload_get_property_idx (MonoProperty *prop); MonoEvent * hot_reload_added_events_iter (MonoClass *klass, gpointer *iter); +static uint32_t +hot_reload_get_event_idx (MonoEvent *evt); + 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); @@ -215,6 +218,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_added_properties_iter, &hot_reload_get_property_idx, &hot_reload_added_events_iter, + &hot_reload_get_event_idx, }; MonoComponentHotReload * @@ -3561,6 +3565,14 @@ hot_reload_get_property_idx (MonoProperty *prop) return mono_metadata_token_index (prop_info->token); } +uint32_t +hot_reload_get_event_idx (MonoEvent *evt) +{ + g_assert (m_event_is_from_update (evt)); + MonoClassMetadataUpdateEvent *event_info = (MonoClassMetadataUpdateEvent *)evt; + return mono_metadata_token_index (event_info->token); +} + MonoEvent * hot_reload_added_events_iter (MonoClass *klass, gpointer *iter) diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 28011489c574c6..76d7f1ab4bc5ec 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -53,6 +53,7 @@ typedef struct _MonoComponentHotReload { MonoProperty* (*added_properties_iter) (MonoClass *klass, gpointer *iter); uint32_t (*get_property_idx) (MonoProperty *prop); MonoEvent* (*added_events_iter) (MonoClass *klass, gpointer *iter); + uint32_t (*get_event_idx) (MonoEvent *evt); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 526da08e1dd647..3a04a9dee77c41 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -2656,11 +2656,17 @@ mono_class_get_event_token (MonoEvent *event) { MonoClass *klass = event->parent; + if (G_UNLIKELY (m_class_get_image (klass)->has_updates)) { + if (G_UNLIKELY (m_event_is_from_update (event))) { + uint32_t idx = mono_metadata_update_get_event_idx (event); + return mono_metadata_make_token (MONO_TABLE_EVENT, idx); + } + } + while (klass) { MonoClassEventInfo *info = mono_class_get_event_info (klass); if (info) { for (guint32 i = 0; i < info->count; ++i) { - /* TODO: metadata-update: get tokens for added props, too */ g_assert (!m_event_is_from_update (&info->events[i])); if (&info->events [i] == event) return mono_metadata_make_token (MONO_TABLE_EVENT, info->first + i + 1); diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 06b77f2ba990f9..c65bab8b55dedf 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -186,6 +186,8 @@ find_property_index (MonoClass *klass, MonoProperty *property) static guint32 find_event_index (MonoClass *klass, MonoEvent *event) { + if (G_UNLIKELY (m_event_is_from_update (event))) + return mono_metadata_update_get_event_idx (event); MonoClassEventInfo *info = mono_class_get_event_info (klass); for (guint32 i = 0; i < info->count; ++i) { diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 8372876fecbb07..eda27cc1c8a62a 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -261,3 +261,9 @@ mono_metadata_update_added_events_iter (MonoClass *klass, gpointer *iter) { return mono_component_hot_reload()->added_events_iter (klass, iter); } + +uint32_t +mono_metadata_update_get_event_idx (MonoEvent *evt) +{ + return mono_component_hot_reload()->get_event_idx (evt); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 9323e91a2dce76..4e45c60439b5c3 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -109,4 +109,8 @@ mono_metadata_update_get_property_idx (MonoProperty *prop); MonoEvent * mono_metadata_update_added_events_iter (MonoClass *klass, gpointer *iter); +uint32_t +mono_metadata_update_get_event_idx (MonoEvent *event); + + #endif /*__MONO_METADATA_UPDATE_H__*/ From b14f5655125ab38c7227006c22fc8d8413f14feb Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 18 Nov 2022 18:54:23 -0500 Subject: [PATCH 54/55] fix dynamic component build --- src/mono/mono/metadata/class-internals.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index 3a15e146e0c0e4..32736a6a980b6b 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1356,7 +1356,7 @@ mono_class_get_property_info (MonoClass *klass); void mono_class_set_property_info (MonoClass *klass, MonoClassPropertyInfo *info); -MonoClassEventInfo* +MONO_COMPONENT_API MonoClassEventInfo* mono_class_get_event_info (MonoClass *klass); void From 0a849ac981909fcab23ff57a0d0cf5f44492953d Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 21 Nov 2022 11:22:13 -0500 Subject: [PATCH 55/55] remove unused ifdefs and fix whitespace --- src/mono/mono/component/hot_reload.c | 11 ----------- src/mono/mono/metadata/custom-attrs.c | 18 +++++++++--------- .../DebuggerTestSuite/HotReloadTests.cs | 4 ---- 3 files changed, 9 insertions(+), 24 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 3bd349fd701ac0..120d227d7ccace 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -34,8 +34,6 @@ #include -#define ALLOW_INSTANCE_FIELD_ADD - typedef struct _BaselineInfo BaselineInfo; typedef struct _DeltaInfo DeltaInfo; @@ -2236,15 +2234,6 @@ apply_enclog_pass2 (Pass2Context *ctx, MonoImage *image_base, BaselineInfo *base uint32_t mapped_token = hot_reload_relative_delta_index (image_dmeta, delta_info, log_token); uint32_t field_flags = mono_metadata_decode_row_col (&image_dmeta->tables [MONO_TABLE_FIELD], mapped_token - 1, MONO_FIELD_FLAGS); -#ifndef ALLOW_INSTANCE_FIELD_ADD - if ((field_flags & FIELD_ATTRIBUTE_STATIC) == 0) { - /* TODO: implement instance (and literal?) fields */ - mono_trace (G_LOG_LEVEL_WARNING, MONO_TRACE_METADATA_UPDATE, "Adding non-static fields isn't implemented yet (token 0x%08x, class %s.%s)", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); - mono_error_set_not_implemented (error, "Adding non-static fields isn't implemented yet (token 0x%08x, class %s.%s)", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); - return FALSE; - } -#endif - add_field_to_baseline (base_info, delta_info, add_member_klass, log_token); /* This actually does slightly more than diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index c65bab8b55dedf..a0c4d5b6e380e3 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -169,8 +169,8 @@ find_field_index (MonoClass *klass, MonoClassField *field) { static guint32 find_property_index (MonoClass *klass, MonoProperty *property) { - if (G_UNLIKELY (m_property_is_from_update (property))) - return mono_metadata_update_get_property_idx (property); + if (G_UNLIKELY (m_property_is_from_update (property))) + return mono_metadata_update_get_property_idx (property); MonoClassPropertyInfo *info = mono_class_get_property_info (klass); for (guint32 i = 0; i < info->count; ++i) { @@ -186,8 +186,8 @@ find_property_index (MonoClass *klass, MonoProperty *property) static guint32 find_event_index (MonoClass *klass, MonoEvent *event) { - if (G_UNLIKELY (m_event_is_from_update (event))) - return mono_metadata_update_get_event_idx (event); + if (G_UNLIKELY (m_event_is_from_update (event))) + return mono_metadata_update_get_event_idx (event); MonoClassEventInfo *info = mono_class_get_event_info (klass); for (guint32 i = 0; i < info->count; ++i) { @@ -364,7 +364,7 @@ load_cattr_value (MonoImage *image, MonoType *t, MonoObject **out_obj, const cha type = mono_class_enum_basetype_internal (t->data.klass)->type; goto handle_enum; } else { - MonoClass *k = t->data.klass; + MonoClass *k = t->data.klass; if (mono_is_corlib_image (m_class_get_image (k)) && strcmp (m_class_get_name_space (k), "System") == 0 && strcmp (m_class_get_name (k), "DateTime") == 0){ guint64 *val = (guint64 *)g_malloc (sizeof (guint64)); @@ -704,7 +704,7 @@ load_cattr_value_noalloc (MonoImage *image, MonoType *t, const char *p, const ch type = mono_class_enum_basetype_internal (t->data.klass)->type; goto handle_enum; } else { - MonoClass *k = t->data.klass; + MonoClass *k = t->data.klass; if (mono_is_corlib_image (m_class_get_image (k)) && strcmp (m_class_get_name_space (k), "System") == 0 && strcmp (m_class_get_name (k), "DateTime") == 0){ guint64 *val = (guint64 *)g_malloc (sizeof (guint64)); @@ -2589,7 +2589,7 @@ mono_reflection_get_custom_attrs_data_checked (MonoObjectHandle obj, MonoError * if (!cinfo->cached) mono_custom_attrs_free (cinfo); goto_if_nok (error, leave); - } else { + } else { MonoClass *cattr_data = try_get_cattr_data_class (error); goto_if_nok (error, return_null); @@ -2894,8 +2894,8 @@ init_weak_fields_inner (MonoImage *image, GHashTable *indexes) } } else { /* FIXME: metadata-update */ - if (G_UNLIKELY (image->has_updates)) - g_warning ("[WeakAttribute] ignored on fields added by hot reload in '%s'", image->name); + if (G_UNLIKELY (image->has_updates)) + g_warning ("[WeakAttribute] ignored on fields added by hot reload in '%s'", image->name); /* Memberref pointing to a typeref */ tdef = &image->tables [MONO_TABLE_MEMBERREF]; diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs index c1740d6f69e3d9..c463cc3baebb43 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/HotReloadTests.cs @@ -577,7 +577,6 @@ public async Task DebugHotReload_NewInstanceFields() await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, locals_fn: async (locals) => { await CheckObject(locals, "c", "ApplyUpdateReferencedAssembly.AddInstanceFields.C"); -#if true var c = await GetObjectOnLocals(locals, "c"); await CheckProps (c, new { Field1 = TNumber(123), @@ -589,14 +588,11 @@ await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", scrip await CheckProps (c, new { Field1 = TNumber("456.5", isDecimal: true), }, "c", num_fields: 1); -#endif }); await SendCommandAndCheck (JObject.FromObject(new { }), "Debugger.resume", script_loc: null, line: -1, column: -1, function_name: null, locals_fn: async (locals) => { - _testOutput.WriteLine ("aleksey cprops1 ---- {0}", locals); await CheckObject(locals, "c", "ApplyUpdateReferencedAssembly.AddInstanceFields.C"); var c = await GetObjectOnLocals(locals, "c"); - _testOutput.WriteLine ("aleksey cprops1 ---- {0}", c); await CheckProps (c, new { Field1 = TNumber(123), Field2 = TString("spqr"),