Skip to content

Commit

Permalink
[wasm][debugger] Return errors from debugger.c's details functions
Browse files Browse the repository at this point in the history
- functions like those for getting object/vt properties, can fail, for
example, if the objectId is invalid.
- We now return that bool result, and that gets surfaced to the caller

- This will also help to differentiate the case where the result of such
a function was a failure vs just an empty result
  • Loading branch information
radical committed Aug 7, 2020
1 parent 89cfe32 commit 8116c9e
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 34 deletions.
39 changes: 21 additions & 18 deletions src/mono/mono/mini/mini-wasm-debugger.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ EMSCRIPTEN_KEEPALIVE int mono_wasm_set_breakpoint (const char *assembly_name, in
EMSCRIPTEN_KEEPALIVE int mono_wasm_remove_breakpoint (int bp_id);
EMSCRIPTEN_KEEPALIVE int mono_wasm_current_bp_id (void);
EMSCRIPTEN_KEEPALIVE void mono_wasm_enum_frames (void);
EMSCRIPTEN_KEEPALIVE void mono_wasm_get_local_vars (int scope, int* pos, int len);
EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_get_local_vars (int scope, int* pos, int len);
EMSCRIPTEN_KEEPALIVE void mono_wasm_clear_all_breakpoints (void);
EMSCRIPTEN_KEEPALIVE int mono_wasm_setup_single_step (int kind);
EMSCRIPTEN_KEEPALIVE void mono_wasm_get_object_properties (int object_id, gboolean expand_value_types);
EMSCRIPTEN_KEEPALIVE void mono_wasm_get_array_values (int object_id, int start_idx, int count, gboolean expand_value_types);
EMSCRIPTEN_KEEPALIVE void mono_wasm_invoke_getter_on_object (int object_id, const char* name);
EMSCRIPTEN_KEEPALIVE void mono_wasm_invoke_getter_on_value (void *value, MonoClass *klass, const char *name);
EMSCRIPTEN_KEEPALIVE void mono_wasm_get_deref_ptr_value (void *value_addr, MonoClass *klass);
EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_get_object_properties (int object_id, gboolean expand_value_types);
EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_get_array_values (int object_id, int start_idx, int count, gboolean expand_value_types);
EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_invoke_getter_on_object (int object_id, const char* name);
EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_invoke_getter_on_value (void *value, MonoClass *klass, const char *name);
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);
Expand Down Expand Up @@ -1286,21 +1286,22 @@ describe_variables_on_frame (MonoStackFrameInfo *info, MonoContext *ctx, gpointe
return TRUE;
}

EMSCRIPTEN_KEEPALIVE void
EMSCRIPTEN_KEEPALIVE gboolean
mono_wasm_get_deref_ptr_value (void *value_addr, MonoClass *klass)
{
MonoType *type = m_class_get_byval_arg (klass);
if (type->type != MONO_TYPE_PTR && type->type != MONO_TYPE_FNPTR) {
DEBUG_PRINTF (2, "BUG: mono_wasm_get_deref_ptr_value: Expected to get a ptr type, but got 0x%x\n", type->type);
return;
return FALSE;
}

mono_wasm_add_properties_var ("deref", -1);
describe_value (type->data.type, value_addr, TRUE);
return TRUE;
}

//FIXME this doesn't support getting the return value pseudo-var
EMSCRIPTEN_KEEPALIVE void
EMSCRIPTEN_KEEPALIVE gboolean
mono_wasm_get_local_vars (int scope, int* pos, int len)
{
FrameDescData data;
Expand All @@ -1310,39 +1311,41 @@ mono_wasm_get_local_vars (int scope, int* pos, int len)
data.pos = pos;

mono_walk_stack_with_ctx (describe_variables_on_frame, NULL, MONO_UNWIND_NONE, &data);

return TRUE;
}

