[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

diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c
index 8e13638..db4e341 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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<string>();
+                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 (file)
--- 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<string>();
             List<string> 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 (file)
--- 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();
+        }
+    }
+}