From: Aleksey Kliger (λgeek) Date: Wed, 4 Aug 2021 13:15:26 +0000 (-0400) Subject: [hot_reload] Allow benign Param table updates (#56800) X-Git-Tag: accepted/tizen/unified/20220110.054933~661 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=95147163dac477da5177f5c5402ae9b93feb5c89;p=platform%2Fupstream%2Fdotnet%2Fruntime.git [hot_reload] Allow benign Param table updates (#56800) * bump hotreload-utils to one that uses the WatchHotReloadService necessary for producing nice deltas for code with lambdas * allow benign param updates as long as the param name and sequence number stays the same, the edit is allowed. * [enc] Add test of changes to lambda and enclosing method --- diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 50d8643..f5a7cb4 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -218,9 +218,9 @@ https://dev.azure.com/dnceng/internal/_git/dotnet-optimization f8742ac0820e98221e7eec5f13989d9538579399 - + https://github.com/dotnet/hotreload-utils - 589840a79ed6335a310a011ee9e244e660798cf5 + 312d3ada595616e44ad827f64231109dc4ea1431 https://github.com/dotnet/runtime-assets diff --git a/eng/Versions.props b/eng/Versions.props index bc6adff..3c0fe9d 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -154,7 +154,7 @@ 16.9.0-preview-20201201-01 1.0.0-prerelease.21373.1 1.0.0-prerelease.21373.1 - 1.0.1-alpha.0.21369.1 + 1.0.2-alpha.0.21402.1 2.4.1 2.4.2 1.3.0 diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange.cs new file mode 100644 index 0000000..9024582 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System; + +namespace System.Reflection.Metadata.ApplyUpdate.Test +{ + public class LambdaBodyChange { + + public LambdaBodyChange () {} + + public string MethodWithLambda () { + Func fn = static (s) => "OLD " + s; + return fn("STRING"); + } + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v1.cs new file mode 100644 index 0000000..470c22c --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v1.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System; + +namespace System.Reflection.Metadata.ApplyUpdate.Test +{ + public class LambdaBodyChange { + + public LambdaBodyChange () {} + + public string MethodWithLambda () { + Func fn = static (s) => s + " STRING"; + return fn("NEW"); + } + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v2.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v2.cs new file mode 100644 index 0000000..f98d0a8 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/LambdaBodyChange_v2.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System; + +namespace System.Reflection.Metadata.ApplyUpdate.Test +{ + public class LambdaBodyChange { + + public LambdaBodyChange () {} + + public string MethodWithLambda () { + Func fn = static (s) => s + " STRING!"; + return fn("NEWEST"); + } + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange.csproj new file mode 100644 index 0000000..7a8ab7f --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange.csproj @@ -0,0 +1,12 @@ + + + System.Runtime.Loader.Tests + $(NetCoreAppCurrent) + true + deltascript.json + true + + + + + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/deltascript.json new file mode 100644 index 0000000..6f020cd --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.LambdaBodyChange/deltascript.json @@ -0,0 +1,7 @@ +{ + "changes": [ + {"document": "LambdaBodyChange.cs", "update": "LambdaBodyChange_v1.cs"}, + {"document": "LambdaBodyChange.cs", "update": "LambdaBodyChange_v2.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index c8cf4b4..7b19a79 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -40,6 +40,33 @@ namespace System.Reflection.Metadata } [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] + void LambdaBodyChange() + { + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof (ApplyUpdate.Test.LambdaBodyChange).Assembly; + + var o = new ApplyUpdate.Test.LambdaBodyChange (); + var r = o.MethodWithLambda(); + + Assert.Equal("OLD STRING", r); + + ApplyUpdateUtil.ApplyUpdate(assm); + + r = o.MethodWithLambda(); + + Assert.Equal("NEW STRING", r); + + ApplyUpdateUtil.ApplyUpdate(assm); + + r = o.MethodWithLambda(); + + Assert.Equal("NEWEST STRING!", r); + }); + } + + [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] [ActiveIssue("https://github.com/dotnet/runtime/issues/52993", TestRuntimes.Mono)] void ClassWithCustomAttributes() { 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 714d83d..bd9f445 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 @@ -35,6 +35,7 @@ + @@ -49,6 +50,9 @@ + diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 1d7bc77..0e7117e 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -1167,6 +1167,32 @@ apply_enclog_pass1 (MonoImage *image_base, MonoImage *image_dmeta, gconstpointer break; } } + 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])) { + /* We only allow modifications where the parameter name doesn't change. */ + uint32_t base_param [MONO_PARAM_SIZE]; + uint32_t upd_param [MONO_PARAM_SIZE]; + int mapped_token = hot_reload_relative_delta_index (image_dmeta, mono_metadata_make_token (token_table, token_index)); + g_assert (mapped_token != -1); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x PARAM update. mapped index = 0x%08x\n", i, log_token, mapped_token); + + mono_metadata_decode_row (&image_dmeta->tables [MONO_TABLE_PARAM], mapped_token - 1, upd_param, MONO_PARAM_SIZE); + mono_metadata_decode_row (&image_base->tables [MONO_TABLE_PARAM], token_index - 1, base_param, MONO_PARAM_SIZE); + + const char *base_name = mono_metadata_string_heap (image_base, base_param [MONO_PARAM_NAME]); + const char *upd_name = mono_metadata_string_heap (image_base, upd_param [MONO_PARAM_NAME]); + mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE, "row[0x%02x: 0x%08x PARAM update: seq = %d (base = %d), name = '%s' (base = '%s')\n", i, log_token, upd_param [MONO_PARAM_SEQUENCE], base_param [MONO_PARAM_SEQUENCE], upd_name, base_name); + if (strcmp (base_name, upd_name) != 0 || base_param [MONO_PARAM_SEQUENCE] != upd_param [MONO_PARAM_SEQUENCE]) { + mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_METADATA_UPDATE, "row[0x%02x]:0x%08x we do not support patching of existing PARAM table cols.", i, log_token); + mono_error_set_type_load_name (error, NULL, image_base->name, "EnC: we do not support patching of existing PARAM table cols. token=0x%08x", log_token); + unsupported_edits = TRUE; + continue; + } + break; + } else + break; /* added a row. ok */ + } 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])) { @@ -1341,6 +1367,10 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen /* ok, pass1 checked for disallowed modifications */ break; } + case MONO_TABLE_PARAM: { + /* ok, pass1 checked for disallowed modifications */ + break; + } default: { g_assert (token_index > table_info_get_rows (&image_base->tables [token_table])); if (mono_trace_is_traced (G_LOG_LEVEL_DEBUG, MONO_TRACE_METADATA_UPDATE))