Skip to content

Commit

Permalink
[wasm][debugger] Add support for surfacing inherited members (dotnet#…
Browse files Browse the repository at this point in the history 
…41480)

* [wasm][debugger][tests] Update to use `TDateTime`

- this ensures that we check the datetime, and some property getters on
it, wherever we have a datetime.

* [wasm][debugger][tests] Add labels to more checks

* [wasm][debugger] Add support for surfacing inherited members

- surface inherited fields, and properties
- we try to support `Runtime.getProperties`'s two arguments:
    - `ownProperties`, and `accessorsOnly`

    - `ownProperties`: for JS, this means return only the object's own
    members (not inherited ones)
    - `accessorsOnly`: for JS, this means return all the getters

Actual implementation:

- In practice, VSCode, and Chrome debugger seem to only send
`{ ownProperties: true, accessorsOnly: false }`,
and `{ ownProperties: false, accessorsOnly: true }`. The combination of
which means - that we don't return any inherited fields!

- But we want to show inherited fields too, so to get that behavior we
essentially *ignore* `ownProperties`. IOW,

    - `ownProperties`: we return all fields, and properties
    - `accessorsOnly`: we return only the getters, including the
    inherited ones

- Another thing to note is the case for auto-properties
    - these have a backing field
    - and we usually return the backing field's value, instead of
    returning a getter
    - To continue with that, auto-properties are *not* returned for
    `accessorsOnly`

- The code in `mini-wasm-debugger.c` does handle these two arguments,
but that is currently disabled by not passing the args to debugger.c at
all
    - Instead, we get the *full* list of members, and try to filter it
    in `library_mono.js`
    - which includes handling property overrides, or shadowing by new
    properties/fields in derived classes

* [wasm][debugger][tests] Fix simple warnings

* [wasm][debugger][tests] Fix warnings introduced in this PR

* [wasm][debugger][tests] Fix indentation

* [wasm][debugger] Correctly handle local structs in async methods

- When we have a struct local in an async instance method, it doesn't
get expanded, since we have a containerId (the async object), and we can
expand/access it later.

- When the IDE asks us to expand it with `{accessorPropertiesOnly: true}`:
    - we get the expanded json, but `_filter_automatic_properties` tries
    to return just the accessors, but that doesn't handle the expanded
    members of nested structs!
    - That is done in `extract_and_cache_value_types`, which is run *after*
    `_filter_automatic_properties`, but by that time we have already
    lost the expanded members!

    - So, `_get_vt_properties` fails with `Unknown valuetype id`,
    because it doesn't have anything to return at that point.

- This is being solved by ignoring the getProperties args in case of
expanding valuetypes.
    - that means that we can correctly extract, and cache the whole
    object.
    - And after that, we can return accessors/others, based on the args.

* [wasm][debugger] Fix warnings in debugger-test-app, and turn on warnAsError

* For some cases, debugger seems to give the actual method name instead of MoveNext for async methods
  • Loading branch information
@radical
radical authored Sep 4, 2020
1 parent 740d59a commit b25b2bc
Show file tree
Hide file tree
Showing 15 changed files with 974 additions and 199 deletions.
121 changes: 80 additions & 41 deletions src/mono/mono/mini/mini-wasm-debugger.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ enum {
EXCEPTION_MODE_ALL
};

// Flags for get_*_properties
#define GPFLAG_NONE 0x0000
#define GPFLAG_OWN_PROPERTIES 0x0001
#define GPFLAG_ACCESSORS_ONLY 0x0002
#define GPFLAG_EXPAND_VALUETYPES 0x0004

//functions exported to be used by JS
G_BEGIN_DECLS

