diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate.cs new file mode 100644 index 00000000000000..47bbcd171ad3b4 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Reflection.Metadata.ApplyUpdate.Test +{ + public class FirstCallAfterUpdate { + public FirstCallAfterUpdate() {} + public string Method1 (string s) { + return s + " STRING"; + } + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate_v1.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate_v1.cs new file mode 100644 index 00000000000000..66d1d336e6baa3 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate_v1.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Reflection.Metadata.ApplyUpdate.Test +{ + public class FirstCallAfterUpdate { + public FirstCallAfterUpdate() {} + public string Method1(string s) { + return "NEW " + s; + } + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate_v2.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate_v2.cs new file mode 100644 index 00000000000000..9ea687752b8026 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/FirstCallAfterUpdate_v2.cs @@ -0,0 +1,12 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Reflection.Metadata.ApplyUpdate.Test +{ + public class FirstCallAfterUpdate { + public FirstCallAfterUpdate() {} + public string Method1(string s) { + return s + "EST STRING"; + } + } +} diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate.csproj b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate.csproj new file mode 100644 index 00000000000000..1cd197f92264de --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate.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.FirstCallAfterUpdate/deltascript.json b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/deltascript.json new file mode 100644 index 00000000000000..64d533be6b7560 --- /dev/null +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdate/System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate/deltascript.json @@ -0,0 +1,7 @@ +{ + "changes": [ + {"document": "FirstCallAfterUpdate.cs", "update": "FirstCallAfterUpdate_v1.cs"}, + {"document": "FirstCallAfterUpdate.cs", "update": "FirstCallAfterUpdate_v2.cs"}, + ] +} + diff --git a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs index 794ace67148c94..3e240e65caabf8 100644 --- a/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs +++ b/src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs @@ -94,6 +94,26 @@ void LambdaCapturesThis() }); } + [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] + [ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))] + void FirstCallAfterUpdate() + { + /* Tests that updating a method that has not been called before works correctly and that + * the JIT/interpreter doesn't have to rely on cached baseline data. */ + ApplyUpdateUtil.TestCase(static () => + { + var assm = typeof (ApplyUpdate.Test.FirstCallAfterUpdate).Assembly; + + var o = new ApplyUpdate.Test.FirstCallAfterUpdate (); + + ApplyUpdateUtil.ApplyUpdate(assm); + ApplyUpdateUtil.ApplyUpdate(assm); + + string r = o.Method1("NEW"); + + Assert.Equal("NEWEST STRING", r); + }); + } [ConditionalFact(typeof(ApplyUpdateUtil), nameof (ApplyUpdateUtil.IsSupported))] [ActiveIssue("https://github.com/dotnet/runtime/issues/52993", TestRuntimes.Mono)] 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 f893446f21d0e2..8b946a20828d2e 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 @@ -38,6 +38,7 @@ + @@ -54,6 +55,7 @@ + diff --git a/src/libraries/System.Threading.Tasks/tests/Task/ExecutionContextFlowTest.cs b/src/libraries/System.Threading.Tasks/tests/Task/ExecutionContextFlowTest.cs index 8374c56056996a..5ba5ec4922ab33 100644 --- a/src/libraries/System.Threading.Tasks/tests/Task/ExecutionContextFlowTest.cs +++ b/src/libraries/System.Threading.Tasks/tests/Task/ExecutionContextFlowTest.cs @@ -29,6 +29,7 @@ public void SuppressFlow_TaskCapturesContextAccordingly(bool suppressFlow) } } + [ActiveIssue("https://github.com/dotnet/runtime/issues/57331")] [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsPreciseGcSupported))] public static async Task TaskDropsExecutionContextUponCompletion() { diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 2cff5c0dec1445..a2661909791b5c 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -8572,7 +8572,7 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g return ERR_INVALID_ARGUMENT; } - locals = mono_debug_lookup_locals (method, FALSE); + locals = mono_debug_lookup_locals (method); if (!locals) { if (CHECK_PROTOCOL_VERSION (2, 43)) { /* Scopes */ @@ -9289,13 +9289,14 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) pos = - pos - 1; cmd_stack_frame_get_parameter (frame, sig, pos, buf, jit); } else { - MonoDebugLocalsInfo *locals; - - locals = mono_debug_lookup_locals (frame->de.method, TRUE); - if (locals) { - g_assert (pos < locals->num_locals); - pos = locals->locals [pos].index; - mono_debug_free_locals (locals); + if (!CHECK_PROTOCOL_VERSION (2, 59)) { //from newer protocol versions it's sent the pdb index + MonoDebugLocalsInfo *locals; + locals = mono_debug_lookup_locals (frame->de.method); + if (locals) { + g_assert (pos < locals->num_locals); + pos = locals->locals [pos].index; + mono_debug_free_locals (locals); + } } PRINT_DEBUG_MSG (4, "[dbg] send local %d.\n", pos); @@ -9343,13 +9344,14 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) var = &jit->params [pos]; is_arg = TRUE; } else { - MonoDebugLocalsInfo *locals; - - locals = mono_debug_lookup_locals (frame->de.method, TRUE); - if (locals) { - g_assert (pos < locals->num_locals); - pos = locals->locals [pos].index; - mono_debug_free_locals (locals); + if (!CHECK_PROTOCOL_VERSION (2, 59)) { //from newer protocol versions it's sent the pdb index + MonoDebugLocalsInfo *locals; + locals = mono_debug_lookup_locals (frame->de.method); + if (locals) { + g_assert (pos < locals->num_locals); + pos = locals->locals [pos].index; + mono_debug_free_locals (locals); + } } g_assert (pos >= 0 && pos < jit->num_locals); diff --git a/src/mono/mono/component/hot_reload.c b/src/mono/mono/component/hot_reload.c index 80374aeef906bc..d43cca86b251a6 100644 --- a/src/mono/mono/component/hot_reload.c +++ b/src/mono/mono/component/hot_reload.c @@ -793,6 +793,13 @@ hot_reload_effective_table_slow (const MonoTableInfo **t, int *idx) if (G_LIKELY (*idx < table_info_get_rows (*t) && !any_modified)) return; + /* FIXME: when adding methods this won't work anymore. We will need to update the delta + * images' suppressed columns (see the Note in pass2 about + * CMiniMdRW::m_SuppressedDeltaColumns) with the baseline values. */ + /* The only column from the updates that matters the RVA, which is looked up elsewhere. */ + if (tbl_index == MONO_TABLE_METHOD) + return; + GList *list = info->delta_image; MonoImage *dmeta; int ridx; @@ -1272,6 +1279,29 @@ apply_enclog_pass2 (MonoImage *image_base, BaselineInfo *base_info, uint32_t gen MonoTableInfo *table_enclog = &image_dmeta->tables [MONO_TABLE_ENCLOG]; int rows = table_info_get_rows (table_enclog); + /* NOTE: Suppressed colums + * + * Certain column values in some tables in the deltas are not meant to be applied over the + * previous generation. See CMiniMdRW::m_SuppressedDeltaColumns in CoreCLR. For example the + * MONO_METHOD_PARAMLIST column in MONO_TABLE_METHOD is always 0 in an update - for modified + * rows the previous value must be carried over. For added rows, it is supposed to be + * initialized to the end of the param table and updated with the "Param create" func code + * in subsequent EnCLog records. + * + * For mono's immutable model (where we don't change the baseline image data), we will need + * to mutate the delta image tables to incorporate the suppressed column values from the + * previous generation. + * + * For Baseline capabilities, the only suppressed column is MONO_METHOD_PARAMLIST - which we + * can ignore because we don't do anything with param updates and the only column we care + * about is MONO_METHOD_RVA which gets special case treatment with set_update_method(). + * + * But when we implement additional capabilities (for example UpdateParameters), we will + * need to start mutating the delta image tables to pick up the suppressed column values. + * Fortunately whether we get the delta from the debugger or from the runtime API, we always + * have it in writable memory (and not mmap-ed pages), so we can rewrite the table values. + */ + gboolean assemblyref_updated = FALSE; for (int i = 0; i < rows ; ++i) { guint32 cols [MONO_ENCLOG_SIZE]; diff --git a/src/mono/mono/metadata/mono-debug.c b/src/mono/mono/metadata/mono-debug.c index b9bea0e78f5d06..03626981f441d0 100644 --- a/src/mono/mono/metadata/mono-debug.c +++ b/src/mono/mono/metadata/mono-debug.c @@ -867,7 +867,7 @@ mono_debug_method_lookup_location (MonoDebugMethodInfo *minfo, int il_offset) * The result should be freed using mono_debug_free_locals (). */ MonoDebugLocalsInfo* -mono_debug_lookup_locals (MonoMethod *method, mono_bool ignore_pdb) +mono_debug_lookup_locals (MonoMethod *method) { MonoDebugMethodInfo *minfo; MonoDebugLocalsInfo *res; @@ -893,18 +893,16 @@ mono_debug_lookup_locals (MonoMethod *method, mono_bool ignore_pdb) return NULL; } - if (ignore_pdb) - res = mono_debug_symfile_lookup_locals (minfo); - else { - if (minfo->handle->ppdb) { - res = mono_ppdb_lookup_locals (minfo); - } else { - if (!minfo->handle->symfile || !mono_debug_symfile_is_loaded (minfo->handle->symfile)) - res = NULL; - else - res = mono_debug_symfile_lookup_locals (minfo); - } + + if (minfo->handle->ppdb) { + res = mono_ppdb_lookup_locals (minfo); + } else { + if (!minfo->handle->symfile || !mono_debug_symfile_is_loaded (minfo->handle->symfile)) + res = NULL; + else + res = mono_debug_symfile_lookup_locals (minfo); } + mono_debugger_unlock (); return res; diff --git a/src/mono/mono/metadata/mono-debug.h b/src/mono/mono/metadata/mono-debug.h index 2ddbce09947153..2fe6a3394afda1 100644 --- a/src/mono/mono/metadata/mono-debug.h +++ b/src/mono/mono/metadata/mono-debug.h @@ -189,7 +189,7 @@ MONO_API void mono_debug_add_delegate_trampoline (void* code, int size); MONO_API MonoDebugLocalsInfo* -mono_debug_lookup_locals (MonoMethod *method, mono_bool ignore_pdb); +mono_debug_lookup_locals (MonoMethod *method); MONO_API MonoDebugMethodAsyncInfo* mono_debug_lookup_method_async_debug_info (MonoMethod *method); diff --git a/src/mono/mono/mini/dwarfwriter.c b/src/mono/mono/mini/dwarfwriter.c index 1bac0d946391e7..d23074199bc65c 100644 --- a/src/mono/mono/mini/dwarfwriter.c +++ b/src/mono/mono/mini/dwarfwriter.c @@ -1899,7 +1899,7 @@ mono_dwarf_writer_emit_method (MonoDwarfWriter *w, MonoCompile *cfg, MonoMethod g_free (names); /* Locals */ - locals_info = mono_debug_lookup_locals (method, FALSE); + locals_info = mono_debug_lookup_locals (method); for (i = 0; i < header->num_locals; ++i) { MonoInst *ins = locals [i]; diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs index 548b012901a5a3..9681a02882574c 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs @@ -845,6 +845,21 @@ public async Task InspectTaskAtLocals() => await CheckInspectLocalsAtBreakpointS }, "t_props", num_fields: 53); }); + + [Fact] + public async Task InspectLocalsWithIndexAndPositionWithDifferentValues() //https://github.com/xamarin/xamarin-android/issues/6161 + { + await EvaluateAndCheck( + "window.setTimeout(function() { invoke_static_method('[debugger-test] MainPage:CallSetValue'); }, 1);", + "dotnet://debugger-test.dll/debugger-test.cs", 758, 16, + "set_SomeValue", + locals_fn: (locals) => + { + CheckNumber(locals, "view", 150); + } + ); + } + //TODO add tests covering basic stepping behavior as step in/out/over } } diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs b/src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs index 4a893f9948feb2..a826d97ce8e664 100644 --- a/src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs @@ -725,5 +725,48 @@ public async System.Threading.Tasks.Task AsyncMethod() Console.WriteLine($"time for await"); return true; } + +} + +public class MainPage +{ + public MainPage() + { + } + + int count = 0; + private int someValue; + + public int SomeValue + { + get + { + return someValue; + } + set + { + someValue = value; + count++; + + if (count == 10) + { + var view = 150; + + if (view != 50) + { + + } + System.Diagnostics.Debugger.Break(); + } + + SomeValue = count; + } + } + + public static void CallSetValue() + { + var mainPage = new MainPage(); + mainPage.SomeValue = 10; + } }