[wasm][debug] Avoid pausing on breakpoint while invoking methods. (#8…

…1162)

* 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

@@ -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)

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

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
 

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,

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);

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
+#endif

src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs

@@ -252,7 +252,38 @@ internal virtual async Task<JObject> WaitFor(string what)
         {
             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();

src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs

@@ -882,5 +882,36 @@ await EvaluateOnCallFrameAndCheck(id,
                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);
+        }
     }
 }

src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs

@@ -170,7 +170,7 @@ void FailAllWaiters(Exception? exception = null)
             }
         }
 
-        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();

src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs

@@ -2123,3 +2123,32 @@ public static class NestedClass3
         }
     }
 }
+[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();
+        }
+    }
+}