Skip to content

[wasm][debugger] - RuntimeError: memory access out of bounds #51859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 7, 2021
Merged

[wasm][debugger] - RuntimeError: memory access out of bounds #51859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4bd84ac
- fix null reference in describe_value() + add test for it
pavelsavara May 5, 2021
9649dbf
code review feedback
pavelsavara May 6, 2021
d0f6482
code review
pavelsavara May 7, 2021
516a769
lint
pavelsavara May 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/mono/mono/mini/mini-wasm-debugger.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))) {
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion src/mono/wasm/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion src/mono/wasm/build/WasmApp.InTree.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<MSBuild Projects="@(WasmAppBuildProject)"
Properties="Configuration=Debug"
Targets="Build;Publish"/>
Targets="Build"/>
</Target>

<Target Name="CopyAppZipToHelixTestDir"
Expand Down
3 changes: 2 additions & 1 deletion src/mono/wasm/build/WasmApp.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -221,7 +222,7 @@

<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>
Expand Down
66 changes: 66 additions & 0 deletions src/mono/wasm/debugger/DebuggerTestSuite/AssignmentTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of the new overloads that use CheckValue, you can use:

await CheckProps(locals, new
{
  r = checkDefault
}, "default", num_fields: 2);

But since this needs to be async, you will need to do:

var pause_location = await StepAndCheck(StepKind.Into, "dotnet://debugger-test.dll/debugger-assignment-test.cs", -1, -1, "TestedMethod");
var locals = await GetProperties(pause_location["callFrames"][0]["callFrameId"].Value<string>());

await CheckProps(locals, new
{
  r = checkDefault
}, "default", num_fields: 2);

}
);

// 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);
}
);
}
}
}
58 changes: 38 additions & 20 deletions src/mono/wasm/debugger/DebuggerTestSuite/DebuggerTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -85,19 +86,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);
Expand Down Expand Up @@ -149,9 +150,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];

Expand Down Expand Up @@ -258,11 +259,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;
Expand Down Expand Up @@ -326,10 +334,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;
}

Expand Down Expand Up @@ -409,7 +417,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);
Expand Down Expand Up @@ -467,7 +475,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;
Expand Down Expand Up @@ -697,9 +712,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;
}
}
Expand Down Expand Up @@ -844,8 +859,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);
Expand Down Expand Up @@ -925,6 +940,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 });

Expand Down
13 changes: 11 additions & 2 deletions src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand All @@ -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()));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/mono/wasm/debugger/tests/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading