From 994ad0d24379f8503ff68c9e18a93d8f3452230d Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 13 Sep 2022 17:43:39 -0300 Subject: [PATCH 1/6] fixing setting a breakpoint in an invalid IL offset --- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 3 +++ src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index ec0488c826262b..d51a4b5d26b4d2 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -1721,6 +1721,9 @@ private async Task OnSetNextIP(MessageId sessionId, SourceLocation targetL return false; var breakpointId = await context.SdbAgent.SetBreakpoint(scope.Method.DebugId, ilOffset.Offset, token); + if (breakpointId == -1) + return false; + context.TempBreakpointForSetNextIP = breakpointId; await SendResume(sessionId, token); return true; diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 2531e96818cf43..d5d301e8bd0209 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -1359,7 +1359,9 @@ public async Task SetBreakpoint(int methodId, long il_offset, CancellationT commandParamsWriter.Write((byte)ModifierKind.LocationOnly); commandParamsWriter.Write(methodId); commandParamsWriter.Write(il_offset); - using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdEventRequest.Set, commandParamsWriter, token); + using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdEventRequest.Set, commandParamsWriter, token, false); + if (retDebuggerCmdReader.HasError) + return -1; return retDebuggerCmdReader.ReadInt32(); } From b65917d43aff31cda890795b1a84cce2df8fe705 Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 13 Sep 2022 18:10:54 -0300 Subject: [PATCH 2/6] addressing @radical comments --- .../debugger/BrowserDebugProxy/MonoProxy.cs | 60 ++++++++++--------- .../BrowserDebugProxy/MonoSDBHelper.cs | 2 +- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index d51a4b5d26b4d2..dd248617b89143 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -361,37 +361,43 @@ protected override async Task AcceptCommand(MessageId id, JObject parms, C SendResponse(id, resp, token); return true; } + try { + string bpid = resp.Value["breakpointId"]?.ToString(); + IEnumerable locations = resp.Value["locations"]?.Values(); + var request = BreakpointRequest.Parse(bpid, args); - string bpid = resp.Value["breakpointId"]?.ToString(); - IEnumerable locations = resp.Value["locations"]?.Values(); - var request = BreakpointRequest.Parse(bpid, args); + // is the store done loading? + bool loaded = context.Source.Task.IsCompleted; + if (!loaded) + { + // Send and empty response immediately if not + // and register the breakpoint for resolution + context.BreakpointRequests[bpid] = request; + SendResponse(id, resp, token); + } - // is the store done loading? - bool loaded = context.Source.Task.IsCompleted; - if (!loaded) - { - // Send and empty response immediately if not - // and register the breakpoint for resolution - context.BreakpointRequests[bpid] = request; - SendResponse(id, resp, token); - } + if (await IsRuntimeAlreadyReadyAlready(id, token)) + { + DebugStore store = await RuntimeReady(id, token); - if (await IsRuntimeAlreadyReadyAlready(id, token)) - { - DebugStore store = await RuntimeReady(id, token); + Log("verbose", $"BP req {args}"); + await SetBreakpoint(id, store, request, !loaded, false, token); + } - Log("verbose", $"BP req {args}"); - await SetBreakpoint(id, store, request, !loaded, false, token); - } + if (loaded) + { + // we were already loaded so we should send a response + // with the locations included and register the request + context.BreakpointRequests[bpid] = request; + var result = Result.OkFromObject(request.AsSetBreakpointByUrlResponse(locations)); + SendResponse(id, result, token); - if (loaded) + } + } + catch (Exception e) { - // we were already loaded so we should send a response - // with the locations included and register the request - context.BreakpointRequests[bpid] = request; - var result = Result.OkFromObject(request.AsSetBreakpointByUrlResponse(locations)); - SendResponse(id, result, token); - + logger.LogDebug($"Debugger.setBreakpointByUrl failed with exception: {e}"); + SendResponse(id, resp, token); } return true; } @@ -1443,7 +1449,7 @@ private async Task SetMonoBreakpoint(SessionId sessionId, string req var assembly_id = await context.SdbAgent.GetAssemblyId(asm_name, token); var methodId = await context.SdbAgent.GetMethodIdByToken(assembly_id, method_token, token); - var breakpoint_id = await context.SdbAgent.SetBreakpoint(methodId, il_offset, token); + var breakpoint_id = await context.SdbAgent.SetBreakpointNoThrow(methodId, il_offset, token); if (breakpoint_id > 0) { @@ -1720,7 +1726,7 @@ private async Task OnSetNextIP(MessageId sessionId, SourceLocation targetL if (!ret) return false; - var breakpointId = await context.SdbAgent.SetBreakpoint(scope.Method.DebugId, ilOffset.Offset, token); + var breakpointId = await context.SdbAgent.SetBreakpointNoThrow(scope.Method.DebugId, ilOffset.Offset, token); if (breakpointId == -1) return false; diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index d5d301e8bd0209..68024419d9b3a9 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -1350,7 +1350,7 @@ public async Task GetParameters(int methodId, CancellationToken token) return parameters; } - public async Task SetBreakpoint(int methodId, long il_offset, CancellationToken token) + public async Task SetBreakpointNoThrow(int methodId, long il_offset, CancellationToken token) { using var commandParamsWriter = new MonoBinaryWriter(); commandParamsWriter.Write((byte)EventKind.Breakpoint); From 6eeccfd560cdfec8e009d04f6084dcd33acc9eef Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 13 Sep 2022 18:14:15 -0300 Subject: [PATCH 3/6] Update src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs Co-authored-by: Ankit Jain --- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index dd248617b89143..8821b6c68d3a4d 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -361,7 +361,8 @@ protected override async Task AcceptCommand(MessageId id, JObject parms, C SendResponse(id, resp, token); return true; } - try { + try + { string bpid = resp.Value["breakpointId"]?.ToString(); IEnumerable locations = resp.Value["locations"]?.Values(); var request = BreakpointRequest.Parse(bpid, args); From 7112b51135cd55eb4139090d68326ec6fbbf864a Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 13 Sep 2022 18:17:08 -0300 Subject: [PATCH 4/6] Update src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs Co-authored-by: Ankit Jain --- src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 68024419d9b3a9..cbe0692125b352 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -1359,7 +1359,7 @@ public async Task SetBreakpointNoThrow(int methodId, long il_offset, Cancel commandParamsWriter.Write((byte)ModifierKind.LocationOnly); commandParamsWriter.Write(methodId); commandParamsWriter.Write(il_offset); - using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdEventRequest.Set, commandParamsWriter, token, false); + using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdEventRequest.Set, commandParamsWriter, token, throwOnError: false); if (retDebuggerCmdReader.HasError) return -1; return retDebuggerCmdReader.ReadInt32(); From 64e86efc86de7c8fef75ccfb183cf97414dec4bd Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 13 Sep 2022 18:19:00 -0300 Subject: [PATCH 5/6] addressing @radical comments --- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 8821b6c68d3a4d..f66adc9694061f 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -398,7 +398,7 @@ protected override async Task AcceptCommand(MessageId id, JObject parms, C catch (Exception e) { logger.LogDebug($"Debugger.setBreakpointByUrl failed with exception: {e}"); - SendResponse(id, resp, token); + SendResponse(id, Result.Err($"Debugger.setBreakpointByUrl failed with exception: {e}"), token); } return true; } From 2ce1a1bf025c7c6d2f75b1d7cb0327ced437b482 Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 13 Sep 2022 18:37:57 -0300 Subject: [PATCH 6/6] addressing @radical comments --- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index f66adc9694061f..b0d316e42dc1a2 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -397,8 +397,8 @@ protected override async Task AcceptCommand(MessageId id, JObject parms, C } catch (Exception e) { - logger.LogDebug($"Debugger.setBreakpointByUrl failed with exception: {e}"); - SendResponse(id, Result.Err($"Debugger.setBreakpointByUrl failed with exception: {e}"), token); + logger.LogDebug($"Debugger.setBreakpointByUrl - {args} - failed with exception: {e}"); + SendResponse(id, Result.Err($"Debugger.setBreakpointByUrl - {args} - failed with exception: {e}"), token); } return true; } @@ -1450,6 +1450,7 @@ private async Task SetMonoBreakpoint(SessionId sessionId, string req var assembly_id = await context.SdbAgent.GetAssemblyId(asm_name, token); var methodId = await context.SdbAgent.GetMethodIdByToken(assembly_id, method_token, token); + //the breakpoint can be invalid because a race condition between the changes already applied on runtime and not applied yet on debugger side var breakpoint_id = await context.SdbAgent.SetBreakpointNoThrow(methodId, il_offset, token); if (breakpoint_id > 0)