From f85f579e36edd527542cbdd6200f91e5b68780aa Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Tue, 20 Jul 2021 22:32:42 -0300 Subject: [PATCH] [wasm][debugger] Fix step behavior (#55915) * Creating test to close 49143. * Creating test to close https://github.com/dotnet/runtime/issues/49141 * Adding test for https://github.com/dotnet/runtime/issues/49218. * Fix behavior of step to be the same of what we see when debugging using debugger-libs+mono or coreclr. Fix error message of evaluate calling methods. Adding test for https://github.com/dotnet/runtime/issues/49142 * Improving test to test what @radical asked. * Changing what was suggested by @radical. --- .../BrowserDebugProxy/MonoSDBHelper.cs | 8 +- .../DebuggerTestSuite/DebuggerTestBase.cs | 17 +++ .../EvaluateOnCallFrameTests.cs | 24 +--- .../DebuggerTestSuite/SteppingTests.cs | 76 ++++++++++++- .../tests/debugger-test/debugger-test.cs | 104 +++++++++++++++--- 5 files changed, 188 insertions(+), 41 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 645a6a4ac2a2c..77b3695ff7d30 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -337,6 +337,12 @@ internal enum StepFilter { DebuggerNonUserCode = 8 } + internal enum StepSize + { + Minimal, + Line + } + internal class MonoBinaryReader : BinaryReader { public MonoBinaryReader(Stream stream) : base(stream) {} @@ -897,7 +903,7 @@ public async Task Step(SessionId sessionId, int thread_id, StepKind kind, commandParamsWriter.Write((byte)1); commandParamsWriter.Write((byte)ModifierKind.Step); commandParamsWriter.Write(thread_id); - commandParamsWriter.Write((int)0); + commandParamsWriter.Write((int)StepSize.Line); commandParamsWriter.Write((int)kind); commandParamsWriter.Write((int)(StepFilter.StaticCtor | StepFilter.DebuggerHidden)); //filter var retDebuggerCmdReader = await SendDebuggerAgentCommand(sessionId, CmdEventRequest.Set, commandParams, token); diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs index b26624807150f..1d30086b6b2d9 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs @@ -902,6 +902,23 @@ internal async Task SetBreakpointInMethod(string assembly, string type, return res; } + internal async Task EvaluateOnCallFrameAndCheck(string call_frame_id, params (string expression, JObject expected)[] args) + { + foreach (var arg in args) + { + var (eval_val, _) = await EvaluateOnCallFrame(call_frame_id, arg.expression); + try + { + await CheckValue(eval_val, arg.expected, arg.expression); + } + catch + { + Console.WriteLine($"CheckValue failed for {arg.expression}. Expected: {arg.expected}, vs {eval_val}"); + throw; + } + } + } + internal void AssertEqual(object expected, object actual, string label) { if (expected?.Equals(actual) == true) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs index d6803ee8670fd..a28080c4c70f7 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs @@ -476,23 +476,6 @@ public async Task EvaluatePropertyThatThrows() await EvaluateOnCallFrameAndCheck(id, ("this.PropertyThrowException", TString("System.Exception: error"))); }); - async Task EvaluateOnCallFrameAndCheck(string call_frame_id, params (string expression, JObject expected)[] args) - { - foreach (var arg in args) - { - var (eval_val, _) = await EvaluateOnCallFrame(call_frame_id, arg.expression); - try - { - await CheckValue(eval_val, arg.expected, arg.expression); - } - catch - { - Console.WriteLine($"CheckValue failed for {arg.expression}. Expected: {arg.expected}, vs {eval_val}"); - throw; - } - } - } - async Task EvaluateOnCallFrameFail(string call_frame_id, params (string expression, string class_name)[] args) { foreach (var arg in args) @@ -513,7 +496,7 @@ public async Task EvaluateSimpleMethodCallsError() => await CheckInspectLocalsAt var id = pause_location["callFrames"][0]["callFrameId"].Value(); var (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethodWrong()", expect_ok: false ); - AssertEqual("Method 'MyMethodWrong' not found in type 'DebuggerTests.EvaluateMethodTestsClass.ParmToTest'", res.Error["message"]?.Value(), "wrong error message"); + AssertEqual("Unable to evaluate method 'MyMethodWrong'", res.Error["message"]?.Value(), "wrong error message"); (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethod(1)", expect_ok: false ); AssertEqual("Unable to evaluate method 'MyMethod'", res.Error["message"]?.Value(), "wrong error message"); @@ -522,10 +505,10 @@ public async Task EvaluateSimpleMethodCallsError() => await CheckInspectLocalsAt AssertEqual("Unable to evaluate method 'CallMethodWithParm'", res.Error["message"]?.Value(), "wrong error message"); (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjNull.MyMethod()", expect_ok: false ); - AssertEqual("Object reference not set to an instance of an object.", res.Error["message"]?.Value(), "wrong error message"); + AssertEqual("Unable to evaluate method 'MyMethod'", res.Error["message"]?.Value(), "wrong error message"); (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjException.MyMethod()", expect_ok: false ); - AssertEqual("Object reference not set to an instance of an object.", res.Error["message"]?.Value(), "wrong error message"); + AssertEqual("Unable to evaluate method 'MyMethod'", res.Error["message"]?.Value(), "wrong error message"); }); [Fact] @@ -540,6 +523,7 @@ await EvaluateOnCallFrameAndCheck(id, ("this.CallMethod()", TNumber(1)), ("this.CallMethod()", TNumber(1)), ("this.ParmToTestObj.MyMethod()", TString("methodOK")), + ("this.ParmToTestObj.ToString()", TString("DebuggerTests.EvaluateMethodTestsClass+ParmToTest")), ("this.objToTest.MyMethod()", TString("methodOK"))); }); diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/SteppingTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/SteppingTests.cs index eed238c6f5902..081cd28732d44 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/SteppingTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/SteppingTests.cs @@ -198,7 +198,7 @@ public async Task InspectLocalsInPreviousFramesDuringSteppingIn(bool use_cfo) CheckObject(locals_m1, "nim", "Math.NestedInMath"); // step back into OuterMethod - await StepAndCheck(StepKind.Over, debugger_test_loc, 91, 8, "OuterMethod", times: 9, + await StepAndCheck(StepKind.Over, debugger_test_loc, 91, 8, "OuterMethod", times: 6, locals_fn: (locals) => { Assert.Equal(5, locals.Count()); @@ -236,7 +236,7 @@ await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", } ); - await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 92, 8, "OuterMethod", times: 2, + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 92, 8, "OuterMethod", times: 1, locals_fn: (locals) => { Assert.Equal(5, locals.Count()); @@ -284,7 +284,7 @@ await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", // Step into InnerMethod await StepAndCheck(StepKind.Into, "dotnet://debugger-test.dll/debugger-test.cs", 105, 8, "InnerMethod"); - await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 109, 12, "InnerMethod", times: 5, + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 110, 12, "InnerMethod", times: 5, locals_fn: (locals) => { Assert.Equal(4, locals.Count()); @@ -297,7 +297,7 @@ await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", ); // Step back to OuterMethod - await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 88, 8, "OuterMethod", times: 6, + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 90, 8, "OuterMethod", times: 6, locals_fn: (locals) => { Assert.Equal(5, locals.Count()); @@ -469,7 +469,7 @@ public async Task InspectValueTypeMethodArgsWhileStepping(bool use_cfo) // ----------- Step back to the caller --------- pause_location = await StepAndCheck(StepKind.Over, debugger_test_loc, 30, 12, "TestStructsAsMethodArgs", - times: 2, locals_fn: (l) => { /* non-null to make sure that locals get fetched */ }); + times: 1, locals_fn: (l) => { /* non-null to make sure that locals get fetched */ }); locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value()); await CheckProps(locals, new { @@ -862,5 +862,71 @@ public async Task BreakpointOnHiddenLineOfMethodWithNoNextVisibleLineShouldNotPa Task t = await Task.WhenAny(pause_task, Task.Delay(2000)); Assert.True(t != pause_task, "Debugger unexpectedly paused"); } + + [Fact] + public async Task SimpleStep_RegressionTest_49141() + { + await SetBreakpoint("dotnet://debugger-test.dll/debugger-test.cs", 674, 0); + + string expression = "window.setTimeout(function() { invoke_static_method ('[debugger-test] Foo:RunBart'); }, 1);"; + await EvaluateAndCheck( + expression, + "dotnet://debugger-test.dll/debugger-test.cs", 674, 12, + "Bart"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 677, 8, "Bart"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 678, 4, "Bart"); + } + + [Fact] + public async Task StepAndEvaluateExpression() + { + await SetBreakpoint("dotnet://debugger-test.dll/debugger-test.cs", 682, 0); + + await EvaluateAndCheck( + "window.setTimeout(function() { invoke_static_method ('[debugger-test] Foo:RunBart'); }, 1);", + "dotnet://debugger-test.dll/debugger-test.cs", 682, 8, + "RunBart"); + var pause_location = await StepAndCheck(StepKind.Into, "dotnet://debugger-test.dll/debugger-test.cs", 671, 4, "Bart"); + var id = pause_location["callFrames"][0]["callFrameId"].Value(); + await EvaluateOnCallFrameAndCheck(id, ("this.Bar", TString("Same of something"))); + pause_location = await StepAndCheck(StepKind.Into, "dotnet://debugger-test.dll/debugger-test.cs", 673, 8, "Bart"); + id = pause_location["callFrames"][0]["callFrameId"].Value(); + await EvaluateOnCallFrameAndCheck(id, ("this.Bar", TString("Same of something"))); + } + + [Fact] + public async Task StepOverWithMoreThanOneCommandInSameLine() + { + await SetBreakpoint("dotnet://debugger-test.dll/debugger-test.cs", 693, 0); + + string expression = "window.setTimeout(function() { invoke_static_method ('[debugger-test] Foo:RunBart'); }, 1);"; + await EvaluateAndCheck( + expression, + "dotnet://debugger-test.dll/debugger-test.cs", 693, 8, + "OtherBar"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 694, 8, "OtherBar"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 696, 8, "OtherBar"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 699, 8, "OtherBar"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 701, 8, "OtherBar"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 702, 4, "OtherBar"); + } + + [Fact] + public async Task StepOverWithMoreThanOneCommandInSameLineAsync() + { + await SetBreakpoint("dotnet://debugger-test.dll/debugger-test.cs", 710, 0); + + string expression = "window.setTimeout(function() { invoke_static_method ('[debugger-test] Foo:RunBart'); }, 1);"; + await EvaluateAndCheck( + expression, + "dotnet://debugger-test.dll/debugger-test.cs", 710, 8, + "MoveNext"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 711, 8, "MoveNext"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 713, 8, "MoveNext"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 716, 8, "MoveNext"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 718, 8, "MoveNext"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 719, 8, "MoveNext"); + await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-test.cs", 720, 4, "MoveNext"); + } } } diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs b/src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs index 228d9d1ff62e5..c0990612185db 100644 --- a/src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test/debugger-test.cs @@ -2,7 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Threading.Tasks; +using System.Linq; public partial class Math { //Only append content to this class as the test suite depends on line info public static int IntAdd(int a, int b) @@ -361,7 +361,7 @@ public static void BoxingTest() Console.WriteLine ($"break here"); } - public static async Task BoxingTestAsync() + public static async System.Threading.Tasks.Task BoxingTestAsync() { int? n_i = 5; object o_i = n_i.Value; @@ -380,7 +380,7 @@ public static async Task BoxingTestAsync() object o_ia = new int[] {918, 58971}; Console.WriteLine ($"break here"); - await Task.CompletedTask; + await System.Threading.Tasks.Task.CompletedTask; } public static void BoxedTypeObjectTest() @@ -395,7 +395,7 @@ public static void BoxedTypeObjectTest() object oo0 = oo; Console.WriteLine ($"break here"); } - public static async Task BoxedTypeObjectTestAsync() + public static async System.Threading.Tasks.Task BoxedTypeObjectTestAsync() { int i = 5; object o0 = i; @@ -406,7 +406,7 @@ public static async Task BoxedTypeObjectTestAsync() object oo = new object(); object oo0 = oo; Console.WriteLine ($"break here"); - await Task.CompletedTask; + await System.Threading.Tasks.Task.CompletedTask; } public static void BoxedAsClass() @@ -419,7 +419,7 @@ public static void BoxedAsClass() Console.WriteLine ($"break here"); } - public static async Task BoxedAsClassAsync() + public static async System.Threading.Tasks.Task BoxedAsClassAsync() { ValueType vt_dt = new DateTime(4819, 5, 6, 7, 8, 9); ValueType vt_gs = new Math.GenericStruct { StringField = "vt_gs#StringField" }; @@ -427,7 +427,7 @@ public static async Task BoxedAsClassAsync() Enum ee = System.IO.FileMode.Append; Console.WriteLine ($"break here"); - await Task.CompletedTask; + await System.Threading.Tasks.Task.CompletedTask; } } @@ -452,14 +452,14 @@ public void Test() TestEvent?.Invoke(this, Delegate?.ToString()); } - public async Task TestAsync() + public async System.Threading.Tasks.Task TestAsync() { TestEvent += (_, s) => Console.WriteLine(s); TestEvent += (_, s) => Console.WriteLine(s + "qwe"); Delegate = TestEvent; TestEvent?.Invoke(this, Delegate?.ToString()); - await Task.CompletedTask; + await System.Threading.Tasks.Task.CompletedTask; } } @@ -470,10 +470,10 @@ public static void StaticMethodWithNoLocals() Console.WriteLine($"break here"); } - public static async Task StaticMethodWithNoLocalsAsync() + public static async System.Threading.Tasks.Task StaticMethodWithNoLocalsAsync() { Console.WriteLine($"break here"); - await Task.CompletedTask; + await System.Threading.Tasks.Task.CompletedTask; } public static void run() @@ -490,10 +490,10 @@ public static void StaticMethodWithNoLocals() Console.WriteLine($"break here"); } - public static async Task StaticMethodWithNoLocalsAsync() + public static async System.Threading.Tasks.Task StaticMethodWithNoLocalsAsync() { Console.WriteLine($"break here"); - await Task.CompletedTask; + await System.Threading.Tasks.Task.CompletedTask; } public static void StaticMethodWithLocalEmptyStruct() @@ -502,11 +502,11 @@ public static void StaticMethodWithLocalEmptyStruct() Console.WriteLine($"break here"); } - public static async Task StaticMethodWithLocalEmptyStructAsync() + public static async System.Threading.Tasks.Task StaticMethodWithLocalEmptyStructAsync() { var es = new EmptyStruct(); Console.WriteLine($"break here"); - await Task.CompletedTask; + await System.Threading.Tasks.Task.CompletedTask; } public static void run() @@ -653,3 +653,77 @@ internal static void ApplyUpdate (System.Reflection.Assembly assm, int version) } } + +public class Something +{ + public string Name { get; set; } + public Something() => Name = "Same of something"; + public override string ToString() => Name; +} + +public class Foo +{ + public string Bar => Stuffs.First(x => x.Name.StartsWith('S')).Name; + public System.Collections.Generic.List Stuffs { get; } = Enumerable.Range(0, 10).Select(x => new Something()).ToList(); + public string Lorem { get; set; } = "Safe"; + public string Ipsum { get; set; } = "Side"; + public Something What { get; } = new Something(); + public int Bart() + { + int ret; + if (Lorem.StartsWith('S')) + ret = 0; + else + ret = 1; + return ret; + } + public static void RunBart() + { + Foo foo = new Foo(); + foo.Bart(); + Console.WriteLine(foo.OtherBar()); + foo.OtherBarAsync().Wait(10); + } + public bool OtherBar() + { + var a = 1; + var b = 2; + var x = "Stew"; + var y = "00.123"; + var c = a + b == 3 || b + a == 2; + var d = TimeSpan.TryParseExact(y, @"ss\.fff", null, out var ts) && x.Contains('S'); + var e = TimeSpan.TryParseExact(y, @"ss\.fff", null, out var ts1) + && x.Contains('S'); + var f = TimeSpan.TryParseExact(y, @"ss\.fff", null, out var ts2) + && + x.Contains('S'); + var g = TimeSpan.TryParseExact(y, @"ss\.fff", null, out var ts3) && + x.Contains('S'); + return d && e == true; + } + public async System.Threading.Tasks.Task OtherBarAsync() + { + var a = 1; + var b = 2; + var x = "Stew"; + var y = "00.123"; + var c = a + b == 3 || b + a == 2; + var d = TimeSpan.TryParseExact(y, @"ss\.fff", null, out var ts) && await AsyncMethod(); + var e = TimeSpan.TryParseExact(y, @"ss\.fff", null, out var ts1) + && await AsyncMethod(); + var f = TimeSpan.TryParseExact(y, @"ss\.fff", null, out var ts2) + && + await AsyncMethod(); + var g = await AsyncMethod() && + await AsyncMethod(); + Console.WriteLine(g); + await System.Threading.Tasks.Task.CompletedTask; + } + public async System.Threading.Tasks.Task AsyncMethod() + { + await System.Threading.Tasks.Task.Delay(1); + Console.WriteLine($"time for await"); + return true; + } +} +