From e3af7876e6581c93ed85510a28e5a1fa3bfc16a1 Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 7 Feb 2023 17:17:02 -0300 Subject: [PATCH] [wasm][debug] Avoid pausing on breakpoint while invoking methods. (#81162) * Trying to avoid pausing on breakpoint while invoking methods. * Adding test case for it as discussed with @radical offline. * Creating new tests and fixing its behavior. * Addressing @radical comments offline. * same behavior for stepping, disable global_stepping while evaluating expression. * Renaming function as suggested by @radical --- src/mono/mono/component/debugger-agent.c | 39 ++++++++++++---------- src/mono/mono/component/debugger-agent.h | 5 +++ src/mono/mono/component/debugger-engine.h | 2 +- src/mono/mono/component/debugger-protocol.h | 2 +- src/mono/mono/component/debugger.h | 2 +- src/mono/mono/component/mini-wasm-debugger.c | 9 +++-- .../debugger/DebuggerTestSuite/DebuggerTestBase.cs | 31 +++++++++++++++++ .../DebuggerTestSuite/EvaluateOnCallFrameTests.cs | 31 +++++++++++++++++ .../wasm/debugger/DebuggerTestSuite/Inspector.cs | 2 +- .../tests/debugger-test/debugger-evaluate-test.cs | 29 ++++++++++++++++ 10 files changed, 128 insertions(+), 24 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 8e13638..db4e341 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -217,7 +217,7 @@ struct _DebuggerTlsData { gboolean terminated; /* Whenever to disable breakpoints (used during invokes) */ - gboolean disable_breakpoints; + gboolean disable_breakpoint_and_stepping; /* * Number of times this thread has been resumed using resume_thread (). @@ -521,7 +521,6 @@ static void mono_init_debugger_agent_common (MonoProfilerHandle *prof); /* Callbacks used by debugger-engine */ static MonoContext* tls_get_restore_state (void *the_tls); static gboolean try_process_suspend (void *tls, MonoContext *ctx, gboolean from_breakpoint); -static gboolean begin_breakpoint_processing (void *tls, MonoContext *ctx, MonoJitInfo *ji, gboolean from_signal); static void begin_single_step_processing (MonoContext *ctx, gboolean from_signal); static gboolean ensure_jit (DbgEngineStackFrame* the_frame); static int ensure_runtime_is_suspended (void); @@ -775,7 +774,7 @@ mono_debugger_agent_init_internal (void) memset (&cbs, 0, sizeof (cbs)); cbs.tls_get_restore_state = tls_get_restore_state; cbs.try_process_suspend = try_process_suspend; - cbs.begin_breakpoint_processing = begin_breakpoint_processing; + cbs.begin_breakpoint_processing = mono_begin_breakpoint_processing; cbs.begin_single_step_processing = begin_single_step_processing; cbs.ss_discard_frame_context = mono_ss_discard_frame_context; cbs.ss_calculate_framecount = mono_ss_calculate_framecount; @@ -2263,6 +2262,12 @@ mono_wasm_get_tls (void) GET_TLS_DATA_FROM_THREAD (thread); return tls; } + +bool +mono_wasm_is_breakpoint_and_stepping_disabled (void) +{ + return mono_wasm_get_tls ()->disable_breakpoint_and_stepping; +} #endif #ifdef HOST_WASI @@ -3702,7 +3707,7 @@ process_event (EventKind event, gpointer arg, gint32 il_offset, MonoContext *ctx GET_DEBUGGER_TLS(); g_assert (tls); // We are already processing a breakpoint event - if (tls->disable_breakpoints) + if (tls->disable_breakpoint_and_stepping) return; mono_stopwatch_stop (&tls->step_time); break; @@ -4266,7 +4271,7 @@ mono_de_frame_async_id (DbgEngineStackFrame *frame) MonoObject *ex; ERROR_DECL (error); MonoObject *obj; - gboolean old_disable_breakpoints = FALSE; + gboolean old_disable_breakpoint_and_stepping = FALSE; DebuggerTlsData *tls; /* @@ -4283,27 +4288,27 @@ mono_de_frame_async_id (DbgEngineStackFrame *frame) tls = (DebuggerTlsData *)mono_native_tls_get_value (debugger_tls_id); if (tls) { - old_disable_breakpoints = tls->disable_breakpoints; - tls->disable_breakpoints = TRUE; + old_disable_breakpoint_and_stepping = tls->disable_breakpoint_and_stepping; + tls->disable_breakpoint_and_stepping = TRUE; } method = get_object_id_for_debugger_method (mono_class_from_mono_type_internal (builder_field->type)); if (!method) { if (tls) - tls->disable_breakpoints = old_disable_breakpoints; + tls->disable_breakpoint_and_stepping = old_disable_breakpoint_and_stepping; return 0; } obj = mono_runtime_try_invoke (method, builder, NULL, &ex, error); mono_error_assert_ok (error); if (tls) - tls->disable_breakpoints = old_disable_breakpoints; + tls->disable_breakpoint_and_stepping = old_disable_breakpoint_and_stepping; return get_objid (obj); } -static gboolean -begin_breakpoint_processing (void *the_tls, MonoContext *ctx, MonoJitInfo *ji, gboolean from_signal) +bool +mono_begin_breakpoint_processing (void *the_tls, MonoContext *ctx, MonoJitInfo *ji, gboolean from_signal) { DebuggerTlsData *tls = (DebuggerTlsData*)the_tls; @@ -4316,7 +4321,7 @@ begin_breakpoint_processing (void *the_tls, MonoContext *ctx, MonoJitInfo *ji, g #else NOT_IMPLEMENTED; #endif - if (tls->disable_breakpoints) + if (tls->disable_breakpoint_and_stepping) return FALSE; return TRUE; } @@ -4856,7 +4861,7 @@ debugger_agent_handle_exception (MonoException *exc, MonoContext *throw_ctx, if (tls != NULL) { if (tls->abort_requested) return; - if (tls->disable_breakpoints) + if (tls->disable_breakpoint_and_stepping) return; } @@ -6380,10 +6385,10 @@ mono_do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, gu if (i < nargs) return err; - if (invoke->flags & INVOKE_FLAG_DISABLE_BREAKPOINTS) - tls->disable_breakpoints = TRUE; + if (invoke->flags & INVOKE_FLAG_DISABLE_BREAKPOINTS_AND_STEPPING) + tls->disable_breakpoint_and_stepping = TRUE; else - tls->disable_breakpoints = FALSE; + tls->disable_breakpoint_and_stepping = FALSE; /* * Add an LMF frame to link the stack frames on the invoke method with our caller. @@ -6476,7 +6481,7 @@ mono_do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, gu } } - tls->disable_breakpoints = FALSE; + tls->disable_breakpoint_and_stepping = FALSE; #ifdef MONO_ARCH_SOFT_DEBUG_SUPPORTED if (invoke->has_ctx) diff --git a/src/mono/mono/component/debugger-agent.h b/src/mono/mono/component/debugger-agent.h index 13a9f50..3514559 100644 --- a/src/mono/mono/component/debugger-agent.h +++ b/src/mono/mono/component/debugger-agent.h @@ -31,6 +31,8 @@ mono_change_log_level (int new_log_level); void mono_wasm_save_thread_context (void); +bool +mono_wasm_is_breakpoint_and_stepping_disabled (void); #endif void @@ -70,4 +72,7 @@ mono_de_frame_async_id (DbgEngineStackFrame *frame); bool mono_debugger_agent_receive_and_process_command (void); + +bool +mono_begin_breakpoint_processing (void *the_tls, MonoContext *ctx, MonoJitInfo *ji, gboolean from_signal); #endif diff --git a/src/mono/mono/component/debugger-engine.h b/src/mono/mono/component/debugger-engine.h index d174f3e..8b06239 100644 --- a/src/mono/mono/component/debugger-engine.h +++ b/src/mono/mono/component/debugger-engine.h @@ -178,7 +178,7 @@ #define INVOKE_FLAG_SINGLE_THREADED MDBGPROT_INVOKE_FLAG_SINGLE_THREADED #define INVOKE_FLAG_VIRTUAL MDBGPROT_INVOKE_FLAG_VIRTUAL -#define INVOKE_FLAG_DISABLE_BREAKPOINTS MDBGPROT_INVOKE_FLAG_DISABLE_BREAKPOINTS +#define INVOKE_FLAG_DISABLE_BREAKPOINTS_AND_STEPPING MDBGPROT_INVOKE_FLAG_DISABLE_BREAKPOINTS_AND_STEPPING #define INVOKE_FLAG_RETURN_OUT_THIS MDBGPROT_INVOKE_FLAG_RETURN_OUT_THIS #define INVOKE_FLAG_RETURN_OUT_ARGS MDBGPROT_INVOKE_FLAG_RETURN_OUT_ARGS diff --git a/src/mono/mono/component/debugger-protocol.h b/src/mono/mono/component/debugger-protocol.h index 0096eb1..bd032ed 100644 --- a/src/mono/mono/component/debugger-protocol.h +++ b/src/mono/mono/component/debugger-protocol.h @@ -96,7 +96,7 @@ typedef enum { } MdbgProtStackFrameFlags; typedef enum { - MDBGPROT_INVOKE_FLAG_DISABLE_BREAKPOINTS = 1, + MDBGPROT_INVOKE_FLAG_DISABLE_BREAKPOINTS_AND_STEPPING = 1, MDBGPROT_INVOKE_FLAG_SINGLE_THREADED = 2, MDBGPROT_INVOKE_FLAG_RETURN_OUT_THIS = 4, MDBGPROT_INVOKE_FLAG_RETURN_OUT_ARGS = 8, diff --git a/src/mono/mono/component/debugger.h b/src/mono/mono/component/debugger.h index f871535..fe14960 100644 --- a/src/mono/mono/component/debugger.h +++ b/src/mono/mono/component/debugger.h @@ -113,7 +113,7 @@ typedef struct { typedef struct { MonoContext *(*tls_get_restore_state) (void *tls); gboolean (*try_process_suspend) (void *tls, MonoContext *ctx, gboolean from_breakpoint); - gboolean (*begin_breakpoint_processing) (void *tls, MonoContext *ctx, MonoJitInfo *ji, gboolean from_signal); + bool (*begin_breakpoint_processing) (void *tls, MonoContext *ctx, MonoJitInfo *ji, gboolean from_signal); void (*begin_single_step_processing) (MonoContext *ctx, gboolean from_signal); void (*ss_discard_frame_context) (void *tls); diff --git a/src/mono/mono/component/mini-wasm-debugger.c b/src/mono/mono/component/mini-wasm-debugger.c index f9071ea..2b38e5c 100644 --- a/src/mono/mono/component/mini-wasm-debugger.c +++ b/src/mono/mono/component/mini-wasm-debugger.c @@ -89,10 +89,10 @@ try_process_suspend (void *tls, MonoContext *ctx, gboolean from_breakpoint) return FALSE; } -static gboolean +static bool begin_breakpoint_processing (void *tls, MonoContext *ctx, MonoJitInfo *ji, gboolean from_signal) { - return TRUE; + return mono_begin_breakpoint_processing(tls, ctx, ji, from_signal); } static void @@ -218,6 +218,8 @@ assembly_loaded (MonoProfiler *prof, MonoAssembly *assembly) static void mono_wasm_single_step_hit (void) { + if (mono_wasm_is_breakpoint_and_stepping_disabled ()) + return; mono_de_process_single_step (mono_wasm_get_tls (), FALSE); } @@ -419,6 +421,7 @@ mono_wasm_send_dbg_command (int id, MdbgProtCommandSet command_set, int command, InvokeData invoke_data; memset (&invoke_data, 0, sizeof (InvokeData)); invoke_data.endp = data + size; + invoke_data.flags = INVOKE_FLAG_DISABLE_BREAKPOINTS_AND_STEPPING; error = mono_do_invoke_method (tls, &buf, &invoke_data, data, &data); } else if (command_set == MDBGPROT_CMD_SET_VM && (command == MDBGPROT_CMD_GET_ASSEMBLY_BYTES)) @@ -491,4 +494,4 @@ mini_wasm_debugger_add_function_pointers (MonoComponentDebugger* fn_table) fn_table->mono_wasm_single_step_hit = mono_wasm_single_step_hit; } -#endif \ No newline at end of file +#endif diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs index c55c2b1..73a6aa0 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs @@ -252,7 +252,38 @@ namespace DebuggerTests { return await insp.WaitFor(what); } + public async Task WaitForConsoleMessage(string message) + { + object llock = new(); + var tcs = new TaskCompletionSource(); + insp.On("Runtime.consoleAPICalled", async (args, c) => + { + (string line, string type) = insp.FormatConsoleAPICalled(args); + if (string.IsNullOrEmpty(line)) + return await Task.FromResult(ProtocolEventHandlerReturn.KeepHandler); + lock (llock) + { + try + { + if (line == message) + { + tcs.SetResult(); + } + } + catch (Exception ex) + { + tcs.SetException(ex); + } + } + + return tcs.Task.IsCompleted + ? await Task.FromResult(ProtocolEventHandlerReturn.RemoveHandler) + : await Task.FromResult(ProtocolEventHandlerReturn.KeepHandler); + }); + + await tcs.Task; + } public async Task WaitForScriptParsedEventsAsync(params string[] paths) { object llock = new(); diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs index f9db1ca..ad413c0 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs @@ -882,5 +882,36 @@ namespace DebuggerTests props = await GetObjectOnFrame(frame, "this"); CheckNumber(props, "a", 11); }); + + [ConditionalTheory(nameof(RunningOnChrome))] + [InlineData(true)] + [InlineData(false)] + public async Task EvaluateMethodWithBPWhilePausedInADifferentMethodAndNotHit(bool setBreakpointBeforePause) + { + await cli.SendCommand("DotnetDebugger.setEvaluationOptions", JObject.FromObject(new { options = new { noFuncEval = false } }), token); + var waitForScript = WaitForConsoleMessage("console.warning: MONO_WASM: Adding an id (0) that already exists in commands_received"); + if (setBreakpointBeforePause) + await SetBreakpointInMethod("debugger-test.dll", "TestEvaluateDontPauseOnBreakpoint", "MyMethod2", 1); + await CheckInspectLocalsAtBreakpointSite( + "TestEvaluateDontPauseOnBreakpoint", "run", 3, "TestEvaluateDontPauseOnBreakpoint.run", + "window.setTimeout(function() { invoke_static_method ('[debugger-test] TestEvaluateDontPauseOnBreakpoint:run'); })", + wait_for_event_fn: async (pause_location) => + { + if (!setBreakpointBeforePause) + await SetBreakpointInMethod("debugger-test.dll", "TestEvaluateDontPauseOnBreakpoint", "MyMethod2", 1); + var id = pause_location["callFrames"][0]["callFrameId"].Value(); + await EvaluateOnCallFrameAndCheck(id, + ("myVar.MyMethod2()", TString("Object 11")), + ("myVar.MyMethod3()", TString("Object 11")), + ("myVar.MyCount", TString("Object 11")), + ("myVar.MyMethod()", TString("Object 10")), + ("myVar", TObject("TestEvaluateDontPauseOnBreakpoint", description: "Object 11"))); + var props = await GetObjectOnFrame(pause_location["callFrames"][0], "myVar"); + await CheckString(props, "MyCount", "Object 11"); + }); + await SendCommandAndCheck(null, "Debugger.resume", null, 0, 0, "TestEvaluateDontPauseOnBreakpoint.MyMethod2"); + await SendCommandAndCheck(null, "Debugger.resume", null, 0, 0, "TestEvaluateDontPauseOnBreakpoint.MyMethod"); + Assert.False(waitForScript.IsCompleted); + } } } diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs index b868384..3dafd0d 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs @@ -170,7 +170,7 @@ namespace DebuggerTests } } - private (string line, string type) FormatConsoleAPICalled(JObject args) + internal (string line, string type) FormatConsoleAPICalled(JObject args) { string? type = args?["type"]?.Value(); List consoleArgs = new(); diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs b/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs index 4e6f597..1960f6b 100644 --- a/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs @@ -2123,3 +2123,32 @@ public static class NoNamespaceClass } } } +[System.Diagnostics.DebuggerDisplay("{MyMethod2(),nq}")] +public class TestEvaluateDontPauseOnBreakpoint +{ + public int count; + public static void run() + { + var myVar = new TestEvaluateDontPauseOnBreakpoint(); + myVar.count = 10; + myVar.MyMethod2(); + myVar.MyMethod(); + } + public string MyMethod() { + System.Diagnostics.Debugger.Break(); + return string.Format("Object {0}", count); + } + public string MyMethod2() { + return string.Format("Object {0}", count + 1); + } + public string MyMethod3() { + return MyMethod2(); + } + public string MyCount + { + get + { + return MyMethod2(); + } + } +} -- 2.7.4