Skip to content

Commit

Permalink
[debugger] Assert when async debug a generic method (mono#17727)
Browse files Browse the repository at this point in the history
* When we try to call a method to get the async_id to do an async debug and we are trying to do this in a generic method like this:

async Task<T> ExecuteAsync_Broken<T>()
 {
            await Task.Delay(2);
            return default;
  }

We need to inflate the generic type before call the method or we will get the error:  Could not execute the method because the containing type 'System.Runtime.CompilerServices.AsyncTaskMethodBuilder1[T_REF]’, is not fully instantiated.

Fixes mono#17549
Fixes mono#17569

Cherry-picked by Alex Thibodeau -- some light massaging required
  • Loading branch information
thaystg authored and spatil-rythmos committed Jun 15, 2020
1 parent e2b47cb commit 51c8c04
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 9 deletions.
12 changes: 12 additions & 0 deletions mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ public static void wait_one ()
new Tests ().invoke_abort ();
new Tests ().evaluate_method ();

test_async_debug_generics();
test_invalid_argument_assembly_get_type ();

return 3;
Expand Down Expand Up @@ -558,6 +559,17 @@ public static void ss7_3 ()
ss_nested_3 ();
}

[MethodImplAttribute (MethodImplOptions.NoInlining)]
public static void test_async_debug_generics () {
ExecuteAsync_Broken<object>().Wait ();
}

async static Task<T> ExecuteAsync_Broken<T>()
{
await Task.Delay(2);
return default;
}

[MethodImplAttribute (MethodImplOptions.NoInlining)]
public static void ss_nested_1 (int i) {
}
Expand Down
9 changes: 9 additions & 0 deletions mcs/class/Mono.Debugger.Soft/Test/dtest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4444,5 +4444,14 @@ public void Hash ()
}
}

[Test]
public void TestAsyncDebugGenerics () {
Event e = run_until ("test_async_debug_generics");
e = step_in_await ("test_async_debug_generics", e);
e = step_in_await ("MoveNext", e);
e = step_in_await ("MoveNext", e);
e = step_in_await ("MoveNext", e);
e = step_in_await ("MoveNext", e);
}
} // class DebuggerTests
} // namespace
34 changes: 30 additions & 4 deletions mono/mini/debugger-agent.c
Original file line number Diff line number Diff line change
Expand Up @@ -5448,6 +5448,30 @@ get_object_id_for_debugger_method (MonoClass* async_builder_class)
return method;
}

static MonoClass *
get_class_to_get_builder_field(StackFrame *frame)
{
MonoError error;
gpointer this_addr = get_this_addr (frame);
MonoClass *original_class = frame->method->klass;
MonoClass *ret;
if (!mono_class_is_valuetype(original_class) && mono_class_is_open_constructed_type (&original_class->byval_arg)) {
MonoObject *this_obj = *(MonoObject**)this_addr;
MonoGenericContext context;
MonoType *inflated_type;

g_assert (this_obj);
context = mono_get_generic_context_from_stack_frame (frame->ji, this_obj->vtable);
inflated_type = mono_class_inflate_generic_type_checked (&original_class->byval_arg, &context, &error);
mono_error_assert_ok (&error); /* FIXME don't swallow the error */

ret = mono_class_from_mono_type (inflated_type);
mono_metadata_free_type (inflated_type);
return ret;
}
return original_class;
}

/* Return the address of the AsyncMethodBuilder struct belonging to the state machine method pointed to by FRAME */
static gpointer
get_async_method_builder (StackFrame *frame)
Expand All @@ -5456,15 +5480,17 @@ get_async_method_builder (StackFrame *frame)
MonoClassField *builder_field;
gpointer builder;
guint8 *this_addr;
MonoClass* klass = frame->method->klass;

builder_field = mono_class_get_field_from_name (frame->method->klass, "<>t__builder");
klass = get_class_to_get_builder_field(frame);
builder_field = mono_class_get_field_from_name_full (klass, "<>t__builder", NULL);
g_assert (builder_field);

this_addr = get_this_addr (frame);
if (!this_addr)
return NULL;

if (mono_class_is_valuetype (frame->method->klass)) {
if (mono_class_is_valuetype (klass)) {
guint8 *vtaddr = *(guint8**)this_addr;
builder = (char*)vtaddr + mono_field_get_offset (builder_field) - sizeof (MonoObject);
} else {
Expand Down Expand Up @@ -5497,7 +5523,7 @@ get_this_async_id (StackFrame *frame)
if (!builder)
return 0;

builder_field = mono_class_get_field_from_name (frame->method->klass, "<>t__builder");
builder_field = mono_class_get_field_from_name (get_class_to_get_builder_field(frame), "<>t__builder");
g_assert (builder_field);

tls = (DebuggerTlsData *)mono_native_tls_get_value (debugger_tls_id);
Expand All @@ -5521,7 +5547,7 @@ get_this_async_id (StackFrame *frame)
static gboolean
set_set_notification_for_wait_completion_flag (StackFrame *frame)
{
MonoClassField *builder_field = mono_class_get_field_from_name (frame->method->klass, "<>t__builder");
MonoClassField *builder_field = mono_class_get_field_from_name (get_class_to_get_builder_field(frame), "<>t__builder");
g_assert (builder_field);
gpointer builder = get_async_method_builder (frame);
g_assert (builder);
Expand Down
11 changes: 6 additions & 5 deletions mono/mini/mini-exceptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -805,8 +805,8 @@ get_generic_info_from_stack_frame (MonoJitInfo *ji, MonoContext *ctx)
/*
* generic_info is either a MonoMethodRuntimeGenericContext or a MonoVTable.
*/
static MonoGenericContext
get_generic_context_from_stack_frame (MonoJitInfo *ji, gpointer generic_info)
MonoGenericContext
mono_get_generic_context_from_stack_frame (MonoJitInfo *ji, gpointer generic_info)
{
MonoGenericContext context = { NULL, NULL };
MonoClass *klass, *method_container_class;
Expand Down Expand Up @@ -851,6 +851,7 @@ get_generic_context_from_stack_frame (MonoJitInfo *ji, gpointer generic_info)
return context;
}


static MonoMethod*
get_method_from_stack_frame (MonoJitInfo *ji, gpointer generic_info)
{
Expand All @@ -860,7 +861,7 @@ get_method_from_stack_frame (MonoJitInfo *ji, gpointer generic_info)

if (!ji->has_generic_jit_info || !mono_jit_info_get_generic_jit_info (ji)->has_this)
return jinfo_get_method (ji);
context = get_generic_context_from_stack_frame (ji, generic_info);
context = mono_get_generic_context_from_stack_frame (ji, generic_info);

method = jinfo_get_method (ji);
method = mono_method_get_declaring_generic_method (method);
Expand Down Expand Up @@ -1386,7 +1387,7 @@ get_exception_catch_class (MonoJitExceptionInfo *ei, MonoJitInfo *ji, MonoContex

if (!ji->has_generic_jit_info || !mono_jit_info_get_generic_jit_info (ji)->has_this)
return catch_class;
context = get_generic_context_from_stack_frame (ji, get_generic_info_from_stack_frame (ji, ctx));
context = mono_get_generic_context_from_stack_frame (ji, get_generic_info_from_stack_frame (ji, ctx));

/* FIXME: we shouldn't inflate but instead put the
type in the rgctx and fetch it from there. It
Expand Down Expand Up @@ -3473,7 +3474,7 @@ mono_llvm_match_exception (MonoJitInfo *jinfo, guint32 region_start, guint32 reg
MonoType *inflated_type;

g_assert (rgctx || this_obj);
context = get_generic_context_from_stack_frame (jinfo, rgctx ? rgctx : this_obj->vtable);
context = mono_get_generic_context_from_stack_frame (jinfo, rgctx ? rgctx : this_obj->vtable);
inflated_type = mono_class_inflate_generic_type_checked (&catch_class->byval_arg, &context, &error);
mono_error_assert_ok (&error); /* FIXME don't swallow the error */

Expand Down
3 changes: 3 additions & 0 deletions mono/mini/mini.h
Original file line number Diff line number Diff line change
Expand Up @@ -2883,4 +2883,7 @@ gboolean MONO_SIG_HANDLER_SIGNATURE (mono_chain_signal);

#endif

MonoGenericContext
mono_get_generic_context_from_stack_frame (MonoJitInfo *ji, gpointer generic_info);

#endif /* __MONO_MINI_H__ */

0 comments on commit 51c8c04

Please sign in to comment.