From 4c564c308fa4333cbd8655e0389f4d3cfd5512ea Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 29 Nov 2021 16:50:20 -0500 Subject: [PATCH 01/45] Add AddStaticField test; cleanup AddLambdaCapturingThis --- .../AddLambdaCapturingThis_v1.cs | 10 +++----- .../AddStaticField.cs | 22 ++++++++++++++++ .../AddStaticField_v1.cs | 25 +++++++++++++++++++ ...ata.ApplyUpdate.Test.AddStaticField.csproj | 11 ++++++++ .../deltascript.json | 6 +++++ .../tests/ApplyUpdateTest.cs | 23 ++++++++++++++++- .../tests/System.Runtime.Loader.Tests.csproj | 1 + 7 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField_v1.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField.csproj create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/deltascript.json diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis_v1.cs index 22dd869021dc3..42642b6a2479d 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis_v1.cs @@ -17,13 +17,9 @@ public AddLambdaCapturingThis () { public string TestMethod () { // capture 'this' but no locals - Func fn = s => NewMethod (s + field, 42); - return fn ("123"); + Func fn = s => field; + Func fn2 = s => "42" + s + field; + return fn2 ("123"); } - - private string NewMethod (string s, int i) { - return i.ToString() + s; - } - } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField.cs new file mode 100644 index 0000000000000..45c03cbb9f427 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField.cs @@ -0,0 +1,22 @@ +// 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 AddStaticField + { + public AddStaticField () { + } + + public string GetField => s_field; + + private static string s_field; + + public void TestMethod () { + s_field = "abcd"; + } + + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField_v1.cs new file mode 100644 index 0000000000000..1491b581a266b --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField_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; + + +namespace System.Reflection.Metadata.ApplyUpdate.Test +{ + public class AddStaticField + { + public AddStaticField () { + } + + public string GetField => s_field2; + + private static string s_field; + + private static string s_field2; + + public void TestMethod () { + s_field = "spqr"; + s_field2 = "4567"; + } + + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField.csproj new file mode 100644 index 0000000000000..0a3f60146cac3 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField.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.AddStaticField/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/deltascript.json new file mode 100644 index 0000000000000..3c0d7a0fc6866 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/deltascript.json @@ -0,0 +1,6 @@ +{ + "changes": [ + {"document": "AddStaticField.cs", "update": "AddStaticField_v1.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index dd5c49c396335..2ea327bbe1bba 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -263,7 +263,6 @@ public void AsyncMethodChanges() }); } - [ActiveIssue ("https://github.com/dotnet/runtime/issues/50249", TestRuntimes.Mono)] [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddLambdaCapturingThis() { @@ -282,6 +281,28 @@ public static void TestAddLambdaCapturingThis() }); } + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestAddStaticField() + { + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField).Assembly; + + var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField(); + + x.TestMethod(); + + Assert.Equal ("abcd", x.GetField); + + ApplyUpdateUtil.ApplyUpdate(assm); + + x.TestMethod(); + + string result = x.GetField; + Assert.Equal("4567", result); + }); + } + class NonRuntimeAssembly : Assembly { 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 d678320e01740..110cedabbbab9 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 @@ -53,6 +53,7 @@ + From 6a329233c4c175a8a915aa07734498af6f856028 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 16 Dec 2021 16:21:51 -0500 Subject: [PATCH 02/45] Add AddNestedClass testcase --- .../AddNestedClass.cs | 20 +++++++++++++ .../AddNestedClass_v1.cs | 29 +++++++++++++++++++ ...ata.ApplyUpdate.Test.AddNestedClass.csproj | 11 +++++++ .../deltascript.json | 6 ++++ .../tests/ApplyUpdateTest.cs | 21 ++++++++++++++ .../tests/System.Runtime.Loader.Tests.csproj | 1 + 6 files changed, 88 insertions(+) create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass.csproj create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/deltascript.json diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass.cs new file mode 100644 index 0000000000000..d5bd6ce27fd0e --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass.cs @@ -0,0 +1,20 @@ +// 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 AddNestedClass + { + public AddNestedClass() + { + } + + public string TestMethod() + { + return "123"; + } + + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs new file mode 100644 index 0000000000000..04fc6992631d9 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/AddNestedClass_v1.cs @@ -0,0 +1,29 @@ +// 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 AddNestedClass + { + public AddNestedClass() + { + } + + public string TestMethod() + { + var n = new Nested(); + n.f = "123"; + return n.M(); + } + + private class Nested { + public Nested() { } + internal string f; + public string M () { + return f + "456"; + } + } + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass.csproj new file mode 100644 index 0000000000000..26f3348462094 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass.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.AddNestedClass/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/deltascript.json new file mode 100644 index 0000000000000..e70a3b3bbafbc --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass/deltascript.json @@ -0,0 +1,6 @@ +{ + "changes": [ + {"document": "AddNestedClass.cs", "update": "AddNestedClass_v1.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 2ea327bbe1bba..a978c23fef3b5 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -303,6 +303,27 @@ public static void TestAddStaticField() }); } + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestAddNestedClass() + { + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass).Assembly; + + var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass(); + + var r = x.TestMethod(); + + Assert.Equal ("123", r); + + ApplyUpdateUtil.ApplyUpdate(assm); + + r = x.TestMethod(); + + Assert.Equal("123456", r); + }); + } + class NonRuntimeAssembly : Assembly { 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 110cedabbbab9..cc0abc2ab0f11 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 @@ -54,6 +54,7 @@ + From c8cd6eb8a08907b062a289d790932246c128e221 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 29 Nov 2021 16:11:09 -0500 Subject: [PATCH 03/45] hot_reload: add a comment about remaining work --- src/mono/mono/component/hot_reload.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index bd8c3db635c88..c52c652b8791a 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1856,6 +1856,20 @@ hot_reload_apply_changes (int origin, MonoImage *image_base, gconstpointer dmeta } mono_error_assert_ok (error); + /* TODO: Need to do the second half of EditAndContinueModule::ApplyEditAndContinue - that + * is, actually inform the execution engine about new methods and fields. In particular + * EditAndContinueModule::AddMethod (and EEClass::AddMethod) to store the MonoMethod on the + * class, and EditAndContinueModule::AddField (and EEClass::AddField) to store the new + * MonoClassField on the class. Also we will need the equivalent of + * EnCFieldDesc::GetAddress in the interpreter to get the field address on a MonoObject + * instance. + * + * Maybe a simpler design (than stashing a dependent handle in the sync block like CoreCLR + * do) is to CWT from the original object to an enc dictionary that will map added fields + * (key: token?) to values, and make transform.c change ldfld/stfld/ldsflda into an icall call to + * get the CWT target and lookup the token in there. + */ + MonoAssemblyLoadContext *alc = mono_image_get_alc (image_base); hot_reload_update_publish (alc, generation); From 6ec639adc4af4972bcb39930fb5c9541104f35cd Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 30 Nov 2021 12:48:03 -0500 Subject: [PATCH 04/45] move per-class metadata update info to MonoClassMetadataUpdateInfo Use a property on the class instead of a hash table in the baseline image to keep track of added fields and methods --- src/mono/mono/component/CMakeLists.txt | 1 + .../mono/component/hot_reload-internals.h | 22 +++++++ src/mono/mono/component/hot_reload.c | 41 ++++++++---- src/mono/mono/metadata/class-accessors.c | 63 ++++++++++++++++++- src/mono/mono/metadata/class-internals.h | 12 ++++ 5 files changed, 127 insertions(+), 12 deletions(-) create mode 100644 src/mono/mono/component/hot_reload-internals.h diff --git a/src/mono/mono/component/CMakeLists.txt b/src/mono/mono/component/CMakeLists.txt index 595f6d889442b..faab69a9b68e9 100644 --- a/src/mono/mono/component/CMakeLists.txt +++ b/src/mono/mono/component/CMakeLists.txt @@ -41,6 +41,7 @@ list(APPEND components ) set(${MONO_HOT_RELOAD_COMPONENT_NAME}-sources ${MONO_COMPONENT_PATH}/hot_reload.c + ${MONO_COMPONENT_PATH}/hot_reload-internals.h ${MONO_COMPONENT_PATH}/hot_reload.h ) set(${MONO_HOT_RELOAD_COMPONENT_NAME}-stub-sources diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h new file mode 100644 index 0000000000000..7e86b64d6685f --- /dev/null +++ b/src/mono/mono/component/hot_reload-internals.h @@ -0,0 +1,22 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// + +#ifndef _MONO_COMPONENT_HOT_RELOAD_INTERNALS_H +#define _MONO_COMPONENT_HOT_RELOAD_INTERNALS_H + +#include +#include "mono/metadata/object-forward.h" +#include "mono/metadata/metadata-internals.h" + +/* Class-specific metadata update info. See + * mono_class_get_metadata_update_info() Note that this info is associated with + * class _definitions_ that can be edited, so primitives, generic instances, + * arrays, pointers, etc do not have this info. + */ +struct _MonoClassMetadataUpdateInfo { + GArray *added_methods; /* a set of Method table tokens of any methods added to this class */ + GArray *added_fields; /* a set of Field table tokens of any methods added to this class */ +}; + +#endif/*_MONO_COMPONENT_HOT_RELOAD_INTERNALS_H*/ diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index c52c652b8791a..267f6aa0d0999 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -8,6 +8,8 @@ #include #include "mono/utils/mono-compiler.h" +#include "mono/component/hot_reload-internals.h" + #include #include "mono/metadata/assembly-internals.h" #include "mono/metadata/metadata-internals.h" @@ -28,7 +30,8 @@ #include -#undef ALLOW_METHOD_ADD +#define ALLOW_METHOD_ADD +#undef ALLOW_FIELD_ADD typedef struct _DeltaInfo DeltaInfo; @@ -202,8 +205,9 @@ typedef struct _BaselineInfo { /* TRUE if any published update modified an existing row */ gboolean any_modified_rows [MONO_TABLE_NUM]; - GHashTable *added_methods; /* maps each MonoClass to a GArray of added method tokens */ + /* Parents for added methods, fields, etc */ GHashTable *method_parent; /* maps added methoddef tokens to typedef tokens */ + GHashTable *field_parent; /* maps added fielddef tokens to typedef tokens */ } BaselineInfo; @@ -660,6 +664,24 @@ hot_reload_update_cancel (uint32_t generation) publish_unlock (); } +MonoClassMetadataUpdateInfo * +mono_class_get_or_add_metadata_update_info (MonoClass *klass) +{ + MonoClassMetadataUpdateInfo *info = NULL; + info = mono_class_get_metadata_update_info (klass); + if (info) + return info; + mono_loader_lock (); + info = mono_class_get_metadata_update_info (klass); + if (!info) { + info = mono_class_alloc0 (klass, sizeof (MonoClassMetadataUpdateInfo)); + mono_class_set_metadata_update_info (klass, info); + } + mono_loader_unlock (); + g_assert (info); + return info; +} + /* * Given a baseline and an (optional) previous delta, allocate space for new tables for the current delta. * @@ -2055,17 +2077,15 @@ hot_reload_table_num_rows_slow (MonoImage *base, int table_index) static void add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t method_token, MonoDebugInformationEnc* pdb_address) { - if (!base_info->added_methods) { - base_info->added_methods = g_hash_table_new (g_direct_hash, g_direct_equal); - } if (!base_info->method_parent) { base_info->method_parent = g_hash_table_new (g_direct_hash, g_direct_equal); } + MonoClassMetadataUpdateInfo *klass_info = mono_class_get_or_add_metadata_update_info (klass); /* FIXME: locking for readers/writers of the GArray */ - GArray *arr = g_hash_table_lookup (base_info->added_methods, klass); + GArray *arr = klass_info->added_methods; if (!arr) { arr = g_array_new (FALSE, FALSE, sizeof(uint32_t)); - g_hash_table_insert (base_info->added_methods, klass, arr); + klass_info->added_methods = arr; } g_array_append_val (arr, method_token); g_hash_table_insert (base_info->method_parent, GUINT_TO_POINTER (method_token), GUINT_TO_POINTER (m_class_get_type_token (klass))); @@ -2081,11 +2101,10 @@ hot_reload_get_added_methods (MonoClass *klass) MonoImage *image = m_class_get_image (klass); if (!image->has_updates) return NULL; - BaselineInfo *base_info = baseline_info_lookup (image); - if (!base_info || base_info->added_methods == NULL) + MonoClassMetadataUpdateInfo *klass_info = mono_class_get_metadata_update_info (klass); + if (!klass_info) return NULL; - - return g_hash_table_lookup (base_info->added_methods, klass); + return klass_info->added_methods; } static uint32_t diff --git a/src/mono/mono/metadata/class-accessors.c b/src/mono/mono/metadata/class-accessors.c index 76065cb45c456..d51b57a811d60 100644 --- a/src/mono/mono/metadata/class-accessors.c +++ b/src/mono/mono/metadata/class-accessors.c @@ -28,7 +28,8 @@ typedef enum { PROP_DIM_CONFLICTS = 10, /* GSList of MonoMethod* */ PROP_FIELD_DEF_VALUES_2BYTESWIZZLE = 11, /* MonoFieldDefaultValue* with default values swizzled at 2 byte boundaries*/ PROP_FIELD_DEF_VALUES_4BYTESWIZZLE = 12, /* MonoFieldDefaultValue* with default values swizzled at 4 byte boundaries*/ - PROP_FIELD_DEF_VALUES_8BYTESWIZZLE = 13 /* MonoFieldDefaultValue* with default values swizzled at 8 byte boundaries*/ + PROP_FIELD_DEF_VALUES_8BYTESWIZZLE = 13, /* MonoFieldDefaultValue* with default values swizzled at 8 byte boundaries*/ + PROP_METADATA_UPDATE_INFO = 14, /* MonoClassMetadataUpdateInfo* */ } InfrequentDataKind; /* Accessors based on class kind*/ @@ -589,6 +590,66 @@ mono_class_publish_gc_descriptor (MonoClass *klass, MonoGCDescriptor gc_descr) return ret; } +MonoClassMetadataUpdateInfo* +mono_class_get_metadata_update_info (MonoClass *klass) +{ + switch (m_class_get_class_kind (klass)) { + case MONO_CLASS_GTD: + return NULL; + case MONO_CLASS_DEF: + return (MonoClassMetadataUpdateInfo *)get_pointer_property (klass, PROP_METADATA_UPDATE_INFO); + case MONO_CLASS_GINST: + case MONO_CLASS_GPARAM: + case MONO_CLASS_POINTER: + case MONO_CLASS_GC_FILLER: + return NULL; + default: + g_assert_not_reached (); + } +} + +/* + * LOCKING: assumes the loader lock is held + */ +void +mono_class_set_metadata_update_info (MonoClass *klass, MonoClassMetadataUpdateInfo *value) +{ + switch (m_class_get_class_kind (klass)) { + case MONO_CLASS_GTD: + g_assertf (0, "%s: EnC metadata update info on generic types is not supported", __func__); + break; + case MONO_CLASS_DEF: + return set_pointer_property (klass, PROP_METADATA_UPDATE_INFO, value); + case MONO_CLASS_GINST: + case MONO_CLASS_GPARAM: + case MONO_CLASS_POINTER: + case MONO_CLASS_GC_FILLER: + g_assert_not_reached (); + break; + default: + g_assert_not_reached (); + } +} + +gboolean +mono_class_has_metadata_update_info (MonoClass *klass) +{ + switch (m_class_get_class_kind (klass)) { + case MONO_CLASS_GTD: + return FALSE; + case MONO_CLASS_DEF: + return get_pointer_property (klass, PROP_METADATA_UPDATE_INFO) != NULL; + case MONO_CLASS_GINST: + case MONO_CLASS_GPARAM: + case MONO_CLASS_POINTER: + case MONO_CLASS_GC_FILLER: + return FALSE; + default: + g_assert_not_reached (); + } +} + + #ifdef MONO_CLASS_DEF_PRIVATE #define MONO_CLASS_GETTER(funcname, rettype, optref, argtype, fieldname) rettype funcname (argtype *klass) { return optref klass-> fieldname ; } #define MONO_CLASS_OFFSET(funcname, argtype, fieldname) intptr_t funcname (void) { return MONO_STRUCT_OFFSET (argtype, fieldname); } diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index 01104274c1f3b..693ff2cddbdfe 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1455,6 +1455,18 @@ mono_class_set_dim_conflicts (MonoClass *klass, GSList *conflicts); GSList* mono_class_get_dim_conflicts (MonoClass *klass); +/* opaque struct of class specific hot reload info */ +typedef struct _MonoClassMetadataUpdateInfo MonoClassMetadataUpdateInfo; + +MONO_COMPONENT_API gboolean +mono_class_has_metadata_update_info (MonoClass *klass); + +MONO_COMPONENT_API MonoClassMetadataUpdateInfo * +mono_class_get_metadata_update_info (MonoClass *klass); + +MONO_COMPONENT_API void +mono_class_set_metadata_update_info (MonoClass *klass, MonoClassMetadataUpdateInfo *value); + MONO_COMPONENT_API MonoMethod * mono_class_get_method_from_name_checked (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); From 560046b43901468cd2279235f5bf31bd627630c4 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 6 Dec 2021 13:33:46 -0500 Subject: [PATCH 05/45] wip --- src/mono/mono/component/hot_reload.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 267f6aa0d0999..76b363694409f 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1317,6 +1317,17 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de #endif /* handled above */ break; +#ifdef ALLOW_FIELD_ADD + case MONO_TABLE_FIELD: + if (func_code == ENC_FUNC_DEFAULT) + continue; /* ok, allowed */ +#else + /* adding or modifying a field */ + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support adding or modifying fields.", i, log_token); + mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding or modifying fields. token=0x%08x", log_token); + unsupported_edits = TRUE; + break; +#endif case MONO_TABLE_PROPERTY: { /* modifying a property, ok */ if (token_index <= table_info_get_rows (&image_base->tables [token_table])) @@ -1441,6 +1452,22 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de i++; /* skip the next record */ continue; } +#endif +#ifdef ALLOW_FIELD_ADD + if (!new_class && func_code == ENC_FUNC_ADD_FIELD) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x AddField to klass 0x%08x, skipping next EnClog record", i, log_token, token_index); + g_assert (i + 1 < rows); + guint32 next_cols [MONO_ENCLOG_SIZE]; + mono_metadata_decode_row (table_enclog, i + 1, next_cols, MONO_ENCLOG_SIZE); + g_assert (next_cols [MONO_ENCLOG_FUNC_CODE] == ENC_FUNC_DEFAULT); + int next_token = next_cols [MONO_ENCLOG_TOKEN]; + int next_table = mono_metadata_token_table (next_token); + int next_index = mono_metadata_token_index (next_token); + g_assert (next_table == MONO_TABLE_FIELD); + g_assert (next_index > table_info_get_rows (&image_base->tables [next_table])); + i++; /* skip the next record */ + continue; + } #endif /* fallthru */ } From 9f5d73bfe095a9b6ad72c03f0a61c58feca5856a Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 17 Dec 2021 16:57:31 -0500 Subject: [PATCH 06/45] Cleanup pass1 to check add/update against prior gen Instead of the baseline image --- src/mono/mono/component/hot_reload.c | 55 +++++++++++++++++----------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 76b363694409f..8080feffb7830 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -30,8 +30,9 @@ #include +#define ALLOW_CLASS_ADD #define ALLOW_METHOD_ADD -#undef ALLOW_FIELD_ADD +#define ALLOW_FIELD_ADD typedef struct _DeltaInfo DeltaInfo; @@ -1270,7 +1271,10 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de int token_table = mono_metadata_token_table (log_token); int token_index = mono_metadata_token_index (log_token); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x (%s idx=0x%02x) (base table has 0x%04x rows)\tfunc=0x%02x (\"%s\")\n", i, log_token, mono_meta_table_name (token_table), token_index, table_info_get_rows (&image_base->tables [token_table]), func_code, funccode_to_str (func_code)); + gboolean is_addition = token_index-1 >= delta_info->count[token_table].prev_gen_rows ; + + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x (%s idx=0x%02x) (base table has 0x%04x rows; prev gen had 0x%04x rows)\t%s\tfunc=0x%02x (\"%s\")\n", i, log_token, mono_meta_table_name (token_table), token_index, table_info_get_rows (&image_base->tables [token_table]), delta_info->count[token_table].prev_gen_rows, (is_addition ? "ADD" : "UPDATE"), func_code, funccode_to_str (func_code)); if (token_table != MONO_TABLE_METHOD) @@ -1278,7 +1282,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de #ifndef ALLOW_METHOD_ADD - if (token_index > table_info_get_rows (&image_base->tables [token_table])) { + if (is_addition) { mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "\tcannot add new method with token 0x%08x", log_token); mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: cannot add new method with token 0x%08x", log_token); unsupported_edits = TRUE; @@ -1288,8 +1292,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de #ifdef ALLOW_METHOD_ADD /* adding a new parameter to a new method is ok */ - /* FIXME: total rows, not just the baseline rows */ - if (func_code == ENC_FUNC_ADD_PARAM && token_index > table_info_get_rows (&image_base->tables [token_table])) + if (func_code == ENC_FUNC_ADD_PARAM && is_addition) continue; #endif @@ -1306,6 +1309,8 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de int token_table = mono_metadata_token_table (log_token); int token_index = mono_metadata_token_index (log_token); + gboolean is_addition = token_index-1 >= delta_info->count[token_table].prev_gen_rows ; + switch (token_table) { case MONO_TABLE_ASSEMBLYREF: /* okay, supported */ @@ -1317,8 +1322,8 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de #endif /* handled above */ break; -#ifdef ALLOW_FIELD_ADD case MONO_TABLE_FIELD: +#ifdef ALLOW_FIELD_ADD if (func_code == ENC_FUNC_DEFAULT) continue; /* ok, allowed */ #else @@ -1330,7 +1335,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de #endif case MONO_TABLE_PROPERTY: { /* modifying a property, ok */ - if (token_index <= table_info_get_rows (&image_base->tables [token_table])) + if (!is_addition) break; /* adding a property */ mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support adding new properties.", i, log_token); @@ -1339,8 +1344,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de continue; } case MONO_TABLE_METHODSEMANTICS: { - /* FIXME: this should get the current table size, not the base stable size */ - if (token_index > table_info_get_rows (&image_base->tables [token_table])) { + if (is_addition) { /* new rows are fine, as long as they point at existing methods */ guint32 sema_cols [MONO_METHOD_SEMA_SIZE]; int mapped_token = hot_reload_relative_delta_index (image_dmeta, delta_info, mono_metadata_make_token (token_table, token_index)); @@ -1352,7 +1356,8 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de case METHOD_SEMANTIC_SETTER: { int prop_method_index = sema_cols [MONO_METHOD_SEMA_METHOD]; /* ok, if it's pointing to an existing getter/setter */ - if (prop_method_index < table_info_get_rows (&image_base->tables [MONO_TABLE_METHOD])) + gboolean is_prop_method_add = prop_method_index-1 >= delta_info->count[MONO_TABLE_METHOD].prev_gen_rows; + if (!is_prop_method_add) break; mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x adding new getter/setter method 0x%08x to a property is not supported", i, log_token, prop_method_index); mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support adding a new getter or setter to a property, token=0x%08x", log_token); @@ -1373,8 +1378,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de } } case MONO_TABLE_CUSTOMATTRIBUTE: { - /* FIXME: this should get the current table size, not the base stable size */ - if (token_index <= table_info_get_rows (&image_base->tables [token_table])) { + if (!is_addition) { /* modifying existing rows is ok, as long as the parent and ctor are the same */ guint32 ca_upd_cols [MONO_CUSTOM_ATTR_SIZE]; guint32 ca_base_cols [MONO_CUSTOM_ATTR_SIZE]; @@ -1408,8 +1412,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de } } case MONO_TABLE_PARAM: { - /* FIXME: this should get the current table size, not the base stable size */ - if (token_index <= table_info_get_rows (&image_base->tables [token_table])) { + if (!is_addition) { /* We only allow modifications where the parameter name doesn't change. */ uint32_t base_param [MONO_PARAM_SIZE]; uint32_t upd_param [MONO_PARAM_SIZE]; @@ -1434,11 +1437,14 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de break; /* added a row. ok */ } case MONO_TABLE_TYPEDEF: { + gboolean new_class = is_addition; #ifdef ALLOW_METHOD_ADD - /* FIXME: wrong for cumulative updates - need to look at DeltaInfo:count.prev_gen_rows */ - gboolean new_class = token_index > table_info_get_rows (&image_base->tables [token_table]); /* only allow adding methods to existing classes for now */ - if (!new_class && func_code == ENC_FUNC_ADD_METHOD) { + if ( +#ifndef ALLOW_CLASS_ADD + !new_class && +#endif + func_code == ENC_FUNC_ADD_METHOD) { /* next record should be a MONO_TABLE_METHOD addition (func == default) */ g_assert (i + 1 < rows); guint32 next_cols [MONO_ENCLOG_SIZE]; @@ -1448,13 +1454,18 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de int next_table = mono_metadata_token_table (next_token); int next_index = mono_metadata_token_index (next_token); g_assert (next_table == MONO_TABLE_METHOD); - g_assert (next_index > table_info_get_rows (&image_base->tables [next_table])); + /* expecting an added method */ + g_assert (next_index-1 >= delta_info->count[next_table].prev_gen_rows); i++; /* skip the next record */ continue; } #endif #ifdef ALLOW_FIELD_ADD - if (!new_class && func_code == ENC_FUNC_ADD_FIELD) { + if ( +#ifndef ALLOW_CLASS_ADD + !new_class && +#endif + func_code == ENC_FUNC_ADD_FIELD) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x AddField to klass 0x%08x, skipping next EnClog record", i, log_token, token_index); g_assert (i + 1 < rows); guint32 next_cols [MONO_ENCLOG_SIZE]; @@ -1464,7 +1475,8 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de int next_table = mono_metadata_token_table (next_token); int next_index = mono_metadata_token_index (next_token); g_assert (next_table == MONO_TABLE_FIELD); - g_assert (next_index > table_info_get_rows (&image_base->tables [next_table])); + /* expecting an added field */ + g_assert (next_index-1 >= delta_info->count[next_table].prev_gen_rows); i++; /* skip the next record */ continue; } @@ -1472,8 +1484,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de /* fallthru */ } default: - /* FIXME: this bounds check is wrong for cumulative updates - need to look at the DeltaInfo:count.prev_gen_rows */ - if (token_index <= table_info_get_rows (&image_base->tables [token_table])) { + if (!is_addition) { mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing table cols.", i, log_token); mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing table cols. token=0x%08x", log_token); unsupported_edits = TRUE; From 5c8908e31ed4a571b5eb393950720746131f1da9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 20 Dec 2021 12:41:14 -0500 Subject: [PATCH 07/45] XXX no merge - mess with sample --- .../tests/ApplyUpdateTest.cs | 2 + src/mono/sample/mbr/console/Makefile | 7 +- src/mono/sample/mbr/console/Program.cs | 87 ++----------------- src/mono/sample/mbr/console/TestClass.cs | 1 - src/mono/sample/mbr/console/TestClass_v1.cs | 9 +- 5 files changed, 21 insertions(+), 85 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index a978c23fef3b5..7e116f6b2fa2a 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -17,6 +17,7 @@ namespace System.Reflection.Metadata [Collection(nameof(DisableParallelization))] public class ApplyUpdateTest { +#if false [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] void StaticMethodBodyUpdate() @@ -302,6 +303,7 @@ public static void TestAddStaticField() Assert.Equal("4567", result); }); } +#endif [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddNestedClass() diff --git a/src/mono/sample/mbr/console/Makefile b/src/mono/sample/mbr/console/Makefile index ee47d81f09b5c..cf4d46e810e2f 100644 --- a/src/mono/sample/mbr/console/Makefile +++ b/src/mono/sample/mbr/console/Makefile @@ -5,7 +5,7 @@ DOTNET_Q_ARGS=--nologo -v:q -consoleloggerparameters:NoSummary # How to build the project. For hot reload this must be Debug CONFIG ?=Debug # How was dotnet/runtime built? should be the same as build.sh -c option -BUILT_RUNTIME_CONFIG ?= Release +BUILT_RUNTIME_CONFIG ?= Debug MONO_ARCH=x64 OS := $(shell uname -s) @@ -17,12 +17,17 @@ endif MONO_ENV_OPTIONS = --interp +ifneq ($V,0) +MONO_VERBOSE_LOGGING=MONO_LOG_LEVEL=debug MONO_LOG_MASK=metadata-update +endif + publish: $(DOTNET) publish -c $(CONFIG) -r $(TARGET_OS)-$(MONO_ARCH) -p:BuiltRuntimeConfiguration=$(BUILT_RUNTIME_CONFIG) run: publish COMPlus_DebugWriteToStdErr=1 \ MONO_ENV_OPTIONS="$(MONO_ENV_OPTIONS)" \ + $(MONO_VERBOSE_LOGGING) \ DOTNET_MODIFIABLE_ASSEMBLIES=debug \ $(TOP)artifacts/bin/ConsoleDelta/$(MONO_ARCH)/$(CONFIG)/$(TARGET_OS)-$(MONO_ARCH)/publish/ConsoleDelta diff --git a/src/mono/sample/mbr/console/Program.cs b/src/mono/sample/mbr/console/Program.cs index 01fba0d8e4ee1..f92dbd5e1550a 100644 --- a/src/mono/sample/mbr/console/Program.cs +++ b/src/mono/sample/mbr/console/Program.cs @@ -11,43 +11,6 @@ namespace HelloWorld { internal class Program { - class State { - public readonly ManualResetEventSlim mreIn; - public readonly ManualResetEventSlim mreOut; - public readonly ManualResetEventSlim mreBusy; - public string res; - private volatile bool busyChanged; - - public State() { - mreIn = new ManualResetEventSlim (); - mreOut = new ManualResetEventSlim (); - mreBusy = new ManualResetEventSlim (); - res = ""; - busyChanged = false; - } - - - public bool BusyChanged {get => busyChanged ; set { busyChanged = value; mreBusy.Set ();} } - - public void WaitForBusy () { - mreBusy.Wait (); - mreBusy.Reset (); - } - - public string ConsumerStep () { - mreIn.Set (); - mreOut.Wait (); - mreOut.Reset (); - return res; - } - - public void ProducerStep (Func step) { - mreIn.Wait (); - mreIn.Reset (); - res = step (); - mreOut.Set (); - } - } private static int Main(string[] args) { @@ -60,56 +23,20 @@ private static int Main(string[] args) Assembly assm = typeof (TestClass).Assembly; var replacer = DeltaHelper.Make (); - var st = new State (); - var t = new Thread (MutatorThread); - t.Start (st); - var t2 = new Thread (BusyThread) { IsBackground = true }; - t2.Start (st); + var s = TestClass.TargetMethod (); - string res = st.ConsumerStep (); - if (res != "OLD STRING") - return 1; + Console.WriteLine (s); replacer.Update (assm); - res = st.ConsumerStep (); - if (res != "NEW STRING") - return 2; - - st.WaitForBusy (); - Console.WriteLine ("BusyChanged: {0}", st.BusyChanged); + s = TestClass.TargetMethod (); - return 0; - } + Console.WriteLine (s); - private static void MutatorThread (object o) - { - var st = (State)o; - static string Step () => TestClass.TargetMethod (); - st.ProducerStep (Step); - st.ProducerStep (Step); - } + if (s != "NEW STRING") + return 2; - // This method is not affected by the update, but it calls the target - // method which is. Still we expect to see "BusyThread" and its - // callees show up in the trace output when it safepoints during an - // update. - private static void BusyThread (object o) - { - State st = (State)o; - string prev = TestClass.TargetMethod (); - while (true) { - Thread.Sleep (0); - for (int i = 0; i < 5000; ++i) { - if (i % 1000 == 0) { - string cur = TestClass.TargetMethod (); - if (cur != prev) { - st.BusyChanged = true; - prev = cur; - } - } - } - } + return 0; } } diff --git a/src/mono/sample/mbr/console/TestClass.cs b/src/mono/sample/mbr/console/TestClass.cs index c18db33b47257..7884de75ec4fe 100644 --- a/src/mono/sample/mbr/console/TestClass.cs +++ b/src/mono/sample/mbr/console/TestClass.cs @@ -5,7 +5,6 @@ public class TestClass { [MethodImpl(MethodImplOptions.NoInlining)] public static string TargetMethod () { string s = "OLD STRING"; - Console.WriteLine (s); return s; } } diff --git a/src/mono/sample/mbr/console/TestClass_v1.cs b/src/mono/sample/mbr/console/TestClass_v1.cs index a41793c3f7dcc..5c552f3e3409f 100644 --- a/src/mono/sample/mbr/console/TestClass_v1.cs +++ b/src/mono/sample/mbr/console/TestClass_v1.cs @@ -4,8 +4,11 @@ public class TestClass { [MethodImpl(MethodImplOptions.NoInlining)] public static string TargetMethod () { - string s = "NEW STRING"; - Console.WriteLine (s); - return s; + var o = new Inner(); + return o.GetIt(); } + + public class Inner { + public string GetIt() => "NEW STRING"; + } } From 520a92c65783e71021ed171b0519829f4c84d674 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 20 Dec 2021 16:54:47 -0500 Subject: [PATCH 08/45] WIP: adding a class Working on making mono_metadata_nested_in_typedef do the right thing for nested classes Contributes to making mono_class_create_from_typedef do the right thing for nested classes. --- src/mono/mono/component/hot_reload-stub.c | 10 ++ src/mono/mono/component/hot_reload.c | 107 +++++++++++++++++++--- src/mono/mono/component/hot_reload.h | 2 + src/mono/mono/metadata/class-init.c | 2 +- src/mono/mono/metadata/metadata-update.c | 6 ++ src/mono/mono/metadata/metadata-update.h | 4 + src/mono/mono/metadata/metadata.c | 6 +- 7 files changed, 124 insertions(+), 13 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 6064a6a78f875..35d38106241e2 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -71,6 +71,9 @@ hot_reload_stub_get_added_methods (MonoClass *klass); static uint32_t hot_reload_stub_method_parent (MonoImage *image, uint32_t method_index); +static void* +hot_reload_stub_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -91,6 +94,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_table_num_rows_slow, &hot_reload_stub_get_added_methods, &hot_reload_stub_method_parent, + &hot_reload_stub_metadata_linear_search, }; static bool @@ -213,6 +217,12 @@ hot_reload_stub_method_parent (MonoImage *image, uint32_t method_index) } +static void* +hot_reload_stub_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer) +{ + 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 8080feffb7830..341c284a55d61 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -24,6 +24,7 @@ #include "mono/metadata/debug-internals.h" #include "mono/metadata/mono-debug.h" #include "mono/metadata/debug-mono-ppdb.h" +#include "mono/utils/bsearch.h" #include @@ -99,6 +100,9 @@ hot_reload_get_added_methods (MonoClass *klass); static uint32_t hot_reload_method_parent (MonoImage *base, uint32_t method_token); +static void* +hot_reload_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_available }, @@ -120,6 +124,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_table_num_rows_slow, &hot_reload_get_added_methods, &hot_reload_method_parent, + &hot_reload_metadata_linear_search, }; MonoComponentHotReload * @@ -665,7 +670,7 @@ hot_reload_update_cancel (uint32_t generation) publish_unlock (); } -MonoClassMetadataUpdateInfo * +static MonoClassMetadataUpdateInfo * mono_class_get_or_add_metadata_update_info (MonoClass *klass) { MonoClassMetadataUpdateInfo *info = NULL; @@ -900,8 +905,13 @@ dump_update_summary (MonoImage *image_base, MonoImage *image_dmeta) } +/* + * Finds the latest mutated version of the table given by tbl_index + * + * On success returns TRUE, modifies *t and optionally updates *delta_out + */ static gboolean -effective_table_mutant (MonoImage *base, BaselineInfo *info, int tbl_index, const MonoTableInfo **t, int idx) +effective_table_mutant (MonoImage *base, BaselineInfo *info, int tbl_index, const MonoTableInfo **t, DeltaInfo **delta_out) { GList *ptr =info->delta_info_last; uint32_t exposed_gen = hot_reload_get_thread_generation (); @@ -922,6 +932,8 @@ effective_table_mutant (MonoImage *base, BaselineInfo *info, int tbl_index, cons g_assert (delta_info != NULL); *t = &delta_info->mutants [tbl_index]; + if (delta_out) + *delta_out = delta_info; return TRUE; } @@ -940,7 +952,7 @@ hot_reload_effective_table_slow (const MonoTableInfo **t, int idx) if (!info) return; - gboolean success = effective_table_mutant (base, info, tbl_index, t, idx); + gboolean success = effective_table_mutant (base, info, tbl_index, t, NULL); g_assert (success); } @@ -1437,7 +1449,7 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *de break; /* added a row. ok */ } case MONO_TABLE_TYPEDEF: { - gboolean new_class = is_addition; + gboolean new_class G_GNUC_UNUSED = is_addition; #ifdef ALLOW_METHOD_ADD /* only allow adding methods to existing classes for now */ if ( @@ -1604,6 +1616,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen * have it in writable memory (and not mmap-ed pages), so we can rewrite the table values. */ + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Pass 2 begin: base '%s' delta image=%p", image_base->name, image_dmeta); #ifdef ALLOW_METHOD_ADD MonoClass *add_method_klass = NULL; @@ -1620,7 +1633,11 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen int token_table = mono_metadata_token_table (log_token); int token_index = mono_metadata_token_index (log_token); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "enclog i=%d: token=0x%08x (table=%s): %d", i, log_token, mono_meta_table_name (token_table), func_code); + + gboolean is_addition = token_index-1 >= delta_info->count[token_table].prev_gen_rows ; + + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "enclog i=%d: token=0x%08x (table=%s): %d:\t%s", i, log_token, mono_meta_table_name (token_table), func_code, (is_addition ? "ADD" : "UPDATE")); + /* TODO: See CMiniMdRW::ApplyDelta for how to drive this. */ @@ -1697,7 +1714,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen if (func_code == ENC_FUNC_ADD_PARAM) break; - if (token_index > table_info_get_rows (&image_base->tables [token_table])) { + if (is_addition) { if (!add_method_klass) g_error ("EnC: new method added but I don't know the class, should be caught by pass1"); g_assert (add_method_klass); @@ -1732,11 +1749,38 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen break; } case MONO_TABLE_TYPEDEF: { - /* TODO: throw? */ - /* TODO: happens changing the class (adding field or method). we ignore it, but dragons are here */ - - /* existing entries are supposed to be patched */ - g_assert (token_index <= table_info_get_rows (&image_base->tables [token_table])); +#ifdef ALLOW_CLASS_ADD + if (is_addition) { + /* Adding a new class. ok */ + switch (func_code) { + case ENC_FUNC_DEFAULT: + /* ok, added a new class */ + /* TODO: do things here */ + break; + case ENC_FUNC_ADD_METHOD: + case ENC_FUNC_ADD_FIELD: + /* ok, adding a new field or method to a new class */ + /* TODO: do we need to do anything special? Conceptually + * this is the same as modifying an existing class - + * especially since from the next generation's point of view + * that's what adding a field/method will be. */ + break; + case ENC_FUNC_ADD_PROPERTY: + case ENC_FUNC_ADD_EVENT: + g_assert_not_reached (); /* FIXME: implement me */ + default: + g_assert_not_reached (); /* unknown func_code */ + } + break; + } +#endif + /* modifying an existing class by adding a method or field, etc. */ + g_assert (!is_addition); +#if !defined(ALLOW_METHOD_ADD) && !defined(ALLOW_FIELD_ADD) + g_assert_not_reached (); +#else + g_assert (func_code != ENC_FUNC_DEFAULT); +#endif break; } case MONO_TABLE_PROPERTY: { @@ -2159,3 +2203,44 @@ hot_reload_method_parent (MonoImage *base_image, uint32_t method_token) return res; } + +/* XXX HACK - keep in sync with locator_t in metadata/metadata.c */ +typedef struct { + int idx; /* The index that we are trying to locate */ + int col_idx; /* The index in the row where idx may be stored */ + MonoTableInfo *t; /* pointer to the table */ + guint32 result; +} upd_locator_t; + +void* +hot_reload_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer) +{ + BaselineInfo *base_info = baseline_info_lookup (base_image); + g_assert (base_info); + + g_assert (base_image->tables < base_table && base_table < &base_image->tables [MONO_TABLE_LAST]); + + int tbl_index; + { + size_t s = ALIGN_TO (sizeof (MonoTableInfo), sizeof (gpointer)); + tbl_index = ((intptr_t) base_table - (intptr_t) base_image->tables) / s; + } + + DeltaInfo *delta_info = NULL; + const MonoTableInfo *latest_mod_table = base_table; + gboolean success = effective_table_mutant (base_image, base_info, tbl_index, &latest_mod_table, &delta_info); + g_assert (success); + uint32_t rows = table_info_get_rows (latest_mod_table); + + upd_locator_t *loc = (upd_locator_t*)key; + g_assert (loc); + loc->result = 0; + /* HACK: this is so that the locator can compute the row index of the given row. but passing the mutant table to other metadata functions could backfire. */ + loc->t = (MonoTableInfo*)latest_mod_table; + for (uint32_t idx = 0; idx < table_info_get_rows (latest_mod_table); ++idx) { + const char *row = latest_mod_table->base + idx * latest_mod_table->row_size; + if (!comparer (loc, row)) + return (void*)row; + } + return NULL; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 416ff56e70eab..27d25223a011a 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -9,6 +9,7 @@ #include "mono/metadata/object-forward.h" #include "mono/metadata/metadata-internals.h" #include "mono/metadata/metadata-update.h" +#include "mono/utils/bsearch.h" #include "mono/utils/mono-error.h" #include "mono/utils/mono-compiler.h" #include "mono/component/component.h" @@ -33,6 +34,7 @@ typedef struct _MonoComponentHotReload { gboolean (*table_num_rows_slow) (MonoImage *base_image, int table_index); GArray* (*get_added_methods) (MonoClass *klass); uint32_t (*method_parent) (MonoImage *base_image, uint32_t method_index); + void* (*metadata_linear_search) (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 6164476ba9a91..1083d535a70c1 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -434,7 +434,7 @@ mono_class_create_from_typedef (MonoImage *image, guint32 type_token, MonoError error_init (error); /* FIXME: metadata-update - this function needs extensive work */ - if (mono_metadata_token_table (type_token) != MONO_TABLE_TYPEDEF || tidx > table_info_get_rows (tt)) { + if (mono_metadata_token_table (type_token) != MONO_TABLE_TYPEDEF || mono_metadata_table_bounds_check (image, MONO_TABLE_TYPEDEF, tidx)) { mono_error_set_bad_image (error, image, "Invalid typedef token %x", type_token); return NULL; } diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 800ffb9f04727..5afcbc42e893f 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -147,3 +147,9 @@ mono_metadata_table_num_rows_slow (MonoImage *base_image, int table_index) { return mono_component_hot_reload()->table_num_rows_slow (base_image, table_index); } + +void* +mono_metadata_update_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer) +{ + return mono_component_hot_reload()->metadata_linear_search (base_image, base_table, key, comparer); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 15d0d51e10f4a..c3e0edba4134c 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -6,6 +6,7 @@ #define __MONO_METADATA_UPDATE_H__ #include "mono/utils/mono-forward.h" +#include "mono/utils/bsearch.h" #include "mono/metadata/loader-internals.h" #include "mono/metadata/metadata-internals.h" @@ -57,4 +58,7 @@ mono_metadata_update_table_bounds_check (MonoImage *base_image, int table_index, gboolean mono_metadata_update_delta_heap_lookup (MonoImage *base_image, MetadataHeapGetterFunc get_heap, uint32_t orig_index, MonoImage **image_out, uint32_t *index_out); +void* +mono_metadata_update_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); + #endif /*__MONO_METADATA_UPDATE_H__*/ diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 04a43f5e0076f..a7b50bc59113f 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -4983,9 +4983,13 @@ mono_metadata_nested_in_typedef (MonoImage *meta, guint32 index) /* FIXME: metadata-update */ - if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) + if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) && !meta->has_updates) return 0; + if (G_UNLIKELY (meta->has_updates)) + if (!mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) + return 0; + /* loc_result is 0..1, needs to be mapped to table index (that is +1) */ return mono_metadata_decode_row_col (tdef, loc.result, MONO_NESTED_CLASS_ENCLOSING) | MONO_TOKEN_TYPE_DEF; } From 8d63497e683ac9ac3124ef3e1ddd1191bc089e00 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 21 Dec 2021 15:24:30 -0500 Subject: [PATCH 09/45] fix: correct mutant table population when the baseline table is empty In that case the table row_size and size_bitmap of the baseline are both 0, so offset calculations don't work. In that case when we know we need to add rows to the mutant table, use the uncompressed layout (32-bit columns, always) same as the EnC delta. --- src/mono/mono/component/hot_reload.c | 34 +++++++++++++++++----------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 341c284a55d61..424a98430ff9f 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -723,12 +723,6 @@ delta_info_initialize_mutants (const MonoImage *base, const BaselineInfo *base_i guint32 rows = count->prev_gen_rows + count->inserted_rows; - MonoTableInfo *tbl = &delta->mutants [i]; - tbl->row_size = base->tables[i].row_size; - tbl->size_bitfield = base->tables[i].size_bitfield; - tbl->rows_ = rows; - - tbl->base = mono_mempool_alloc (delta->pool, tbl->row_size * rows); const MonoTableInfo *prev_table; if (!prev_delta || prev_delta->mutants [i].base == NULL) prev_table = &base->tables [i]; @@ -736,6 +730,21 @@ delta_info_initialize_mutants (const MonoImage *base, const BaselineInfo *base_i prev_table = &prev_delta->mutants [i]; g_assert (prev_table != NULL); + + MonoTableInfo *tbl = &delta->mutants [i]; + if (prev_table->rows_ == 0) { + /* table was empty in the baseline and it was empty in the prior generation, but now we have some rows. Use the format of the mutant table. */ + g_assert (prev_table->row_size == 0); + tbl->row_size = delta->delta_image->tables [i].row_size; + tbl->size_bitfield = delta->delta_image->tables [i].size_bitfield; + } else { + tbl->row_size = prev_table->row_size; + tbl->size_bitfield = prev_table->size_bitfield; + } + tbl->rows_ = rows; + g_assert (tbl->rows_ > 0 && tbl->row_size != 0); + + tbl->base = mono_mempool_alloc (delta->pool, tbl->row_size * rows); g_assert (table_info_get_rows (prev_table) == count->prev_gen_rows); /* copy the old rows and zero out the new ones */ @@ -1164,7 +1173,7 @@ funccode_to_str (int func_code) * */ static void -delta_info_mutate_row (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo *cur_delta, guint32 log_token) +delta_info_mutate_row (MonoImage *image_dmeta, DeltaInfo *cur_delta, guint32 log_token) { int token_table = mono_metadata_token_table (log_token); int token_index = mono_metadata_token_index (log_token); /* 1-based */ @@ -1174,16 +1183,16 @@ delta_info_mutate_row (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo int delta_index = hot_reload_relative_delta_index (image_dmeta, cur_delta, log_token); /* The complication here is that we want the mutant table to look like the table in - * image_base with respect to column widths, but the delta tables are generally coming in + * the baseline image with respect to column widths, but the delta tables are generally coming in * uncompressed (4-byte columns). So we have to copy one column at a time and adjust the * widths as we go. */ - guint32 dst_bitfield = image_base->tables [token_table].size_bitfield; + guint32 dst_bitfield = cur_delta->mutants [token_table].size_bitfield; guint32 src_bitfield = image_dmeta->tables [token_table].size_bitfield; const char *src_base = image_dmeta->tables [token_table].base + (delta_index - 1) * image_dmeta->tables [token_table].row_size; - char *dst_base = (char*)cur_delta->mutants [token_table].base + (token_index - 1) * image_base->tables [token_table].row_size; + char *dst_base = (char*)cur_delta->mutants [token_table].base + (token_index - 1) * cur_delta->mutants [token_table].row_size; guint32 src_offset = 0, dst_offset = 0; for (int col = 0; col < mono_metadata_table_count (dst_bitfield); ++col) { @@ -1225,12 +1234,11 @@ delta_info_mutate_row (MonoImage *image_base, MonoImage *image_dmeta, DeltaInfo default: g_assert_not_reached (); } - } src_offset += src_col_size; dst_offset += dst_col_size; } - g_assert (dst_offset == image_base->tables [token_table].row_size); + g_assert (dst_offset == cur_delta->mutants [token_table].row_size); mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "mutate: table=0x%02x row=0x%04x delta row=0x%04x %s", token_table, token_index, delta_index, modified ? "Mod" : "Add"); } @@ -1250,7 +1258,7 @@ prepare_mutated_rows (const MonoTableInfo *table_enclog, MonoImage *image_base, if (func_code != ENC_FUNC_DEFAULT) continue; - delta_info_mutate_row (image_base, image_dmeta, delta_info, log_token); + delta_info_mutate_row (image_dmeta, delta_info, log_token); } } From 3ed1f3939243b0d6692eeaf3f40fb3361436d186 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 21 Dec 2021 15:28:03 -0500 Subject: [PATCH 10/45] Calling new methods on a nested class works --- src/mono/mono/component/hot_reload.c | 2 +- src/mono/mono/metadata/class-init.c | 46 ++++++++++++++++------------ src/mono/mono/metadata/metadata.c | 4 +-- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 424a98430ff9f..77f48f186c50f 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -2245,7 +2245,7 @@ hot_reload_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_ta loc->result = 0; /* HACK: this is so that the locator can compute the row index of the given row. but passing the mutant table to other metadata functions could backfire. */ loc->t = (MonoTableInfo*)latest_mod_table; - for (uint32_t idx = 0; idx < table_info_get_rows (latest_mod_table); ++idx) { + for (uint32_t idx = 0; idx < rows; ++idx) { const char *row = latest_mod_table->base + idx * latest_mod_table->row_size; if (!comparer (loc, row)) return (void*)row; diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index 1083d535a70c1..d87d277c78a8c 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -614,27 +614,33 @@ mono_class_create_from_typedef (MonoImage *image, guint32 type_token, MonoError /* * Compute the field and method lists */ - int first_field_idx; - first_field_idx = cols [MONO_TYPEDEF_FIELD_LIST] - 1; - mono_class_set_first_field_idx (klass, first_field_idx); - int first_method_idx; - first_method_idx = cols [MONO_TYPEDEF_METHOD_LIST] - 1; - mono_class_set_first_method_idx (klass, first_method_idx); - - if (table_info_get_rows (tt) > tidx){ - mono_metadata_decode_row (tt, tidx, cols_next, MONO_TYPEDEF_SIZE); - field_last = cols_next [MONO_TYPEDEF_FIELD_LIST] - 1; - method_last = cols_next [MONO_TYPEDEF_METHOD_LIST] - 1; - } else { - field_last = table_info_get_rows (&image->tables [MONO_TABLE_FIELD]); - method_last = table_info_get_rows (&image->tables [MONO_TABLE_METHOD]); - } + /* + * EnC metadata-update: new classes are added with method and field indices set to 0, new + * methods are added using the EnCLog AddMethod or AddField functions that will be added to + * MonoClassMetadataUpdateInfo + */ + if (G_LIKELY (cols [MONO_TYPEDEF_FIELD_LIST] != 0 || cols [MONO_TYPEDEF_METHOD_LIST] != 0)) { + int first_field_idx; + first_field_idx = cols [MONO_TYPEDEF_FIELD_LIST] - 1; + mono_class_set_first_field_idx (klass, first_field_idx); + int first_method_idx; + first_method_idx = cols [MONO_TYPEDEF_METHOD_LIST] - 1; + mono_class_set_first_method_idx (klass, first_method_idx); + if (table_info_get_rows (tt) > tidx) { + mono_metadata_decode_row (tt, tidx, cols_next, MONO_TYPEDEF_SIZE); + field_last = cols_next [MONO_TYPEDEF_FIELD_LIST] - 1; + method_last = cols_next [MONO_TYPEDEF_METHOD_LIST] - 1; + } else { + field_last = table_info_get_rows (&image->tables [MONO_TABLE_FIELD]); + method_last = table_info_get_rows (&image->tables [MONO_TABLE_METHOD]); + } - if (cols [MONO_TYPEDEF_FIELD_LIST] && - cols [MONO_TYPEDEF_FIELD_LIST] <= table_info_get_rows (&image->tables [MONO_TABLE_FIELD])) - mono_class_set_field_count (klass, field_last - first_field_idx); - if (cols [MONO_TYPEDEF_METHOD_LIST] <= table_info_get_rows (&image->tables [MONO_TABLE_METHOD])) - mono_class_set_method_count (klass, method_last - first_method_idx); + if (cols [MONO_TYPEDEF_FIELD_LIST] && + cols [MONO_TYPEDEF_FIELD_LIST] <= table_info_get_rows (&image->tables [MONO_TABLE_FIELD])) + mono_class_set_field_count (klass, field_last - first_field_idx); + if (cols [MONO_TYPEDEF_METHOD_LIST] <= table_info_get_rows (&image->tables [MONO_TABLE_METHOD])) + mono_class_set_method_count (klass, method_last - first_method_idx); + } /* reserve space to store vector pointer in arrays */ if (mono_is_corlib_image (image) && !strcmp (nspace, "System") && !strcmp (name, "Array")) { diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index a7b50bc59113f..87c9762b5cd32 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -4974,7 +4974,7 @@ mono_metadata_nested_in_typedef (MonoImage *meta, guint32 index) MonoTableInfo *tdef = &meta->tables [MONO_TABLE_NESTEDCLASS]; locator_t loc; - if (!tdef->base) + if (!tdef->base && !meta->has_updates) return 0; loc.idx = mono_metadata_token_index (index); @@ -4983,7 +4983,7 @@ mono_metadata_nested_in_typedef (MonoImage *meta, guint32 index) /* FIXME: metadata-update */ - if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) && !meta->has_updates) + if (tdef->base && !mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) && !meta->has_updates) return 0; if (G_UNLIKELY (meta->has_updates)) From e4a3f5734dc096a1702fd572539d796f6e69a7bd Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 21 Dec 2021 16:37:51 -0500 Subject: [PATCH 11/45] XXX no merge - change the sample to add a static lambda --- src/mono/sample/mbr/console/TestClass_v1.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/mono/sample/mbr/console/TestClass_v1.cs b/src/mono/sample/mbr/console/TestClass_v1.cs index 5c552f3e3409f..96bbebd87b7b4 100644 --- a/src/mono/sample/mbr/console/TestClass_v1.cs +++ b/src/mono/sample/mbr/console/TestClass_v1.cs @@ -4,11 +4,18 @@ public class TestClass { [MethodImpl(MethodImplOptions.NoInlining)] public static string TargetMethod () { +#if false var o = new Inner(); return o.GetIt(); +#endif + return NewFunc (() => "NEW STRING"); } + public static string NewFunc(Func f) => f (); + +#if false public class Inner { public string GetIt() => "NEW STRING"; } +#endif } From 0b1f9275f18a6da8d691843f1d92c676f7bd55ca Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 21 Dec 2021 16:39:03 -0500 Subject: [PATCH 12/45] WIP: adding fields Doesn't work yet. Dies in mono_class_get_field, called from mono_field_from_token_checked. The problem we're going to have to deal with is that unlike MonoMethod which is allocated upon lookup, the MonoClassFields of a class are allocated contiguously in class initialization. And there are not really enough fields in MonoClassField to indicate "this is not an ordinary field, don't do pointer arithmetic with it". One idea is to steal a bit from MonoClassField:parent and always check it as a trigger to go on a slower path. --- src/mono/mono/component/hot_reload-stub.c | 10 +++ src/mono/mono/component/hot_reload.c | 89 ++++++++++++++++++++--- src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/metadata.c | 7 +- 4 files changed, 95 insertions(+), 12 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 35d38106241e2..556ed2f360d0e 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -74,6 +74,9 @@ hot_reload_stub_method_parent (MonoImage *image, uint32_t method_index); static void* hot_reload_stub_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); +static uint32_t +hot_reload_stub_field_parent (MonoImage *image, uint32_t field_index); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -95,6 +98,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_get_added_methods, &hot_reload_stub_method_parent, &hot_reload_stub_metadata_linear_search, + &hot_reload_stub_field_parent, }; static bool @@ -223,6 +227,12 @@ hot_reload_stub_metadata_linear_search (MonoImage *base_image, MonoTableInfo *ba return NULL; } +static uint32_t +hot_reload_stub_field_parent (MonoImage *image, uint32_t field_index) +{ + 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 77f48f186c50f..d8c79003175b9 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -103,6 +103,9 @@ hot_reload_method_parent (MonoImage *base, uint32_t method_token); static void* hot_reload_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); +static uint32_t +hot_reload_field_parent (MonoImage *base, uint32_t field_token); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_available }, @@ -125,6 +128,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_added_methods, &hot_reload_method_parent, &hot_reload_metadata_linear_search, + &hot_reload_field_parent, }; MonoComponentHotReload * @@ -528,6 +532,8 @@ image_append_delta (MonoImage *base, BaselineInfo *base_info, MonoImage *delta, static void add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t method_token, MonoDebugInformationEnc* pdb_address); +static void +add_field_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t field_token, MonoError *error); void hot_reload_init (void) @@ -1626,8 +1632,10 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Pass 2 begin: base '%s' delta image=%p", image_base->name, image_dmeta); +#if defined(ALLOW_METHOD_ADD) || defined(ALLOW_FIELD_ADD) + MonoClass *add_member_klass = NULL; +#endif #ifdef ALLOW_METHOD_ADD - MonoClass *add_method_klass = NULL; uint32_t add_param_method_index = 0; #endif @@ -1655,14 +1663,12 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen #ifdef ALLOW_METHOD_ADD case ENC_FUNC_ADD_METHOD: { g_assert (token_table == MONO_TABLE_TYPEDEF); - /* FIXME: this bounds check is wrong for cumulative updates - need to look at the DeltaInfo:count.prev_gen_rows */ - /* should've been caught by pass1 if we're adding a new method to a new class. */ MonoClass *klass = mono_class_get_checked (image_base, log_token, error); if (!is_ok (error)) { mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Can't get class with token 0x%08x due to: %s", log_token, mono_error_get_message (error)); return FALSE; } - add_method_klass = klass; + add_member_klass = klass; break; } @@ -1671,6 +1677,18 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen add_param_method_index = token_index; break; } +#endif +#ifdef ALLOW_FIELD_ADD + case ENC_FUNC_ADD_FIELD: { + g_assert (token_table == MONO_TABLE_TYPEDEF); + MonoClass *klass = mono_class_get_checked (image_base, log_token, error); + if (!is_ok (error)) { + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Can't get class with token 0x%08x due to: %s", log_token, mono_error_get_message (error)); + return FALSE; + } + add_member_klass = klass; + break; + } #endif default: g_error ("EnC: unsupported FuncCode, should be caught by pass1"); @@ -1723,13 +1741,13 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen break; if (is_addition) { - if (!add_method_klass) + if (!add_member_klass) g_error ("EnC: new method added but I don't know the class, should be caught by pass1"); - g_assert (add_method_klass); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to class %s.%s", token_index, m_class_get_name_space (add_method_klass), m_class_get_name (add_method_klass)); + g_assert (add_member_klass); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to class %s.%s", token_index, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); MonoDebugInformationEnc *method_debug_information = hot_reload_get_method_debug_information (delta_info->ppdb_file, token_index); - add_method_to_baseline (base_info, delta_info, add_method_klass, token_index, method_debug_information); - add_method_klass = NULL; + add_method_to_baseline (base_info, delta_info, add_member_klass, token_index, method_debug_information); + add_member_klass = NULL; } #endif @@ -1751,8 +1769,25 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen /* rva points probably into image_base IL stream. can this ever happen? */ g_print ("TODO: this case is still a bit contrived. token=0x%08x with rva=0x%04x\n", log_token, rva); } -#ifdef ALLOW_METHOD_ADD - add_method_klass = NULL; +#if defined(ALLOW_METHOD_ADD) || defined(ALLOW_FIELD_ADD) + add_member_klass = NULL; +#endif + break; + } + case MONO_TABLE_FIELD: { +#ifdef ALLOW_FIELD_ADD + g_assert (is_addition); + g_assert (add_member_klass); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new field 0x%08x to class %s.%s", token_index, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); + + add_field_to_baseline (base_info, delta_info, add_member_klass, token_index, error); + if (!is_ok (error)) { + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "Could not add new field (delta token 0x%08x) to class %s.%s, due to: %s", token_index, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass), mono_error_get_message (error)); + return FALSE; + } + add_member_klass = NULL; +#else + g_assert_not_reached (); #endif break; } @@ -2212,6 +2247,38 @@ hot_reload_method_parent (MonoImage *base_image, uint32_t method_token) return res; } +static void +add_field_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t field_token, MonoError *error) +{ + if (!base_info->field_parent) { + base_info->field_parent = g_hash_table_new (g_direct_hash, g_direct_equal); + } + MonoClassMetadataUpdateInfo *klass_info = mono_class_get_or_add_metadata_update_info (klass); + GArray *arr = klass_info->added_fields; + if (!arr) { + arr = g_array_new (FALSE, FALSE, sizeof(uint32_t)); + klass_info->added_fields = arr; + } + g_array_append_val (arr, field_token); + g_hash_table_insert (base_info->field_parent, GUINT_TO_POINTER (field_token), GUINT_TO_POINTER (m_class_get_type_token (klass))); +} + +static uint32_t +hot_reload_field_parent (MonoImage *base_image, uint32_t field_token) +{ + if (!base_image->has_updates) + return 0; + BaselineInfo *base_info = baseline_info_lookup (base_image); + if (!base_info || base_info->field_parent == NULL) + return 0; + + uint32_t res = GPOINTER_TO_UINT (g_hash_table_lookup (base_info->field_parent, GUINT_TO_POINTER (field_token))); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "field_parent lookup: 0x%08x returned 0x%08x\n", field_token, res); + + return res; +} + + /* XXX HACK - keep in sync with locator_t in metadata/metadata.c */ typedef struct { int idx; /* The index that we are trying to locate */ diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 27d25223a011a..63c783c0ffca4 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -35,6 +35,7 @@ typedef struct _MonoComponentHotReload { GArray* (*get_added_methods) (MonoClass *klass); uint32_t (*method_parent) (MonoImage *base_image, uint32_t method_index); void* (*metadata_linear_search) (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); + uint32_t (*field_parent) (MonoImage *base_image, uint32_t method_index); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 87c9762b5cd32..19717ea5e9f8a 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -4801,7 +4801,12 @@ mono_metadata_typedef_from_field (MonoImage *meta, guint32 index) if (meta->uncompressed_metadata) loc.idx = search_ptr_table (meta, MONO_TABLE_FIELD_POINTER, loc.idx); - /* FIXME: metadata-update */ + /* if it's not in the base image, look in the hot reload table */ + gboolean added = (loc.idx > table_info_get_rows (&meta->tables [MONO_TABLE_FIELD])); + if (added) { + uint32_t res = mono_component_hot_reload()->field_parent (meta, loc.idx); + return res; /* 0 if not found, otherwise 1-based */ + } if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, typedef_locator)) return 0; From eec222a74b1b8a1fca7009d331aeef43c7373361 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Wed, 22 Dec 2021 17:03:05 -0500 Subject: [PATCH 13/45] WIP: adding static fields Rework the delta info to store info about "members" (rather than methods and fields separately). Add a MonoClassMetadataUpdateField subclass of MonoClassField that has extra info. Add new component methods to go from a MonoClassField to an index, and from a field token to a MonoClassField. Set MonoClassField:offset to -1 to generally go down the "special static field" codepath since we will need to do some one-off work to make a storage area for added static fields. Instance fields are not supported yet and error out during the update pass for now. --- .../mono/component/hot_reload-internals.h | 15 +- src/mono/mono/component/hot_reload-stub.c | 27 ++- src/mono/mono/component/hot_reload.c | 190 +++++++++++++----- src/mono/mono/component/hot_reload.h | 4 +- src/mono/mono/metadata/class-internals.h | 6 +- src/mono/mono/metadata/class.c | 22 +- src/mono/mono/metadata/metadata-update.c | 16 ++ src/mono/mono/metadata/metadata-update.h | 6 + src/mono/mono/utils/mono-error-internals.h | 1 + src/mono/sample/mbr/console/TestClass_v1.cs | 22 +- 10 files changed, 247 insertions(+), 62 deletions(-) diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index 7e86b64d6685f..2347827ceefe4 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -8,6 +8,7 @@ #include #include "mono/metadata/object-forward.h" #include "mono/metadata/metadata-internals.h" +#include "mono/metadata/class-internals.h" /* Class-specific metadata update info. See * mono_class_get_metadata_update_info() Note that this info is associated with @@ -15,8 +16,18 @@ * arrays, pointers, etc do not have this info. */ struct _MonoClassMetadataUpdateInfo { - GArray *added_methods; /* a set of Method table tokens of any methods added to this class */ - GArray *added_fields; /* a set of Field table tokens of any methods added to this class */ + /* FIXME: use a struct that allocates out of the MonoClass mempool! or maybe add the GArray + * to the BaselineInfo for the image and cleanup from there. */ + GArray *added_members; /* a set of Method or Field table tokens of any methods or fields added to this class */ + + GPtrArray *added_fields; /* a set of MonoClassMetadataUpdateField* values for every added field. */ }; +typedef struct _MonoClassMetadataUpdateField { + MonoClassField field; + uint32_t generation; /* when this field was added */ + uint32_t token; /* the Field table token where this field was defined. (this won't make + * sense for generic instances, once EnC is supported there) */ +} MonoClassMetadataUpdateField; + #endif/*_MONO_COMPONENT_HOT_RELOAD_INTERNALS_H*/ diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 556ed2f360d0e..671a507ccc474 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -66,7 +66,7 @@ static int hot_reload_stub_table_num_rows_slow (MonoImage *image, int table_index); static GArray* -hot_reload_stub_get_added_methods (MonoClass *klass); +hot_reload_stub_get_added_members (MonoClass *klass); static uint32_t hot_reload_stub_method_parent (MonoImage *image, uint32_t method_index); @@ -77,6 +77,12 @@ hot_reload_stub_metadata_linear_search (MonoImage *base_image, MonoTableInfo *ba static uint32_t hot_reload_stub_field_parent (MonoImage *image, uint32_t field_index); +static uint32_t +hot_reload_stub_get_field_idx (MonoClassField *field); + +static MonoClassField * +hot_reload_stub_get_field (MonoClass *klass, uint32_t fielddef_token); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -95,10 +101,12 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_get_updated_method_ppdb, &hot_reload_stub_has_modified_rows, &hot_reload_stub_table_num_rows_slow, - &hot_reload_stub_get_added_methods, + &hot_reload_stub_get_added_members, &hot_reload_stub_method_parent, &hot_reload_stub_metadata_linear_search, &hot_reload_stub_field_parent, + &hot_reload_stub_get_field_idx, + &hot_reload_stub_get_field, }; static bool @@ -209,7 +217,7 @@ hot_reload_stub_table_num_rows_slow (MonoImage *image, int table_index) } static GArray* -hot_reload_stub_get_added_methods (MonoClass *klass) +hot_reload_stub_get_added_members (MonoClass *klass) { return NULL; } @@ -233,6 +241,19 @@ hot_reload_stub_field_parent (MonoImage *image, uint32_t field_index) return 0; } +static uint32_t +hot_reload_stub_get_field_idx (MonoClassField *field) +{ + return 0; +} + +static MonoClassField * +hot_reload_stub_get_field (MonoClass *klass, uint32_t fielddef_token) +{ + 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 d8c79003175b9..79a18ce33ed24 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -35,6 +35,7 @@ #define ALLOW_METHOD_ADD #define ALLOW_FIELD_ADD +typedef struct _BaselineInfo BaselineInfo; typedef struct _DeltaInfo DeltaInfo; static void @@ -95,7 +96,7 @@ static int hot_reload_table_num_rows_slow (MonoImage *image, int table_index); static GArray* -hot_reload_get_added_methods (MonoClass *klass); +hot_reload_get_added_members (MonoClass *klass); static uint32_t hot_reload_method_parent (MonoImage *base, uint32_t method_token); @@ -106,6 +107,15 @@ hot_reload_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_ta static uint32_t hot_reload_field_parent (MonoImage *base, uint32_t field_token); +static uint32_t +hot_reload_get_field_idx (MonoClassField *field); + +static MonoClassField * +hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token); + +static MonoClassMetadataUpdateField * +metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, MonoError *error); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_available }, @@ -125,10 +135,12 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_updated_method_ppdb, &hot_reload_has_modified_rows, &hot_reload_table_num_rows_slow, - &hot_reload_get_added_methods, + &hot_reload_get_added_members, &hot_reload_method_parent, &hot_reload_metadata_linear_search, &hot_reload_field_parent, + &hot_reload_get_field_idx, + &hot_reload_get_field, }; MonoComponentHotReload * @@ -203,7 +215,7 @@ struct _DeltaInfo { /* Additional informaiton for baseline MonoImages */ -typedef struct _BaselineInfo { +struct _BaselineInfo { /* List of DeltaInfos of deltas*/ GList *delta_info; /* Tail of delta_info for fast appends */ @@ -216,9 +228,8 @@ typedef struct _BaselineInfo { gboolean any_modified_rows [MONO_TABLE_NUM]; /* Parents for added methods, fields, etc */ - GHashTable *method_parent; /* maps added methoddef tokens to typedef tokens */ - GHashTable *field_parent; /* maps added fielddef tokens to typedef tokens */ -} BaselineInfo; + GHashTable *member_parent; /* maps added methoddef or fielddef tokens to typedef tokens */ +}; #define DOTNET_MODIFIABLE_ASSEMBLIES "DOTNET_MODIFIABLE_ASSEMBLIES" @@ -529,11 +540,15 @@ image_open_dmeta_from_data (MonoImage *base_image, uint32_t generation, gconstpo static DeltaInfo* image_append_delta (MonoImage *base, BaselineInfo *base_info, MonoImage *delta, DeltaInfo *delta_info); +/* common method, don't use directly, use add_method_to_baseline, add_field_to_baseline, etc */ +static void +add_member_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t member_token); + static void add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t method_token, MonoDebugInformationEnc* pdb_address); static void -add_field_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t field_token, MonoError *error); +add_field_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t field_token); void hot_reload_init (void) @@ -1744,9 +1759,9 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen if (!add_member_klass) g_error ("EnC: new method added but I don't know the class, should be caught by pass1"); g_assert (add_member_klass); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to class %s.%s", token_index, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new method 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); MonoDebugInformationEnc *method_debug_information = hot_reload_get_method_debug_information (delta_info->ppdb_file, token_index); - add_method_to_baseline (base_info, delta_info, add_member_klass, token_index, method_debug_information); + add_method_to_baseline (base_info, delta_info, add_member_klass, log_token, method_debug_information); add_member_klass = NULL; } #endif @@ -1778,13 +1793,30 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen #ifdef ALLOW_FIELD_ADD g_assert (is_addition); g_assert (add_member_klass); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new field 0x%08x to class %s.%s", token_index, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "Adding new field 0x%08x to class %s.%s", log_token, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass)); + + 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); - add_field_to_baseline (base_info, delta_info, add_member_klass, token_index, error); + 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; + } + + add_field_to_baseline (base_info, delta_info, add_member_klass, log_token); + + /* This actually does more than mono_class_setup_basic_field_info and + * resolves MonoClassField:type and sets MonoClassField:offset to -1 to make + * it easier to spot that the field is special. + */ + metadata_update_field_setup_basic_info_and_resolve (image_base, base_info, generation, delta_info, add_member_klass, log_token, error); if (!is_ok (error)) { - mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "Could not add new field (delta token 0x%08x) to class %s.%s, due to: %s", token_index, m_class_get_name_space (add_member_klass), m_class_get_name (add_member_klass), mono_error_get_message (error)); + mono_trace (G_LOG_LEVEL_WARNING, MONO_TRACE_METADATA_UPDATE, "Could not setup field (token 0x%08x) due to: %s", log_token, mono_error_get_message (error)); return FALSE; } + add_member_klass = NULL; #else g_assert_not_reached (); @@ -1856,7 +1888,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen * * So by the time we see the param additions, the methods are already in. * - * FIXME: we need a lookaside table (like method_parent) for every place + * FIXME: we need a lookaside table (like member_parent) for every place * that looks at MONO_METHOD_PARAMLIST */ break; @@ -2070,8 +2102,9 @@ hot_reload_get_updated_method_ppdb (MonoImage *base_image, uint32_t idx) if (G_UNLIKELY (gen > 0)) { loc = get_method_update_rva (base_image, info, idx, TRUE); } - /* Check the method_parent table as a way of checking if the method was added by a later generation. If so, still look for its PPDB info in our update tables */ - if (G_UNLIKELY (loc == 0 && info->method_parent && GPOINTER_TO_UINT (g_hash_table_lookup (info->method_parent, GUINT_TO_POINTER (idx))) > 0)) { + /* Check the member_parent table as a way of checking if the method was added by a later generation. If so, still look for its PPDB info in our update tables */ + uint32_t token = mono_metadata_make_token (MONO_TABLE_METHOD, mono_metadata_token_index (idx)); + if (G_UNLIKELY (loc == 0 && info->member_parent && GPOINTER_TO_UINT (g_hash_table_lookup (info->member_parent, GUINT_TO_POINTER (token))) > 0)) { loc = get_method_update_rva (base_image, info, idx, TRUE); } } @@ -2200,27 +2233,36 @@ hot_reload_table_num_rows_slow (MonoImage *base, int table_index) } static void -add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t method_token, MonoDebugInformationEnc* pdb_address) +add_member_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t member_token) { - if (!base_info->method_parent) { - base_info->method_parent = g_hash_table_new (g_direct_hash, g_direct_equal); + /* Check they really passed a table token, not just a table row index */ + g_assert (mono_metadata_token_table (member_token) != 0); + + if (!base_info->member_parent) { + base_info->member_parent = g_hash_table_new (g_direct_hash, g_direct_equal); } MonoClassMetadataUpdateInfo *klass_info = mono_class_get_or_add_metadata_update_info (klass); /* FIXME: locking for readers/writers of the GArray */ - GArray *arr = klass_info->added_methods; + GArray *arr = klass_info->added_members; if (!arr) { arr = g_array_new (FALSE, FALSE, sizeof(uint32_t)); - klass_info->added_methods = arr; + klass_info->added_members = arr; } - g_array_append_val (arr, method_token); - g_hash_table_insert (base_info->method_parent, GUINT_TO_POINTER (method_token), GUINT_TO_POINTER (m_class_get_type_token (klass))); + g_array_append_val (arr, member_token); + g_hash_table_insert (base_info->member_parent, GUINT_TO_POINTER (member_token), GUINT_TO_POINTER (m_class_get_type_token (klass))); +} +static void +add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t method_token, MonoDebugInformationEnc* pdb_address) +{ + add_member_to_baseline (base_info, delta_info, klass, method_token); + if (pdb_address) set_delta_method_debug_info (delta_info, method_token, pdb_address); } static GArray* -hot_reload_get_added_methods (MonoClass *klass) +hot_reload_get_added_members (MonoClass *klass) { /* FIXME: locking for the GArray? */ MonoImage *image = m_class_get_image (klass); @@ -2229,53 +2271,49 @@ hot_reload_get_added_methods (MonoClass *klass) MonoClassMetadataUpdateInfo *klass_info = mono_class_get_metadata_update_info (klass); if (!klass_info) return NULL; - return klass_info->added_methods; + return klass_info->added_members; } static uint32_t -hot_reload_method_parent (MonoImage *base_image, uint32_t method_token) +hot_reload_member_parent (MonoImage *base_image, uint32_t member_token) { + /* make sure they passed a token, not just a table row index */ + g_assert (mono_metadata_token_table (member_token) != 0); + if (!base_image->has_updates) return 0; BaselineInfo *base_info = baseline_info_lookup (base_image); - if (!base_info || base_info->method_parent == NULL) + if (!base_info || base_info->member_parent == NULL) return 0; - uint32_t res = GPOINTER_TO_UINT (g_hash_table_lookup (base_info->method_parent, GUINT_TO_POINTER (method_token))); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "method_parent lookup: 0x%08x returned 0x%08x\n", method_token, res); + uint32_t res = GPOINTER_TO_UINT (g_hash_table_lookup (base_info->member_parent, GUINT_TO_POINTER (member_token))); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "member_parent lookup: 0x%08x returned 0x%08x\n", member_token, res); return res; } +static uint32_t +hot_reload_method_parent (MonoImage *base_image, uint32_t method_token) +{ + /* the callers might pass just an index without a table */ + uint32_t lookup_token = mono_metadata_make_token (MONO_TABLE_METHOD, mono_metadata_token_index (method_token)); + + return hot_reload_member_parent (base_image, lookup_token); +} + static void -add_field_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t field_token, MonoError *error) +add_field_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t field_token) { - if (!base_info->field_parent) { - base_info->field_parent = g_hash_table_new (g_direct_hash, g_direct_equal); - } - MonoClassMetadataUpdateInfo *klass_info = mono_class_get_or_add_metadata_update_info (klass); - GArray *arr = klass_info->added_fields; - if (!arr) { - arr = g_array_new (FALSE, FALSE, sizeof(uint32_t)); - klass_info->added_fields = arr; - } - g_array_append_val (arr, field_token); - g_hash_table_insert (base_info->field_parent, GUINT_TO_POINTER (field_token), GUINT_TO_POINTER (m_class_get_type_token (klass))); + add_member_to_baseline (base_info, delta_info, klass, field_token); } static uint32_t hot_reload_field_parent (MonoImage *base_image, uint32_t field_token) { - if (!base_image->has_updates) - return 0; - BaselineInfo *base_info = baseline_info_lookup (base_image); - if (!base_info || base_info->field_parent == NULL) - return 0; - - uint32_t res = GPOINTER_TO_UINT (g_hash_table_lookup (base_info->field_parent, GUINT_TO_POINTER (field_token))); - mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "field_parent lookup: 0x%08x returned 0x%08x\n", field_token, res); + /* the callers might pass just an index without a table */ + uint32_t lookup_token = mono_metadata_make_token (MONO_TABLE_FIELD, mono_metadata_token_index (field_token)); - return res; + return hot_reload_member_parent (base_image, lookup_token); } @@ -2319,3 +2357,57 @@ hot_reload_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_ta } return NULL; } + +static uint32_t +hot_reload_get_field_idx (MonoClassField *field) +{ + g_assert (m_field_is_from_update (field)); + MonoClassMetadataUpdateField *field_info = (MonoClassMetadataUpdateField*)field; + return mono_metadata_token_index (field_info->token); +} + +static MonoClassField * +hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token) { + MonoClassMetadataUpdateInfo *info = mono_class_get_or_add_metadata_update_info (klass); + g_assert (mono_metadata_token_table (fielddef_token) == MONO_TABLE_FIELD); + /* FIXME: this needs locking in the multi-threaded case. There could be an update happening that resizes the array. */ + GPtrArray *added_fields = info->added_fields; + uint32_t count = added_fields->len; + for (uint32_t i = 0; i < count; ++i) { + MonoClassMetadataUpdateField *field = (MonoClassMetadataUpdateField *)g_ptr_array_index (added_fields, i); + if (field->token == fielddef_token) + return &field->field; + } + return NULL; +} + + +static MonoClassMetadataUpdateField * +metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, MonoError *error) +{ + MonoClassMetadataUpdateInfo *parent_info = mono_class_get_or_add_metadata_update_info (parent_klass); + + MonoClassMetadataUpdateField *field = mono_class_alloc0 (parent_klass, sizeof (MonoClassMetadataUpdateField)); + + m_field_set_parent (&field->field, parent_klass); + m_field_set_meta_flags (&field->field, MONO_CLASS_FIELD_META_FLAG_FROM_UPDATE); + /* It's a special field */ + field->field.offset = -1; + field->generation = generation; + field->token = fielddef_token; + + uint32_t name_idx = mono_metadata_decode_table_row_col (image_base, MONO_TABLE_FIELD, mono_metadata_token_index (fielddef_token) - 1, MONO_FIELD_NAME); + field->field.name = mono_metadata_string_heap (image_base, name_idx); + + mono_field_resolve_type (&field->field, error); + if (!is_ok (error)) + return NULL; + + if (!parent_info->added_fields) { + parent_info->added_fields = g_ptr_array_new (); + } + + g_ptr_array_add (parent_info->added_fields, field); + + return field; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 63c783c0ffca4..d2ba7e27c1ea7 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -32,10 +32,12 @@ typedef struct _MonoComponentHotReload { gpointer (*get_updated_method_ppdb) (MonoImage *base_image, uint32_t idx); gboolean (*has_modified_rows) (const MonoTableInfo *table); gboolean (*table_num_rows_slow) (MonoImage *base_image, int table_index); - GArray* (*get_added_methods) (MonoClass *klass); + GArray* (*get_added_members) (MonoClass *klass); uint32_t (*method_parent) (MonoImage *base_image, uint32_t method_index); void* (*metadata_linear_search) (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); uint32_t (*field_parent) (MonoImage *base_image, uint32_t method_index); + uint32_t (*get_field_idx) (MonoClassField *field); + MonoClassField* (*get_field) (MonoClass *klass, uint32_t fielddef_token); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index 693ff2cddbdfe..44d2ce4b6b381 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1504,7 +1504,7 @@ mono_class_get_default_finalize_method (void); const char * mono_field_get_rva (MonoClassField *field, int swizzle); -void +MONO_COMPONENT_API void mono_field_resolve_type (MonoClassField *field, MonoError *error); gboolean @@ -1587,10 +1587,10 @@ m_field_get_meta_flags (MonoClassField *field) return (unsigned int)(field->parent_and_flags & MONO_CLASS_FIELD_META_FLAG_MASK); } -void +MONO_COMPONENT_API void m_field_set_parent (MonoClassField *field, MonoClass *klass); -void +MONO_COMPONENT_API void m_field_set_meta_flags (MonoClassField *field, unsigned int flags); static inline gboolean diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 04af2642f252d..13213cc75ebb7 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -2401,6 +2402,10 @@ mono_class_get_field_idx (MonoClass *klass, int idx) return &klass_fields [idx - first_field_idx]; } } + if (G_UNLIKELY (m_class_get_image (klass)->has_updates && mono_class_has_metadata_update_info (klass))) { + uint32_t token = mono_metadata_make_token (MONO_TABLE_FIELD, idx + 1); + return mono_metadata_update_get_field (klass, token); + } } klass = m_class_get_parent (klass); } @@ -6417,11 +6422,18 @@ mono_field_resolve_type (MonoClassField *field, MonoError *error) MonoImage *image = m_class_get_image (klass); MonoClass *gtd = mono_class_is_ginst (klass) ? mono_class_get_generic_type_definition (klass) : NULL; MonoType *ftype; - int field_idx = field - m_class_get_fields (klass); + int field_idx; + + if (G_UNLIKELY (m_field_is_from_update (field))) { + field_idx = -1; + } else { + field_idx = field - m_class_get_fields (klass); + } error_init (error); if (gtd) { + g_assert (field_idx != -1); MonoClassField *gfield = &m_class_get_fields (gtd) [field_idx]; MonoType *gtype = mono_field_get_type_checked (gfield, error); if (!is_ok (error)) { @@ -6440,7 +6452,13 @@ mono_field_resolve_type (MonoClassField *field, MonoError *error) const char *sig; guint32 cols [MONO_FIELD_SIZE]; MonoGenericContainer *container = NULL; - int idx = mono_class_get_first_field_idx (klass) + field_idx; + int idx; + + if (G_UNLIKELY (m_field_is_from_update (field))) { + idx = mono_metadata_update_get_field_idx (field) - 1; + } else { + idx = mono_class_get_first_field_idx (klass) + field_idx; + } /*FIXME, in theory we do not lazy load SRE fields*/ g_assert (!image_is_dynamic (image)); diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 5afcbc42e893f..7015bf749a7a4 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -153,3 +153,19 @@ mono_metadata_update_metadata_linear_search (MonoImage *base_image, MonoTableInf { return mono_component_hot_reload()->metadata_linear_search (base_image, base_table, key, comparer); } + +/* + * Returns the (1-based) table row index of the fielddef of the given field + * (which must have m_field_is_from_update set). + */ +uint32_t +mono_metadata_update_get_field_idx (MonoClassField *field) +{ + return mono_component_hot_reload()->get_field_idx (field); +} + +MonoClassField * +mono_metadata_update_get_field (MonoClass *klass, uint32_t fielddef_token) +{ + return mono_component_hot_reload()->get_field (klass, fielddef_token); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index c3e0edba4134c..92614d9d0805b 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -61,4 +61,10 @@ mono_metadata_update_delta_heap_lookup (MonoImage *base_image, MetadataHeapGette void* mono_metadata_update_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); +uint32_t +mono_metadata_update_get_field_idx (MonoClassField *field); + +MonoClassField * +mono_metadata_update_get_field (MonoClass *klass, uint32_t fielddef_token); + #endif /*__MONO_METADATA_UPDATE_H__*/ diff --git a/src/mono/mono/utils/mono-error-internals.h b/src/mono/mono/utils/mono-error-internals.h index a26558689ae1c..f43727dde76bf 100644 --- a/src/mono/mono/utils/mono-error-internals.h +++ b/src/mono/mono/utils/mono-error-internals.h @@ -202,6 +202,7 @@ mono_error_set_generic_error (MonoError *error, const char * name_space, const c void mono_error_set_execution_engine (MonoError *error, const char *msg_format, ...) MONO_ATTR_FORMAT_PRINTF(2,3); +MONO_COMPONENT_API void mono_error_set_not_implemented (MonoError *error, const char *msg_format, ...) MONO_ATTR_FORMAT_PRINTF(2,3); diff --git a/src/mono/sample/mbr/console/TestClass_v1.cs b/src/mono/sample/mbr/console/TestClass_v1.cs index 96bbebd87b7b4..51a4d42a2b1a0 100644 --- a/src/mono/sample/mbr/console/TestClass_v1.cs +++ b/src/mono/sample/mbr/console/TestClass_v1.cs @@ -4,16 +4,34 @@ public class TestClass { [MethodImpl(MethodImplOptions.NoInlining)] public static string TargetMethod () { -#if false +#if ADD_INNER_INST var o = new Inner(); return o.GetIt(); #endif +#if true return NewFunc (() => "NEW STRING"); +#endif +#if ADD_MUTUALS + return ClassOne.F1.s; +#endif } +#if true public static string NewFunc(Func f) => f (); +#endif + +#if ADD_MUTUALS + public class ClassOne { + public static ClassTwo F1; + } + + public class ClassTwo { + public static ClassOne F2; + public string s; + } +#endif -#if false +#if ADD_INNER_INST public class Inner { public string GetIt() => "NEW STRING"; } From 2b92f0b8d9cd6a81f48e850c9964219c1836ceb8 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 23 Dec 2021 09:55:27 -0500 Subject: [PATCH 14/45] WIP: custom attribute lookups --- src/mono/mono/metadata/custom-attrs.c | 16 +++++++++++----- src/mono/mono/metadata/metadata.c | 21 +++++++++++++-------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 580a51b497d37..498ed9a53ed99 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -25,6 +25,7 @@ #include "mono/metadata/tabledefs.h" #include "mono/metadata/tokentype.h" #include "mono/metadata/icall-decl.h" +#include "mono/metadata/metadata-update.h" #include "mono/utils/checked-build.h" #define CHECK_ADD4_OVERFLOW_UN(a, b) ((guint32)(0xFFFFFFFFU) - (guint32)(b) < (guint32)(a)) @@ -150,6 +151,8 @@ free_param_data (MonoMethodSignature *sig, void **params) { */ static guint32 find_field_index (MonoClass *klass, MonoClassField *field) { + if (G_UNLIKELY (m_field_is_from_update (field))) + return mono_metadata_update_get_field_idx (field); int fcount = mono_class_get_field_count (klass); MonoClassField *klass_fields = m_class_get_fields (klass); int index = field - klass_fields; @@ -1626,8 +1629,6 @@ mono_custom_attrs_from_index_checked (MonoImage *image, guint32 idx, gboolean ig error_init (error); ca = &image->tables [MONO_TABLE_CUSTOMATTRIBUTE]; - /* FIXME: metadata-update */ - int rows = table_info_get_rows (ca); i = mono_metadata_custom_attrs_from_index (image, idx); if (!i) @@ -1635,9 +1636,14 @@ mono_custom_attrs_from_index_checked (MonoImage *image, guint32 idx, gboolean ig i --; // initial size chosen arbitrarily, but default is 16 which is rather small attr_array = g_array_sized_new (TRUE, TRUE, sizeof (guint32), 128); - while (i < rows) { - if (mono_metadata_decode_row_col (ca, i, MONO_CUSTOM_ATTR_PARENT) != idx) - break; + while (!mono_metadata_table_bounds_check (image, MONO_TABLE_CUSTOMATTRIBUTE, i + 1)) { + if (mono_metadata_decode_row_col (ca, i, MONO_CUSTOM_ATTR_PARENT) != idx) { + /* if there are updates, the new custom attributes are not sorted, so we have to go until the end. */ + if (G_LIKELY (!image->has_updates)) + break; + else + continue; + } attr_array = g_array_append_val (attr_array, i); ++i; } diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 19717ea5e9f8a..8aa7a921ab48c 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -4986,14 +4986,14 @@ mono_metadata_nested_in_typedef (MonoImage *meta, guint32 index) loc.col_idx = MONO_NESTED_CLASS_NESTED; loc.t = tdef; - /* FIXME: metadata-update */ - - if (tdef->base && !mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) && !meta->has_updates) + gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) != NULL; + if (!found && !meta->has_updates) return 0; - if (G_UNLIKELY (meta->has_updates)) - if (!mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) + if (G_UNLIKELY (meta->has_updates)) { + if (!found && !mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) return 0; + } /* loc_result is 0..1, needs to be mapped to table index (that is +1) */ return mono_metadata_decode_row_col (tdef, loc.result, MONO_NESTED_CLASS_ENCLOSING) | MONO_TOKEN_TYPE_DEF; @@ -5086,19 +5086,24 @@ mono_metadata_custom_attrs_from_index (MonoImage *meta, guint32 index) MonoTableInfo *tdef = &meta->tables [MONO_TABLE_CUSTOMATTRIBUTE]; locator_t loc; - if (!tdef->base) + if (!tdef->base && !meta->has_updates) return 0; loc.idx = index; loc.col_idx = MONO_CUSTOM_ATTR_PARENT; loc.t = tdef; - /* FIXME: metadata-update */ /* FIXME: Index translation */ - if (!mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator)) + gboolean found = tdef->base && mono_binary_search (&loc, tdef->base, table_info_get_rows (tdef), tdef->row_size, table_locator) != NULL; + if (!found && !meta->has_updates) return 0; + if (G_UNLIKELY (meta->has_updates)) { + if (!found && !mono_metadata_update_metadata_linear_search (meta, tdef, &loc, table_locator)) + return 0; + } + /* Find the first entry by searching backwards */ while ((loc.result > 0) && (mono_metadata_decode_row_col (tdef, loc.result - 1, MONO_CUSTOM_ATTR_PARENT) == index)) loc.result --; From bf5701145a6712b5e79bb6afe49824358835c291 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 23 Dec 2021 16:02:55 -0500 Subject: [PATCH 15/45] WIP: Add a sketch of HotReloadInstanceFieldTable This is a class that manages the lifetime for added instance fields for reference types that had new fields added to them by EnC It's basically: ConditionalWeakTable> where the outer key is the object instance, the inner key is the fielddef token, and the value is a Store. The store has a single object? field which the location where the field's value will go. For reference types this is the reference directly, and for valuetypes and primitives, this is the boxed object (so to get the actual storage address you need to add sizeof(MonoObject)). --- .../System.Private.CoreLib.csproj | 1 + .../src/Mono/HotReloadInstanceFieldTable.cs | 104 ++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs diff --git a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj index cb143f5d381be..62cbafcb8fc4a 100644 --- a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -294,6 +294,7 @@ + diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs new file mode 100644 index 0000000000000..b41e48212f3fc --- /dev/null +++ b/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs @@ -0,0 +1,104 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Reflection; +using System.Runtime.CompilerServices; + +namespace Mono; + +internal class HotReloadInstanceFieldTable +{ + // 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. + + // See EnCAddedField::Allocate for a description of the CoreCLR version of this. + // + // This is substantially the same design, except instead of using dependent handles + // (ephemerons) directly from native (storing a linked list of ephemerons off the sync block + // of the instance), we use a ConditionalWeakTable from managed that's keyed on the + // instances (so the value dies when the instance dies) and whose value is another + // dictionary, keyed on the fielddef token with values that are storage for the actual field values. + // + // for reference types, the storage just stores it as an object. For valuetypes and + // primitives, the storage stores the value as a boxed value. + // + // The whole thing is basically a ConditionalWeakTable> but + // with locking on the inner dictionary. + // + + // 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 Store GetInstanceFieldStore(object inst, RuntimeTypeHandle type, uint fielddef_token) + => _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(type, fielddef_token); + + private static HotReloadInstanceFieldTable _singleton = new(); + + private ConditionalWeakTable _table; + + private HotReloadInstanceFieldTable() + { + _table = new(); + } + + private InstanceFields GetOrCreateInstanceFields(object key) + => _table.GetOrCreateValue(key); + + private class InstanceFields + { + private Dictionary _fields; + private object _lock; + + public InstanceFields() + { + _fields = new(); + _lock = new(); + } + + public Store LookupOrAdd(RuntimeTypeHandle type, uint key) + { + if (_fields.TryGetValue(key, out Store? v)) + return v; + lock (_lock) + { + if (_fields.TryGetValue (key, out Store? v2)) + return v2; + + Store s = Store.Create(type); + _fields.Add(key, s); + return s; + } + } + } + + // This is similar to System.Diagnostics.EditAndContinueHelper in CoreCLR, except instead of + // having the allocation logic in native (see EditAndContinueModule::ResolveOrAllocateField, + // and EnCSyncBlockInfo::ResolveOrAllocateField), the logic is in managed. + internal class Store + { + private object? _loc; + + private Store (object? loc) + { + _loc = loc; + } + + public object? Location => _loc; + + public static Store Create (RuntimeTypeHandle type) + { + Type t = Type.GetTypeFromHandle(type) ?? throw new ArgumentException(nameof(type), "Type handle was null"); + object? loc; + if (t.IsPrimitive || t.IsValueType) + loc = RuntimeHelpers.GetUninitializedObject(t); + else if (t.IsClass || t.IsInterface) + loc = null; + else + throw new ArgumentException("EnC: Expected a primitive, valuetype, class or interface field"); + return new Store(loc); + } + } +} From df207dc68c0f5c6b4d5611217341c03e24fb2fae Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 23 Dec 2021 16:09:17 -0500 Subject: [PATCH 16/45] WIP: placeholder for mono_metadata_update_get_static_field_addr and two reminders: 1. The static field area needs to be GC-visible 2. We should really handle added fields on un-inited classes by allocating the instances in the normal way - in the layout algorithm. --- src/mono/mono/component/hot_reload-stub.c | 9 +++++++++ src/mono/mono/component/hot_reload.c | 20 ++++++++++++++++++++ src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/metadata-update.c | 6 ++++++ src/mono/mono/metadata/metadata-update.h | 3 +++ src/mono/mono/metadata/object.c | 4 ++++ 6 files changed, 43 insertions(+) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 671a507ccc474..7c316b045f9fa 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -83,6 +83,9 @@ hot_reload_stub_get_field_idx (MonoClassField *field); static MonoClassField * hot_reload_stub_get_field (MonoClass *klass, uint32_t fielddef_token); +static gpointer +hot_reload_stub_get_static_field_addr (MonoClassField *field); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -107,6 +110,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_field_parent, &hot_reload_stub_get_field_idx, &hot_reload_stub_get_field, + &hot_reload_stub_get_static_field_addr, }; static bool @@ -253,6 +257,11 @@ hot_reload_stub_get_field (MonoClass *klass, uint32_t fielddef_token) return NULL; } +static gpointer +hot_reload_get_static_field_addr (MonoClassField *field) +{ + return NULL; +} MONO_COMPONENT_EXPORT_ENTRYPOINT MonoComponentHotReload * diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 79a18ce33ed24..3a7ed4dbd715e 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -113,6 +113,9 @@ hot_reload_get_field_idx (MonoClassField *field); static MonoClassField * hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token); +static gpointer +hot_reload_get_static_field_addr (MonoClassField *field); + static MonoClassMetadataUpdateField * metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, MonoError *error); @@ -141,6 +144,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_field_parent, &hot_reload_get_field_idx, &hot_reload_get_field, + &hot_reload_get_static_field_addr, }; MonoComponentHotReload * @@ -2385,6 +2389,9 @@ hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token) { static MonoClassMetadataUpdateField * metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, MonoError *error) { + // TODO: hang a "pending field" struct off the parent_klass if !parent_klass->fields + // In that case we can do things simpler, maybe by just creating the MonoClassField array as usual, and just relying on the normal layout algorithm to make space for the instance. + MonoClassMetadataUpdateInfo *parent_info = mono_class_get_or_add_metadata_update_info (parent_klass); MonoClassMetadataUpdateField *field = mono_class_alloc0 (parent_klass, sizeof (MonoClassMetadataUpdateField)); @@ -2411,3 +2418,16 @@ metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, Basel return field; } + +static gpointer +hot_reload_get_static_field_addr (MonoClassField *field) +{ + g_assert (m_field_is_from_update (field)); + MonoClassMetadataUpdateField *f = (MonoClassMetadataUpdateField *)field; + g_assert ((f->field.type->attrs & FIELD_ATTRIBUTE_STATIC) != 0); + // FIXME: this needs to be GC-visible + + g_assert_not_reached (); // FIXME: finish me + + return NULL; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index d2ba7e27c1ea7..204ea2427a815 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -38,6 +38,7 @@ typedef struct _MonoComponentHotReload { uint32_t (*field_parent) (MonoImage *base_image, uint32_t method_index); uint32_t (*get_field_idx) (MonoClassField *field); MonoClassField* (*get_field) (MonoClass *klass, uint32_t fielddef_token); + gpointer (*get_static_field_addr) (MonoClassField *field); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 7015bf749a7a4..2a1d5a891eece 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -169,3 +169,9 @@ mono_metadata_update_get_field (MonoClass *klass, uint32_t fielddef_token) { return mono_component_hot_reload()->get_field (klass, fielddef_token); } + +gpointer +mono_metadata_update_get_static_field_addr (MonoClassField *field) +{ + return mono_component_hot_reload()->get_static_field_addr (field); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index 92614d9d0805b..f66b4bcb745e5 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -67,4 +67,7 @@ mono_metadata_update_get_field_idx (MonoClassField *field); MonoClassField * mono_metadata_update_get_field (MonoClass *klass, uint32_t fielddef_token); +gpointer +mono_metadata_update_get_static_field_addr (MonoClassField *field); + #endif /*__MONO_METADATA_UPDATE_H__*/ diff --git a/src/mono/mono/metadata/object.c b/src/mono/mono/metadata/object.c index 284e15b1ade8e..55afb98f2f194 100644 --- a/src/mono/mono/metadata/object.c +++ b/src/mono/mono/metadata/object.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -2854,6 +2855,9 @@ mono_static_field_get_addr (MonoVTable *vt, MonoClassField *field) g_assert (field->type->attrs & FIELD_ATTRIBUTE_STATIC); if (field->offset == -1) { + if (G_UNLIKELY (m_field_is_from_update (field))) { + return mono_metadata_update_get_static_field_addr (field); + } /* Special static */ ERROR_DECL (error); gpointer addr = mono_special_static_field_get_offset (field, error); From e8c021d5010c96c4f3133b6481b34d4856e36093 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 3 Jan 2022 09:07:25 -0500 Subject: [PATCH 17/45] Free MonoClassMetadataUpdateInfo when BaselineInfo is destroyed --- src/mono/mono/component/hot_reload.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 3a7ed4dbd715e..bb90758fbdd78 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -231,6 +231,9 @@ struct _BaselineInfo { /* TRUE if any published update modified an existing row */ gboolean any_modified_rows [MONO_TABLE_NUM]; + /* A list of MonoClassMetadataUpdateInfo* that need to be cleaned up */ + GSList *klass_info; + /* Parents for added methods, fields, etc */ GHashTable *member_parent; /* maps added methoddef or fielddef tokens to typedef tokens */ }; @@ -341,11 +344,27 @@ baseline_info_init (MonoImage *image_base) return baseline_info; } +static void +klass_info_destroy (gpointer value, gpointer user_data G_GNUC_UNUSED) +{ + MonoClassMetadataUpdateInfo *info = (MonoClassMetadataUpdateInfo *)value; + g_array_free (info->added_members, TRUE); + /* The MonoClassMetadataUpdateField is allocated from the class mempool, don't free it here */ + g_ptr_array_free (info->added_fields, TRUE); + + /* The MonoClassMetadataUpdateInfo itself is allocated from the class mempool, don't free it here */ +} + static void baseline_info_destroy (BaselineInfo *info) { if (info->method_table_update) g_hash_table_destroy (info->method_table_update); + + if (info->klass_info) { + g_slist_foreach (info->klass_info, klass_info_destroy, NULL); + g_slist_free (info->klass_info); + } g_free (info); } @@ -695,6 +714,14 @@ hot_reload_update_cancel (uint32_t generation) publish_unlock (); } +static void +add_class_info_to_baseline (MonoClass *klass, MonoClassMetadataUpdateInfo *klass_info) +{ + MonoImage *image = m_class_get_image (klass); + BaselineInfo *baseline_info = baseline_info_lookup (image); + baseline_info->klass_info = g_slist_prepend (baseline_info->klass_info, klass_info); +} + static MonoClassMetadataUpdateInfo * mono_class_get_or_add_metadata_update_info (MonoClass *klass) { @@ -706,6 +733,7 @@ mono_class_get_or_add_metadata_update_info (MonoClass *klass) info = mono_class_get_metadata_update_info (klass); if (!info) { info = mono_class_alloc0 (klass, sizeof (MonoClassMetadataUpdateInfo)); + add_class_info_to_baseline (klass, info); mono_class_set_metadata_update_info (klass, info); } mono_loader_unlock (); From ebeb21e0a8d68eee7f4b0e453b4d0b9a87f7a2e8 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 3 Jan 2022 09:08:11 -0500 Subject: [PATCH 18/45] WIP: placeholder struct for runtime class data; instance offset placeholder --- src/mono/mono/component/hot_reload-internals.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index 2347827ceefe4..67bd42f25fbed 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -21,6 +21,10 @@ struct _MonoClassMetadataUpdateInfo { GArray *added_members; /* a set of Method or Field table tokens of any methods or fields added to this class */ GPtrArray *added_fields; /* a set of MonoClassMetadataUpdateField* values for every added field. */ + + struct _MonoClassRuntimeMetadataUpdateInfo { + + } runtime; }; typedef struct _MonoClassMetadataUpdateField { @@ -28,6 +32,10 @@ typedef struct _MonoClassMetadataUpdateField { uint32_t generation; /* when this field was added */ uint32_t token; /* the Field table token where this field was defined. (this won't make * sense for generic instances, once EnC is supported there) */ + /* if non-zero the EnC update came before the parent class was initialized. The field is + * stored in the instance at this offset. MonoClassField:offset is -1. Not used for static + * fields. */ + int before_init_instance_offset; } MonoClassMetadataUpdateField; #endif/*_MONO_COMPONENT_HOT_RELOAD_INTERNALS_H*/ From 5fc749677225d19fb0de73668a8adb3dcf333463 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 4 Jan 2022 23:07:05 -0500 Subject: [PATCH 19/45] WIP: static field storage Need to store the fields in a GC root, and in pinned allocations. --- .../src/Mono/HotReloadInstanceFieldTable.cs | 72 ++++++++++--------- .../mono/component/hot_reload-internals.h | 9 ++- 2 files changed, 47 insertions(+), 34 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs index b41e48212f3fc..fc31a88cc48ac 100644 --- a/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs +++ b/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs @@ -6,9 +6,9 @@ using System.Reflection; using System.Runtime.CompilerServices; -namespace Mono; +namespace Mono.HotReload; -internal class HotReloadInstanceFieldTable +internal 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. @@ -24,7 +24,7 @@ internal class HotReloadInstanceFieldTable // for reference types, the storage just stores it as an object. For valuetypes and // primitives, the storage stores the value as a boxed value. // - // The whole thing is basically a ConditionalWeakTable> but + // The whole thing is basically a ConditionalWeakTable> but // with locking on the inner dictionary. // @@ -32,7 +32,7 @@ internal class HotReloadInstanceFieldTable // 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 Store GetInstanceFieldStore(object inst, RuntimeTypeHandle type, uint fielddef_token) + internal static FieldStore GetInstanceFieldFieldStore(object inst, RuntimeTypeHandle type, uint fielddef_token) => _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(type, fielddef_token); private static HotReloadInstanceFieldTable _singleton = new(); @@ -49,7 +49,7 @@ private InstanceFields GetOrCreateInstanceFields(object key) private class InstanceFields { - private Dictionary _fields; + private Dictionary _fields; private object _lock; public InstanceFields() @@ -57,48 +57,54 @@ public InstanceFields() _fields = new(); _lock = new(); } - - public Store LookupOrAdd(RuntimeTypeHandle type, uint key) +x1 + public FieldStore LookupOrAdd(RuntimeTypeHandle type, uint key) { - if (_fields.TryGetValue(key, out Store? v)) + if (_fields.TryGetValue(key, out FieldStore? v)) return v; lock (_lock) { - if (_fields.TryGetValue (key, out Store? v2)) + if (_fields.TryGetValue (key, out FieldStore? v2)) return v2; - Store s = Store.Create(type); + FieldStore s = FieldStore.Create(type); _fields.Add(key, s); return s; } } } - // This is similar to System.Diagnostics.EditAndContinueHelper in CoreCLR, except instead of - // having the allocation logic in native (see EditAndContinueModule::ResolveOrAllocateField, - // and EnCSyncBlockInfo::ResolveOrAllocateField), the logic is in managed. - internal class Store - { - private object? _loc; +} - private Store (object? loc) - { - _loc = loc; - } +// This is similar to System.Diagnostics.EditAndContinueHelper in CoreCLR, except instead of +// having the allocation logic in native (see EditAndContinueModule::ResolveOrAllocateField, +// and EnCSyncBlockInfo::ResolveOrAllocateField), the logic is in managed. +// +// Additionally Mono uses this for storing added static fields. +internal class FieldStore +{ + // keep in sync with hot_reload-internals.h + private object? _loc; - public object? Location => _loc; + private FieldStore (object? loc) + { + _loc = loc; + } - public static Store Create (RuntimeTypeHandle type) - { - Type t = Type.GetTypeFromHandle(type) ?? throw new ArgumentException(nameof(type), "Type handle was null"); - object? loc; - if (t.IsPrimitive || t.IsValueType) - loc = RuntimeHelpers.GetUninitializedObject(t); - else if (t.IsClass || t.IsInterface) - loc = null; - else - throw new ArgumentException("EnC: Expected a primitive, valuetype, class or interface field"); - return new Store(loc); - } + public object? Location => _loc; + + public static FieldStore Create (RuntimeTypeHandle type) + { + Type t = Type.GetTypeFromHandle(type) ?? throw new ArgumentException(nameof(type), "Type handle was null"); + object? loc; + if (t.IsPrimitive || t.IsValueType) + loc = RuntimeHelpers.GetUninitializedObject(t); + else if (t.IsClass || t.IsInterface) + loc = null; + else + throw new ArgumentException("EnC: Expected a primitive, valuetype, class or interface field"); + /* FIXME: do we want FieldStore to be pinned? */ + return new FieldStore(loc); } } + diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index 67bd42f25fbed..2ad3cda41b57c 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -23,10 +23,17 @@ struct _MonoClassMetadataUpdateInfo { GPtrArray *added_fields; /* a set of MonoClassMetadataUpdateField* values for every added field. */ struct _MonoClassRuntimeMetadataUpdateInfo { - + MonoCoopMutex *static_fields_lock; /* protects the static_fields hashtable. Values can be used outside the lock (since they're allocated pinned). */ + MonoGHashTable *static_fields; /* key is field token, value is a pinned managed object: either a boxed valuetype (the static field address is the value address) or a Mono.HotReload.FieldStore object (in which case the static field address is the address of the _loc field in the object.) */ } runtime; }; +/* Keep in sync with Mono.HotReload.FieldStore in managed */ +typedef struct _MonoHotReloadFieldStoreObject { + MonoObject object; + MonoObject *_loc; +} MonoHotReloadFieldStoreObject; + typedef struct _MonoClassMetadataUpdateField { MonoClassField field; uint32_t generation; /* when this field was added */ From 7d4c8f04c367325036b7036a2c1222a4fe87f27a Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Thu, 6 Jan 2022 13:52:29 -0500 Subject: [PATCH 20/45] Implement hot_reload_get_static_field_addr() --- .../src/ILLink/ILLink.Descriptors.xml | 3 + .../src/Mono/HotReloadInstanceFieldTable.cs | 9 +- .../mono/component/hot_reload-internals.h | 13 ++- src/mono/mono/component/hot_reload.c | 93 ++++++++++++++++++- 4 files changed, 107 insertions(+), 11 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 4a8ffce034109..2f578bdf50ab6 100644 --- a/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml +++ b/src/mono/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.xml @@ -647,5 +647,8 @@ + + + diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs index fc31a88cc48ac..02820b7dd6da1 100644 --- a/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs +++ b/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Reflection; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; namespace Mono.HotReload; @@ -35,11 +36,11 @@ internal class InstanceFieldTable internal static FieldStore GetInstanceFieldFieldStore(object inst, RuntimeTypeHandle type, uint fielddef_token) => _singleton.GetOrCreateInstanceFields(inst).LookupOrAdd(type, fielddef_token); - private static HotReloadInstanceFieldTable _singleton = new(); + private static InstanceFieldTable _singleton = new(); private ConditionalWeakTable _table; - private HotReloadInstanceFieldTable() + private InstanceFieldTable() { _table = new(); } @@ -57,7 +58,7 @@ public InstanceFields() _fields = new(); _lock = new(); } -x1 + public FieldStore LookupOrAdd(RuntimeTypeHandle type, uint key) { if (_fields.TryGetValue(key, out FieldStore? v)) @@ -81,6 +82,7 @@ public FieldStore LookupOrAdd(RuntimeTypeHandle type, uint key) // and EnCSyncBlockInfo::ResolveOrAllocateField), the logic is in managed. // // Additionally Mono uses this for storing added static fields. +[StructLayout(LayoutKind.Sequential)] internal class FieldStore { // keep in sync with hot_reload-internals.h @@ -107,4 +109,3 @@ public static FieldStore Create (RuntimeTypeHandle type) return new FieldStore(loc); } } - diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index 2ad3cda41b57c..a69a632660b32 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -10,6 +10,13 @@ #include "mono/metadata/metadata-internals.h" #include "mono/metadata/class-internals.h" +/* Execution-time info for an updated class. */ +typedef struct _MonoClassRuntimeMetadataUpdateInfo { + MonoCoopMutex static_fields_lock; /* protects the static_fields hashtable. Values can be used outside the lock (since they're allocated pinned). */ + MonoGHashTable *static_fields; /* key is field token, value is a pinned managed object: either a boxed valuetype (the static field address is the value address) or a Mono.HotReload.FieldStore object (in which case the static field address is the address of the _loc field in the object.) */ + gboolean inited; +} MonoClassRuntimeMetadataUpdateInfo; + /* Class-specific metadata update info. See * mono_class_get_metadata_update_info() Note that this info is associated with * class _definitions_ that can be edited, so primitives, generic instances, @@ -22,12 +29,10 @@ struct _MonoClassMetadataUpdateInfo { GPtrArray *added_fields; /* a set of MonoClassMetadataUpdateField* values for every added field. */ - struct _MonoClassRuntimeMetadataUpdateInfo { - MonoCoopMutex *static_fields_lock; /* protects the static_fields hashtable. Values can be used outside the lock (since they're allocated pinned). */ - MonoGHashTable *static_fields; /* key is field token, value is a pinned managed object: either a boxed valuetype (the static field address is the value address) or a Mono.HotReload.FieldStore object (in which case the static field address is the address of the _loc field in the object.) */ - } runtime; + MonoClassRuntimeMetadataUpdateInfo runtime; }; + /* Keep in sync with Mono.HotReload.FieldStore in managed */ typedef struct _MonoHotReloadFieldStoreObject { MonoObject object; diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index bb90758fbdd78..6296b4a2633a9 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -12,6 +12,7 @@ #include #include "mono/metadata/assembly-internals.h" +#include "mono/metadata/mono-hash-internals.h" #include "mono/metadata/metadata-internals.h" #include "mono/metadata/metadata-update.h" #include "mono/metadata/object-internals.h" @@ -2447,15 +2448,101 @@ metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, Basel return field; } +static void +ensure_class_runtime_info_inited (MonoClass *klass, MonoClassRuntimeMetadataUpdateInfo *runtime_info) +{ + if (runtime_info->inited) + return; + mono_loader_lock (); + if (runtime_info->inited) { + mono_loader_unlock (); + return; + } + + mono_coop_mutex_init (&runtime_info->static_fields_lock); + + /* FIXME: is it ok to re-use MONO_ROOT_SOURCE_STATIC here? */ + runtime_info->static_fields = mono_g_hash_table_new_type_internal (NULL, NULL, MONO_HASH_VALUE_GC, MONO_ROOT_SOURCE_STATIC, NULL, "Hot Reload Static Fields"); + + runtime_info->inited = TRUE; + + mono_loader_unlock (); +} + +static void +class_runtime_info_static_fields_lock (MonoClassRuntimeMetadataUpdateInfo *runtime_info) +{ + mono_coop_mutex_lock (&runtime_info->static_fields_lock); +} + +static void +class_runtime_info_static_fields_unlock (MonoClassRuntimeMetadataUpdateInfo *runtime_info) +{ + mono_coop_mutex_unlock (&runtime_info->static_fields_lock); +} + +static GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_field_store); + +GENERATE_GET_CLASS_WITH_CACHE(hot_reload_field_store, "Mono.HotReload", "FieldStore"); + + +static MonoObject* +create_static_field_storage (MonoType *t, MonoError *error) +{ + MonoClass *klass; + if (!mono_type_is_reference (t)) + klass = mono_class_from_mono_type_internal (t); + else + klass = mono_class_get_hot_reload_field_store_class (); + + return mono_object_new_pinned (klass, error); +} + static gpointer hot_reload_get_static_field_addr (MonoClassField *field) { g_assert (m_field_is_from_update (field)); MonoClassMetadataUpdateField *f = (MonoClassMetadataUpdateField *)field; g_assert ((f->field.type->attrs & FIELD_ATTRIBUTE_STATIC) != 0); - // FIXME: this needs to be GC-visible + g_assert (!m_type_is_byref(f->field.type)); // byref fields only in ref structs, which aren't allowed in EnC updates + + MonoClass *parent = m_field_get_parent (&f->field); + MonoClassMetadataUpdateInfo *parent_info = mono_class_get_or_add_metadata_update_info (parent); + MonoClassRuntimeMetadataUpdateInfo *runtime_info = &parent_info->runtime; + + ensure_class_runtime_info_inited (parent, runtime_info); + + MonoObject *obj = NULL; + class_runtime_info_static_fields_lock (runtime_info); + obj = (MonoObject*) mono_g_hash_table_lookup (runtime_info->static_fields, GUINT_TO_POINTER (f->token)); + class_runtime_info_static_fields_unlock (runtime_info); + if (!obj) { + ERROR_DECL (error); + obj = create_static_field_storage (f->field.type, error); + class_runtime_info_static_fields_lock (runtime_info); + mono_error_assert_ok (error); + MonoObject *obj2 = (MonoObject*) mono_g_hash_table_lookup (runtime_info->static_fields, GUINT_TO_POINTER (f->token)); + if (!obj2) { + // Noone else created it, use ours + mono_g_hash_table_insert_internal (runtime_info->static_fields, GUINT_TO_POINTER (f->token), obj); + } else { + /* beaten by another thread, silently drop our storage object and use theirs */ + obj = obj2; + } + class_runtime_info_static_fields_unlock (runtime_info); + } + g_assert (obj); - g_assert_not_reached (); // FIXME: finish me + gpointer addr = NULL; + if (!mono_type_is_reference (f->field.type)) { + // object is just the boxed value + addr = mono_object_unbox_internal (obj); + } else { + // object is a Mono.HotReload.FieldStore, and the static field value is obj._loc + MonoHotReloadFieldStoreObject *store = (MonoHotReloadFieldStoreObject *)obj; + addr = (gpointer)&store->_loc; + } + g_assert (addr); - return NULL; + return addr; } From 30c075a1659fbb2b4c26a4f9a63580a6cebb0128 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jan 2022 02:02:33 -0500 Subject: [PATCH 21/45] Add mono_metadata_update_find_method_by_name Now we can find .cctor This is enough that a basic sample with an added static lambda works --- .../tests/ApplyUpdateTest.cs | 3 +- src/mono/mono/component/hot_reload-stub.c | 11 ++++ src/mono/mono/component/hot_reload.c | 51 ++++++++++++++++++- src/mono/mono/component/hot_reload.h | 1 + src/mono/mono/metadata/class.c | 14 ++++- src/mono/mono/metadata/metadata-update.c | 6 +++ src/mono/mono/metadata/metadata-update.h | 3 ++ 7 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 7e116f6b2fa2a..cf27a019c0b99 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -17,7 +17,6 @@ namespace System.Reflection.Metadata [Collection(nameof(DisableParallelization))] public class ApplyUpdateTest { -#if false [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] void StaticMethodBodyUpdate() @@ -303,8 +302,8 @@ public static void TestAddStaticField() Assert.Equal("4567", result); }); } -#endif + [ActiveIssue("no instance fields", TestRuntimes.Mono)] [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddNestedClass() { diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 7c316b045f9fa..d3801a8e7089b 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -86,6 +86,9 @@ hot_reload_stub_get_field (MonoClass *klass, uint32_t fielddef_token); static gpointer hot_reload_stub_get_static_field_addr (MonoClassField *field); +static MonoMethod * +hot_reload_stub_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); + static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_stub_available }, &hot_reload_stub_set_fastpath_data, @@ -111,6 +114,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_get_field_idx, &hot_reload_stub_get_field, &hot_reload_stub_get_static_field_addr, + &hot_reload_stub_find_method_by_name, }; static bool @@ -263,6 +267,13 @@ hot_reload_get_static_field_addr (MonoClassField *field) return NULL; } +static MonoMethod * +hot_reload_stub_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error) +{ + 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 6296b4a2633a9..5f36ff5784a7c 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -117,10 +117,12 @@ hot_reload_get_field (MonoClass *klass, uint32_t fielddef_token); static gpointer hot_reload_get_static_field_addr (MonoClassField *field); +static MonoMethod * +hot_reload_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); + static MonoClassMetadataUpdateField * metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, BaselineInfo *base_info, uint32_t generation, DeltaInfo *delta_info, MonoClass *parent_klass, uint32_t fielddef_token, MonoError *error); - static MonoComponentHotReload fn_table = { { MONO_COMPONENT_ITF_VERSION, &hot_reload_available }, &hot_reload_set_fastpath_data, @@ -146,6 +148,7 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_field_idx, &hot_reload_get_field, &hot_reload_get_static_field_addr, + &hot_reload_find_method_by_name, }; MonoComponentHotReload * @@ -2546,3 +2549,49 @@ hot_reload_get_static_field_addr (MonoClassField *field) return addr; } + +static MonoMethod * +hot_reload_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error) +{ + GArray *arr = hot_reload_get_added_members (klass); + if (!arr) + return NULL; + + /* FIXME: locking if the array grows? */ + + MonoImage *image = m_class_get_image (klass); + int count = arr->len; + MonoMethod *res = NULL; + for (int i = 0; i < count; i++) { + uint32_t token = g_array_index (arr, uint32_t, i); + if (mono_metadata_token_table (token) != MONO_TABLE_METHOD) + continue; + uint32_t idx = mono_metadata_token_index (token); + uint32_t cols [MONO_METHOD_SIZE]; + mono_metadata_decode_table_row (image, MONO_TABLE_METHOD, idx - 1, cols, MONO_METHOD_SIZE); + + if (!strcmp (mono_metadata_string_heap (image, cols [MONO_METHOD_NAME]), name)) { + ERROR_DECL (local_error); + MonoMethod *method = mono_get_method_checked (image, MONO_TOKEN_METHOD_DEF | idx, klass, NULL, local_error); + if (!method) { + mono_error_cleanup (local_error); + continue; + } + if (param_count == -1) { + res = method; + break; + } + MonoMethodSignature *sig = mono_method_signature_checked (method, local_error); + if (!sig) { + mono_error_cleanup (error); + continue; + } + if ((method->flags & flags) == flags && sig->param_count == param_count) { + res = method; + break; + } + } + } + + return res; +} diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 204ea2427a815..a3faaa730814c 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -39,6 +39,7 @@ typedef struct _MonoComponentHotReload { uint32_t (*get_field_idx) (MonoClassField *field); MonoClassField* (*get_field) (MonoClass *klass, uint32_t fielddef_token); gpointer (*get_static_field_addr) (MonoClassField *field); + MonoMethod* (*find_method_by_name) (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); } MonoComponentHotReload; MONO_COMPONENT_EXPORT_ENTRYPOINT diff --git a/src/mono/mono/metadata/class.c b/src/mono/mono/metadata/class.c index 13213cc75ebb7..895a654cbfeaa 100644 --- a/src/mono/mono/metadata/class.c +++ b/src/mono/mono/metadata/class.c @@ -5720,6 +5720,14 @@ mono_find_method_in_metadata (MonoClass *klass, const char *name, int param_coun } } + if (G_UNLIKELY (!res && klass_image->has_updates)) { + if (mono_class_has_metadata_update_info (klass)) { + ERROR_DECL (error); + res = mono_metadata_update_find_method_by_name (klass, name, param_count, flags, error); + mono_error_cleanup (error); + } + } + return res; } @@ -5782,7 +5790,8 @@ mono_class_get_method_from_name_checked (MonoClass *klass, const char *name, FIXME we should better report this error to the caller */ MonoMethod **klass_methods = m_class_get_methods (klass); - if (!klass_methods) + gboolean has_updates = m_class_get_image (klass)->has_updates; + if (!klass_methods && !has_updates) return NULL; int mcount = mono_class_get_method_count (klass); for (i = 0; i < mcount; ++i) { @@ -5796,6 +5805,9 @@ mono_class_get_method_from_name_checked (MonoClass *klass, const char *name, break; } } + if (G_UNLIKELY (!res && has_updates && mono_class_has_metadata_update_info (klass))) { + res = mono_metadata_update_find_method_by_name (klass, name, param_count, flags, error); + } } else { res = mono_find_method_in_metadata (klass, name, param_count, flags); diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 2a1d5a891eece..f0f61bd2be75f 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -175,3 +175,9 @@ mono_metadata_update_get_static_field_addr (MonoClassField *field) { return mono_component_hot_reload()->get_static_field_addr (field); } + +MonoMethod * +mono_metadata_update_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error) +{ + return mono_component_hot_reload()->find_method_by_name (klass, name, param_count, flags, error); +} diff --git a/src/mono/mono/metadata/metadata-update.h b/src/mono/mono/metadata/metadata-update.h index f66b4bcb745e5..ee999a4f0a10f 100644 --- a/src/mono/mono/metadata/metadata-update.h +++ b/src/mono/mono/metadata/metadata-update.h @@ -61,6 +61,9 @@ mono_metadata_update_delta_heap_lookup (MonoImage *base_image, MetadataHeapGette void* mono_metadata_update_metadata_linear_search (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); +MonoMethod* +mono_metadata_update_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error); + uint32_t mono_metadata_update_get_field_idx (MonoClassField *field); From ab5143e4296128bfbe434154ba1422f73d28dab5 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jan 2022 14:14:05 -0500 Subject: [PATCH 22/45] Fix infinite loop --- src/mono/mono/metadata/custom-attrs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 498ed9a53ed99..6cf030bd29f88 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -1641,8 +1641,10 @@ mono_custom_attrs_from_index_checked (MonoImage *image, guint32 idx, gboolean ig /* if there are updates, the new custom attributes are not sorted, so we have to go until the end. */ if (G_LIKELY (!image->has_updates)) break; - else + else { + ++i; continue; + } } attr_array = g_array_append_val (attr_array, i); ++i; From 8c8bdd50d5a4df4cbfa26ea02169d936de8b22ea Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jan 2022 14:39:42 -0500 Subject: [PATCH 23/45] fix build --- src/mono/mono/component/hot_reload-stub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index d3801a8e7089b..0f73d672ad646 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -262,7 +262,7 @@ hot_reload_stub_get_field (MonoClass *klass, uint32_t fielddef_token) } static gpointer -hot_reload_get_static_field_addr (MonoClassField *field) +hot_reload_stub_get_static_field_addr (MonoClassField *field) { return NULL; } From 805b4dd1ceea2249def792916087306a35dac8c5 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jan 2022 18:11:56 -0500 Subject: [PATCH 24/45] fix dynamic components builds --- src/mono/mono/component/hot_reload-stub.c | 4 +- src/mono/mono/component/hot_reload.c | 59 +++++++++++++++++++++-- src/mono/mono/component/hot_reload.h | 3 +- src/mono/mono/metadata/class-internals.h | 2 +- src/mono/mono/metadata/metadata-update.c | 3 +- src/mono/mono/metadata/object-internals.h | 2 +- 6 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 0f73d672ad646..2351b1f6c7a72 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -21,7 +21,7 @@ static MonoComponentHotReload * component_hot_reload_stub_init (void); static void -hot_reload_stub_set_fastpath_data (MonoMetadataUpdateData *ptr); +hot_reload_stub_set_fastpath_data (MonoMetadataUpdateData *ptr, MonoDefaults *mono_defaults); static gboolean hot_reload_stub_update_enabled (int *modifiable_assemblies_out); @@ -130,7 +130,7 @@ component_hot_reload_stub_init (void) } void -hot_reload_stub_set_fastpath_data (MonoMetadataUpdateData *ptr) +hot_reload_stub_set_fastpath_data (MonoMetadataUpdateData *ptr, MonoDefaults *mono_defaults) { } diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 5f36ff5784a7c..018390f7772ff 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -46,7 +46,7 @@ static bool hot_reload_available (void); static void -hot_reload_set_fastpath_data (MonoMetadataUpdateData *data); +hot_reload_set_fastpath_data (MonoMetadataUpdateData *data, MonoDefaults *mono_defaults); static gboolean hot_reload_update_enabled (int *modifiable_assemblies_out); @@ -151,6 +151,56 @@ static MonoComponentHotReload fn_table = { &hot_reload_find_method_by_name, }; +static MonoDefaults *hr_mono_defaults; + +#define HR_GENERATE_GET_CLASS_WITH_CACHE_DECL(shortname) \ +MonoClass* mono_class_get_##shortname##_class (void); + +#define HR_GENERATE_TRY_GET_CLASS_WITH_CACHE_DECL(shortname) \ +MonoClass* mono_class_try_get_##shortname##_class (void); + +// GENERATE_GET_CLASS_WITH_CACHE attempts mono_class_load_from_name whenever +// its cache is null. i.e. potentially repeatedly, though it is expected to succeed +// the first time. +// +#define HR_GENERATE_GET_CLASS_WITH_CACHE(shortname,name_space,name) \ +MonoClass* \ +mono_class_get_##shortname##_class (void) \ +{ \ + static MonoClass *tmp_class; \ + MonoClass *klass = tmp_class; \ + if (!klass) { \ + klass = mono_class_load_from_name (hr_mono_defaults->corlib, name_space, name); \ + mono_memory_barrier (); /* FIXME excessive? */ \ + tmp_class = klass; \ + } \ + return klass; \ +} + +// GENERATE_TRY_GET_CLASS_WITH_CACHE attempts mono_class_load_from_name approximately +// only once. i.e. if it fails, it will return null and not retry. +// In a race it might try a few times, but not indefinitely. +// +// FIXME This maybe has excessive volatile/barriers. +// +#define HR_GENERATE_TRY_GET_CLASS_WITH_CACHE(shortname,name_space,name) \ +MonoClass* \ +mono_class_try_get_##shortname##_class (void) \ +{ \ + static volatile MonoClass *tmp_class; \ + static volatile gboolean inited; \ + MonoClass *klass = (MonoClass *)tmp_class; \ + mono_memory_barrier (); \ + if (!inited) { \ + klass = mono_class_try_load_from_name (hr_mono_defaults->corlib, name_space, name); \ + tmp_class = klass; \ + mono_memory_barrier (); \ + inited = TRUE; \ + } \ + return klass; \ +} + + MonoComponentHotReload * mono_component_hot_reload_init (void) { @@ -167,9 +217,10 @@ hot_reload_available (void) static MonoMetadataUpdateData* metadata_update_data_ptr; static void -hot_reload_set_fastpath_data (MonoMetadataUpdateData *ptr) +hot_reload_set_fastpath_data (MonoMetadataUpdateData *ptr, MonoDefaults *mono_defaults) { metadata_update_data_ptr = ptr; + hr_mono_defaults = mono_defaults; } /* TLS value is a uint32_t of the latest published generation that the thread can see */ @@ -2484,9 +2535,9 @@ class_runtime_info_static_fields_unlock (MonoClassRuntimeMetadataUpdateInfo *run mono_coop_mutex_unlock (&runtime_info->static_fields_lock); } -static GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_field_store); +static HR_GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_field_store); -GENERATE_GET_CLASS_WITH_CACHE(hot_reload_field_store, "Mono.HotReload", "FieldStore"); +static HR_GENERATE_GET_CLASS_WITH_CACHE(hot_reload_field_store, "Mono.HotReload", "FieldStore"); static MonoObject* diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index a3faaa730814c..80642071b922e 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -8,6 +8,7 @@ #include #include "mono/metadata/object-forward.h" #include "mono/metadata/metadata-internals.h" +#include "mono/metadata/class-internals.h" #include "mono/metadata/metadata-update.h" #include "mono/utils/bsearch.h" #include "mono/utils/mono-error.h" @@ -16,7 +17,7 @@ typedef struct _MonoComponentHotReload { MonoComponent component; - void (*set_fastpath_data) (MonoMetadataUpdateData *data); + void (*set_fastpath_data) (MonoMetadataUpdateData *data, MonoDefaults *mono_defaults); gboolean (*update_enabled) (int *modifiable_assemblies_out); gboolean (*no_inline) (MonoMethod *caller, MonoMethod *callee); uint32_t (*thread_expose_published) (void); diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index 44d2ce4b6b381..bb43ccde7dd4e 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1231,7 +1231,7 @@ mono_class_get_generic_container (MonoClass *klass); gpointer mono_class_alloc (MonoClass *klass, int size); -gpointer +MONO_COMPONENT_API gpointer mono_class_alloc0 (MonoClass *klass, int size); #define mono_class_alloc0(klass, size) (g_cast (mono_class_alloc0 ((klass), (size)))) diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index f0f61bd2be75f..8fe4eb9d154b5 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -12,6 +12,7 @@ #include "mono/metadata/metadata-update.h" #include "mono/metadata/components.h" +#include "mono/metadata/class-internals.h" #include "mono/component/hot_reload.h" gboolean @@ -27,7 +28,7 @@ mono_metadata_update_init (void) { memset (&mono_metadata_update_data_private, 0, sizeof (mono_metadata_update_data_private)); MonoComponentHotReload *comp = mono_component_hot_reload (); - comp->set_fastpath_data (&mono_metadata_update_data_private); + comp->set_fastpath_data (&mono_metadata_update_data_private, &mono_defaults); } gboolean diff --git a/src/mono/mono/metadata/object-internals.h b/src/mono/mono/metadata/object-internals.h index 6c6d089ee2122..c1c4d939df2a1 100644 --- a/src/mono/mono/metadata/object-internals.h +++ b/src/mono/mono/metadata/object-internals.h @@ -1666,7 +1666,7 @@ mono_class_set_ref_info (MonoClass *klass, MonoObjectHandle obj); void mono_class_free_ref_info (MonoClass *klass); -MonoObject * +MONO_COMPONENT_API MonoObject * mono_object_new_pinned (MonoClass *klass, MonoError *error); MonoObjectHandle From 6dbaa867a4e42cb91c38a26a462343d597bc33e2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 7 Jan 2022 18:39:11 -0500 Subject: [PATCH 25/45] fix windows build --- src/mono/mono/metadata/class-accessors.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/mono/metadata/class-accessors.c b/src/mono/mono/metadata/class-accessors.c index d51b57a811d60..e4fa09a40d190 100644 --- a/src/mono/mono/metadata/class-accessors.c +++ b/src/mono/mono/metadata/class-accessors.c @@ -619,7 +619,7 @@ mono_class_set_metadata_update_info (MonoClass *klass, MonoClassMetadataUpdateIn g_assertf (0, "%s: EnC metadata update info on generic types is not supported", __func__); break; case MONO_CLASS_DEF: - return set_pointer_property (klass, PROP_METADATA_UPDATE_INFO, value); + set_pointer_property (klass, PROP_METADATA_UPDATE_INFO, value); case MONO_CLASS_GINST: case MONO_CLASS_GPARAM: case MONO_CLASS_POINTER: From 9a572c2c5074b324da4537d5c96da99f96ac047a Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Mon, 10 Jan 2022 14:12:37 -0500 Subject: [PATCH 26/45] Fix inadvertent fallthru in previous fix --- src/mono/mono/metadata/class-accessors.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mono/mono/metadata/class-accessors.c b/src/mono/mono/metadata/class-accessors.c index e4fa09a40d190..609ad55b40494 100644 --- a/src/mono/mono/metadata/class-accessors.c +++ b/src/mono/mono/metadata/class-accessors.c @@ -620,6 +620,7 @@ mono_class_set_metadata_update_info (MonoClass *klass, MonoClassMetadataUpdateIn break; case MONO_CLASS_DEF: set_pointer_property (klass, PROP_METADATA_UPDATE_INFO, value); + return; case MONO_CLASS_GINST: case MONO_CLASS_GPARAM: case MONO_CLASS_POINTER: From f694875905d788398714ea944faded69cef9f54e Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 11:38:40 -0500 Subject: [PATCH 27/45] Report new capabilities NewTypeDefinition is not completely functional because adding instance fields is not supported yet. But adding classes with only static methods works. --- .../src/System/Reflection/Metadata/MetadataUpdater.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs index 973eb43d91ca7..3f20b2c7cd18a 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs @@ -55,7 +55,8 @@ public static void ApplyUpdate(Assembly assembly, ReadOnlySpan metadataDel private static string InitializeApplyUpdateCapabilities() { - return ApplyUpdateEnabled(justComponentCheck: 1) != 0 ? "Baseline" : string.Empty ; + const string caps = "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition"; + return ApplyUpdateEnabled(justComponentCheck: 1) != 0 ? caps : string.Empty ; } [MethodImpl (MethodImplOptions.InternalCall)] From ad51dd6b235c8cc43c45fa6969e9b2d408e2b92b Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 12:51:48 -0500 Subject: [PATCH 28/45] tests: describe what existing tests do, add placeholder static lambda test --- .../System.Runtime.Loader/tests/ApplyUpdateTest.cs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index cf27a019c0b99..8f5c0c4cc41f8 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -71,6 +71,7 @@ void LambdaBodyChange() [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] void LambdaCapturesThis() { + // Tests that changes to the body of a lambda that captures 'this' is supported. ApplyUpdateUtil.TestCase(static () => { var assm = typeof (ApplyUpdate.Test.LambdaCapturesThis).Assembly; @@ -266,6 +267,7 @@ public void AsyncMethodChanges() [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddLambdaCapturingThis() { + // Test that adding a lambda that captures 'this' (to a method that already has a lambda that captures 'this') is supported ApplyUpdateUtil.TestCase(static () => { var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis).Assembly; @@ -284,6 +286,7 @@ public static void TestAddLambdaCapturingThis() [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddStaticField() { + // Test that adding a new static field to an existing class is supported ApplyUpdateUtil.TestCase(static () => { var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField).Assembly; @@ -307,6 +310,7 @@ public static void TestAddStaticField() [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddNestedClass() { + // Test that adding a new nested class to an existing class is supported ApplyUpdateUtil.TestCase(static () => { var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass).Assembly; @@ -325,6 +329,15 @@ public static void TestAddNestedClass() }); } + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestAddStaticLambda() + { + // Test that adding a new static lambda to an existing method body is supported + ApplyUpdateUtil.TestCase(static () => + { + Assert.True (false); + }); + } class NonRuntimeAssembly : Assembly { From d4e506ecb9f694e5f172e58e7f89ebaec7acaa02 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 14:28:26 -0500 Subject: [PATCH 29/45] tests: Add AddStaticLambda test case Adding a brand new static lambda to an existing method body is supported --- .../AddStaticLambda.cs | 17 +++++++++++++++++ .../AddStaticLambda_v1.cs | 17 +++++++++++++++++ ...data.ApplyUpdate.Test.AddStaticLambda.csproj | 11 +++++++++++ .../deltascript.json | 6 ++++++ .../tests/ApplyUpdateTest.cs | 14 +++++++++++++- .../tests/System.Runtime.Loader.Tests.csproj | 1 + 6 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda.csproj create mode 100644 src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/deltascript.json diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs new file mode 100644 index 0000000000000..b4d2e5a901361 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs @@ -0,0 +1,17 @@ +// 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 AddStaticLambda + { + public string TestMethod () { + return "abcd"; + } + + public string Double(Func f) => f() + f(); + + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs new file mode 100644 index 0000000000000..ca02f2f98429d --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs @@ -0,0 +1,17 @@ +// 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 AddStaticLambda + { + public string TestMethod () { + return Double (static () => "abcd"); + } + + public string Double(Func f) => f() + f(); + + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda.csproj new file mode 100644 index 0000000000000..3b402585aa298 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda.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.AddStaticLambda/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/deltascript.json new file mode 100644 index 0000000000000..1d60513f4a43f --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/deltascript.json @@ -0,0 +1,6 @@ +{ + "changes": [ + {"document": "AddStaticLambda.cs", "update": "AddStaticLambda_v1.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 8f5c0c4cc41f8..fcc7054a23ab6 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -335,7 +335,19 @@ public static void TestAddStaticLambda() // Test that adding a new static lambda to an existing method body is supported ApplyUpdateUtil.TestCase(static () => { - Assert.True (false); + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda).Assembly; + + var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda(); + + var r = x.TestMethod(); + + Assert.Equal ("abcd", r); + + ApplyUpdateUtil.ApplyUpdate(assm); + + r = x.TestMethod(); + + Assert.Equal("abcdabcd", r); }); } 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 cc0abc2ab0f11..e21bc7e673f5f 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 @@ -55,6 +55,7 @@ + From b84b38b659baad0efdf978bc09de916575313935 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 14:50:18 -0500 Subject: [PATCH 30/45] Revert "WIP: adding static fields" changes to sample --- src/mono/sample/mbr/console/TestClass_v1.cs | 22 ++------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/src/mono/sample/mbr/console/TestClass_v1.cs b/src/mono/sample/mbr/console/TestClass_v1.cs index 51a4d42a2b1a0..96bbebd87b7b4 100644 --- a/src/mono/sample/mbr/console/TestClass_v1.cs +++ b/src/mono/sample/mbr/console/TestClass_v1.cs @@ -4,34 +4,16 @@ public class TestClass { [MethodImpl(MethodImplOptions.NoInlining)] public static string TargetMethod () { -#if ADD_INNER_INST +#if false var o = new Inner(); return o.GetIt(); #endif -#if true return NewFunc (() => "NEW STRING"); -#endif -#if ADD_MUTUALS - return ClassOne.F1.s; -#endif } -#if true public static string NewFunc(Func f) => f (); -#endif - -#if ADD_MUTUALS - public class ClassOne { - public static ClassTwo F1; - } - - public class ClassTwo { - public static ClassOne F2; - public string s; - } -#endif -#if ADD_INNER_INST +#if false public class Inner { public string GetIt() => "NEW STRING"; } From 10d83a05489a3d02e5e98e60baecbf9455bfa1a9 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 14:51:03 -0500 Subject: [PATCH 31/45] Revert changes to mbr sample New code is tested by new tests --- src/mono/sample/mbr/console/Makefile | 7 +- src/mono/sample/mbr/console/Program.cs | 87 +++++++++++++++++++-- src/mono/sample/mbr/console/TestClass.cs | 1 + src/mono/sample/mbr/console/TestClass_v1.cs | 16 +--- 4 files changed, 85 insertions(+), 26 deletions(-) diff --git a/src/mono/sample/mbr/console/Makefile b/src/mono/sample/mbr/console/Makefile index cf4d46e810e2f..ee47d81f09b5c 100644 --- a/src/mono/sample/mbr/console/Makefile +++ b/src/mono/sample/mbr/console/Makefile @@ -5,7 +5,7 @@ DOTNET_Q_ARGS=--nologo -v:q -consoleloggerparameters:NoSummary # How to build the project. For hot reload this must be Debug CONFIG ?=Debug # How was dotnet/runtime built? should be the same as build.sh -c option -BUILT_RUNTIME_CONFIG ?= Debug +BUILT_RUNTIME_CONFIG ?= Release MONO_ARCH=x64 OS := $(shell uname -s) @@ -17,17 +17,12 @@ endif MONO_ENV_OPTIONS = --interp -ifneq ($V,0) -MONO_VERBOSE_LOGGING=MONO_LOG_LEVEL=debug MONO_LOG_MASK=metadata-update -endif - publish: $(DOTNET) publish -c $(CONFIG) -r $(TARGET_OS)-$(MONO_ARCH) -p:BuiltRuntimeConfiguration=$(BUILT_RUNTIME_CONFIG) run: publish COMPlus_DebugWriteToStdErr=1 \ MONO_ENV_OPTIONS="$(MONO_ENV_OPTIONS)" \ - $(MONO_VERBOSE_LOGGING) \ DOTNET_MODIFIABLE_ASSEMBLIES=debug \ $(TOP)artifacts/bin/ConsoleDelta/$(MONO_ARCH)/$(CONFIG)/$(TARGET_OS)-$(MONO_ARCH)/publish/ConsoleDelta diff --git a/src/mono/sample/mbr/console/Program.cs b/src/mono/sample/mbr/console/Program.cs index f92dbd5e1550a..01fba0d8e4ee1 100644 --- a/src/mono/sample/mbr/console/Program.cs +++ b/src/mono/sample/mbr/console/Program.cs @@ -11,6 +11,43 @@ namespace HelloWorld { internal class Program { + class State { + public readonly ManualResetEventSlim mreIn; + public readonly ManualResetEventSlim mreOut; + public readonly ManualResetEventSlim mreBusy; + public string res; + private volatile bool busyChanged; + + public State() { + mreIn = new ManualResetEventSlim (); + mreOut = new ManualResetEventSlim (); + mreBusy = new ManualResetEventSlim (); + res = ""; + busyChanged = false; + } + + + public bool BusyChanged {get => busyChanged ; set { busyChanged = value; mreBusy.Set ();} } + + public void WaitForBusy () { + mreBusy.Wait (); + mreBusy.Reset (); + } + + public string ConsumerStep () { + mreIn.Set (); + mreOut.Wait (); + mreOut.Reset (); + return res; + } + + public void ProducerStep (Func step) { + mreIn.Wait (); + mreIn.Reset (); + res = step (); + mreOut.Set (); + } + } private static int Main(string[] args) { @@ -23,22 +60,58 @@ private static int Main(string[] args) Assembly assm = typeof (TestClass).Assembly; var replacer = DeltaHelper.Make (); - var s = TestClass.TargetMethod (); + var st = new State (); + var t = new Thread (MutatorThread); + t.Start (st); + var t2 = new Thread (BusyThread) { IsBackground = true }; + t2.Start (st); - Console.WriteLine (s); + string res = st.ConsumerStep (); + if (res != "OLD STRING") + return 1; replacer.Update (assm); - s = TestClass.TargetMethod (); - - Console.WriteLine (s); - - if (s != "NEW STRING") + res = st.ConsumerStep (); + if (res != "NEW STRING") return 2; + st.WaitForBusy (); + Console.WriteLine ("BusyChanged: {0}", st.BusyChanged); + return 0; } + private static void MutatorThread (object o) + { + var st = (State)o; + static string Step () => TestClass.TargetMethod (); + st.ProducerStep (Step); + st.ProducerStep (Step); + } + + // This method is not affected by the update, but it calls the target + // method which is. Still we expect to see "BusyThread" and its + // callees show up in the trace output when it safepoints during an + // update. + private static void BusyThread (object o) + { + State st = (State)o; + string prev = TestClass.TargetMethod (); + while (true) { + Thread.Sleep (0); + for (int i = 0; i < 5000; ++i) { + if (i % 1000 == 0) { + string cur = TestClass.TargetMethod (); + if (cur != prev) { + st.BusyChanged = true; + prev = cur; + } + } + } + } + } + } } diff --git a/src/mono/sample/mbr/console/TestClass.cs b/src/mono/sample/mbr/console/TestClass.cs index 7884de75ec4fe..c18db33b47257 100644 --- a/src/mono/sample/mbr/console/TestClass.cs +++ b/src/mono/sample/mbr/console/TestClass.cs @@ -5,6 +5,7 @@ public class TestClass { [MethodImpl(MethodImplOptions.NoInlining)] public static string TargetMethod () { string s = "OLD STRING"; + Console.WriteLine (s); return s; } } diff --git a/src/mono/sample/mbr/console/TestClass_v1.cs b/src/mono/sample/mbr/console/TestClass_v1.cs index 96bbebd87b7b4..a41793c3f7dcc 100644 --- a/src/mono/sample/mbr/console/TestClass_v1.cs +++ b/src/mono/sample/mbr/console/TestClass_v1.cs @@ -4,18 +4,8 @@ public class TestClass { [MethodImpl(MethodImplOptions.NoInlining)] public static string TargetMethod () { -#if false - var o = new Inner(); - return o.GetIt(); -#endif - return NewFunc (() => "NEW STRING"); + string s = "NEW STRING"; + Console.WriteLine (s); + return s; } - - public static string NewFunc(Func f) => f (); - -#if false - public class Inner { - public string GetIt() => "NEW STRING"; - } -#endif } From a445e5bef661211fb33db0cf3fdcee5a7f38fd1a Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 14:57:12 -0500 Subject: [PATCH 32/45] fix whitespace --- .../AddLambdaCapturingThis.cs | 22 +-- .../AddLambdaCapturingThis_v1.cs | 24 ++-- .../AddStaticField.cs | 14 +- .../AddStaticField_v1.cs | 18 +-- .../AddStaticLambda.cs | 3 +- .../AddStaticLambda_v1.cs | 5 +- .../tests/ApplyUpdateTest.cs | 126 +++++++++--------- 7 files changed, 109 insertions(+), 103 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis.cs index 186144e290e35..a5ec0d11b1f5c 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis.cs @@ -7,19 +7,21 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test { public class AddLambdaCapturingThis { - public AddLambdaCapturingThis () { - field = "abcd"; - } + public AddLambdaCapturingThis() + { + field = "abcd"; + } - public string GetField => field; + public string GetField => field; - private string field; + private string field; - public string TestMethod () { - // capture 'this' but no locals - Func fn = s => field; - return "123"; - } + public string TestMethod() + { + // capture 'this' but no locals + Func fn = s => field; + return "123"; + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis_v1.cs index 42642b6a2479d..44ff73ab1d591 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis/AddLambdaCapturingThis_v1.cs @@ -7,19 +7,21 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test { public class AddLambdaCapturingThis { - public AddLambdaCapturingThis () { - field = "abcd"; - } + public AddLambdaCapturingThis() + { + field = "abcd"; + } - public string GetField => field; + public string GetField => field; - private string field; + private string field; - public string TestMethod () { - // capture 'this' but no locals - Func fn = s => field; - Func fn2 = s => "42" + s + field; - return fn2 ("123"); - } + public string TestMethod() + { + // capture 'this' but no locals + Func fn = s => field; + Func fn2 = s => "42" + s + field; + return fn2 ("123"); + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField.cs index 45c03cbb9f427..6ac1360c3581b 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField.cs @@ -7,16 +7,16 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test { public class AddStaticField { - public AddStaticField () { - } + public AddStaticField () { + } - public string GetField => s_field; + public string GetField => s_field; - private static string s_field; + private static string s_field; - public void TestMethod () { - s_field = "abcd"; - } + public void TestMethod () { + s_field = "abcd"; + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField_v1.cs index 1491b581a266b..f9282469a4fce 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField/AddStaticField_v1.cs @@ -7,19 +7,19 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test { public class AddStaticField { - public AddStaticField () { - } + public AddStaticField () { + } - public string GetField => s_field2; + public string GetField => s_field2; - private static string s_field; + private static string s_field; - private static string s_field2; + private static string s_field2; - public void TestMethod () { - s_field = "spqr"; - s_field2 = "4567"; - } + public void TestMethod () { + s_field = "spqr"; + s_field2 = "4567"; + } } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs index b4d2e5a901361..f7e2363296224 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs @@ -7,7 +7,8 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test { public class AddStaticLambda { - public string TestMethod () { + public string TestMethod() + { return "abcd"; } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs index ca02f2f98429d..65f82639ababb 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs @@ -7,8 +7,9 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test { public class AddStaticLambda { - public string TestMethod () { - return Double (static () => "abcd"); + public string TestMethod() + { + return Double(static () => "abcd"); } public string Double(Func f) => f() + f(); diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index fcc7054a23ab6..729793ebcdfb3 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -71,7 +71,7 @@ void LambdaBodyChange() [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] void LambdaCapturesThis() { - // Tests that changes to the body of a lambda that captures 'this' is supported. + // Tests that changes to the body of a lambda that captures 'this' is supported. ApplyUpdateUtil.TestCase(static () => { var assm = typeof (ApplyUpdate.Test.LambdaCapturesThis).Assembly; @@ -264,92 +264,92 @@ public void AsyncMethodChanges() }); } - [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] - public static void TestAddLambdaCapturingThis() - { - // Test that adding a lambda that captures 'this' (to a method that already has a lambda that captures 'this') is supported - ApplyUpdateUtil.TestCase(static () => - { - var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis).Assembly; + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestAddLambdaCapturingThis() + { + // Test that adding a lambda that captures 'this' (to a method that already has a lambda that captures 'this') is supported + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis).Assembly; - var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis(); + var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis(); - Assert.Equal("123", x.TestMethod()); + Assert.Equal("123", x.TestMethod()); - ApplyUpdateUtil.ApplyUpdate(assm); + ApplyUpdateUtil.ApplyUpdate(assm); - string result = x.TestMethod(); - Assert.Equal("42123abcd", result); - }); - } + string result = x.TestMethod(); + Assert.Equal("42123abcd", result); + }); + } - [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] - public static void TestAddStaticField() - { - // Test that adding a new static field to an existing class is supported - ApplyUpdateUtil.TestCase(static () => - { - var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField).Assembly; + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestAddStaticField() + { + // Test that adding a new static field to an existing class is supported + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField).Assembly; - var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField(); + var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField(); - x.TestMethod(); + x.TestMethod(); - Assert.Equal ("abcd", x.GetField); + Assert.Equal ("abcd", x.GetField); - ApplyUpdateUtil.ApplyUpdate(assm); + ApplyUpdateUtil.ApplyUpdate(assm); - x.TestMethod(); + x.TestMethod(); - string result = x.GetField; - Assert.Equal("4567", result); - }); - } + string result = x.GetField; + Assert.Equal("4567", result); + }); + } - [ActiveIssue("no instance fields", TestRuntimes.Mono)] - [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] - public static void TestAddNestedClass() - { - // Test that adding a new nested class to an existing class is supported - ApplyUpdateUtil.TestCase(static () => - { - var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass).Assembly; + [ActiveIssue("no instance fields", TestRuntimes.Mono)] + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestAddNestedClass() + { + // Test that adding a new nested class to an existing class is supported + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass).Assembly; + + var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass(); - var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass(); + var r = x.TestMethod(); - var r = x.TestMethod(); + Assert.Equal ("123", r); - Assert.Equal ("123", r); + ApplyUpdateUtil.ApplyUpdate(assm); - ApplyUpdateUtil.ApplyUpdate(assm); + r = x.TestMethod(); - r = x.TestMethod(); + Assert.Equal("123456", r); + }); + } - Assert.Equal("123456", r); - }); - } + [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] + public static void TestAddStaticLambda() + { + // Test that adding a new static lambda to an existing method body is supported + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda).Assembly; - [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] - public static void TestAddStaticLambda() - { - // Test that adding a new static lambda to an existing method body is supported - ApplyUpdateUtil.TestCase(static () => - { - var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda).Assembly; + var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda(); - var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda(); + var r = x.TestMethod(); - var r = x.TestMethod(); + Assert.Equal ("abcd", r); - Assert.Equal ("abcd", r); + ApplyUpdateUtil.ApplyUpdate(assm); - ApplyUpdateUtil.ApplyUpdate(assm); + r = x.TestMethod(); - r = x.TestMethod(); - - Assert.Equal("abcdabcd", r); - }); - } + Assert.Equal("abcdabcd", r); + }); + } class NonRuntimeAssembly : Assembly { From c58b0617ba52ce27a2edf8d85f5f4112010875de Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 15:04:34 -0500 Subject: [PATCH 33/45] fix whitespace and comments --- src/mono/mono/component/hot_reload.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 018390f7772ff..9e06080c47776 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1753,7 +1753,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen int token_index = mono_metadata_token_index (log_token); gboolean is_addition = token_index-1 >= delta_info->count[token_table].prev_gen_rows ; - + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "enclog i=%d: token=0x%08x (table=%s): %d:\t%s", i, log_token, mono_meta_table_name (token_table), func_code, (is_addition ? "ADD" : "UPDATE")); @@ -1911,7 +1911,7 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen break; } case MONO_TABLE_TYPEDEF: { -#ifdef ALLOW_CLASS_ADD +#ifdef ALLOW_CLASS_ADD if (is_addition) { /* Adding a new class. ok */ switch (func_code) { @@ -2122,20 +2122,6 @@ hot_reload_apply_changes (int origin, MonoImage *image_base, gconstpointer dmeta } mono_error_assert_ok (error); - /* TODO: Need to do the second half of EditAndContinueModule::ApplyEditAndContinue - that - * is, actually inform the execution engine about new methods and fields. In particular - * EditAndContinueModule::AddMethod (and EEClass::AddMethod) to store the MonoMethod on the - * class, and EditAndContinueModule::AddField (and EEClass::AddField) to store the new - * MonoClassField on the class. Also we will need the equivalent of - * EnCFieldDesc::GetAddress in the interpreter to get the field address on a MonoObject - * instance. - * - * Maybe a simpler design (than stashing a dependent handle in the sync block like CoreCLR - * do) is to CWT from the original object to an enc dictionary that will map added fields - * (key: token?) to values, and make transform.c change ldfld/stfld/ldsflda into an icall call to - * get the CWT target and lookup the token in there. - */ - MonoAssemblyLoadContext *alc = mono_image_get_alc (image_base); hot_reload_update_publish (alc, generation); @@ -2343,7 +2329,7 @@ static void add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClass *klass, uint32_t method_token, MonoDebugInformationEnc* pdb_address) { add_member_to_baseline (base_info, delta_info, klass, method_token); - + if (pdb_address) set_delta_method_debug_info (delta_info, method_token, pdb_address); } @@ -2366,7 +2352,7 @@ hot_reload_member_parent (MonoImage *base_image, uint32_t member_token) { /* make sure they passed a token, not just a table row index */ g_assert (mono_metadata_token_table (member_token) != 0); - + if (!base_image->has_updates) return 0; BaselineInfo *base_info = baseline_info_lookup (base_image); @@ -2404,7 +2390,7 @@ hot_reload_field_parent (MonoImage *base_image, uint32_t field_token) } -/* XXX HACK - keep in sync with locator_t in metadata/metadata.c */ +/* HACK - keep in sync with locator_t in metadata/metadata.c */ typedef struct { int idx; /* The index that we are trying to locate */ int col_idx; /* The index in the row where idx may be stored */ @@ -2496,7 +2482,7 @@ metadata_update_field_setup_basic_info_and_resolve (MonoImage *image_base, Basel if (!parent_info->added_fields) { parent_info->added_fields = g_ptr_array_new (); } - + g_ptr_array_add (parent_info->added_fields, field); return field; From 613527461abb381115ffd81e15e0bd637f347db2 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 15:07:58 -0500 Subject: [PATCH 34/45] Destroy the runtime part of MonoClassMetadataUpdateInfo, too --- src/mono/mono/component/hot_reload.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 9e06080c47776..8ebca103144ec 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -407,6 +407,13 @@ klass_info_destroy (gpointer value, gpointer user_data G_GNUC_UNUSED) /* The MonoClassMetadataUpdateField is allocated from the class mempool, don't free it here */ g_ptr_array_free (info->added_fields, TRUE); + if (info->runtime.static_fields) { + mono_g_hash_table_destroy (info->runtime.static_fields); + info->runtime.static_fields = NULL; + } + + mono_coop_mutex_destroy (&info->runtime.static_fields_lock); + /* The MonoClassMetadataUpdateInfo itself is allocated from the class mempool, don't free it here */ } From e0c3376c320d5d300c1cff0939e2358439df6e2a Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 15:08:53 -0500 Subject: [PATCH 35/45] fix whitespace --- .../src/System/Reflection/Metadata/MetadataUpdater.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs index 3f20b2c7cd18a..afac9bf5e88da 100644 --- a/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs +++ b/src/mono/System.Private.CoreLib/src/System/Reflection/Metadata/MetadataUpdater.cs @@ -55,7 +55,7 @@ public static void ApplyUpdate(Assembly assembly, ReadOnlySpan metadataDel private static string InitializeApplyUpdateCapabilities() { - const string caps = "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition"; + const string caps = "Baseline AddMethodToExistingType AddStaticFieldToExistingType NewTypeDefinition"; return ApplyUpdateEnabled(justComponentCheck: 1) != 0 ? caps : string.Empty ; } From a2b674e3e1e5b02eacfc3f40ce37f01a1f5c1c98 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 15:15:34 -0500 Subject: [PATCH 36/45] rename Mono.HotReload file it has more classes than just the instance table --- src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj | 2 +- .../src/Mono/{HotReloadInstanceFieldTable.cs => HotReload.cs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/mono/System.Private.CoreLib/src/Mono/{HotReloadInstanceFieldTable.cs => HotReload.cs} (100%) diff --git a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj index 62cbafcb8fc4a..da99b06e81b8e 100644 --- a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -290,11 +290,11 @@ + - diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs similarity index 100% rename from src/mono/System.Private.CoreLib/src/Mono/HotReloadInstanceFieldTable.cs rename to src/mono/System.Private.CoreLib/src/Mono/HotReload.cs From 2a0be0783657f16e8a0c23548409955b48d07469 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 15:17:16 -0500 Subject: [PATCH 37/45] tests: add ActiveIssue for supporting adding instance fields --- src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 729793ebcdfb3..47d754a4e769f 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -306,7 +306,7 @@ public static void TestAddStaticField() }); } - [ActiveIssue("no instance fields", TestRuntimes.Mono)] + [ActiveIssue("https://github.com/dotnet/runtime/issues/63643", TestRuntimes.Mono)] [ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))] public static void TestAddNestedClass() { From 08fa426bbb83bed5016e415efcae2813dbf22e78 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 15:24:25 -0500 Subject: [PATCH 38/45] ifdef out Mono.HotReload.InstanceFieldTable it's not ready yet --- src/mono/System.Private.CoreLib/src/Mono/HotReload.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs index 02820b7dd6da1..c1cd977ceb6b2 100644 --- a/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs +++ b/src/mono/System.Private.CoreLib/src/Mono/HotReload.cs @@ -9,6 +9,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 { // Q: Does CoreCLR EnC allow adding fields to a valuetype? @@ -76,6 +78,7 @@ 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 493341f88ae5a0e8e4933b2ff26d80d7d5fcc152 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 20:21:43 -0500 Subject: [PATCH 39/45] Remove get_added_members from hot reload component interface Doesn't seem necessary to expose this yet. It's only used in hot_reload.c right now --- src/mono/mono/component/hot_reload-stub.c | 10 ---------- src/mono/mono/component/hot_reload.c | 1 - src/mono/mono/component/hot_reload.h | 1 - 3 files changed, 12 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 2351b1f6c7a72..3298ad03db9b2 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -65,9 +65,6 @@ hot_reload_stub_has_modified_rows (const MonoTableInfo *table); static int hot_reload_stub_table_num_rows_slow (MonoImage *image, int table_index); -static GArray* -hot_reload_stub_get_added_members (MonoClass *klass); - static uint32_t hot_reload_stub_method_parent (MonoImage *image, uint32_t method_index); @@ -107,7 +104,6 @@ static MonoComponentHotReload fn_table = { &hot_reload_stub_get_updated_method_ppdb, &hot_reload_stub_has_modified_rows, &hot_reload_stub_table_num_rows_slow, - &hot_reload_stub_get_added_members, &hot_reload_stub_method_parent, &hot_reload_stub_metadata_linear_search, &hot_reload_stub_field_parent, @@ -224,12 +220,6 @@ hot_reload_stub_table_num_rows_slow (MonoImage *image, int table_index) g_assert_not_reached (); /* should always take the fast path */ } -static GArray* -hot_reload_stub_get_added_members (MonoClass *klass) -{ - return NULL; -} - static uint32_t hot_reload_stub_method_parent (MonoImage *image, uint32_t method_index) { diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 8ebca103144ec..597850c31e2ca 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -141,7 +141,6 @@ static MonoComponentHotReload fn_table = { &hot_reload_get_updated_method_ppdb, &hot_reload_has_modified_rows, &hot_reload_table_num_rows_slow, - &hot_reload_get_added_members, &hot_reload_method_parent, &hot_reload_metadata_linear_search, &hot_reload_field_parent, diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index 80642071b922e..d034f8c963eb4 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -33,7 +33,6 @@ typedef struct _MonoComponentHotReload { gpointer (*get_updated_method_ppdb) (MonoImage *base_image, uint32_t idx); gboolean (*has_modified_rows) (const MonoTableInfo *table); gboolean (*table_num_rows_slow) (MonoImage *base_image, int table_index); - GArray* (*get_added_members) (MonoClass *klass); uint32_t (*method_parent) (MonoImage *base_image, uint32_t method_index); void* (*metadata_linear_search) (MonoImage *base_image, MonoTableInfo *base_table, const void *key, BinarySearchComparer comparer); uint32_t (*field_parent) (MonoImage *base_image, uint32_t method_index); From 79f22439697417679505f5db72c3a118f73a355c Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 20:22:37 -0500 Subject: [PATCH 40/45] Change the AddStaticLambda sample to use Func To check that lambdas with parameters work --- .../AddStaticLambda.cs | 2 +- .../AddStaticLambda_v1.cs | 4 ++-- src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs index f7e2363296224..9e759e6075b5b 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda.cs @@ -12,7 +12,7 @@ public string TestMethod() return "abcd"; } - public string Double(Func f) => f() + f(); + public string Double(Func f) => f("") + f("1"); } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs index 65f82639ababb..6f9e94b08a7f4 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda/AddStaticLambda_v1.cs @@ -9,10 +9,10 @@ public class AddStaticLambda { public string TestMethod() { - return Double(static () => "abcd"); + return Double(static (s) => s + "abcd"); } - public string Double(Func f) => f() + f(); + public string Double(Func f) => f("") + f("1"); } } diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 47d754a4e769f..9e28894b6bbf3 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -347,7 +347,7 @@ public static void TestAddStaticLambda() r = x.TestMethod(); - Assert.Equal("abcdabcd", r); + Assert.Equal("abcd1abcd", r); }); } From d2f009aa01837ffd1a748d928c8fbe64857ce8db Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 11 Jan 2022 20:47:08 -0500 Subject: [PATCH 41/45] Use a mempool allocated GSlist for the added members This avoids invalidating iterators in case we add members on one thread while another thread is iterating. If it turns out we need random access, we can switch to a GArray with locking. But so far we only ever iterate. --- .../mono/component/hot_reload-internals.h | 2 +- src/mono/mono/component/hot_reload.c | 26 +++++++------------ src/mono/mono/metadata/loader-internals.h | 12 +++++++++ 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/mono/mono/component/hot_reload-internals.h b/src/mono/mono/component/hot_reload-internals.h index a69a632660b32..36d3606e17e4a 100644 --- a/src/mono/mono/component/hot_reload-internals.h +++ b/src/mono/mono/component/hot_reload-internals.h @@ -25,7 +25,7 @@ typedef struct _MonoClassRuntimeMetadataUpdateInfo { struct _MonoClassMetadataUpdateInfo { /* FIXME: use a struct that allocates out of the MonoClass mempool! or maybe add the GArray * to the BaselineInfo for the image and cleanup from there. */ - GArray *added_members; /* a set of Method or Field table tokens of any methods or fields added to this class */ + GSList *added_members; /* a set of Method or Field table tokens of any methods or fields added to this class, allocated from the MonoClass mempool */ GPtrArray *added_fields; /* a set of MonoClassMetadataUpdateField* values for every added field. */ diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 597850c31e2ca..2403a0a3cdfb0 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -96,7 +96,7 @@ hot_reload_has_modified_rows (const MonoTableInfo *table); static int hot_reload_table_num_rows_slow (MonoImage *image, int table_index); -static GArray* +static GSList* hot_reload_get_added_members (MonoClass *klass); static uint32_t @@ -402,7 +402,7 @@ static void klass_info_destroy (gpointer value, gpointer user_data G_GNUC_UNUSED) { MonoClassMetadataUpdateInfo *info = (MonoClassMetadataUpdateInfo *)value; - g_array_free (info->added_members, TRUE); + /* added_members is allocated from the class mempool, don't free it here */ /* The MonoClassMetadataUpdateField is allocated from the class mempool, don't free it here */ g_ptr_array_free (info->added_fields, TRUE); @@ -2321,13 +2321,8 @@ add_member_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClas base_info->member_parent = g_hash_table_new (g_direct_hash, g_direct_equal); } MonoClassMetadataUpdateInfo *klass_info = mono_class_get_or_add_metadata_update_info (klass); - /* FIXME: locking for readers/writers of the GArray */ - GArray *arr = klass_info->added_members; - if (!arr) { - arr = g_array_new (FALSE, FALSE, sizeof(uint32_t)); - klass_info->added_members = arr; - } - g_array_append_val (arr, member_token); + GSList *members = klass_info->added_members; + klass_info->added_members = g_slist_prepend_mem_manager (m_class_get_mem_manager (klass), members, GUINT_TO_POINTER (member_token)); g_hash_table_insert (base_info->member_parent, GUINT_TO_POINTER (member_token), GUINT_TO_POINTER (m_class_get_type_token (klass))); } @@ -2340,7 +2335,7 @@ add_method_to_baseline (BaselineInfo *base_info, DeltaInfo *delta_info, MonoClas set_delta_method_debug_info (delta_info, method_token, pdb_address); } -static GArray* +static GSList* hot_reload_get_added_members (MonoClass *klass) { /* FIXME: locking for the GArray? */ @@ -2596,17 +2591,14 @@ hot_reload_get_static_field_addr (MonoClassField *field) static MonoMethod * hot_reload_find_method_by_name (MonoClass *klass, const char *name, int param_count, int flags, MonoError *error) { - GArray *arr = hot_reload_get_added_members (klass); - if (!arr) + GSList *members = hot_reload_get_added_members (klass); + if (!members) return NULL; - /* FIXME: locking if the array grows? */ - MonoImage *image = m_class_get_image (klass); - int count = arr->len; MonoMethod *res = NULL; - for (int i = 0; i < count; i++) { - uint32_t token = g_array_index (arr, uint32_t, i); + for (GSList *ptr = members; ptr; ptr = ptr->next) { + uint32_t token = GPOINTER_TO_UINT(ptr->data); if (mono_metadata_token_table (token) != MONO_TABLE_METHOD) continue; uint32_t idx = mono_metadata_token_index (token); diff --git a/src/mono/mono/metadata/loader-internals.h b/src/mono/mono/metadata/loader-internals.h index bc79d38d4eb5b..772c2dc875ebb 100644 --- a/src/mono/mono/metadata/loader-internals.h +++ b/src/mono/mono/metadata/loader-internals.h @@ -356,6 +356,18 @@ mono_mem_manager_get_generic (MonoImage **images, int nimages); MonoMemoryManager* mono_mem_manager_merge (MonoMemoryManager *mm1, MonoMemoryManager *mm2); +static inline GSList* +g_slist_prepend_mem_manager (MonoMemoryManager *memory_manager, GSList *list, gpointer data) +{ + GSList *new_list; + + new_list = (GSList *) mono_mem_manager_alloc (memory_manager, sizeof (GSList)); + new_list->data = data; + new_list->next = list; + + return new_list; +} + G_END_DECLS #endif From f83ecac75174df94712f2018553cc27691d81ff0 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Fri, 14 Jan 2022 14:22:37 -0500 Subject: [PATCH 42/45] Use mono_get_corlib instead of passing MonoDefaults to hot_reload --- src/mono/mono/component/hot_reload-stub.c | 4 ++-- src/mono/mono/component/hot_reload.c | 11 ++++------- src/mono/mono/component/hot_reload.h | 2 +- src/mono/mono/metadata/metadata-update.c | 2 +- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/mono/mono/component/hot_reload-stub.c b/src/mono/mono/component/hot_reload-stub.c index 3298ad03db9b2..2377b948a7b0a 100644 --- a/src/mono/mono/component/hot_reload-stub.c +++ b/src/mono/mono/component/hot_reload-stub.c @@ -21,7 +21,7 @@ static MonoComponentHotReload * component_hot_reload_stub_init (void); static void -hot_reload_stub_set_fastpath_data (MonoMetadataUpdateData *ptr, MonoDefaults *mono_defaults); +hot_reload_stub_set_fastpath_data (MonoMetadataUpdateData *ptr); static gboolean hot_reload_stub_update_enabled (int *modifiable_assemblies_out); @@ -126,7 +126,7 @@ component_hot_reload_stub_init (void) } void -hot_reload_stub_set_fastpath_data (MonoMetadataUpdateData *ptr, MonoDefaults *mono_defaults) +hot_reload_stub_set_fastpath_data (MonoMetadataUpdateData *ptr) { } diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 2403a0a3cdfb0..5258ed8b32d16 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -46,7 +46,7 @@ static bool hot_reload_available (void); static void -hot_reload_set_fastpath_data (MonoMetadataUpdateData *data, MonoDefaults *mono_defaults); +hot_reload_set_fastpath_data (MonoMetadataUpdateData *data); static gboolean hot_reload_update_enabled (int *modifiable_assemblies_out); @@ -150,8 +150,6 @@ static MonoComponentHotReload fn_table = { &hot_reload_find_method_by_name, }; -static MonoDefaults *hr_mono_defaults; - #define HR_GENERATE_GET_CLASS_WITH_CACHE_DECL(shortname) \ MonoClass* mono_class_get_##shortname##_class (void); @@ -169,7 +167,7 @@ mono_class_get_##shortname##_class (void) \ static MonoClass *tmp_class; \ MonoClass *klass = tmp_class; \ if (!klass) { \ - klass = mono_class_load_from_name (hr_mono_defaults->corlib, name_space, name); \ + klass = mono_class_load_from_name (mono_get_corlib (), name_space, name); \ mono_memory_barrier (); /* FIXME excessive? */ \ tmp_class = klass; \ } \ @@ -191,7 +189,7 @@ mono_class_try_get_##shortname##_class (void) \ MonoClass *klass = (MonoClass *)tmp_class; \ mono_memory_barrier (); \ if (!inited) { \ - klass = mono_class_try_load_from_name (hr_mono_defaults->corlib, name_space, name); \ + klass = mono_class_try_load_from_name (mono_get_corlib (), name_space, name); \ tmp_class = klass; \ mono_memory_barrier (); \ inited = TRUE; \ @@ -216,10 +214,9 @@ hot_reload_available (void) static MonoMetadataUpdateData* metadata_update_data_ptr; static void -hot_reload_set_fastpath_data (MonoMetadataUpdateData *ptr, MonoDefaults *mono_defaults) +hot_reload_set_fastpath_data (MonoMetadataUpdateData *ptr) { metadata_update_data_ptr = ptr; - hr_mono_defaults = mono_defaults; } /* TLS value is a uint32_t of the latest published generation that the thread can see */ diff --git a/src/mono/mono/component/hot_reload.h b/src/mono/mono/component/hot_reload.h index d034f8c963eb4..790234e557718 100644 --- a/src/mono/mono/component/hot_reload.h +++ b/src/mono/mono/component/hot_reload.h @@ -17,7 +17,7 @@ typedef struct _MonoComponentHotReload { MonoComponent component; - void (*set_fastpath_data) (MonoMetadataUpdateData *data, MonoDefaults *mono_defaults); + void (*set_fastpath_data) (MonoMetadataUpdateData *data); gboolean (*update_enabled) (int *modifiable_assemblies_out); gboolean (*no_inline) (MonoMethod *caller, MonoMethod *callee); uint32_t (*thread_expose_published) (void); diff --git a/src/mono/mono/metadata/metadata-update.c b/src/mono/mono/metadata/metadata-update.c index 8fe4eb9d154b5..e24823ead81f3 100644 --- a/src/mono/mono/metadata/metadata-update.c +++ b/src/mono/mono/metadata/metadata-update.c @@ -28,7 +28,7 @@ mono_metadata_update_init (void) { memset (&mono_metadata_update_data_private, 0, sizeof (mono_metadata_update_data_private)); MonoComponentHotReload *comp = mono_component_hot_reload (); - comp->set_fastpath_data (&mono_metadata_update_data_private, &mono_defaults); + comp->set_fastpath_data (&mono_metadata_update_data_private); } gboolean From 7250d362d6ea5c556b2515a1f69c389aacd7b4bb Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Sat, 15 Jan 2022 10:20:45 -0500 Subject: [PATCH 43/45] use normal GENERATE_TRY_GET_CLASS_WITH_CACHE --- src/mono/mono/component/hot_reload.c | 52 ++-------------------------- 1 file changed, 2 insertions(+), 50 deletions(-) diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 5258ed8b32d16..e44ae56e5f4af 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -150,54 +150,6 @@ static MonoComponentHotReload fn_table = { &hot_reload_find_method_by_name, }; -#define HR_GENERATE_GET_CLASS_WITH_CACHE_DECL(shortname) \ -MonoClass* mono_class_get_##shortname##_class (void); - -#define HR_GENERATE_TRY_GET_CLASS_WITH_CACHE_DECL(shortname) \ -MonoClass* mono_class_try_get_##shortname##_class (void); - -// GENERATE_GET_CLASS_WITH_CACHE attempts mono_class_load_from_name whenever -// its cache is null. i.e. potentially repeatedly, though it is expected to succeed -// the first time. -// -#define HR_GENERATE_GET_CLASS_WITH_CACHE(shortname,name_space,name) \ -MonoClass* \ -mono_class_get_##shortname##_class (void) \ -{ \ - static MonoClass *tmp_class; \ - MonoClass *klass = tmp_class; \ - if (!klass) { \ - klass = mono_class_load_from_name (mono_get_corlib (), name_space, name); \ - mono_memory_barrier (); /* FIXME excessive? */ \ - tmp_class = klass; \ - } \ - return klass; \ -} - -// GENERATE_TRY_GET_CLASS_WITH_CACHE attempts mono_class_load_from_name approximately -// only once. i.e. if it fails, it will return null and not retry. -// In a race it might try a few times, but not indefinitely. -// -// FIXME This maybe has excessive volatile/barriers. -// -#define HR_GENERATE_TRY_GET_CLASS_WITH_CACHE(shortname,name_space,name) \ -MonoClass* \ -mono_class_try_get_##shortname##_class (void) \ -{ \ - static volatile MonoClass *tmp_class; \ - static volatile gboolean inited; \ - MonoClass *klass = (MonoClass *)tmp_class; \ - mono_memory_barrier (); \ - if (!inited) { \ - klass = mono_class_try_load_from_name (mono_get_corlib (), name_space, name); \ - tmp_class = klass; \ - mono_memory_barrier (); \ - inited = TRUE; \ - } \ - return klass; \ -} - - MonoComponentHotReload * mono_component_hot_reload_init (void) { @@ -2519,9 +2471,9 @@ class_runtime_info_static_fields_unlock (MonoClassRuntimeMetadataUpdateInfo *run mono_coop_mutex_unlock (&runtime_info->static_fields_lock); } -static HR_GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_field_store); +static GENERATE_GET_CLASS_WITH_CACHE_DECL (hot_reload_field_store); -static HR_GENERATE_GET_CLASS_WITH_CACHE(hot_reload_field_store, "Mono.HotReload", "FieldStore"); +static GENERATE_GET_CLASS_WITH_CACHE(hot_reload_field_store, "Mono.HotReload", "FieldStore"); static MonoObject* From 924c6e35490327b19af5a899511d787dedfdfa00 Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 18 Jan 2022 13:39:14 -0500 Subject: [PATCH 44/45] fix formatting --- src/mono/mono/metadata/custom-attrs.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/metadata/custom-attrs.c b/src/mono/mono/metadata/custom-attrs.c index 6cf030bd29f88..59a3ce25983e1 100644 --- a/src/mono/mono/metadata/custom-attrs.c +++ b/src/mono/mono/metadata/custom-attrs.c @@ -1638,10 +1638,11 @@ mono_custom_attrs_from_index_checked (MonoImage *image, guint32 idx, gboolean ig attr_array = g_array_sized_new (TRUE, TRUE, sizeof (guint32), 128); while (!mono_metadata_table_bounds_check (image, MONO_TABLE_CUSTOMATTRIBUTE, i + 1)) { if (mono_metadata_decode_row_col (ca, i, MONO_CUSTOM_ATTR_PARENT) != idx) { - /* if there are updates, the new custom attributes are not sorted, so we have to go until the end. */ - if (G_LIKELY (!image->has_updates)) + if (G_LIKELY (!image->has_updates)) { break; - else { + } else { + // if there are updates, the new custom attributes are not sorted, + // so we have to go until the end. ++i; continue; } From 0a56e88ba06dfebf769dedca75adf596908f8d3c Mon Sep 17 00:00:00 2001 From: Aleksey Kliger Date: Tue, 18 Jan 2022 14:17:42 -0500 Subject: [PATCH 45/45] [metadata] make m_field_set_parent and m_field_set_meta_flags inline m_ prefix functions are supposed to be inline --- src/mono/mono/metadata/class-init.c | 13 ------------- src/mono/mono/metadata/class-inlines.h | 14 ++++++++++++++ src/mono/mono/metadata/class-internals.h | 6 ------ 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/mono/mono/metadata/class-init.c b/src/mono/mono/metadata/class-init.c index d87d277c78a8c..750dcff22d70b 100644 --- a/src/mono/mono/metadata/class-init.c +++ b/src/mono/mono/metadata/class-init.c @@ -4086,16 +4086,3 @@ mono_classes_init (void) mono_counters_register ("MonoClass size", MONO_COUNTER_METADATA | MONO_COUNTER_INT, &classes_size); } - -void -m_field_set_parent (MonoClassField *field, MonoClass *klass) -{ - uintptr_t old_flags = m_field_get_meta_flags (field); - field->parent_and_flags = ((uintptr_t)klass) | old_flags; -} - -void -m_field_set_meta_flags (MonoClassField *field, unsigned int flags) -{ - field->parent_and_flags |= (field->parent_and_flags & ~MONO_CLASS_FIELD_META_FLAG_MASK) | flags; -} diff --git a/src/mono/mono/metadata/class-inlines.h b/src/mono/mono/metadata/class-inlines.h index a9aa3c992a541..e45350a8bc4aa 100644 --- a/src/mono/mono/metadata/class-inlines.h +++ b/src/mono/mono/metadata/class-inlines.h @@ -253,4 +253,18 @@ m_method_is_wrapper (MonoMethod *method) return method->wrapper_type != 0; } + +static inline void +m_field_set_parent (MonoClassField *field, MonoClass *klass) +{ + uintptr_t old_flags = m_field_get_meta_flags (field); + field->parent_and_flags = ((uintptr_t)klass) | old_flags; +} + +static inline void +m_field_set_meta_flags (MonoClassField *field, unsigned int flags) +{ + field->parent_and_flags |= (field->parent_and_flags & ~MONO_CLASS_FIELD_META_FLAG_MASK) | flags; +} + #endif diff --git a/src/mono/mono/metadata/class-internals.h b/src/mono/mono/metadata/class-internals.h index bb43ccde7dd4e..4a7992875bf68 100644 --- a/src/mono/mono/metadata/class-internals.h +++ b/src/mono/mono/metadata/class-internals.h @@ -1587,12 +1587,6 @@ m_field_get_meta_flags (MonoClassField *field) return (unsigned int)(field->parent_and_flags & MONO_CLASS_FIELD_META_FLAG_MASK); } -MONO_COMPONENT_API void -m_field_set_parent (MonoClassField *field, MonoClass *klass); - -MONO_COMPONENT_API void -m_field_set_meta_flags (MonoClassField *field, unsigned int flags); - static inline gboolean m_field_get_offset (MonoClassField *field) {