EMSCRIPTEN_KEEPALIVE void
EMSCRIPTEN_KEEPALIVE gboolean
mono_wasm_get_object_properties (int object_id, gboolean expand_value_types)
{
DEBUG_PRINTF (2, "getting properties of object %d\n", object_id);

describe_object_properties (object_id, FALSE, expand_value_types);
return describe_object_properties (object_id, FALSE, expand_value_types);
}

EMSCRIPTEN_KEEPALIVE void
EMSCRIPTEN_KEEPALIVE gboolean
mono_wasm_get_array_values (int object_id, int start_idx, int count, gboolean expand_value_types)
{
DEBUG_PRINTF (2, "getting array values %d, startIdx: %d, count: %d, expandValueType: %d\n", object_id, start_idx, count, expand_value_types);

describe_array_values (object_id, start_idx, count, expand_value_types);
return describe_array_values (object_id, start_idx, count, expand_value_types);
}

EMSCRIPTEN_KEEPALIVE void
EMSCRIPTEN_KEEPALIVE
mono_wasm_invoke_getter_on_object (int object_id, const char* name)
{
MonoObject *obj = get_object_from_id (object_id);
if (!obj)
return;
return FALSE;

invoke_getter (obj, mono_object_class (obj), name);
return invoke_getter (obj, mono_object_class (obj), name);
}

EMSCRIPTEN_KEEPALIVE void
EMSCRIPTEN_KEEPALIVE gboolean
mono_wasm_invoke_getter_on_value (void *value, MonoClass *klass, const char *name)
{
DEBUG_PRINTF (2, "mono_wasm_invoke_getter_on_value: v: %p klass: %p, name: %s\n", value, klass, name);
invoke_getter (value, klass, name);
return invoke_getter (value, klass, name);
}

// Functions required by debugger-state-machine.
Expand Down
29 changes: 28 additions & 1 deletion src/mono/wasm/debugger/DebuggerTestSuite/ArrayTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -601,5 +601,32 @@ await CompareObjectPropertiesFor(frame_locals, "this",
});
}

[Fact]
public async Task InvalidArrayId() => await CheckInspectLocalsAtBreakpointSite(
"DebuggerTests.Container", "PlaceholderMethod", 1, "PlaceholderMethod",
"window.setTimeout(function() { invoke_static_method ('[debugger-test] DebuggerTests.ArrayTestsClass:ObjectArrayMembers'); }, 1);",
wait_for_event_fn : async(pause_location) =>
{
int frame_idx = 1;
var frame_locals = await GetProperties(pause_location["callFrames"][frame_idx]["callFrameId"].Value<string>());
var c_obj = GetAndAssertObjectWithName(frame_locals, "c");
var c_obj_id = c_obj["value"] ? ["objectId"]?.Value<string>();
Assert.NotNull(c_obj_id);
// Invalid format
await GetProperties("dotnet:array:4123", expect_ok : false);
// Invalid object id
await GetProperties("dotnet:array:{ \"arrayId\": 234980 }", expect_ok : false);
// Trying to access object as an array
if (!DotnetObjectId.TryParse (c_obj_id, out var id) || id.Scheme != "object")
Assert.True(false, "Unexpected object id format. Maybe this test is out of sync with the object id format in library_mono.js?");
if (!int.TryParse(id.Value, out var idNum))
Assert.True(false, "Expected a numeric value part of the object id: {c_obj_id}");
await GetProperties($"dotnet:array:{{\"arrayId\":{idNum}}}", expect_ok : false);
});
}
}
}
60 changes: 60 additions & 0 deletions src/mono/wasm/debugger/DebuggerTestSuite/Support.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Microsoft.WebAssembly.Diagnostics;
using Xunit;
Expand Down Expand Up @@ -962,6 +963,65 @@ public DebugTestContext(InspectorClient cli, Inspector insp, CancellationToken t
}
}