Expand All @@ -41,8 +47,8 @@ EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_get_local_vars (int scope, int* pos, int
EMSCRIPTEN_KEEPALIVE void mono_wasm_clear_all_breakpoints (void);
EMSCRIPTEN_KEEPALIVE int mono_wasm_setup_single_step (int kind);
EMSCRIPTEN_KEEPALIVE int mono_wasm_pause_on_exceptions (int state);
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_get_object_properties (int object_id, int gpflags);
EMSCRIPTEN_KEEPALIVE gboolean mono_wasm_get_array_values (int object_id, int start_idx, int count, int gpflags);
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);
Expand All @@ -61,7 +67,7 @@ extern void mono_wasm_add_typed_value (const char *type, const char *str_value,

G_END_DECLS

static void describe_object_properties_for_klass (void *obj, MonoClass *klass, gboolean isAsyncLocalThis, gboolean expandValueType);
static void describe_object_properties_for_klass (void *obj, MonoClass *klass, gboolean isAsyncLocalThis, int gpflags);
static void handle_exception (MonoException *exc, MonoContext *throw_ctx, MonoContext *catch_ctx, StackFrameInfo *catch_frame);

//FIXME move all of those fields to the profiler object
Expand Down Expand Up @@ -823,7 +829,8 @@ read_enum_value (const char *mem, int type)
return 0;
}

static gboolean describe_value(MonoType * type, gpointer addr, gboolean expandValueType)
static gboolean
describe_value(MonoType * type, gpointer addr, int gpflags)
{
ERROR_DECL (error);
switch (type->type) {
Expand Down Expand Up @@ -984,7 +991,7 @@ static gboolean describe_value(MonoType * type, gpointer addr, gboolean expandVa
} else {
char *to_string_val = get_to_string_description (class_name, klass, addr);

if (expandValueType) {
if (gpflags & GPFLAG_EXPAND_VALUETYPES) {
int32_t size = mono_class_value_size (klass, NULL);
void *value_buf = g_malloc0 (size);
mono_value_copy_internal (value_buf, addr, klass);
Expand All @@ -996,7 +1003,7 @@ static gboolean describe_value(MonoType * type, gpointer addr, gboolean expandVa
g_free (value_buf);

// FIXME: isAsyncLocalThis
describe_object_properties_for_klass (addr, klass, FALSE, expandValueType);
describe_object_properties_for_klass (addr, klass, FALSE, gpflags);
mono_wasm_add_typed_value ("end_vt", NULL, 0);
} else {
EM_ASM ({
Expand Down Expand Up @@ -1043,35 +1050,45 @@ 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, TRUE);
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, TRUE);
describe_value (sig->ret, &res, GPFLAG_EXPAND_VALUETYPES);
else
describe_value (sig->ret, mono_object_unbox_internal (res), TRUE);
describe_value (sig->ret, mono_object_unbox_internal (res), GPFLAG_EXPAND_VALUETYPES);
}

static void
describe_object_properties_for_klass (void *obj, MonoClass *klass, gboolean isAsyncLocalThis, gboolean expandValueType)
describe_object_properties_for_klass (void *obj, MonoClass *klass, gboolean isAsyncLocalThis, int gpflags)
{
MonoClassField *f;
MonoProperty *p;
MonoMethodSignature *sig;
gpointer iter = NULL;
gboolean is_valuetype;
int pnum;
char *klass_name;
gboolean auto_invoke_getters;
gboolean is_own;
gboolean only_backing_fields;

g_assert (klass);
MonoClass *start_klass = klass;

only_backing_fields = gpflags & GPFLAG_ACCESSORS_ONLY;
is_valuetype = m_class_is_valuetype(klass);
if (is_valuetype)
gpflags |= GPFLAG_EXPAND_VALUETYPES;

handle_parent:
is_own = (start_klass == klass);
klass_name = mono_class_full_name (klass);
gpointer iter = NULL;
while (obj && (f = mono_class_get_fields_internal (klass, &iter))) {
if (isAsyncLocalThis && f->name[0] == '<' && f->name[1] == '>') {
if (g_str_has_suffix (f->name, "__this")) {
mono_wasm_add_properties_var ("this", f->offset);
gpointer field_value = (guint8*)obj + f->offset;

describe_value (f->type, field_value, is_valuetype | expandValueType);
describe_value (f->type, field_value, gpflags);
}

continue;
Expand All @@ -1081,29 +1098,39 @@ describe_object_properties_for_klass (void *obj, MonoClass *klass, gboolean isAs
if (mono_field_is_deleted (f))
continue;

mono_wasm_add_properties_var (f->name, f->offset);
if (only_backing_fields && !g_str_has_suffix(f->name, "k__BackingField"))
continue;

EM_ASM ({
MONO.mono_wasm_add_properties_var ($0, { field_offset: $1, is_own: $2, attr: $3, owner_class: $4 });
}, f->name, f->offset, is_own, f->type->attrs, klass_name);

gpointer field_addr;
if (is_valuetype)
field_addr = mono_vtype_get_field_addr (obj, f);
else
field_addr = (guint8*)obj + f->offset;
describe_value (f->type, field_addr, is_valuetype | expandValueType);

describe_value (f->type, field_addr, gpflags);
}

klass_name = mono_class_full_name (klass);
auto_invoke_getters = are_getters_allowed (klass_name);

iter = NULL;
pnum = 0;
while ((p = mono_class_get_properties (klass, &iter))) {
if (p->get->name) { //if get doesn't have name means that doesn't have a getter implemented and we don't want to show value, like VS debug
if (isAsyncLocalThis && (p->name[0] != '<' || (p->name[0] == '<' && p->name[1] == '>')))
continue;

mono_wasm_add_properties_var (p->name, pnum);
sig = mono_method_signature_internal (p->get);
if (sig->param_count != 0) {
// getters with params are not shown
continue;
}

EM_ASM ({
MONO.mono_wasm_add_properties_var ($0, { field_offset: $1, is_own: $2, attr: $3, owner_class: $4 });
}, p->name, pnum, is_own, p->attrs, klass_name);

gboolean vt_self_type_getter = is_valuetype && mono_class_from_mono_type_internal (sig->ret) == klass;
if (auto_invoke_getters && !vt_self_type_getter) {
Expand All @@ -1112,8 +1139,7 @@ describe_object_properties_for_klass (void *obj, MonoClass *klass, gboolean isAs
// not allowed to call the getter here
char *ret_class_name = mono_class_full_name (mono_class_from_mono_type_internal (sig->ret));

gboolean invokable = sig->param_count == 0;
mono_wasm_add_typed_value ("getter", ret_class_name, invokable);
mono_wasm_add_typed_value ("getter", ret_class_name, -1);

g_free (ret_class_name);
continue;
Expand All @@ -1123,6 +1149,14 @@ describe_object_properties_for_klass (void *obj, MonoClass *klass, gboolean isAs
}

g_free (klass_name);

// ownProperties
// Note: ownProperties should mean that we return members of the klass itself,
// but we are going to ignore that here, because otherwise vscode/chrome don't
// seem to ask for inherited fields at all.
// if (!is_valuetype && !(gpflags & GPFLAG_OWN_PROPERTIES) && (klass = m_class_get_parent (klass)))
if (!is_valuetype && (klass = m_class_get_parent (klass)))
goto handle_parent;
}

/*
Expand All @@ -1148,9 +1182,9 @@ describe_delegate_properties (MonoObject *obj)
}

static gboolean
describe_object_properties (guint64 objectId, gboolean isAsyncLocalThis, gboolean expandValueType)
describe_object_properties (guint64 objectId, gboolean isAsyncLocalThis, int gpflags)
{
DEBUG_PRINTF (2, "describe_object_properties %llu\n", objectId);
DEBUG_PRINTF (2, "describe_object_properties %llu, gpflags: %d\n", objectId, gpflags);

MonoObject *obj = get_object_from_id (objectId);
if (!obj)
Expand All @@ -1160,7 +1194,7 @@ describe_object_properties (guint64 objectId, gboolean isAsyncLocalThis, gboolea
// delegates get the same id format as regular objects
describe_delegate_properties (obj);
} else {
describe_object_properties_for_klass (obj, obj->vtable->klass, isAsyncLocalThis, expandValueType);
describe_object_properties_for_klass (obj, obj->vtable->klass, isAsyncLocalThis, gpflags);
}

return TRUE;
Expand All @@ -1174,7 +1208,9 @@ invoke_getter (void *obj_or_value, MonoClass *klass, const char *name)
return FALSE;
}

gpointer iter = NULL;
gpointer iter;
handle_parent:
iter = NULL;
MonoProperty *p;
while ((p = mono_class_get_properties (klass, &iter))) {
//if get doesn't have name means that doesn't have a getter implemented and we don't want to show value, like VS debug
Expand All @@ -1185,11 +1221,14 @@ invoke_getter (void *obj_or_value, MonoClass *klass, const char *name)
return TRUE;
}

if ((klass = m_class_get_parent(klass)))
goto handle_parent;

return FALSE;
}

static gboolean
describe_array_values (guint64 objectId, int startIdx, int count, gboolean expandValueType)
static gboolean
describe_array_values (guint64 objectId, int startIdx, int count, int gpflags)
{
if (count == 0)
return TRUE;
Expand Down Expand Up @@ -1229,22 +1268,22 @@ describe_array_values (guint64 objectId, int startIdx, int count, gboolean expan
for (int i = startIdx; i < endIdx; i ++) {
mono_wasm_add_array_item(i);
elem = (gpointer*)((char*)arr->vector + (i * esize));
describe_value (m_class_get_byval_arg (m_class_get_element_class (klass)), elem, expandValueType);
describe_value (m_class_get_byval_arg (m_class_get_element_class (klass)), elem, gpflags);
}
return TRUE;
}

static void
describe_async_method_locals (InterpFrame *frame, MonoMethod *method)
{
//Async methods are special in the way that local variables can be lifted to generated class fields
//Async methods are special in the way that local variables can be lifted to generated class fields
gpointer addr = NULL;
if (mono_debug_lookup_method_async_debug_info (method)) {
addr = mini_get_interp_callbacks ()->frame_get_this (frame);
MonoObject *obj = *(MonoObject**)addr;
int objId = get_object_id (obj);
mono_wasm_set_is_async_method (objId);
describe_object_properties (objId, TRUE, FALSE);
describe_object_properties (objId, TRUE, GPFLAG_NONE);
}
}

Expand All @@ -1264,17 +1303,17 @@ describe_non_async_this (InterpFrame *frame, MonoMethod *method)
mono_wasm_add_properties_var ("this", -1);

if (m_class_is_valuetype (klass)) {
describe_value (type, obj, TRUE);
describe_value (type, obj, GPFLAG_EXPAND_VALUETYPES);
} else {
// this is an object, and we can retrieve the valuetypes in it later
// through the object id
describe_value (type, addr, FALSE);
describe_value (type, addr, GPFLAG_NONE);
}
}
}

static gboolean
describe_variable (InterpFrame *frame, MonoMethod *method, int pos, gboolean expandValueType)
describe_variable (InterpFrame *frame, MonoMethod *method, int pos, int gpflags)
{
ERROR_DECL (error);
MonoMethodHeader *header = NULL;
Expand All @@ -1295,7 +1334,7 @@ describe_variable (InterpFrame *frame, MonoMethod *method, int pos, gboolean exp

DEBUG_PRINTF (2, "adding val %p type [%p] %s\n", addr, type, mono_type_full_name (type));

describe_value(type, addr, expandValueType);
describe_value(type, addr, gpflags);
if (header)
mono_metadata_free_mh (header);

Expand Down Expand Up @@ -1324,7 +1363,7 @@ describe_variables_on_frame (MonoStackFrameInfo *info, MonoContext *ctx, gpointe

for (int i = 0; i < data->len; i++)
{
describe_variable (frame, method, data->pos[i], TRUE);
describe_variable (frame, method, data->pos[i], GPFLAG_EXPAND_VALUETYPES);
}

describe_async_method_locals (frame, method);
Expand All @@ -1343,7 +1382,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, TRUE);
describe_value (type->data.type, value_addr, GPFLAG_EXPAND_VALUETYPES);
return TRUE;
}

Expand All @@ -1363,19 +1402,19 @@ mono_wasm_get_local_vars (int scope, int* pos, int len)
}

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

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

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

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

EMSCRIPTEN_KEEPALIVE gboolean
Expand Down
Loading

0 comments on commit b25b2bc

Please sign in to comment.