From 704c3c7212f02252a63c14f43857fc7fa77eb575 Mon Sep 17 00:00:00 2001 From: Thays Date: Thu, 19 Aug 2021 17:24:31 -0300 Subject: [PATCH 1/4] Fix xamarin-android 6161 --- src/mono/mono/component/debugger-agent.c | 10 +++++----- src/mono/mono/metadata/mono-debug.c | 22 ++++++++++------------ src/mono/mono/metadata/mono-debug.h | 2 +- src/mono/mono/mini/dwarfwriter.c | 2 +- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 2cff5c0dec1445..15fddaa9da2a85 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 */ @@ -9291,8 +9291,8 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) } else { MonoDebugLocalsInfo *locals; - locals = mono_debug_lookup_locals (frame->de.method, TRUE); - if (locals) { + locals = mono_debug_lookup_locals (frame->de.method); + if (locals && CHECK_ICORDBG (FALSE)) { //on icordbg the index value is correct, we don't need to fix it. g_assert (pos < locals->num_locals); pos = locals->locals [pos].index; mono_debug_free_locals (locals); @@ -9345,8 +9345,8 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) } else { MonoDebugLocalsInfo *locals; - locals = mono_debug_lookup_locals (frame->de.method, TRUE); - if (locals) { + locals = mono_debug_lookup_locals (frame->de.method); + if (locals && CHECK_ICORDBG (FALSE)) { g_assert (pos < locals->num_locals); pos = locals->locals [pos].index; mono_debug_free_locals (locals); 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]; From c296b5c98be24e057cc7e81ae8ae1ca9c3301913 Mon Sep 17 00:00:00 2001 From: Thays Date: Thu, 19 Aug 2021 17:47:57 -0300 Subject: [PATCH 2/4] Fixing leak. --- src/mono/mono/component/debugger-agent.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 15fddaa9da2a85..63a79597f5fa80 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -9292,9 +9292,12 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) MonoDebugLocalsInfo *locals; locals = mono_debug_lookup_locals (frame->de.method); - if (locals && CHECK_ICORDBG (FALSE)) { //on icordbg the index value is correct, we don't need to fix it. - g_assert (pos < locals->num_locals); - pos = locals->locals [pos].index; + if (locals) { + if (CHECK_ICORDBG (FALSE)) //on icordbg the index value is correct, we don't need to fix it. + { + g_assert (pos < locals->num_locals); + pos = locals->locals [pos].index; + } mono_debug_free_locals (locals); } @@ -9346,9 +9349,12 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) MonoDebugLocalsInfo *locals; locals = mono_debug_lookup_locals (frame->de.method); - if (locals && CHECK_ICORDBG (FALSE)) { - g_assert (pos < locals->num_locals); - pos = locals->locals [pos].index; + if (locals) { + if (CHECK_ICORDBG (FALSE)) + { + g_assert (pos < locals->num_locals); + pos = locals->locals [pos].index; + } mono_debug_free_locals (locals); } g_assert (pos >= 0 && pos < jit->num_locals); From b8472b4309f81583f76882e874ba9a996858f6df Mon Sep 17 00:00:00 2001 From: Thays Date: Thu, 19 Aug 2021 18:47:51 -0300 Subject: [PATCH 3/4] Adding wasm debugger test. Fixing using protocol version. --- src/mono/mono/component/debugger-agent.c | 4 +- .../wasm/debugger/DebuggerTestSuite/Tests.cs | 15 +++++++ .../tests/debugger-test/debugger-test.cs | 43 +++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 63a79597f5fa80..02698090638cd6 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -9293,7 +9293,7 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) locals = mono_debug_lookup_locals (frame->de.method); if (locals) { - if (CHECK_ICORDBG (FALSE)) //on icordbg the index value is correct, we don't need to fix it. + if (!CHECK_PROTOCOL_VERSION (2, 59)) //from newer protocol versions it's sent the pdb index { g_assert (pos < locals->num_locals); pos = locals->locals [pos].index; @@ -9350,7 +9350,7 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) locals = mono_debug_lookup_locals (frame->de.method); if (locals) { - if (CHECK_ICORDBG (FALSE)) + if (!CHECK_PROTOCOL_VERSION (2, 59)) //from newer protocol versions it's sent the pdb index { g_assert (pos < locals->num_locals); pos = locals->locals [pos].index; 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; + } } From 8bd038dad0e0278f09261bd60c4ce58d270a26f0 Mon Sep 17 00:00:00 2001 From: Thays Date: Thu, 19 Aug 2021 19:41:27 -0300 Subject: [PATCH 4/4] only get locals in older protocol versions. --- src/mono/mono/component/debugger-agent.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 02698090638cd6..a2661909791b5c 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -9289,16 +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); - if (locals) { - if (!CHECK_PROTOCOL_VERSION (2, 59)) //from newer protocol versions it's sent the pdb index - { + 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); } - mono_debug_free_locals (locals); } PRINT_DEBUG_MSG (4, "[dbg] send local %d.\n", pos); @@ -9346,16 +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); - if (locals) { - if (!CHECK_PROTOCOL_VERSION (2, 59)) //from newer protocol versions it's sent the pdb index - { + 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); } - mono_debug_free_locals (locals); } g_assert (pos >= 0 && pos < jit->num_locals);