class DotnetObjectId
{
public string Scheme { get; }
public string Value { get; }

JObject value_json;
public JObject ValueAsJson
{
get
{
if (value_json == null)
{
try
{
value_json = JObject.Parse(Value);
}
catch (JsonReaderException) { }
}

return value_json;
}
}

public static bool TryParse(JToken jToken, out DotnetObjectId objectId) => TryParse(jToken?.Value<string>(), out objectId);

public static bool TryParse(string id, out DotnetObjectId objectId)
{
objectId = null;
if (id == null)
{
return false;
}

if (!id.StartsWith("dotnet:"))
{
return false;
}

var parts = id.Split(":", 3);

if (parts.Length < 3)
{
return false;
}

objectId = new DotnetObjectId(parts[1], parts[2]);

return true;
}

public DotnetObjectId(string scheme, string value)
{
Scheme = scheme;
Value = value;
}

public override string ToString() => $"dotnet:{Scheme}:{Value}";
}

enum StepKind
{
Into,
Expand Down
57 changes: 42 additions & 15 deletions src/mono/wasm/runtime/library_mono.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,8 +255,10 @@ var MonoSupportLib = {
}

this._async_method_objectId = 0;
let res = this.mono_wasm_get_local_vars_info (scope, heapBytes.byteOffset, var_list.length);
let { res_ok, res } = this.mono_wasm_get_local_vars_info (scope, heapBytes.byteOffset, var_list.length);
Module._free(heapBytes.byteOffset);
if (!res_ok)
throw new Error (`Failed to get locals for scope ${scope}`);

if (this._async_method_objectId != 0)
this._assign_vt_ids (res, v => ({ containerId: this._async_method_objectId, fieldOffset: v.fieldOffset }));
Expand Down Expand Up @@ -288,7 +290,10 @@ var MonoSupportLib = {
* @returns {object}
*/
_get_object_properties: function(idNum, expandValueTypes) {
let res = this.mono_wasm_get_object_properties_info (idNum, expandValueTypes);
let { res_ok, res } = this.mono_wasm_get_object_properties_info (idNum, expandValueTypes);
if (!res_ok)
throw new Error (`Failed to get properties for ${idNum}`);

res = MONO._filter_automatic_properties (res);
res = this._assign_vt_ids (res, v => ({ containerId: idNum, fieldOffset: v.fieldOffset }));
res = this._post_process_details (res);
Expand All @@ -307,7 +312,10 @@ var MonoSupportLib = {
if (isNaN (id.o.arrayId))
throw new Error (`Invalid array id: ${id.idStr}`);

let res = this.mono_wasm_get_array_values_info (id.o.arrayId, startIdx, count, expandValueTypes);
let { res_ok, res } = this.mono_wasm_get_array_values_info (id.o.arrayId, startIdx, count, expandValueTypes);
if (!res_ok)
throw new Error (`Failed to get properties for array id ${id.idStr}`);

res = this._assign_vt_ids (res, (_, i) => ({ arrayId: id.o.arrayId, arrayIdx: Number (startIdx) + i}));

for (let i = 0; i < res.length; i ++) {
Expand Down Expand Up @@ -501,7 +509,10 @@ var MonoSupportLib = {
throw new Error (`Both ptr_addr and klass_addr need to be non-zero, to dereference a pointer. objectId: ${objectId}`);

const value_addr = new DataView (Module.HEAPU8.buffer).getUint32 (ptr_args.ptr_addr, /* littleEndian */ true);
let res = this.mono_wasm_get_deref_ptr_value_info (value_addr, ptr_args.klass_addr);
let { res_ok, res } = this.mono_wasm_get_deref_ptr_value_info (value_addr, ptr_args.klass_addr);
if (!res_ok)
throw new Error (`Failed to dereference pointer ${objectId}`);

if (res.length > 0) {
if (ptr_args.varName === undefined)
throw new Error (`Bug: no varName found for the pointer. objectId: ${objectId}`);
Expand Down Expand Up @@ -569,7 +580,11 @@ var MonoSupportLib = {
if (isNaN (id.o) || id.o < 0)
throw new Error (`Invalid object id: ${objectIdStr}`);

getter_res = this.mono_wasm_invoke_getter_on_object_info (id.o, name);
let { res_ok, res } = this.mono_wasm_invoke_getter_on_object_info (id.o, name);
if (!res_ok)
throw new Error (`Invoking getter on ${objectIdStr} failed`);

getter_res = res;
} else if (id.scheme == 'valuetype') {
const id_props = this._get_id_props (objectIdStr);
if (id_props === undefined)
Expand All @@ -582,8 +597,14 @@ var MonoSupportLib = {
const dataHeap = new Uint8Array (Module.HEAPU8.buffer, dataPtr, id_props.value64.length);
dataHeap.set (new Uint8Array (this._base64_to_uint8 (id_props.value64)));

getter_res = this.mono_wasm_invoke_getter_on_value_info (dataHeap.byteOffset, id_props.klass, name);
let { res_ok, res } = this.mono_wasm_invoke_getter_on_value_info (dataHeap.byteOffset, id_props.klass, name);
Module._free (dataHeap.byteOffset);

if (!res_ok) {
console.debug (`Invoking getter on valuetype ${objectIdStr}, with props: ${JSON.stringify (id_props)} failed`);
throw new Error (`Invoking getter on valuetype ${objectIdStr} failed`);
}
getter_res = res;
} else {
throw new Error (`Only object, and valuetypes supported for getters, id: ${objectIdStr}`);
}
Expand Down Expand Up @@ -711,16 +732,22 @@ var MonoSupportLib = {
* @returns {void}
*/
_register_c_var_fn: function (name, ret_type, params) {
if (ret_type !== 'bool')
throw new Error (`Bug: Expected a C function signature that returns bool`);

this._register_c_fn (name, ret_type, params);
Object.defineProperty (this, name + '_info', {
value: function (...args) {
MONO.var_info = [];
MONO._c_fn_table [name + '_wrapper'] (...args);
const res_ok = MONO._c_fn_table [name + '_wrapper'] (...args);
let res = MONO.var_info;
MONO.var_info = [];
res = this._fixup_name_value_objects (res);
if (res_ok) {
res = this._fixup_name_value_objects (res);
return { res_ok, res };
}

return res;
return { res_ok, res: undefined };
}
});
},
Expand All @@ -737,12 +764,12 @@ var MonoSupportLib = {
this._call_function_res_cache = {};

this._c_fn_table = {};
this._register_c_var_fn ('mono_wasm_get_object_properties', null, [ 'number', 'bool' ]);
this._register_c_var_fn ('mono_wasm_get_array_values', null, [ 'number', 'number', 'number', 'bool' ]);
this._register_c_var_fn ('mono_wasm_invoke_getter_on_object', null, [ 'number', 'string' ]);
this._register_c_var_fn ('mono_wasm_invoke_getter_on_value', null, [ 'number', 'number', 'string' ]);
this._register_c_var_fn ('mono_wasm_get_local_vars', null, [ 'number', 'number', 'number']);
this._register_c_var_fn ('mono_wasm_get_deref_ptr_value', null, [ 'number', 'number']);
this._register_c_var_fn ('mono_wasm_get_object_properties', 'bool', [ 'number', 'bool' ]);
this._register_c_var_fn ('mono_wasm_get_array_values', 'bool', [ 'number', 'number', 'number', 'bool' ]);
this._register_c_var_fn ('mono_wasm_invoke_getter_on_object', 'bool', [ 'number', 'string' ]);
this._register_c_var_fn ('mono_wasm_invoke_getter_on_value', 'bool', [ 'number', 'number', 'string' ]);
this._register_c_var_fn ('mono_wasm_get_local_vars', 'bool', [ 'number', 'number', 'number']);
this._register_c_var_fn ('mono_wasm_get_deref_ptr_value', 'bool', [ 'number', 'number']);
},

mono_wasm_set_breakpoint: function (assembly, method_token, il_offset) {
Expand Down

0 comments on commit 8116c9e

Please sign in to comment.