[wasm][debugger] - RuntimeError: memory access out of bounds (dotnet#…

…51859)

- fix null reference in describe_value() + add tests for it
- fix null reference in PDB load
- improved handling of targetCrashed in Inspector
- add windows chrome to probe path
- include debugging payload into test failure exception for easier debugging
- make it possible to compile with WasmBuildNative=true
- make it possible to compile with Debug runtime

src/mono/mono/mini/mini-wasm-debugger.c

@@ -541,7 +541,7 @@ assembly_loaded (MonoProfiler *prof, MonoAssembly *assembly)
 		MonoDebugHandle *handle = mono_debug_get_handle (assembly_image);
 		if (handle) {
 			MonoPPDBFile *ppdb = handle->ppdb;
-			if (!mono_ppdb_is_embedded (ppdb)) { //if it's an embedded pdb we don't need to send pdb extrated to DebuggerProxy. 
+			if (ppdb && !mono_ppdb_is_embedded (ppdb)) { //if it's an embedded pdb we don't need to send pdb extrated to DebuggerProxy. 
 				pdb_image = mono_ppdb_get_image (ppdb);
 				mono_wasm_asm_loaded (assembly_image->assembly_name, assembly_image->raw_data, assembly_image->raw_data_len, pdb_image->raw_data, pdb_image->raw_data_len);
 				return;
@@ -1023,6 +1023,10 @@ describe_value(MonoType * type, gpointer addr, int gpflags)
 
 		case MONO_TYPE_OBJECT: {
 			MonoObject *obj = *(MonoObject**)addr;
+			if (!obj) {
+				mono_wasm_add_obj_var ("object", NULL, 0);
+				break;
+			}
 			MonoClass *klass = obj->vtable->klass;
 			if (!klass) {
 				// boxed null
@@ -1070,6 +1074,12 @@ describe_value(MonoType * type, gpointer addr, int gpflags)
 		case MONO_TYPE_ARRAY:
 		case MONO_TYPE_CLASS: {
 			MonoObject *obj = *(MonoObject**)addr;
+			if (!obj) {
+				char *class_name = mono_type_full_name (type);
+				mono_wasm_add_func_var (class_name, NULL, 0);
+				g_free (class_name);
+				return TRUE;
+			}
 			MonoClass *klass = type->data.klass;
 
 			if (m_class_is_valuetype (mono_object_class (obj))) {
@@ -1523,7 +1533,7 @@ describe_variable (InterpFrame *frame, MonoMethod *method, MonoMethodHeader *hea
 		addr = mini_get_interp_callbacks ()->frame_get_local (frame, pos);
 	}
 
-	PRINT_DEBUG_MSG (2, "adding val %p type [%p] %s\n", addr, type, mono_type_full_name (type));
+	PRINT_DEBUG_MSG (2, "adding val %p type 0x%x %s\n", addr, type->type, mono_type_full_name (type));
 
 	return describe_value(type, addr, gpflags);
 }

src/mono/wasm/README.md

@@ -137,7 +137,7 @@ To run a test with `FooBar` in the name:
 
 (See https://docs.microsoft.com/en-us/dotnet/core/testing/selective-unit-tests?pivots=xunit for filter options)
 
-Additional arguments for `dotnet test` can be passed via `TEST_ARGS`. Though only one of `TEST_ARGS`, or `TEST_FILTER` can be used at a time.
+Additional arguments for `dotnet test` can be passed via `MSBUILD_ARGS` or `TEST_ARGS`. For example `MSBUILD_ARGS="/p:WasmDebugLevel=5"`. Though only one of `TEST_ARGS`, or `TEST_FILTER` can be used at a time.
 
 ## Run samples
 

src/mono/wasm/build/WasmApp.InTree.targets

@@ -25,7 +25,7 @@
 
     <MSBuild Projects="@(WasmAppBuildProject)"
          Properties="Configuration=Debug"
-         Targets="Build;Publish"/>
+         Targets="Build"/>
   </Target>
 
   <Target Name="CopyAppZipToHelixTestDir"

src/mono/wasm/build/WasmApp.targets

@@ -86,6 +86,7 @@
     <Error Condition="'@(_WasmAssembliesInternal)' == ''" Text="Item _WasmAssembliesInternal is empty" />
     <Error Condition="'$(_IsEMSDKMissing)' == 'true'"
            Text="$(_EMSDKMissingErrorMessage) Emscripten SDK is required for AOT'ing assemblies." />
+    <Error Condition="!Exists('$(MonoAotCrossCompilerPath)')" Text="MonoAotCrossCompilerPath=$(MonoAotCrossCompilerPath) doesn't exist" />
 
     <ItemGroup>
       <MonoAOTCompilerDefaultAotArguments Include="no-opt" />
@@ -222,7 +223,7 @@
       <WasmAppDir>$([MSBuild]::NormalizeDirectory($(WasmAppDir)))</WasmAppDir>
       <MicrosoftNetCoreAppRuntimePackRidDir>$([MSBuild]::NormalizeDirectory($(MicrosoftNetCoreAppRuntimePackRidDir)))</MicrosoftNetCoreAppRuntimePackRidDir>
       <MicrosoftNetCoreAppRuntimePackRidNativeDir>$([MSBuild]::NormalizeDirectory($(MicrosoftNetCoreAppRuntimePackRidDir), 'native'))</MicrosoftNetCoreAppRuntimePackRidNativeDir>
-      <MonoAotCrossCompilerPath Condition="'$(MonoAotCrossCompilerPath)' == ''">$([MSBuild]::NormalizePath($(MicrosoftNetCoreAppRuntimePackRidNativeDir), 'cross', $(PackageRID), 'mono-aot-cross$(_ExeExt)'))</MonoAotCrossCompilerPath>
+      <MonoAotCrossCompilerPath Condition="'$(MonoAotCrossCompilerPath)' == ''">$([MSBuild]::NormalizePath($(MicrosoftNetCoreAppRuntimePackRidNativeDir), 'cross', $(RuntimeIdentifier), 'mono-aot-cross$(_ExeExt)'))</MonoAotCrossCompilerPath>
       <!-- emcc, and mono-aot-cross don't like relative paths for output files -->
       <_WasmIntermediateOutputPath>$([MSBuild]::NormalizeDirectory($(IntermediateOutputPath), 'wasm'))</_WasmIntermediateOutputPath>
     </PropertyGroup>

src/mono/wasm/debugger/DebuggerTestSuite/AssignmentTests.cs

@@ -0,0 +1,66 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Newtonsoft.Json.Linq;
+using Xunit;
+
+namespace DebuggerTests
+{
+    public class AssignmentTests : DebuggerTestBase
+    {
+        public static TheoryData<string, JObject, JObject> GetTestData => new TheoryData<string, JObject, JObject>
+        {
+            { "MONO_TYPE_OBJECT",      TObject("object", is_null: true),                        TObject("object") },
+            { "MONO_TYPE_CLASS",       TObject("DebuggerTests.MONO_TYPE_CLASS", is_null: true), TObject("DebuggerTests.MONO_TYPE_CLASS") },
+            { "MONO_TYPE_BOOLEAN",     TBool(default),                                          TBool(true) },
+            { "MONO_TYPE_CHAR",        TSymbol("0 '\u0000'"),                                   TSymbol("97 'a'") },
+            { "MONO_TYPE_STRING",      TString(default),                                        TString("hello") },
+            { "MONO_TYPE_ENUM",        TEnum("DebuggerTests.RGB", "Red"),                       TEnum("DebuggerTests.RGB", "Blue") },
+            { "MONO_TYPE_ARRAY",       TObject("byte[]", is_null: true),                        TArray("byte[]", 2) },
+            { "MONO_TYPE_VALUETYPE",   TValueType("DebuggerTests.Point"),                       TValueType("DebuggerTests.Point") },
+            { "MONO_TYPE_VALUETYPE2",  TValueType("System.Decimal","0"),                        TValueType("System.Decimal", "1.1") },
+            { "MONO_TYPE_GENERICINST", TObject("System.Func<int>", is_null: true),              TDelegate("System.Func<int>", "int Prepare ()") },
+            { "MONO_TYPE_FNPTR",       TPointer("*()",  is_null: true),                         TPointer("*()") },
+            { "MONO_TYPE_PTR",         TPointer("int*", is_null: true),                         TPointer("int*") },
+            { "MONO_TYPE_I1",          TNumber(0),                                              TNumber(-1) },
+            { "MONO_TYPE_I2",          TNumber(0),                                              TNumber(-1) },
+            { "MONO_TYPE_I4",          TNumber(0),                                              TNumber(-1) },
+            { "MONO_TYPE_I8",          TNumber(0),                                              TNumber(-1) },
+            { "MONO_TYPE_U1",          TNumber(0),                                              TNumber(1) },
+            { "MONO_TYPE_U2",          TNumber(0),                                              TNumber(1) },
+            { "MONO_TYPE_U4",          TNumber(0),                                              TNumber(1) },
+            { "MONO_TYPE_U8",          TNumber(0),                                              TNumber(1) },
+            { "MONO_TYPE_R4",          TNumber(0),                                              TNumber("3.1414999961853027") },
+            { "MONO_TYPE_R8",          TNumber(0),                                              TNumber("3.1415") },
+        };
+
+        [Theory]
+        [MemberData("GetTestData")]
+        async Task InspectVariableBeforeAndAfterAssignment(string clazz, JObject checkDefault, JObject checkValue)
+        {
+            await SetBreakpointInMethod("debugger-test", "DebuggerTests." + clazz, "Prepare", 2);
+            await EvaluateAndCheck("window.setTimeout(function() { invoke_static_method('[debugger-test] DebuggerTests." + clazz + ":Prepare'); })", null, -1, -1, "Prepare");
+
+            // 1) check un-assigned variables
+            await StepAndCheck(StepKind.Into, "dotnet://debugger-test.dll/debugger-assignment-test.cs", -1, -1, "TestedMethod",
+                locals_fn: (locals) =>
+                {
+                    Assert.Equal(2, locals.Count());
+                    Check(locals, "r", checkDefault);
+                }
+            );
+
+            // 2) check assigned variables
+            await StepAndCheck(StepKind.Over, "dotnet://debugger-test.dll/debugger-assignment-test.cs", -1, -1, "TestedMethod", times: 3,
+                locals_fn: (locals) =>
+                {
+                    Assert.Equal(2, locals.Count());
+                    Check(locals, "r", checkValue);
+                }
+            );
+        }
+    }
+}

src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs

@@ -53,6 +53,7 @@ static protected string FindTestPath()
             "/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge",
             "/Applications/Google Chrome Canary.app/Contents/MacOS/Google Chrome Canary",
             "/usr/bin/chromium",
+            "C:/Program Files/Google/Chrome/Application/chrome.exe",
             "/usr/bin/chromium-browser",
         };
         static string chrome_path;
@@ -89,19 +90,19 @@ public DebuggerTestBase(string driver = "debugger-driver.html")
 
         public virtual async Task InitializeAsync()
         {
-           Func<InspectorClient, CancellationToken, List<(string, Task<Result>)>> fn = (client, token) =>
-            {
-                Func<string, (string, Task<Result>)> getInitCmdFn = (cmd) => (cmd, client.SendCommand(cmd, null, token));
-                var init_cmds = new List<(string, Task<Result>)>
-                {
+            Func<InspectorClient, CancellationToken, List<(string, Task<Result>)>> fn = (client, token) =>
+             {
+                 Func<string, (string, Task<Result>)> getInitCmdFn = (cmd) => (cmd, client.SendCommand(cmd, null, token));
+                 var init_cmds = new List<(string, Task<Result>)>
+                 {
                     getInitCmdFn("Profiler.enable"),
                     getInitCmdFn("Runtime.enable"),
                     getInitCmdFn("Debugger.enable"),
                     getInitCmdFn("Runtime.runIfWaitingForDebugger")
-                };
+                 };
 
-                return init_cmds;
-            };
+                 return init_cmds;
+             };
 
             await Ready();
             await insp.OpenSessionAsync(fn);
@@ -153,9 +154,9 @@ await EvaluateAndCheck(
                 function_name,
                 wait_for_event_fn: async (pause_location) =>
                {
-                       //make sure we're on the right bp
+                   //make sure we're on the right bp
 
-                       Assert.Equal(bp.Value["breakpointId"]?.ToString(), pause_location["hitBreakpoints"]?[0]?.Value<string>());
+                   Assert.Equal(bp.Value["breakpointId"]?.ToString(), pause_location["hitBreakpoints"]?[0]?.Value<string>());
 
                    var top_frame = pause_location!["callFrames"]?[0];
 
@@ -262,11 +263,18 @@ internal JToken CheckSymbol(JToken locals, string name, string value)
             return l;
         }
 
-        internal JToken CheckObject(JToken locals, string name, string class_name, string subtype = null, bool is_null = false)
+        internal JToken Check(JToken locals, string name, JObject expected)
+        {
+            var l = GetAndAssertObjectWithName(locals, name);
+            CheckValue(l["value"], expected, name).Wait();
+            return l;
+        }
+
+        internal JToken CheckObject(JToken locals, string name, string class_name, string subtype = null, bool is_null = false, string description = null)
         {
             var l = GetAndAssertObjectWithName(locals, name);
             var val = l["value"];
-            CheckValue(val, TObject(class_name, is_null: is_null), name).Wait();
+            CheckValue(val, TObject(class_name, is_null: is_null, description: description), name).Wait();
             Assert.True(val["isValueType"] == null || !val["isValueType"].Value<bool>());
 
             return l;
@@ -330,10 +338,10 @@ internal void CheckContentValue(JToken token, string value)
             Assert.Equal(value, val);
         }
 
-        internal JToken CheckValueType(JToken locals, string name, string class_name)
+        internal JToken CheckValueType(JToken locals, string name, string class_name, string description=null)
         {
             var l = GetAndAssertObjectWithName(locals, name);
-            CheckValue(l["value"], TValueType(class_name), name).Wait();
+            CheckValue(l["value"], TValueType(class_name, description: description), name).Wait();
             return l;
         }
 
@@ -413,7 +421,7 @@ internal async Task<Result> InvokeGetter(JToken obj, object arguments, string fn
             {
                 functionDeclaration = fn,
                 objectId = obj["value"]?["objectId"]?.Value<string>(),
-                arguments = new[] { new { value = property } , new { value = newvalue } },
+                arguments = new[] { new { value = property }, new { value = newvalue } },
                 silent = true
             });
             var res = await cli.SendCommand("Runtime.callFunctionOn", req, token);
@@ -471,7 +479,14 @@ internal async Task<JObject> SendCommandAndCheck(JObject args, string method, st
             if (locals_fn != null)
             {
                 var locals = await GetProperties(wait_res["callFrames"][0]["callFrameId"].Value<string>());
-                locals_fn(locals);
+                try
+                {
+                    locals_fn(locals);
+                }
+                catch (System.AggregateException ex)
+                {
+                    throw new AggregateException(ex.Message + " \n" + locals.ToString(), ex);
+                }
             }
 
             return wait_res;
@@ -701,9 +716,9 @@ internal async Task CheckValue(JToken actual_val, JToken exp_val, string label)
                     AssertEqual(exp_val_str, actual_field_val_str, $"[{label}] Value for json property named {jp.Name} didn't match.");
                 }
             }
-            catch
+            catch (Exception ex)
             {
-                Console.WriteLine($"Expected: {exp_val}. Actual: {actual_val}");
+                Console.WriteLine($"{ex.Message} \nExpected: {exp_val} \nActual: {actual_val}");
                 throw;
             }
         }
@@ -848,8 +863,8 @@ internal async Task<Result> RemoveBreakpoint(string id, bool expect_ok = true)
         internal async Task<Result> SetBreakpoint(string url_key, int line, int column, bool expect_ok = true, bool use_regex = false, string condition = "")
         {
             var bp1_req = !use_regex ?
-                JObject.FromObject(new { lineNumber = line, columnNumber = column, url = dicFileToUrl[url_key], condition}) :
-                JObject.FromObject(new { lineNumber = line, columnNumber = column, urlRegex = url_key, condition});
+                JObject.FromObject(new { lineNumber = line, columnNumber = column, url = dicFileToUrl[url_key], condition }) :
+                JObject.FromObject(new { lineNumber = line, columnNumber = column, urlRegex = url_key, condition });
 
             var bp1_res = await cli.SendCommand("Debugger.setBreakpointByUrl", bp1_req, token);
             Assert.True(expect_ok ? bp1_res.IsOk : bp1_res.IsErr);
@@ -929,6 +944,9 @@ internal static JObject TNumber(int value) =>
         internal static JObject TNumber(uint value) =>
             JObject.FromObject(new { type = "number", value = @value.ToString(), description = value.ToString() });
 
+        internal static JObject TNumber(string value) =>
+            JObject.FromObject(new { type = "number", value = @value.ToString(), description = value });
+
         internal static JObject TValueType(string className, string description = null, object members = null) =>
             JObject.FromObject(new { type = "object", isValueType = true, className = className, description = description ?? className });
 

src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs

@@ -147,6 +147,7 @@ private static string FormatConsoleAPICalled(JObject args)
 
         async Task OnMessage(string method, JObject args, CancellationToken token)
         {
+            bool fail = false;
             switch (method)
             {
                 case "Debugger.paused":
@@ -158,14 +159,22 @@ async Task OnMessage(string method, JObject args, CancellationToken token)
                 case "Runtime.consoleAPICalled":
                     _logger.LogInformation(FormatConsoleAPICalled(args));
                     break;
+                case "Inspector.detached":
+                case "Inspector.targetCrashed":
+                case "Inspector.targetReloadedAfterCrash":
+                    fail = true;
+                    break;
+                case "Runtime.exceptionThrown":
+                    _logger.LogDebug($"Failing all waiters because: {method}: {args}");
+                    fail = true;
+                    break;
             }
             if (eventListeners.TryGetValue(method, out var listener))
             {
                 await listener(args, token).ConfigureAwait(false);
             }
-            else if (String.Compare(method, "Runtime.exceptionThrown") == 0)
+            else if (fail)
             {
-                _logger.LogDebug($"Failing all waiters because: {method}: {args}");
                 FailAllWaiters(new ArgumentException(args.ToString()));
             }
         }

src/mono/wasm/debugger/tests/Directory.Build.props

@@ -5,7 +5,7 @@
     <TargetFramework>$(AspNetCoreAppCurrent)</TargetFramework>
     <OutputType>Library</OutputType>
     <Configuration>Debug</Configuration>
-    <RuntimeConfiguration>Release</RuntimeConfiguration>
+    <RuntimeConfiguration Condition="'$(RuntimeConfiguration)'==''">Release</RuntimeConfiguration>
 
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <NoWarn>219</NoWarn>