From a92e44e85546b5921dac55d5977c4215ce08ec49 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 1130f53efc97d8..48ea126cb53e4c 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -1727,6 +1727,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 3a25a7fac07929..0d436f3d4f280c 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -1365,7 +1365,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 013c6c5a8dbce5e481748f144af40c54a65fdfbc 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 48ea126cb53e4c..ff050e210976f5 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; } @@ -1449,7 +1455,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) { @@ -1726,7 +1732,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 0d436f3d4f280c..1d70dc557c835c 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -1356,7 +1356,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 50f60142d1288ca3cb37fe54b19af87b9378b558 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 ff050e210976f5..0015997f2e2f61 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 273c0dc0d55766ddfdd92874a4d68b24e2e8d1e0 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 1d70dc557c835c..8ff9f8ae9af99e 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -1365,7 +1365,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 d8a6a1c3cc262be4d17f13b6feda39ec719deb45 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 0015997f2e2f61..b2616c622bf86c 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 b28920d42db67e7fc1ffff710e0fed41f4cb8089 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 b2616c622bf86c..cf4bb725cdc348 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; } @@ -1456,6 +1456,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)