From e927c14d162c3693bcc0e3b0e8976cc7f8a6ed4a Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Thu, 16 Feb 2023 10:21:52 -0300 Subject: [PATCH] [wasm][debugger] Skip wasm frames if just my code is enabled (#81732) * Trying to avoid stepping out when there is no source available and justmycode is enabled. * Skip only wasm frames. * Fix test after using DebuggerNonUserCode on JSImport and JSExport. * Fix stepping out when JustMyCode is enabled and the function that should be skipped is the last in the callstack. * Fix side effect. * addressing radical comments offline --- .../BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs | 2 +- .../wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 161 ++++++++++++--------- .../debugger/DebuggerTestSuite/SteppingTests.cs | 21 +++ src/mono/wasm/runtime/rollup.config.js | 4 +- 4 files changed, 120 insertions(+), 68 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs index 41a1ea5..25c7dc8 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs @@ -895,7 +895,7 @@ internal sealed class FirefoxMonoProxy : MonoProxy if (method is null) return false; - if (await ShouldSkipMethod(sessionId, context, event_kind, 0, method, token)) + if (await ShouldSkipMethod(sessionId, context, event_kind, 0, frame_count, method, token)) { await SendResume(sessionId, token); return true; diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 0f9385e..148bde4 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -154,52 +154,7 @@ namespace Microsoft.WebAssembly.Diagnostics case "Debugger.paused": { - if (args["asyncStackTraceId"] != null) - { - if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) - return false; - if (context.CopyDataFromParentContext()) - { - var store = await LoadStore(sessionId, true, token); - foreach (var source in store.AllSources()) - { - await OnSourceFileAdded(sessionId, source, context, token, false); - } - } - } - - //TODO figure out how to stich out more frames and, in particular what happens when real wasm is on the stack - string top_func = args?["callFrames"]?[0]?["functionName"]?.Value(); - switch (top_func) { - // keep function names un-mangled via src\mono\wasm\runtime\rollup.config.js - case "mono_wasm_set_entrypoint_breakpoint": - case "_mono_wasm_set_entrypoint_breakpoint": - { - await OnSetEntrypointBreakpoint(sessionId, args, token); - return true; - } - case "mono_wasm_runtime_ready": - case "_mono_wasm_runtime_ready": - { - await RuntimeReady(sessionId, token); - await SendResume(sessionId, token); - if (!JustMyCode) - await ReloadSymbolsFromSymbolServer(sessionId, GetContext(sessionId), token); - return true; - } - case "mono_wasm_fire_debugger_agent_message_with_data_to_pause": - case "_mono_wasm_fire_debugger_agent_message_with_data_to_pause": - try - { - return await OnReceiveDebuggerAgentEvent(sessionId, args, await GetLastDebuggerAgentBuffer(sessionId, args, token), token); - } - catch (Exception) //if the page is refreshed maybe it stops here. - { - await SendResume(sessionId, token); - return true; - } - } - break; + return await OnDebuggerPaused(sessionId, args, token); } case "Debugger.breakpointResolved": @@ -223,7 +178,74 @@ namespace Microsoft.WebAssembly.Diagnostics break; } } + return false; + } + protected async Task OnDebuggerPaused(SessionId sessionId, JObject args, CancellationToken token) + { + if (args["asyncStackTraceId"] != null) + { + if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) + return false; + if (context.CopyDataFromParentContext()) + { + var store = await LoadStore(sessionId, true, token); + foreach (var source in store.AllSources()) + { + await OnSourceFileAdded(sessionId, source, context, token, false); + } + } + } + + //TODO figure out how to stich out more frames and, in particular what happens when real wasm is on the stack + string top_func = args?["callFrames"]?[0]?["functionName"]?.Value(); + switch (top_func) { + // keep function names un-mangled via src\mono\wasm\runtime\rollup.config.js + case "mono_wasm_set_entrypoint_breakpoint": + case "_mono_wasm_set_entrypoint_breakpoint": + { + await OnSetEntrypointBreakpoint(sessionId, args, token); + return true; + } + case "mono_wasm_runtime_ready": + case "_mono_wasm_runtime_ready": + { + await RuntimeReady(sessionId, token); + await SendResume(sessionId, token); + if (!JustMyCode) + await ReloadSymbolsFromSymbolServer(sessionId, GetContext(sessionId), token); + return true; + } + case "mono_wasm_fire_debugger_agent_message_with_data_to_pause": + case "_mono_wasm_fire_debugger_agent_message_with_data_to_pause": + try + { + return await OnReceiveDebuggerAgentEvent(sessionId, args, await GetLastDebuggerAgentBuffer(sessionId, args, token), token); + } + catch (Exception) //if the page is refreshed maybe it stops here. + { + await SendResume(sessionId, token); + return true; + } + case "mono_wasm_fire_debugger_agent_message_with_data": + case "_mono_wasm_fire_debugger_agent_message_with_data": + { + //the only reason that we would get pause in this method is because the user is stepping out + //and as we don't want to pause in a debugger related function we continue stepping out + await SendCommand(sessionId, "Debugger.stepOut", new JObject(), token); + return true; + } + default: + { + //avoid pausing when justMyCode is enabled and it's a wasm function + if (JustMyCode && args?["callFrames"]?[0]?["scopeChain"]?[0]?["type"]?.Value()?.Equals("wasm-expression-stack") == true) + { + await SendCommand(sessionId, "Debugger.stepOut", new JObject(), token); + return true; + } + break; + } + } return false; } @@ -945,7 +967,7 @@ namespace Microsoft.WebAssembly.Diagnostics return true; } - protected virtual async Task ShouldSkipMethod(SessionId sessionId, ExecutionContext context, EventKind event_kind, int frameNumber, MethodInfoWithDebugInformation method, CancellationToken token) + protected virtual async Task ShouldSkipMethod(SessionId sessionId, ExecutionContext context, EventKind event_kind, int frameNumber, int totalFrames, MethodInfoWithDebugInformation method, CancellationToken token) { var shouldReturn = await SkipMethod( isSkippable: context.IsSkippingHiddenMethod, @@ -1006,8 +1028,10 @@ namespace Microsoft.WebAssembly.Diagnostics { if (isSkippable && shouldBeSkipped) { - await context.SdbAgent.Step(context.ThreadId, stepKind, token); - await SendResume(sessionId, token); + if (frameNumber + 1 == totalFrames && stepKind == StepKind.Out) //is the last managed frame + await SendCommand(sessionId, "Debugger.stepOut", new JObject(), token); + else + await TryStepOnManagedCodeAndStepOutIfNotPossible(sessionId, context, stepKind, token); return true; } return false; @@ -1035,7 +1059,7 @@ namespace Microsoft.WebAssembly.Diagnostics DebugStore store = await LoadStore(sessionId, true, token); var method = await context.SdbAgent.GetMethodInfo(methodId, token); - if (await ShouldSkipMethod(sessionId, context, event_kind, j, method, token)) + if (await ShouldSkipMethod(sessionId, context, event_kind, j, frame_count, method, token)) return true; SourceLocation location = method?.Info.GetLocationByIl(il_pos); @@ -1092,10 +1116,13 @@ namespace Microsoft.WebAssembly.Diagnostics { string function_name = frame["functionName"]?.Value(); string url = frame["url"]?.Value(); + var isWasmExpressionStack = frame["scopeChain"]?[0]?["type"]?.Value()?.Equals("wasm-expression-stack") == true; if (!(function_name.StartsWith("wasm-function", StringComparison.Ordinal) || url.StartsWith("wasm://", StringComparison.Ordinal) || url.EndsWith(".wasm", StringComparison.Ordinal) || - function_name == "_mono_wasm_fire_debugger_agent_message")) + JustMyCode && isWasmExpressionStack || + function_name.StartsWith("_mono_wasm_fire_debugger_agent_message", StringComparison.Ordinal) || + function_name.StartsWith("mono_wasm_fire_debugger_agent_message", StringComparison.Ordinal))) { callFrames.Add(frame); } @@ -1255,7 +1282,21 @@ namespace Microsoft.WebAssembly.Diagnostics //discard managed frames GetContext(msg_id).ClearState(); } + protected async Task TryStepOnManagedCodeAndStepOutIfNotPossible(SessionId sessionId, ExecutionContext context, StepKind kind, CancellationToken token) + { + var step = await context.SdbAgent.Step(context.ThreadId, kind, token); + if (step == false) //it will return false if it's the last managed frame and the runtime added the single step breakpoint in a MONO_WRAPPER_RUNTIME_INVOKE + { + context.ClearState(); + await SendCommand(sessionId, "Debugger.stepOut", new JObject(), token); + return false; + } + + context.ClearState(); + await SendResume(sessionId, token); + return true; + } protected async Task Step(MessageId msgId, StepKind kind, CancellationToken token) { @@ -1265,20 +1306,10 @@ namespace Microsoft.WebAssembly.Diagnostics if (context.CallStack.Count <= 1 && kind == StepKind.Out) return false; - - var step = await context.SdbAgent.Step(context.ThreadId, kind, token); - if (step == false) { - context.ClearState(); - await SendCommand(msgId, "Debugger.stepOut", new JObject(), token); - return false; - } - - SendResponse(msgId, Result.Ok(new JObject()), token); - - context.ClearState(); - - await SendResume(msgId, token); - return true; + var ret = await TryStepOnManagedCodeAndStepOutIfNotPossible(msgId, context, kind, token); + if (ret) + SendResponse(msgId, Result.Ok(new JObject()), token); + return ret; } private async Task OnJSEventRaised(SessionId sessionId, JObject eventArgs, CancellationToken token) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/SteppingTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/SteppingTests.cs index a3f58e4..7778d31 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/SteppingTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/SteppingTests.cs @@ -1215,5 +1215,26 @@ namespace DebuggerTests }, times: 2 ); } + + [ConditionalTheory(nameof(RunningOnChrome))] + [InlineData(true)] + [InlineData(false)] + public async Task SkipWasmFunctionsAccordinglyJustMyCode(bool justMyCode) + { + await SetJustMyCode(justMyCode); + var bp = await SetBreakpoint("dotnet://debugger-test.dll/debugger-test.cs", 10, 8); + + var pause_location = await EvaluateAndCheck( + "window.setTimeout(function() { invoke_add(); invoke_add(); }, 1);", + "dotnet://debugger-test.dll/debugger-test.cs", 10, 8, + "Math.IntAdd" + ); + if (justMyCode) + Assert.False(pause_location["callFrames"].Value().Any(f => f?["scopeChain"]?[0]?["type"]?.Value()?.Equals("wasm-expression-stack") == true)); + else + Assert.True(pause_location["callFrames"].Value().Any(f => f?["scopeChain"]?[0]?["type"]?.Value()?.Equals("wasm-expression-stack") == true)); + if (justMyCode) + await StepAndCheck(StepKind.Out, "dotnet://debugger-test.dll/debugger-test.cs", 10, 8, "Math.IntAdd", times: 4); + } } } diff --git a/src/mono/wasm/runtime/rollup.config.js b/src/mono/wasm/runtime/rollup.config.js index 299c253..d9b5269 100644 --- a/src/mono/wasm/runtime/rollup.config.js +++ b/src/mono/wasm/runtime/rollup.config.js @@ -27,7 +27,7 @@ const terserConfig = { mangle: { // because of stack walk at src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs // and unit test at src\libraries\System.Runtime.InteropServices.JavaScript\tests\System.Runtime.InteropServices.JavaScript.Legacy.UnitTests\timers.mjs - keep_fnames: /(mono_wasm_runtime_ready|mono_wasm_fire_debugger_agent_message_with_data_to_pause|mono_wasm_set_timeout_exec)/, + keep_fnames: /(mono_wasm_runtime_ready|mono_wasm_fire_debugger_agent_message_with_data|mono_wasm_fire_debugger_agent_message_with_data_to_pause|mono_wasm_set_timeout_exec)/, keep_classnames: /(ManagedObject|ManagedError|Span|ArraySegment|WasmRootBuffer|SessionOptionsBuilder)/, }, format: { @@ -328,4 +328,4 @@ function onwarn(warning) { } console.warn(`(!) ${warning.toString()}`); -} \ No newline at end of file +} -- 2.7.4