From efed0da1efeeee37552cd523bbb3a76eaddf75ef Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Wed, 20 Sep 2023 11:38:11 +0000 Subject: [PATCH 1/6] Fix. --- .../wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 2 +- .../DebuggerTestSuite/EvaluateOnCallFrameTests.cs | 9 +++++---- .../tests/debugger-test/debugger-evaluate-test.cs | 2 ++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 82282501cfedf7..b63143aff7e48f 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -564,7 +564,7 @@ private void Write(T1 type, T2 value) where T1 : struct where T2 : struc public void WriteObj(DotnetObjectId objectId, MonoSDBHelper SdbHelper) { - if (objectId.Scheme == "object") + if (objectId.Scheme == "object" || objectId.Scheme == "array") { Write(ElementType.Class, objectId.Value); } diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs index 2ff9bd26a28272..846eea5b1cf9eb 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs @@ -585,7 +585,7 @@ public async Task EvaluateIndexingNegative() => await CheckInspectLocalsAtBreakp Assert.Equal("Unable to evaluate element access 'f.idx0[2]': Cannot apply indexing with [] to a primitive object of type 'number'", res.Error["result"]?["description"]?.Value()); var exceptionDetailsStack = res.Error["exceptionDetails"]?["stackTrace"]?["callFrames"]?[0]; Assert.Equal("DebuggerTests.EvaluateLocalsWithIndexingTests.EvaluateLocals", exceptionDetailsStack?["functionName"]?.Value()); - Assert.Equal(556, exceptionDetailsStack?["lineNumber"]?.Value()); + Assert.Equal(559, exceptionDetailsStack?["lineNumber"]?.Value()); Assert.Equal(12, exceptionDetailsStack?["columnNumber"]?.Value()); (_, res) = await EvaluateOnCallFrame(id, "f[1]", expect_ok: false ); Assert.Equal( "Unable to evaluate element access 'f[1]': Cannot apply indexing with [] to an object of type 'DebuggerTests.EvaluateLocalsWithIndexingTests.TestEvaluate'", res.Error["result"]?["description"]?.Value()); @@ -650,7 +650,7 @@ await EvaluateOnCallFrameAndCheck(id, [Fact] public async Task EvaluateObjectByNonIntLocals() => await CheckInspectLocalsAtBreakpointSite( - "DebuggerTests.EvaluateLocalsWithIndexingTests", "EvaluateLocals", 12, "DebuggerTests.EvaluateLocalsWithIndexingTests.EvaluateLocals", + "DebuggerTests.EvaluateLocalsWithIndexingTests", "EvaluateLocals", 13, "DebuggerTests.EvaluateLocalsWithIndexingTests.EvaluateLocals", "window.setTimeout(function() { invoke_static_method ('[debugger-test] DebuggerTests.EvaluateLocalsWithIndexingTests:EvaluateLocals'); })", wait_for_event_fn: async (pause_location) => { @@ -662,7 +662,8 @@ await EvaluateOnCallFrameAndCheck(id, ("f[shortString]", TBool(false)), ("f[aFloat]", TNumber(1)), ("f[aDouble]", TNumber(2)), - ("f[aDecimal]", TNumber(3)) // object + ("f[aDecimal]", TNumber(3)), + ("f[arr]", TChar('t')) ); }); @@ -722,7 +723,7 @@ public async Task EvaluateIndexingByExpressionNegative() => await CheckInspectLo Assert.Equal("Unable to evaluate element access 'f.numList[\"a\" + 1]': Cannot index with an object of type 'string'", res.Error["result"]?["description"]?.Value()); var exceptionDetailsStack = res.Error["exceptionDetails"]?["stackTrace"]?["callFrames"]?[0]; Assert.Equal("DebuggerTests.EvaluateLocalsWithIndexingTests.EvaluateLocals", exceptionDetailsStack?["functionName"]?.Value()); - Assert.Equal(556, exceptionDetailsStack?["lineNumber"]?.Value()); + Assert.Equal(559, exceptionDetailsStack?["lineNumber"]?.Value()); Assert.Equal(12, exceptionDetailsStack?["columnNumber"]?.Value()); }); diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs b/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs index 480dc30115c430..d83cbc7a6d1d0e 100644 --- a/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test/debugger-evaluate-test.cs @@ -529,6 +529,7 @@ public class TestEvaluate public int this[double key] => (int)key; public int this[float key] => (int)key; public int this[decimal key] => (int)key; + public char this[char[] arr] => arr.Length == 0 ? '0' : arr[0]; public void run() { @@ -561,6 +562,7 @@ public static void EvaluateLocals() float aFloat = 1.23f; double aDouble = 2.34; decimal aDecimal = 3.34m; + char[] arr = new char[] { 't', 'e', 's', 't' }; } } From 0fae47801abb945eba69ac497e0bf95479671ca5 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Thu, 21 Sep 2023 05:59:42 +0000 Subject: [PATCH 2/6] Array should have its own ElementType passed. --- .../BrowserDebugProxy/MonoSDBHelper.cs | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index b63143aff7e48f..844ed5b4d54193 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -564,16 +564,29 @@ private void Write(T1 type, T2 value) where T1 : struct where T2 : struc public void WriteObj(DotnetObjectId objectId, MonoSDBHelper SdbHelper) { - if (objectId.Scheme == "object" || objectId.Scheme == "array") + switch (objectId.Scheme) { - Write(ElementType.Class, objectId.Value); - } - else if (objectId.Scheme == "valuetype") - { - if (SdbHelper.ValueCreator.TryGetValueTypeById(objectId.Value, out ValueTypeClass vt)) + case "object": + { + Write(ElementType.Class, objectId.Value); + break; + } + case "array": + { + Write(ElementType.Array, objectId.Value); + break; + } + case "valuetype": + { + if (!SdbHelper.ValueCreator.TryGetValueTypeById(objectId.Value, out ValueTypeClass vt)) + throw new ArgumentException($"Could not find any valuetype with id: {objectId.Value}", nameof(objectId.Value)); Write(vt.Buffer); - else - throw new ArgumentException($"Could not find any valuetype with id: {objectId.Value}", nameof(objectId.Value)); + break; + } + default: + { + throw new NotImplementedException($"Writing object of scheme: {objectId.Scheme} is not supported"); + } } } From 305def5676fa6eeca60c981d9f30bd88d59e29be Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Thu, 21 Sep 2023 07:20:18 +0000 Subject: [PATCH 3/6] Revert "array" to be in the same case as "object". --- src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 844ed5b4d54193..bca6e331e68e7d 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -567,13 +567,9 @@ public void WriteObj(DotnetObjectId objectId, MonoSDBHelper SdbHelper) switch (objectId.Scheme) { case "object": - { - Write(ElementType.Class, objectId.Value); - break; - } case "array": { - Write(ElementType.Array, objectId.Value); + Write(ElementType.Class, objectId.Value); break; } case "valuetype": From f994a26530cffe956a66c82baaf63b89764210f8 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Thu, 21 Sep 2023 14:51:33 +0000 Subject: [PATCH 4/6] Apply @thaystg's feedback. --- src/mono/mono/component/debugger-agent.c | 18 ++++++++++++++---- .../BrowserDebugProxy/MonoSDBHelper.cs | 6 +++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index c840e1b7ae2613..d1f6f5475ac530 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -5512,7 +5512,9 @@ decode_value_compute_size (MonoType *t, int type, MonoDomain *domain, guint8 *bu !(t->type == MONO_TYPE_PTR && type == MONO_TYPE_I8) && !(t->type == MONO_TYPE_FNPTR && type == MONO_TYPE_I8) && !(t->type == MONO_TYPE_GENERICINST && type == MONO_TYPE_VALUETYPE) && - !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_OBJECT)) { + !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_OBJECT) && + !(t->type == MONO_TYPE_SZARRAY && type == MONO_TYPE_SZARRAY) && + !(t->type == MONO_TYPE_ARRAY && type == MONO_TYPE_ARRAY)) { char *name = mono_type_full_name (t); PRINT_DEBUG_MSG (1, "[%p] Expected value of type %s, got 0x%0x.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, type); g_free (name); @@ -5580,16 +5582,20 @@ decode_value_compute_size (MonoType *t, int type, MonoDomain *domain, guint8 *bu handle_ref: default: if (MONO_TYPE_IS_REFERENCE (t)) { - if (type == MONO_TYPE_CLASS || type == MONO_TYPE_OBJECT || type == MONO_TYPE_STRING) { + PRINT_DEBUG_MSG (1, "[ILONA] MONO_TYPE_IS_REFERENCE\n"); + if (type == MONO_TYPE_CLASS || type == MONO_TYPE_OBJECT || type == MONO_TYPE_STRING || type == MONO_TYPE_ARRAY || type == MONO_TYPE_SZARRAY) { + PRINT_DEBUG_MSG (1, "[ILONA] inside of if\n"); ret += sizeof(MonoObject*); decode_objid (buf, &buf, limit); } else if (type == VALUE_TYPE_ID_NULL) { + PRINT_DEBUG_MSG (1, "[ILONA] inside of if VALUE_TYPE_ID_NULL\n"); ret += sizeof(MonoObject*); if (CHECK_PROTOCOL_VERSION (2, 59)) { decode_byte (buf, &buf, limit); decode_int (buf, &buf, limit); //not used } } else if (type == MONO_TYPE_VALUETYPE) { + PRINT_DEBUG_MSG (1, "[ILONA] inside of if MONO_TYPE_VALUETYPE\n"); MonoDomain *d; decode_byte (buf, &buf, limit); if (CHECK_PROTOCOL_VERSION(2, 61)) @@ -5604,8 +5610,10 @@ decode_value_compute_size (MonoType *t, int type, MonoDomain *domain, guint8 *bu } else if ((t->type == MONO_TYPE_GENERICINST) && mono_metadata_generic_class_is_valuetype (t->data.generic_class) && m_class_is_enumtype (t->data.generic_class->container_class)){ + PRINT_DEBUG_MSG (1, "[ILONA] inside of if MONO_TYPE_GENERICINST\n"); ret += decode_vtype_compute_size (t, domain, buf, &buf, limit, from_by_ref_value_type); } else { + PRINT_DEBUG_MSG (1, "[ILONA] inside of if NOT_IMPLEMENTED\n"); NOT_IMPLEMENTED; } break; @@ -5635,7 +5643,9 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr, !(t->type == MONO_TYPE_PTR && type == MONO_TYPE_I8) && !(t->type == MONO_TYPE_FNPTR && type == MONO_TYPE_I8) && !(t->type == MONO_TYPE_GENERICINST && type == MONO_TYPE_VALUETYPE) && - !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_OBJECT)) { + !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_OBJECT) && + !(t->type == MONO_TYPE_SZARRAY && type == MONO_TYPE_SZARRAY) && + !(t->type == MONO_TYPE_ARRAY && type == MONO_TYPE_ARRAY)) { char *name = mono_type_full_name (t); PRINT_DEBUG_MSG (1, "[%p] Expected value of type %s, got 0x%0x.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, type); g_free (name); @@ -5728,7 +5738,7 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr, handle_ref: default: if (MONO_TYPE_IS_REFERENCE (t)) { - if (type == MONO_TYPE_CLASS || type == MONO_TYPE_OBJECT || type == MONO_TYPE_STRING) { + if (type == MONO_TYPE_CLASS || type == MONO_TYPE_OBJECT || type == MONO_TYPE_STRING || type == MONO_TYPE_ARRAY || type == MONO_TYPE_SZARRAY) { int objid = decode_objid (buf, &buf, limit); MonoObject *obj; diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index bca6e331e68e7d..844ed5b4d54193 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -567,11 +567,15 @@ public void WriteObj(DotnetObjectId objectId, MonoSDBHelper SdbHelper) switch (objectId.Scheme) { case "object": - case "array": { Write(ElementType.Class, objectId.Value); break; } + case "array": + { + Write(ElementType.Array, objectId.Value); + break; + } case "valuetype": { if (!SdbHelper.ValueCreator.TryGetValueTypeById(objectId.Value, out ValueTypeClass vt)) From 5e9c2749d2519c716923c66a3d8cb56fcac33e36 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Thu, 21 Sep 2023 14:53:04 +0000 Subject: [PATCH 5/6] But without debuging logs. --- src/mono/mono/component/debugger-agent.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index d1f6f5475ac530..d0ec04551009b9 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -5582,20 +5582,16 @@ decode_value_compute_size (MonoType *t, int type, MonoDomain *domain, guint8 *bu handle_ref: default: if (MONO_TYPE_IS_REFERENCE (t)) { - PRINT_DEBUG_MSG (1, "[ILONA] MONO_TYPE_IS_REFERENCE\n"); if (type == MONO_TYPE_CLASS || type == MONO_TYPE_OBJECT || type == MONO_TYPE_STRING || type == MONO_TYPE_ARRAY || type == MONO_TYPE_SZARRAY) { - PRINT_DEBUG_MSG (1, "[ILONA] inside of if\n"); ret += sizeof(MonoObject*); decode_objid (buf, &buf, limit); } else if (type == VALUE_TYPE_ID_NULL) { - PRINT_DEBUG_MSG (1, "[ILONA] inside of if VALUE_TYPE_ID_NULL\n"); ret += sizeof(MonoObject*); if (CHECK_PROTOCOL_VERSION (2, 59)) { decode_byte (buf, &buf, limit); decode_int (buf, &buf, limit); //not used } } else if (type == MONO_TYPE_VALUETYPE) { - PRINT_DEBUG_MSG (1, "[ILONA] inside of if MONO_TYPE_VALUETYPE\n"); MonoDomain *d; decode_byte (buf, &buf, limit); if (CHECK_PROTOCOL_VERSION(2, 61)) @@ -5610,10 +5606,8 @@ decode_value_compute_size (MonoType *t, int type, MonoDomain *domain, guint8 *bu } else if ((t->type == MONO_TYPE_GENERICINST) && mono_metadata_generic_class_is_valuetype (t->data.generic_class) && m_class_is_enumtype (t->data.generic_class->container_class)){ - PRINT_DEBUG_MSG (1, "[ILONA] inside of if MONO_TYPE_GENERICINST\n"); ret += decode_vtype_compute_size (t, domain, buf, &buf, limit, from_by_ref_value_type); } else { - PRINT_DEBUG_MSG (1, "[ILONA] inside of if NOT_IMPLEMENTED\n"); NOT_IMPLEMENTED; } break; From 776bd1fedbf133f119c32537863cd2062ca3cb70 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Mon, 25 Sep 2023 07:28:24 +0000 Subject: [PATCH 6/6] Feedback. --- src/mono/mono/component/debugger-agent.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index d0ec04551009b9..e5fd65378515aa 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -5513,8 +5513,8 @@ decode_value_compute_size (MonoType *t, int type, MonoDomain *domain, guint8 *bu !(t->type == MONO_TYPE_FNPTR && type == MONO_TYPE_I8) && !(t->type == MONO_TYPE_GENERICINST && type == MONO_TYPE_VALUETYPE) && !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_OBJECT) && - !(t->type == MONO_TYPE_SZARRAY && type == MONO_TYPE_SZARRAY) && - !(t->type == MONO_TYPE_ARRAY && type == MONO_TYPE_ARRAY)) { + !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_SZARRAY) && + !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_ARRAY)) { char *name = mono_type_full_name (t); PRINT_DEBUG_MSG (1, "[%p] Expected value of type %s, got 0x%0x.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, type); g_free (name); @@ -5638,8 +5638,8 @@ decode_value_internal (MonoType *t, int type, MonoDomain *domain, guint8 *addr, !(t->type == MONO_TYPE_FNPTR && type == MONO_TYPE_I8) && !(t->type == MONO_TYPE_GENERICINST && type == MONO_TYPE_VALUETYPE) && !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_OBJECT) && - !(t->type == MONO_TYPE_SZARRAY && type == MONO_TYPE_SZARRAY) && - !(t->type == MONO_TYPE_ARRAY && type == MONO_TYPE_ARRAY)) { + !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_SZARRAY) && + !(t->type == MONO_TYPE_VALUETYPE && type == MONO_TYPE_ARRAY)) { char *name = mono_type_full_name (t); PRINT_DEBUG_MSG (1, "[%p] Expected value of type %s, got 0x%0x.\n", (gpointer) (gsize) mono_native_thread_id_get (), name, type); g_free (name);