From c1dd1228f545a4122b0202cc1b2497a663b88895 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Tue, 1 Sep 2020 02:29:13 -0400 Subject: [PATCH 01/10] [wasm][debugger] Instead of failing completely, skip the problematic .. property. Some times we might not get a `value`, or `name`, or it might instead have a `get`. Handle those cases correctly when combining the name/value/get objects. This showed up in case of a `MulticastDelegate`, where we didn't have a `value`, and ended up incorrectly combining the name/value objects, thus returning incorrect locals. --- src/mono/wasm/runtime/library_mono.js | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/mono/wasm/runtime/library_mono.js b/src/mono/wasm/runtime/library_mono.js index 9783e7d027137b..9e23d3083fdf1f 100644 --- a/src/mono/wasm/runtime/library_mono.js +++ b/src/mono/wasm/runtime/library_mono.js @@ -362,17 +362,21 @@ var MonoSupportLib = { var i = 0; while (i < var_list.length) { let o = var_list [i]; - const name = o.name; - if (name == null || name == undefined) { + const this_has_name = o.name !== undefined; + let next_has_value_or_get_set = false; + + if (i + 1 < var_list.length) { + const next = var_list [i+1]; + next_has_value_or_get_set = next.value !== undefined || next.get !== undefined || next.set !== undefined; + } + + if (!this_has_name || !next_has_value_or_get_set) { i ++; out_list.push (o); continue; } - if (i + 1 < var_list.length) { - o = Object.assign (o, var_list [i + 1]); - } - + o = Object.assign (o, var_list [i + 1]); out_list.push (o); i += 2; } @@ -396,6 +400,11 @@ var MonoSupportLib = { // Split props into the 3 groups - backing_fields, getters, and all_fields_except_backing_fields props.forEach(p => { + if (p.name === undefined) { + console.debug(`Bug: Found a member with no name. Skipping it. p: ${JSON.stringify(p)}`); + return; + } + if (p.name.endsWith('k__BackingField')) { const auto_prop_name = p.name.replace ('k__BackingField', '') .replace ('<', '') From cc5f8447121dc1c8e66622410c89f31d24222590 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Tue, 1 Sep 2020 15:17:26 -0400 Subject: [PATCH 02/10] [wasm][debugger] Handle MulticastDelegate, and events - Essentially we just surface these as a symbol showing the type name --- src/mono/mono/mini/mini-wasm-debugger.c | 19 +++++----- .../debugger/DebuggerTestSuite/Support.cs | 36 +++++++++++-------- .../wasm/debugger/DebuggerTestSuite/Tests.cs | 18 +++++++++- src/mono/wasm/debugger/tests/debugger-test.cs | 32 +++++++++++++++++ src/mono/wasm/runtime/library_mono.js | 5 +-- 5 files changed, 83 insertions(+), 27 deletions(-) diff --git a/src/mono/mono/mini/mini-wasm-debugger.c b/src/mono/mono/mini/mini-wasm-debugger.c index 3c60fcdd6cd8cb..f18167676f0e35 100644 --- a/src/mono/mono/mini/mini-wasm-debugger.c +++ b/src/mono/mono/mini/mini-wasm-debugger.c @@ -930,17 +930,16 @@ describe_value(MonoType * type, gpointer addr, int gpflags) method = mono_get_delegate_invoke_internal (klass); if (!method) { - DEBUG_PRINTF (2, "Could not get a method for the delegate for %s\n", class_name); - break; - } - - MonoMethod *tm = ((MonoDelegate *)obj)->method; - char *tm_desc = NULL; - if (tm) - tm_desc = mono_method_to_desc_for_js (tm, FALSE); + mono_wasm_add_func_var (class_name, NULL, -1); + } else { + MonoMethod *tm = ((MonoDelegate *)obj)->method; + char *tm_desc = NULL; + if (tm) + tm_desc = mono_method_to_desc_for_js (tm, FALSE); - mono_wasm_add_func_var (class_name, tm_desc, obj_id); - g_free (tm_desc); + mono_wasm_add_func_var (class_name, tm_desc, obj_id); + g_free (tm_desc); + } } else { char *to_string_val = get_to_string_description (class_name, klass, addr); mono_wasm_add_obj_var (class_name, to_string_val, obj_id); diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Support.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Support.cs index e1970e8ab48b09..2a6a64d18d0c82 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Support.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Support.cs @@ -719,26 +719,34 @@ internal async Task CheckValue(JToken actual_val, JToken exp_val, string label) return; } - foreach (var jp in exp_val.Values()) + try { - if (jp.Value.Type == JTokenType.Object) + foreach (var jp in exp_val.Values()) { - var new_val = await GetProperties(actual_val["objectId"].Value()); - await CheckProps(new_val, jp.Value, $"{label}-{actual_val["objectId"]?.Value()}"); + if (jp.Value.Type == JTokenType.Object) + { + var new_val = await GetProperties(actual_val["objectId"].Value()); + await CheckProps(new_val, jp.Value, $"{label}-{actual_val["objectId"]?.Value()}"); - continue; - } + continue; + } - var exp_val_str = jp.Value.Value(); - bool null_or_empty_exp_val = String.IsNullOrEmpty(exp_val_str); + var exp_val_str = jp.Value.Value(); + bool null_or_empty_exp_val = String.IsNullOrEmpty(exp_val_str); - var actual_field_val = actual_val?.Values()?.FirstOrDefault(a_jp => a_jp.Name == jp.Name); - var actual_field_val_str = actual_field_val?.Value?.Value(); - if (null_or_empty_exp_val && String.IsNullOrEmpty(actual_field_val_str)) - continue; + var actual_field_val = actual_val?.Values()?.FirstOrDefault(a_jp => a_jp.Name == jp.Name); + var actual_field_val_str = actual_field_val?.Value?.Value(); + if (null_or_empty_exp_val && String.IsNullOrEmpty(actual_field_val_str)) + continue; - Assert.True(actual_field_val != null, $"[{label}] Could not find value field named {jp.Name}"); - AssertEqual(exp_val_str, actual_field_val_str, $"[{label}] Value for json property named {jp.Name} didn't match."); + Assert.True(actual_field_val != null, $"[{label}] Could not find value field named {jp.Name}"); + AssertEqual(exp_val_str, actual_field_val_str, $"[{label}] Value for json property named {jp.Name} didn't match."); + } + } + catch + { + Console.WriteLine ($"Expected: {exp_val}. Actual: {actual_val}"); + throw; } } diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs index 958b1761a56861..3fc828f19198e1 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs @@ -1624,9 +1624,25 @@ await insp.Ready(async (cli, token) => await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", "dotnet://debugger-test.dll/debugger-test.cs", 12, 8, "IntAdd"); bp = await SetBreakpoint("dotnet://debugger-test.dll/debugger-test.cs", 10, 8); await SendCommandAndCheck(JObject.FromObject(new { }), "Debugger.resume", "dotnet://debugger-test.dll/debugger-test.cs", 10, 8, "IntAdd"); - + }); } + + [Fact] + public async Task MulticastDelegateTest() => await CheckInspectLocalsAtBreakpointSite( + "MulticastDelegateTestClass", "Test", 5, "Test", + "window.setTimeout(function() { invoke_static_method('[debugger-test] MulticastDelegateTestClass:run'); })", + wait_for_event_fn: async (pause_location) => + { + var frame_locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value()); + var this_props = await GetObjectOnLocals(frame_locals, "this"); + await CheckProps(this_props, new + { + TestEvent = TSymbol("System.EventHandler"), + Delegate = TSymbol("System.MulticastDelegate") + }, "this_props"); + }); + //TODO add tests covering basic stepping behavior as step in/out/over } } diff --git a/src/mono/wasm/debugger/tests/debugger-test.cs b/src/mono/wasm/debugger/tests/debugger-test.cs index 4fb58be0166330..ccc51277e12197 100644 --- a/src/mono/wasm/debugger/tests/debugger-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test.cs @@ -340,3 +340,35 @@ public static int locals() static void locals_inner() { } } + +public class MulticastDelegateTestClass +{ + event EventHandler TestEvent; + MulticastDelegate Delegate; + + public static void run() + { + var obj = new MulticastDelegateTestClass(); + obj.Test(); + obj.TestAsync().Wait(); + } + + public void Test() + { + TestEvent += (_, s) => Console.WriteLine (s); + TestEvent += (_, s) => Console.WriteLine (s + "qwe"); + Delegate = TestEvent; + + TestEvent?.Invoke(this, Delegate?.ToString()); + } + + public async Task TestAsync() + { + TestEvent += (_, s) => Console.WriteLine (s); + TestEvent += (_, s) => Console.WriteLine (s + "qwe"); + Delegate = TestEvent; + + TestEvent?.Invoke(this, Delegate?.ToString()); + await Task.CompletedTask; + } +} diff --git a/src/mono/wasm/runtime/library_mono.js b/src/mono/wasm/runtime/library_mono.js index 9e23d3083fdf1f..31fc3c1b2be41c 100644 --- a/src/mono/wasm/runtime/library_mono.js +++ b/src/mono/wasm/runtime/library_mono.js @@ -2086,14 +2086,15 @@ var MonoSupportLib = { var args_sig = parts.splice (1).join (', '); return `${ret_sig} ${method_name} (${args_sig})`; } - let tgt_sig; if (targetName != 0) tgt_sig = args_to_sig (Module.UTF8ToString (targetName)); const type_name = MONO._mono_csharp_fixup_class_name (Module.UTF8ToString (className)); + if (tgt_sig === undefined) + tgt_sig = type_name; - if (objectId == -1) { + if (objectId == -1 || targetName === 0) { // Target property MONO.var_info.push ({ value: { From a7cf0e95d8f7cae2560baadbf7873003ddaa69c0 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Tue, 1 Sep 2020 17:32:38 -0400 Subject: [PATCH 03/10] [wasm][debugger] Fail gracefully when vt could not be expanded --- src/mono/wasm/runtime/library_mono.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/mono/wasm/runtime/library_mono.js b/src/mono/wasm/runtime/library_mono.js index 31fc3c1b2be41c..3226d6789ec8a4 100644 --- a/src/mono/wasm/runtime/library_mono.js +++ b/src/mono/wasm/runtime/library_mono.js @@ -768,6 +768,13 @@ var MonoSupportLib = { if (value.type != "object" || value.isValueType != true || value.expanded != true) // undefined would also give us false continue; + if (value.members === undefined) { + // this could happen for valuetypes that maybe + // we were not able to describe, like `ref` parameters + // So, skip that + continue; + } + // Generate objectId for expanded valuetypes value.objectId = value.objectId || this._new_or_add_id_props ({ scheme: 'valuetype' }); From 6af061394e7901bc3e7c302b02f893e089ba78b2 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 3 Sep 2020 14:14:38 -0400 Subject: [PATCH 04/10] [wasm][debugger] Handle invalid scope ids scope<0, or scope too high - This does get filtered at the proxy level, but app side should be able to handle it too --- src/mono/mono/mini/mini-wasm-debugger.c | 9 +++- .../debugger/DebuggerTestSuite/MonoJsTests.cs | 48 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs diff --git a/src/mono/mono/mini/mini-wasm-debugger.c b/src/mono/mono/mini/mini-wasm-debugger.c index f18167676f0e35..a64df749953011 100644 --- a/src/mono/mono/mini/mini-wasm-debugger.c +++ b/src/mono/mono/mini/mini-wasm-debugger.c @@ -769,6 +769,7 @@ typedef struct { int target_frame; int len; int *pos; + gboolean found; } FrameDescData; /* @@ -1355,6 +1356,8 @@ describe_variables_on_frame (MonoStackFrameInfo *info, MonoContext *ctx, gpointe return FALSE; } + data->found = TRUE; + InterpFrame *frame = (InterpFrame*)info->interp_frame; g_assert (frame); MonoMethod *method = frame->imethod->method; @@ -1389,15 +1392,19 @@ mono_wasm_get_deref_ptr_value (void *value_addr, MonoClass *klass) EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_get_local_vars (int scope, int* pos, int len) { + if (scope < 0) + return FALSE; + FrameDescData data; data.target_frame = scope; data.cur_frame = 0; data.len = len; data.pos = pos; + data.found = FALSE; mono_walk_stack_with_ctx (describe_variables_on_frame, NULL, MONO_UNWIND_NONE, &data); - return TRUE; + return data.found; } EMSCRIPTEN_KEEPALIVE gboolean diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs new file mode 100644 index 00000000000000..1ce2e87d1b170a --- /dev/null +++ b/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs @@ -0,0 +1,48 @@ +using System; +using System.Threading.Tasks; +using Xunit; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; + +namespace DebuggerTests +{ + public class MonoJsTests : DebuggerTestBase + { + [Fact] + public async Task InvalidScopeId() + { + var insp = new Inspector(); + var scripts = SubscribeToScripts(insp); + + await Ready(); + await insp.Ready(async (cli, token) => + { + ctx = new DebugTestContext(cli, insp, token, scripts); + + var bp1_res = await SetBreakpointInMethod("debugger-test.dll", "Math", "IntAdd", 3); + await EvaluateAndCheck( + "window.setTimeout(function() { invoke_static_method('[debugger-test] Math:IntAdd', 1, 2); })", + null, -1, -1, "IntAdd"); + + var var_ids = new[] + { + new { index = 0, name = "one" }, + }; + + var scope_id = "-12"; + var expression = $"MONO.mono_wasm_get_variables({scope_id}, {JsonConvert.SerializeObject(var_ids)})"; + var res = await ctx.cli.SendCommand($"Runtime.evaluate", JObject.FromObject(new { expression, returnByValue = true }), ctx.token); + Assert.False(res.IsOk); + + Console.WriteLine (res); + + scope_id = "30000"; + expression = $"MONO.mono_wasm_get_variables({scope_id}, {JsonConvert.SerializeObject(var_ids)})"; + res = await ctx.cli.SendCommand($"Runtime.evaluate", JObject.FromObject(new { expression, returnByValue = true }), ctx.token); + Console.WriteLine (res); + Assert.False(res.IsOk); + + }); + } + } +} From cfbe65ac998e0fb2f666255da1117a1c273ee36d Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 3 Sep 2020 14:30:20 -0400 Subject: [PATCH 05/10] [wasm][debugger] Handle invalid/missing/failed value descriptions - Eg. missing because of invalid param/local id, or value description failed because of some as yet unsupported type --- src/mono/mono/mini/mini-wasm-debugger.c | 43 +++++--- .../debugger/DebuggerTestSuite/MonoJsTests.cs | 102 +++++++++++++++++- .../wasm/debugger/DebuggerTestSuite/Tests.cs | 39 +++++++ src/mono/wasm/debugger/tests/debugger-test.cs | 56 ++++++++++ src/mono/wasm/runtime/library_mono.js | 35 +++++- 5 files changed, 249 insertions(+), 26 deletions(-) diff --git a/src/mono/mono/mini/mini-wasm-debugger.c b/src/mono/mono/mini/mini-wasm-debugger.c index a64df749953011..7da835e54191ee 100644 --- a/src/mono/mono/mini/mini-wasm-debugger.c +++ b/src/mono/mono/mini/mini-wasm-debugger.c @@ -1037,7 +1037,7 @@ are_getters_allowed (const char *class_name) return FALSE; } -static void +static gboolean invoke_and_describe_getter_value (MonoObject *obj, MonoProperty *p) { ERROR_DECL (error); @@ -1050,11 +1050,11 @@ invoke_and_describe_getter_value (MonoObject *obj, MonoProperty *p) if (!is_ok (error) && exc == NULL) exc = (MonoObject*) mono_error_convert_to_exception (error); if (exc) - describe_value (mono_get_object_type (), &exc, GPFLAG_EXPAND_VALUETYPES); + return describe_value (mono_get_object_type (), &exc, GPFLAG_EXPAND_VALUETYPES); else if (!res || !m_class_is_valuetype (mono_object_class (res))) - describe_value (sig->ret, &res, GPFLAG_EXPAND_VALUETYPES); + return describe_value (sig->ret, &res, GPFLAG_EXPAND_VALUETYPES); else - describe_value (sig->ret, mono_object_unbox_internal (res), GPFLAG_EXPAND_VALUETYPES); + return describe_value (sig->ret, mono_object_unbox_internal (res), GPFLAG_EXPAND_VALUETYPES); } static void @@ -1313,20 +1313,28 @@ describe_non_async_this (InterpFrame *frame, MonoMethod *method) } static gboolean -describe_variable (InterpFrame *frame, MonoMethod *method, int pos, int gpflags) +describe_variable (InterpFrame *frame, MonoMethod *method, MonoMethodHeader *header, int pos, int gpflags) { ERROR_DECL (error); - MonoMethodHeader *header = NULL; MonoType *type = NULL; gpointer addr = NULL; if (pos < 0) { + MonoMethodSignature *sig = mono_method_signature_internal (method); pos = -pos - 1; - type = mono_method_signature_internal (method)->params [pos]; + + if (pos >= sig->param_count) { + DEBUG_PRINTF(1, "BUG: describe_variable, trying to access param indexed %d, but the method (%s) has only %d params\n", pos, method->name, sig->param_count); + return FALSE; + } + + type = sig->params [pos]; addr = mini_get_interp_callbacks ()->frame_get_arg (frame, pos); } else { - header = mono_method_get_header_checked (method, error); - mono_error_assert_ok (error); /* FIXME report error */ + if (pos >= header->num_locals) { + DEBUG_PRINTF(1, "BUG: describe_variable, trying to access local indexed %d, but the method (%s) has only %d locals\n", pos, method->name, header->num_locals); + return FALSE; + } type = header->locals [pos]; addr = mini_get_interp_callbacks ()->frame_get_local (frame, pos); @@ -1334,16 +1342,13 @@ describe_variable (InterpFrame *frame, MonoMethod *method, int pos, int gpflags) DEBUG_PRINTF (2, "adding val %p type [%p] %s\n", addr, type, mono_type_full_name (type)); - describe_value(type, addr, gpflags); - if (header) - mono_metadata_free_mh (header); - - return TRUE; + return describe_value(type, addr, gpflags); } static gboolean describe_variables_on_frame (MonoStackFrameInfo *info, MonoContext *ctx, gpointer ud) { + ERROR_DECL (error); FrameDescData *data = (FrameDescData*)ud; //skip wrappers @@ -1363,14 +1368,19 @@ describe_variables_on_frame (MonoStackFrameInfo *info, MonoContext *ctx, gpointe MonoMethod *method = frame->imethod->method; g_assert (method); + MonoMethodHeader *header = mono_method_get_header_checked (method, error); + mono_error_assert_ok (error); /* FIXME report error */ + for (int i = 0; i < data->len; i++) { - describe_variable (frame, method, data->pos[i], GPFLAG_EXPAND_VALUETYPES); + if (!describe_variable (frame, method, header, data->pos[i], GPFLAG_EXPAND_VALUETYPES)) + mono_wasm_add_typed_value("symbol", "", 0); } describe_async_method_locals (frame, method); describe_non_async_this (frame, method); + mono_metadata_free_mh (header); return TRUE; } @@ -1384,8 +1394,7 @@ mono_wasm_get_deref_ptr_value (void *value_addr, MonoClass *klass) } mono_wasm_add_properties_var ("deref", -1); - describe_value (type->data.type, value_addr, GPFLAG_EXPAND_VALUETYPES); - return TRUE; + return describe_value (type->data.type, value_addr, GPFLAG_EXPAND_VALUETYPES); } //FIXME this doesn't support getting the return value pseudo-var diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs index 1ce2e87d1b170a..f5cad4d9c4e4c6 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Threading.Tasks; using Xunit; using Newtonsoft.Json; @@ -8,6 +9,103 @@ namespace DebuggerTests { public class MonoJsTests : DebuggerTestBase { + [Fact] + public async Task FixupNameValueObjectsWithMissingParts() + { + var insp = new Inspector(); + var scripts = SubscribeToScripts(insp); + + await Ready(); + await insp.Ready(async (cli, token) => + { + ctx = new DebugTestContext(cli, insp, token, scripts); + + var bp1_res = await SetBreakpointInMethod("debugger-test.dll", "Math", "IntAdd", 3); + + var names = new JObject[] + { + JObject.FromObject(new { name = "Abc" }), + JObject.FromObject(new { name = "Def" }), + JObject.FromObject(new { name = "Xyz" }) + }; + + var values = new JObject[] + { + JObject.FromObject(new { value = TObject("testclass") }), + JObject.FromObject(new { value = TString("test string") }), + }; + + var getters = new JObject[] + { + GetterRes("xyz"), + GetterRes("unattached") + }; + + var list = new[] { names[0], names[1], values[0], names[2], getters[0], getters[1] }; + var res = await ctx.cli.SendCommand($"Runtime.evaluate", JObject.FromObject(new { expression = $"MONO._fixup_name_value_objects({JsonConvert.SerializeObject(list)})", returnByValue = true }), ctx.token); + Assert.True(res.IsOk); + + await CheckProps(res.Value["result"]["value"], new + { + Abc = TSymbol(""), + Def = TObject("testclass"), + Xyz = TGetter("xyz") + }, "#1", num_fields: 4); + + JObject.DeepEquals(getters[1], res.Value["result"]["value"].Values().ToArray()[3]); + + JObject GetterRes(string name) => JObject.FromObject(new + { + get = new + { + className = "Function", + description = $"get {name} () {{}}", + type = "function" + } + }); + }); + } + + [Fact] + public async Task GetParamsAndLocalsWithInvalidIndices() + { + var insp = new Inspector(); + var scripts = SubscribeToScripts(insp); + + await Ready(); + await insp.Ready(async (cli, token) => + { + ctx = new DebugTestContext(cli, insp, token, scripts); + + var bp1_res = await SetBreakpointInMethod("debugger-test.dll", "Math", "IntAdd", 3); + var pause_location = await EvaluateAndCheck( + "window.setTimeout(function() { invoke_static_method('[debugger-test] Math:IntAdd', 1, 2); })", + null, -1, -1, "IntAdd"); + + var scope_id = pause_location["callFrames"][0]["callFrameId"].Value(); + var scope = int.Parse(scope_id.Split(new[] {':'}, StringSplitOptions.RemoveEmptyEntries)[2]); + + var var_ids = new[] + { + new { index = 0, name = "one" }, + new { index = -12, name = "bad0" }, + new { index = 1231, name = "bad1" } + }; + + var expression = $"MONO.mono_wasm_get_variables({scope}, {JsonConvert.SerializeObject(var_ids)})"; + + var res = await ctx.cli.SendCommand($"Runtime.evaluate", JObject.FromObject(new { expression, returnByValue = true }), ctx.token); + Assert.True(res.IsOk); + + await CheckProps(res.Value["result"]?["value"], new + { + one = TNumber(3), + bad0 = TSymbol(""), + bad1 = TSymbol("") + }, "results"); + }); + } + [Fact] public async Task InvalidScopeId() { @@ -34,14 +132,10 @@ await EvaluateAndCheck( var res = await ctx.cli.SendCommand($"Runtime.evaluate", JObject.FromObject(new { expression, returnByValue = true }), ctx.token); Assert.False(res.IsOk); - Console.WriteLine (res); - scope_id = "30000"; expression = $"MONO.mono_wasm_get_variables({scope_id}, {JsonConvert.SerializeObject(var_ids)})"; res = await ctx.cli.SendCommand($"Runtime.evaluate", JObject.FromObject(new { expression, returnByValue = true }), ctx.token); - Console.WriteLine (res); Assert.False(res.IsOk); - }); } } diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs index 3fc828f19198e1..d3c8ef2f21c6eb 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs @@ -1643,6 +1643,45 @@ public async Task MulticastDelegateTest() => await CheckInspectLocalsAtBreakpoin }, "this_props"); }); + [Theory] + [InlineData("EmptyClass", false)] + [InlineData("EmptyClass", true)] + [InlineData("EmptyStruct", false)] + [InlineData("EmptyStruct", true)] + public async Task EmptyTypeWithNoLocalsOrParams(string type_name, bool is_async) => await CheckInspectLocalsAtBreakpointSite( + type_name, + $"StaticMethodWithNoLocals{ (is_async ? "Async" : "") }", + 1, + is_async ? "MoveNext" : "StaticMethodWithNoLocals", + $"window.setTimeout(function() {{ invoke_static_method('[debugger-test] {type_name}:run'); }})", + wait_for_event_fn: async (pause_location) => + { + var frame_locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value()); + AssertEqual(0, frame_locals.Values().Count(), "locals"); + }); + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task StaticMethodWithLocalEmptyStructThatWillGetExpanded(bool is_async) => await CheckInspectLocalsAtBreakpointSite( + "EmptyStruct", + $"StaticMethodWithLocalEmptyStruct{ (is_async ? "Async" : "") }", + 1, + is_async ? "MoveNext" : "StaticMethodWithLocalEmptyStruct", + $"window.setTimeout(function() {{ invoke_static_method('[debugger-test] EmptyStruct:run'); }})", + wait_for_event_fn: async (pause_location) => + { + var frame_locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value()); + await CheckProps(frame_locals, new + { + es = TValueType("EmptyStruct") + }, "locals"); + + var es = GetAndAssertObjectWithName(frame_locals, "es"); + var es_props = await GetProperties(es["value"]["objectId"]?.Value()); + AssertEqual(0, es_props.Values().Count(), "es_props"); + }); + //TODO add tests covering basic stepping behavior as step in/out/over } } diff --git a/src/mono/wasm/debugger/tests/debugger-test.cs b/src/mono/wasm/debugger/tests/debugger-test.cs index ccc51277e12197..d22bfeb97ee379 100644 --- a/src/mono/wasm/debugger/tests/debugger-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test.cs @@ -372,3 +372,59 @@ public async Task TestAsync() await Task.CompletedTask; } } + +public class EmptyClass +{ + public static void StaticMethodWithNoLocals() + { + Console.WriteLine ($"break here"); + } + + public static async Task StaticMethodWithNoLocalsAsync() + { + Console.WriteLine ($"break here"); + await Task.CompletedTask; + } + + public static void run() + { + StaticMethodWithNoLocals(); + StaticMethodWithNoLocalsAsync().Wait(); + } +} + +public struct EmptyStruct +{ + public static void StaticMethodWithNoLocals() + { + Console.WriteLine ($"break here"); + } + + public static async Task StaticMethodWithNoLocalsAsync() + { + Console.WriteLine ($"break here"); + await Task.CompletedTask; + } + + public static void StaticMethodWithLocalEmptyStruct() + { + var es = new EmptyStruct(); + Console.WriteLine ($"break here"); + } + + public static async Task StaticMethodWithLocalEmptyStructAsync() + { + var es = new EmptyStruct(); + Console.WriteLine ($"break here"); + await Task.CompletedTask; + } + + public static void run() + { + StaticMethodWithNoLocals(); + StaticMethodWithNoLocalsAsync().Wait(); + + StaticMethodWithLocalEmptyStruct(); + StaticMethodWithLocalEmptyStructAsync().Wait(); + } +} diff --git a/src/mono/wasm/runtime/library_mono.js b/src/mono/wasm/runtime/library_mono.js index 3226d6789ec8a4..530fec0145257e 100644 --- a/src/mono/wasm/runtime/library_mono.js +++ b/src/mono/wasm/runtime/library_mono.js @@ -370,15 +370,26 @@ var MonoSupportLib = { next_has_value_or_get_set = next.value !== undefined || next.get !== undefined || next.set !== undefined; } - if (!this_has_name || !next_has_value_or_get_set) { + if (!this_has_name) { + // insert the object as-is + // Eg. in case of locals, the names are added + // later + i ++; + } else if (next_has_value_or_get_set) { + // found a {name} followed by a {value/get} + o = Object.assign (o, var_list [i + 1]); + i += 2; + } else { + // missing value/get, so add a placeholder one + o.value = { + type: "symbol", + value: "", + description: "" + }; i ++; - out_list.push (o); - continue; } - o = Object.assign (o, var_list [i + 1]); out_list.push (o); - i += 2; } return out_list; @@ -1933,6 +1944,20 @@ var MonoSupportLib = { } break; + case "symbol": { + if (typeof value === 'object' && value.isClassName) + str_value = MONO._mono_csharp_fixup_class_name (str_value); + + MONO.var_info.push ({ + value: { + type: "symbol", + value: str_value, + description: str_value + } + }); + } + break; + default: { const msg = `'${str_value}' ${value}`; From 012c1b4eeb40deab993d4fa71935cc0858221bee Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 3 Sep 2020 16:37:15 -0400 Subject: [PATCH 06/10] [wasm][debugger] Fix frame indexing - `collect_frames`, `list_frames` - both iterate over stack frames. They skip some frames. This debug proxy assigns a simple index to each of the received(filtered) frames. - so, if we had `[ frame0, (skipped frame1), frame2 ]`, the proxy will have `[ frame0(index:0), frame2(index:1) ]` - `describe_variables_on_frame` also iterates over stack frames, and tries to find a given frame by an index. And this index is what the proxy had assigned. - because some frames were skipped, this function also needs to skip the *same* ones, so that it can find the intended frame. - Instead of trying to keep these in sync, here the indexing is changed to be the real index found as we iterate over the frames. - And the proxy just uses that. - So, we will have `[ frame0(index:0), frame2(index:2) ]` This showed up in a trace in aspnetcore, which was called via reflection. And that frame didn't get added to the list because it was not `MONO_WRAPPER_NONE`, which caused the indexing to get out of sync. Fixes: https://github.com/dotnet/runtime/issues/41818 --- src/mono/mono/mini/mini-wasm-debugger.c | 27 +++++++------- .../debugger/BrowserDebugProxy/MonoProxy.cs | 4 +- .../debugger/DebuggerTestSuite/Support.cs | 2 - .../wasm/debugger/DebuggerTestSuite/Tests.cs | 37 ++++++++++++++++++- .../tests/debugger-get-properties-test.cs | 29 +++++++++++++++ src/mono/wasm/runtime/library_mono.js | 5 ++- 6 files changed, 82 insertions(+), 22 deletions(-) diff --git a/src/mono/mono/mini/mini-wasm-debugger.c b/src/mono/mono/mini/mini-wasm-debugger.c index 7da835e54191ee..f2e398666616fe 100644 --- a/src/mono/mono/mini/mini-wasm-debugger.c +++ b/src/mono/mono/mini/mini-wasm-debugger.c @@ -54,7 +54,7 @@ EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_invoke_getter_on_value (void *value, Mon EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_get_deref_ptr_value (void *value_addr, MonoClass *klass); //JS functions imported that we use -extern void mono_wasm_add_frame (int il_offset, int method_token, const char *assembly_name, const char *method_name); +extern void mono_wasm_add_frame (int il_offset, int method_token, int frame_id, const char *assembly_name, const char *method_name); extern void mono_wasm_fire_bp (void); extern void mono_wasm_fire_exception (int exception_obj_id, const char* message, const char* class_name, gboolean uncaught); extern void mono_wasm_add_obj_var (const char*, const char*, guint64); @@ -143,7 +143,7 @@ collect_frames (MonoStackFrameInfo *info, MonoContext *ctx, gpointer data) if (!method) return FALSE; - DEBUG_PRINTF (2, "Reporting method %s native_offset %d\n", method->name, info->native_offset); + DEBUG_PRINTF (2, "collect_frames: Reporting method %s native_offset %d, wrapper_type: %d\n", method->name, info->native_offset, method->wrapper_type); if (!mono_find_prev_seq_point_for_native_offset (mono_get_root_domain (), method, info->native_offset, NULL, &sp)) DEBUG_PRINTF (2, "collect_frames: Failed to lookup sequence point. method: %s, native_offset: %d \n", method->name, info->native_offset); @@ -657,20 +657,22 @@ list_frames (MonoStackFrameInfo *info, MonoContext *ctx, gpointer data) MonoMethod *method; char *method_full_name; + int* frame_id_p = (int*)data; + (*frame_id_p)++; + //skip wrappers if (info->type != FRAME_TYPE_MANAGED && info->type != FRAME_TYPE_INTERP) return FALSE; - if (info->ji) method = jinfo_get_method (info->ji); else method = info->method; - if (!method) + if (!method || method->wrapper_type != MONO_WRAPPER_NONE) return FALSE; - DEBUG_PRINTF (2, "Reporting method %s native_offset %d\n", method->name, info->native_offset); + DEBUG_PRINTF (2, "list_frames: Reporting method %s native_offset %d, wrapper_type: %d\n", method->name, info->native_offset, method->wrapper_type); if (!mono_find_prev_seq_point_for_native_offset (mono_get_root_domain (), method, info->native_offset, NULL, &sp)) DEBUG_PRINTF (2, "list_frames: Failed to lookup sequence point. method: %s, native_offset: %d\n", method->name, info->native_offset); @@ -682,10 +684,8 @@ list_frames (MonoStackFrameInfo *info, MonoContext *ctx, gpointer data) char *assembly_name = g_strdup (m_class_get_image (method->klass)->module_name); inplace_tolower (assembly_name); - if (method->wrapper_type == MONO_WRAPPER_NONE) { - DEBUG_PRINTF (2, "adding off %d token %d assembly name %s\n", sp.il_offset, mono_metadata_token_index (method->token), assembly_name); - mono_wasm_add_frame (sp.il_offset, mono_metadata_token_index (method->token), assembly_name, method_full_name); - } + DEBUG_PRINTF (2, "adding off %d token %d assembly name %s\n", sp.il_offset, mono_metadata_token_index (method->token), assembly_name); + mono_wasm_add_frame (sp.il_offset, mono_metadata_token_index (method->token), *frame_id_p, assembly_name, method_full_name); g_free (assembly_name); @@ -695,7 +695,8 @@ list_frames (MonoStackFrameInfo *info, MonoContext *ctx, gpointer data) EMSCRIPTEN_KEEPALIVE void mono_wasm_enum_frames (void) { - mono_walk_stack_with_ctx (list_frames, NULL, MONO_UNWIND_NONE, NULL); + int frame_id = 0; + mono_walk_stack_with_ctx (list_frames, NULL, MONO_UNWIND_NONE, &frame_id); } static char* @@ -1351,15 +1352,15 @@ describe_variables_on_frame (MonoStackFrameInfo *info, MonoContext *ctx, gpointe ERROR_DECL (error); FrameDescData *data = (FrameDescData*)ud; + ++data->cur_frame; + //skip wrappers if (info->type != FRAME_TYPE_MANAGED && info->type != FRAME_TYPE_INTERP) { return FALSE; } - if (data->cur_frame < data->target_frame) { - ++data->cur_frame; + if ((data->cur_frame - 1) != data->target_frame) return FALSE; - } data->found = TRUE; diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index e47697ea8039dc..333253ac661c0e 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -457,7 +457,6 @@ async Task RuntimeGetProperties(MessageId id, DotnetObjectId objectId, J return res; } - //static int frame_id=0; async Task OnPause(SessionId sessionId, JObject args, CancellationToken token) { //FIXME we should send release objects every now and then? Or intercept those we inject and deal in the runtime @@ -518,12 +517,11 @@ async Task OnPause(SessionId sessionId, JObject args, CancellationToken to } var frames = new List(); - int frame_id = 0; var the_mono_frames = res.Value?["result"]?["value"]?["frames"]?.Values(); foreach (var mono_frame in the_mono_frames) { - ++frame_id; + int frame_id = mono_frame["frame_id"].Value(); var il_pos = mono_frame["il_pos"].Value(); var method_token = mono_frame["method_token"].Value(); var assembly_name = mono_frame["assembly_name"].Value(); diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Support.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Support.cs index 2a6a64d18d0c82..c4ec6daae3e582 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Support.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Support.cs @@ -222,7 +222,6 @@ await EvaluateAndCheck( var top_frame = pause_location["callFrames"][0]; var scope = top_frame["scopeChain"][0]; - Assert.Equal("dotnet:scope:0", scope["object"]["objectId"]); if (wait_for_event_fn != null) await wait_for_event_fn(pause_location); else @@ -271,7 +270,6 @@ await insp.Ready(async (cli, token) => var top_frame = pause_location["callFrames"][0]; var scope = top_frame["scopeChain"][0]; - Assert.Equal("dotnet:scope:0", scope["object"]["objectId"]); if (wait_for_event_fn != null) await wait_for_event_fn(pause_location); diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs index d3c8ef2f21c6eb..b80f0a43d704fd 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs @@ -235,7 +235,6 @@ await EvaluateAndCheck( Assert.Equal("IntAdd", scope["name"]); Assert.Equal("object", scope["object"]["type"]); - Assert.Equal("dotnet:scope:0", scope["object"]["objectId"]); CheckLocation("dotnet://debugger-test.dll/debugger-test.cs", 8, 4, scripts, scope["startLocation"]); CheckLocation("dotnet://debugger-test.dll/debugger-test.cs", 14, 4, scripts, scope["endLocation"]); return Task.CompletedTask; @@ -451,7 +450,6 @@ await EvaluateAndCheck( var top_frame = pause_location["callFrames"][0]; var scope = top_frame["scopeChain"][0]; - Assert.Equal("dotnet:scope:0", scope["object"]["objectId"]); // Try to get an invalid scope! var get_prop_req = JObject.FromObject(new @@ -1682,6 +1680,41 @@ public async Task StaticMethodWithLocalEmptyStructThatWillGetExpanded(bool is_as AssertEqual(0, es_props.Values().Count(), "es_props"); }); + [Fact] + public async Task PreviousFrameForAReflectedCall() => await CheckInspectLocalsAtBreakpointSite( + "DebuggerTests.GetPropertiesTests.CloneableStruct", "SimpleStaticMethod", 1, "SimpleStaticMethod", + "window.setTimeout(function() { invoke_static_method('[debugger-test] DebuggerTests.GetPropertiesTests.TestWithReflection:run'); })", + wait_for_event_fn: async (pause_location) => + { + var frame = FindFrame(pause_location, "InvokeReflectedStaticMethod"); + Assert.NotNull(frame); + + var frame_locals = await GetProperties(frame["callFrameId"].Value()); + + await CheckProps(frame_locals, new + { + mi = TObject("System.Reflection.MethodInfo"), + dt = TDateTime(new DateTime(4210, 3, 4, 5, 6, 7)), + i = TNumber(4), + strings = TArray("string[]", 1), + cs = TValueType("DebuggerTests.GetPropertiesTests.CloneableStruct"), + + num = TNumber(10), + name = TString("foobar"), + some_date = TDateTime(new DateTime(1234, 6, 7, 8, 9, 10)), + num1 = TNumber(100), + str2 = TString("xyz"), + num3 = TNumber(345), + str3 = TString("abc") + }, "InvokeReflectedStaticMethod#locals"); + }); + + JObject FindFrame(JObject pause_location, string function_name) + => pause_location["callFrames"] + ?.Values() + ?.Where(f => f["functionName"]?.Value() == function_name) + ?.FirstOrDefault(); + //TODO add tests covering basic stepping behavior as step in/out/over } } diff --git a/src/mono/wasm/debugger/tests/debugger-get-properties-test.cs b/src/mono/wasm/debugger/tests/debugger-get-properties-test.cs index 60e2d485becd44..b00558c09f2a05 100644 --- a/src/mono/wasm/debugger/tests/debugger-get-properties-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-get-properties-test.cs @@ -3,6 +3,8 @@ using System; using System.Threading.Tasks; +using System.Reflection; + namespace DebuggerTests.GetPropertiesTests { @@ -154,6 +156,12 @@ public async Task InstanceMethodAsync() Console.WriteLine ($"break here"); await Task.CompletedTask; } + + public static void SimpleStaticMethod(DateTime dateTimeArg, string stringArg) + { + Console.WriteLine ($"break here"); + } + } public struct NestedStruct @@ -203,4 +211,25 @@ public static void run() Console.WriteLine ($"break here"); } } + + public class TestWithReflection + { + public static void run() + { + InvokeReflectedStaticMethod(10, "foobar", new DateTime(1234, 6, 7, 8, 9, 10), 100, "xyz", 345, "abc"); + } + + public static void InvokeReflectedStaticMethod(int num, string name, DateTime some_date, int num1, string str2, int num3, string str3) + { + var mi = typeof(CloneableStruct).GetMethod("SimpleStaticMethod"); + var dt = new DateTime(4210, 3, 4, 5, 6, 7); + int i = 4; + + string[] strings = new[]{ "abc" }; + CloneableStruct cs = new CloneableStruct(); + + // var cs = new CloneableStruct(); + mi.Invoke(null, new object[]{ dt, "called from run" }); + } + } } diff --git a/src/mono/wasm/runtime/library_mono.js b/src/mono/wasm/runtime/library_mono.js index 530fec0145257e..d871cf24997e77 100644 --- a/src/mono/wasm/runtime/library_mono.js +++ b/src/mono/wasm/runtime/library_mono.js @@ -2147,14 +2147,15 @@ var MonoSupportLib = { } }, - mono_wasm_add_frame: function(il, method, assembly_name, method_full_name) { + mono_wasm_add_frame: function(il, method, frame_id, assembly_name, method_full_name) { var parts = Module.UTF8ToString (method_full_name).split (":", 2); MONO.active_frames.push( { il_pos: il, method_token: method, assembly_name: Module.UTF8ToString (assembly_name), // Extract just the method name from `{class_name}:{method_name}` - method_name: parts [parts.length - 1] + method_name: parts [parts.length - 1], + frame_id }); }, From d0c675dcaddb70cf1a78a8e2048d80ab286a9785 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 4 Sep 2020 13:07:58 -0400 Subject: [PATCH 07/10] fix warning: remove unused var --- src/mono/mono/mini/mini-wasm-debugger.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/mono/mono/mini/mini-wasm-debugger.c b/src/mono/mono/mini/mini-wasm-debugger.c index f2e398666616fe..b6c5a7aa577e2f 100644 --- a/src/mono/mono/mini/mini-wasm-debugger.c +++ b/src/mono/mono/mini/mini-wasm-debugger.c @@ -1316,8 +1316,6 @@ describe_non_async_this (InterpFrame *frame, MonoMethod *method) static gboolean describe_variable (InterpFrame *frame, MonoMethod *method, MonoMethodHeader *header, int pos, int gpflags) { - ERROR_DECL (error); - MonoType *type = NULL; gpointer addr = NULL; if (pos < 0) { From cd443c1c53f58f3712277c8e4a2aa85a2462c823 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 4 Sep 2020 16:13:31 -0400 Subject: [PATCH 08/10] rebase on master, fix errors --- src/mono/wasm/debugger/tests/debugger-test.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/tests/debugger-test.cs b/src/mono/wasm/debugger/tests/debugger-test.cs index d22bfeb97ee379..ffdbffc0442153 100644 --- a/src/mono/wasm/debugger/tests/debugger-test.cs +++ b/src/mono/wasm/debugger/tests/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; 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) From 1ad4b0f13ae6614d6d92e2ab5381c83d5579be7d Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 4 Sep 2020 17:11:06 -0400 Subject: [PATCH 09/10] Make frame indices returned from debugger.c, 0-based - Earlier this 1-based, and it was being adjusted in `MonoProxy`. - Based on @lewing's suggestion, changing this to be 0-based in debugger.c, itself, thus removing the need to "fixup" in `MonoProxy`. --- src/mono/mono/mini/mini-wasm-debugger.c | 6 +++--- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/mono/mono/mini/mini-wasm-debugger.c b/src/mono/mono/mini/mini-wasm-debugger.c index b6c5a7aa577e2f..ed3181837c0c26 100644 --- a/src/mono/mono/mini/mini-wasm-debugger.c +++ b/src/mono/mono/mini/mini-wasm-debugger.c @@ -695,7 +695,7 @@ list_frames (MonoStackFrameInfo *info, MonoContext *ctx, gpointer data) EMSCRIPTEN_KEEPALIVE void mono_wasm_enum_frames (void) { - int frame_id = 0; + int frame_id = -1; mono_walk_stack_with_ctx (list_frames, NULL, MONO_UNWIND_NONE, &frame_id); } @@ -1357,7 +1357,7 @@ describe_variables_on_frame (MonoStackFrameInfo *info, MonoContext *ctx, gpointe return FALSE; } - if ((data->cur_frame - 1) != data->target_frame) + if (data->cur_frame != data->target_frame) return FALSE; data->found = TRUE; @@ -1405,7 +1405,7 @@ mono_wasm_get_local_vars (int scope, int* pos, int len) FrameDescData data; data.target_frame = scope; - data.cur_frame = 0; + data.cur_frame = -1; data.len = len; data.pos = pos; data.found = FALSE; diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 333253ac661c0e..a0cc9dc8926c46 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -558,12 +558,12 @@ async Task OnPause(SessionId sessionId, JObject args, CancellationToken to Log("info", $"frame il offset: {il_pos} method token: {method_token} assembly name: {assembly_name}"); Log("info", $"\tmethod {method_name} location: {location}"); - frames.Add(new Frame(method, location, frame_id - 1)); + frames.Add(new Frame(method, location, frame_id)); callFrames.Add(new { functionName = method_name, - callFrameId = $"dotnet:scope:{frame_id - 1}", + callFrameId = $"dotnet:scope:{frame_id}", functionLocation = method.StartLocation.AsLocation(), location = location.AsLocation(), @@ -580,7 +580,7 @@ async Task OnPause(SessionId sessionId, JObject args, CancellationToken to @type = "object", className = "Object", description = "Object", - objectId = $"dotnet:scope:{frame_id-1}", + objectId = $"dotnet:scope:{frame_id}", }, name = method_name, startLocation = method.StartLocation.AsLocation(), From 91964dd9d42d116c935966716099f0bb8e779975 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 4 Sep 2020 18:12:04 -0400 Subject: [PATCH 10/10] dotnet-format fixes --- .../debugger/DebuggerTestSuite/MonoJsTests.cs | 4 +- .../wasm/debugger/DebuggerTestSuite/Tests.cs | 56 +++++++++---------- .../tests/debugger-get-properties-test.cs | 32 +++++------ src/mono/wasm/debugger/tests/debugger-test.cs | 22 ++++---- 4 files changed, 57 insertions(+), 57 deletions(-) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs index f5cad4d9c4e4c6..ca530287ccacf6 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/MonoJsTests.cs @@ -1,9 +1,9 @@ using System; using System.Linq; using System.Threading.Tasks; -using Xunit; using Newtonsoft.Json; using Newtonsoft.Json.Linq; +using Xunit; namespace DebuggerTests { @@ -83,7 +83,7 @@ await insp.Ready(async (cli, token) => null, -1, -1, "IntAdd"); var scope_id = pause_location["callFrames"][0]["callFrameId"].Value(); - var scope = int.Parse(scope_id.Split(new[] {':'}, StringSplitOptions.RemoveEmptyEntries)[2]); + var scope = int.Parse(scope_id.Split(new[] { ':' }, StringSplitOptions.RemoveEmptyEntries)[2]); var var_ids = new[] { diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs index b80f0a43d704fd..0d8e902be1a77c 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Tests.cs @@ -1632,13 +1632,13 @@ public async Task MulticastDelegateTest() => await CheckInspectLocalsAtBreakpoin "window.setTimeout(function() { invoke_static_method('[debugger-test] MulticastDelegateTestClass:run'); })", wait_for_event_fn: async (pause_location) => { - var frame_locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value()); - var this_props = await GetObjectOnLocals(frame_locals, "this"); - await CheckProps(this_props, new - { - TestEvent = TSymbol("System.EventHandler"), - Delegate = TSymbol("System.MulticastDelegate") - }, "this_props"); + var frame_locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value()); + var this_props = await GetObjectOnLocals(frame_locals, "this"); + await CheckProps(this_props, new + { + TestEvent = TSymbol("System.EventHandler"), + Delegate = TSymbol("System.MulticastDelegate") + }, "this_props"); }); [Theory] @@ -1686,27 +1686,27 @@ public async Task PreviousFrameForAReflectedCall() => await CheckInspectLocalsAt "window.setTimeout(function() { invoke_static_method('[debugger-test] DebuggerTests.GetPropertiesTests.TestWithReflection:run'); })", wait_for_event_fn: async (pause_location) => { - var frame = FindFrame(pause_location, "InvokeReflectedStaticMethod"); - Assert.NotNull(frame); - - var frame_locals = await GetProperties(frame["callFrameId"].Value()); - - await CheckProps(frame_locals, new - { - mi = TObject("System.Reflection.MethodInfo"), - dt = TDateTime(new DateTime(4210, 3, 4, 5, 6, 7)), - i = TNumber(4), - strings = TArray("string[]", 1), - cs = TValueType("DebuggerTests.GetPropertiesTests.CloneableStruct"), - - num = TNumber(10), - name = TString("foobar"), - some_date = TDateTime(new DateTime(1234, 6, 7, 8, 9, 10)), - num1 = TNumber(100), - str2 = TString("xyz"), - num3 = TNumber(345), - str3 = TString("abc") - }, "InvokeReflectedStaticMethod#locals"); + var frame = FindFrame(pause_location, "InvokeReflectedStaticMethod"); + Assert.NotNull(frame); + + var frame_locals = await GetProperties(frame["callFrameId"].Value()); + + await CheckProps(frame_locals, new + { + mi = TObject("System.Reflection.MethodInfo"), + dt = TDateTime(new DateTime(4210, 3, 4, 5, 6, 7)), + i = TNumber(4), + strings = TArray("string[]", 1), + cs = TValueType("DebuggerTests.GetPropertiesTests.CloneableStruct"), + + num = TNumber(10), + name = TString("foobar"), + some_date = TDateTime(new DateTime(1234, 6, 7, 8, 9, 10)), + num1 = TNumber(100), + str2 = TString("xyz"), + num3 = TNumber(345), + str3 = TString("abc") + }, "InvokeReflectedStaticMethod#locals"); }); JObject FindFrame(JObject pause_location, string function_name) diff --git a/src/mono/wasm/debugger/tests/debugger-get-properties-test.cs b/src/mono/wasm/debugger/tests/debugger-get-properties-test.cs index b00558c09f2a05..d591a34ce140ae 100644 --- a/src/mono/wasm/debugger/tests/debugger-get-properties-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-get-properties-test.cs @@ -2,8 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Threading.Tasks; using System.Reflection; +using System.Threading.Tasks; namespace DebuggerTests.GetPropertiesTests { @@ -19,7 +19,7 @@ public interface ILastName } public interface IName : IFirstName, ILastName - {} + { } public class BaseBaseClass { @@ -93,20 +93,20 @@ public DerivedClass() public static void run() { - new DerivedClass().InstanceMethod (); - new DerivedClass().InstanceMethodAsync ().Wait(); + new DerivedClass().InstanceMethod(); + new DerivedClass().InstanceMethodAsync().Wait(); } public string GetStringField() => _stringField; public void InstanceMethod() { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } public async Task InstanceMethodAsync() { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); await Task.CompletedTask; } } @@ -140,26 +140,26 @@ public CloneableStruct(int bias) public static void run() { - new CloneableStruct(3).InstanceMethod (); - new CloneableStruct(3).InstanceMethodAsync ().Wait(); + new CloneableStruct(3).InstanceMethod(); + new CloneableStruct(3).InstanceMethodAsync().Wait(); } public string GetStringField() => _stringField; public void InstanceMethod() { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } public async Task InstanceMethodAsync() { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); await Task.CompletedTask; } public static void SimpleStaticMethod(DateTime dateTimeArg, string stringArg) { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } } @@ -182,13 +182,13 @@ public static void run() public static void TestNestedStructStatic() { var ns = new NestedStruct(3); - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } public static async Task TestNestedStructStaticAsync() { var ns = new NestedStruct(3); - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); await Task.CompletedTask; } } @@ -208,7 +208,7 @@ class DerivedClassForJSTest : BaseClassForJSTest public static void run() { var obj = new DerivedClassForJSTest(); - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } } @@ -225,11 +225,11 @@ public static void InvokeReflectedStaticMethod(int num, string name, DateTime so var dt = new DateTime(4210, 3, 4, 5, 6, 7); int i = 4; - string[] strings = new[]{ "abc" }; + string[] strings = new[] { "abc" }; CloneableStruct cs = new CloneableStruct(); // var cs = new CloneableStruct(); - mi.Invoke(null, new object[]{ dt, "called from run" }); + mi.Invoke(null, new object[] { dt, "called from run" }); } } } diff --git a/src/mono/wasm/debugger/tests/debugger-test.cs b/src/mono/wasm/debugger/tests/debugger-test.cs index ffdbffc0442153..75ce392a5ebec6 100644 --- a/src/mono/wasm/debugger/tests/debugger-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test.cs @@ -316,7 +316,7 @@ public static void TestSimpleStrings() str_spaces, str_esc }; - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } } @@ -355,8 +355,8 @@ public static void run() public void Test() { - TestEvent += (_, s) => Console.WriteLine (s); - TestEvent += (_, s) => Console.WriteLine (s + "qwe"); + TestEvent += (_, s) => Console.WriteLine(s); + TestEvent += (_, s) => Console.WriteLine(s + "qwe"); Delegate = TestEvent; TestEvent?.Invoke(this, Delegate?.ToString()); @@ -364,8 +364,8 @@ public void Test() public async Task TestAsync() { - TestEvent += (_, s) => Console.WriteLine (s); - TestEvent += (_, s) => Console.WriteLine (s + "qwe"); + TestEvent += (_, s) => Console.WriteLine(s); + TestEvent += (_, s) => Console.WriteLine(s + "qwe"); Delegate = TestEvent; TestEvent?.Invoke(this, Delegate?.ToString()); @@ -377,12 +377,12 @@ public class EmptyClass { public static void StaticMethodWithNoLocals() { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } public static async Task StaticMethodWithNoLocalsAsync() { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); await Task.CompletedTask; } @@ -397,25 +397,25 @@ public struct EmptyStruct { public static void StaticMethodWithNoLocals() { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } public static async Task StaticMethodWithNoLocalsAsync() { - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); await Task.CompletedTask; } public static void StaticMethodWithLocalEmptyStruct() { var es = new EmptyStruct(); - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); } public static async Task StaticMethodWithLocalEmptyStructAsync() { var es = new EmptyStruct(); - Console.WriteLine ($"break here"); + Console.WriteLine($"break here"); await Task.CompletedTask; }