Skip to content

Commit

Permalink
[debugger] Removing some asserts (mono#19758)
Browse files Browse the repository at this point in the history
* Removing some asserts and returning err_invalid_argument with an error message when it's possible.

* if we don't find method get_ObjectIdForDebugger we try to find the property Task to continue async debug.

Cherry-picked by Alex Thibodeau with some moderate changes
  • Loading branch information
thaystg authored and UnityAlex committed May 14, 2020
1 parent 127b402 commit 4fc45ee
Showing 1 changed file with 67 additions and 18 deletions.
85 changes: 67 additions & 18 deletions mono/mini/debugger-agent.c
Original file line number Diff line number Diff line change
Expand Up @@ -5442,7 +5442,16 @@ get_object_id_for_debugger_method (MonoClass* async_builder_class)
MonoError error;
GPtrArray *array = mono_class_get_methods_by_name (async_builder_class, "get_ObjectIdForDebugger", 0x24, FALSE, FALSE, &error);
mono_error_assert_ok (&error);
g_assert (array->len == 1);
if (array->len != 1) {
g_ptr_array_free (array, TRUE);
//if we don't find method get_ObjectIdForDebugger we try to find the property Task to continue async debug.
MonoProperty *prop = mono_class_get_property_from_name (async_builder_class, "Task");
if (!prop) {
DEBUG_PRINTF (1, "Impossible to debug async methods.\n");
return NULL;
}
return prop->get;
}
MonoMethod *method = (MonoMethod *)g_ptr_array_index (array, 0);
g_ptr_array_free (array, TRUE);
return method;
Expand All @@ -5460,7 +5469,9 @@ get_class_to_get_builder_field(StackFrame *frame)
MonoGenericContext context;
MonoType *inflated_type;

g_assert (this_obj);
if (!this_obj)
return NULL;

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 */
Expand All @@ -5484,7 +5495,8 @@ get_async_method_builder (StackFrame *frame)

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);
if (!builder_field)
return NULL;

this_addr = get_this_addr (frame);
if (!this_addr)
Expand Down Expand Up @@ -5524,7 +5536,8 @@ get_this_async_id (StackFrame *frame)
return 0;

builder_field = mono_class_get_field_from_name (get_class_to_get_builder_field(frame), "<>t__builder");
g_assert (builder_field);
if (!builder_field)
return 0;

tls = (DebuggerTlsData *)mono_native_tls_get_value (debugger_tls_id);
if (tls) {
Expand All @@ -5533,6 +5546,11 @@ get_this_async_id (StackFrame *frame)
}

method = get_object_id_for_debugger_method (mono_class_from_mono_type (mono_field_get_type (builder_field)));
if (!method) {
if (tls)
tls->disable_breakpoints = old_disable_breakpoints;
return 0;
}
obj = mono_runtime_try_invoke (method, builder, NULL, &ex, &error);
mono_error_assert_ok (&error);

Expand All @@ -5548,9 +5566,11 @@ static gboolean
set_set_notification_for_wait_completion_flag (StackFrame *frame)
{
MonoClassField *builder_field = mono_class_get_field_from_name (get_class_to_get_builder_field(frame), "<>t__builder");
g_assert (builder_field);
if (!builder_field)
return FALSE;
gpointer builder = get_async_method_builder (frame);
g_assert (builder);
if (!builder)
return FALSE;

void* args [1];
gboolean arg = TRUE;
Expand Down Expand Up @@ -6042,7 +6062,6 @@ process_single_step_inner (DebuggerTlsData *tls, gboolean from_signal)
return;
}


/*
* The ip points to the instruction causing the single step event, which is before
* the offset recorded in the seq point map, so find the next seq point after ip.
Expand Down Expand Up @@ -6814,7 +6833,10 @@ ss_create (MonoInternalThread *thread, StepSize size, StepDepth depth, StepFilte
* We are stopped at a throw site. Stepping should go to the catch site.
*/
frame = tls->catch_frame;
g_assert (frame.type == FRAME_TYPE_MANAGED || frame.type == FRAME_TYPE_INTERP);
if (frame.type != FRAME_TYPE_MANAGED && frame.type != FRAME_TYPE_INTERP) {
DEBUG_PRINTF (1, "Current frame is not managed nor interpreter.\n");
return ERR_INVALID_ARGUMENT;
}

/*
* Find the seq point corresponding to the landing site ip, which is the first seq
Expand All @@ -6824,7 +6846,11 @@ ss_create (MonoInternalThread *thread, StepSize size, StepDepth depth, StepFilte
sp = (found_sp)? &local_sp : NULL;
if (!sp)
no_seq_points_found (frame.method, frame.native_offset);
g_assert (sp);

if (!sp) {
DEBUG_PRINTF (1, "Could not find next sequence point.\n");
return ERR_INVALID_ARGUMENT;
}

method = frame.method;

Expand Down Expand Up @@ -6870,7 +6896,10 @@ ss_create (MonoInternalThread *thread, StepSize size, StepDepth depth, StepFilte
sp = (found_sp)? &local_sp : NULL;
if (!sp)
no_seq_points_found (frame->method, frame->native_offset);
g_assert (sp);
if (!sp) {
DEBUG_PRINTF (1, "Could not find next sequence point.\n");
return ERR_INVALID_ARGUMENT;
}
method = frame->method;
}
}
Expand Down Expand Up @@ -10838,7 +10867,11 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g
if (mono_class_get_context (klass)) {
MonoError error;
result = mono_class_inflate_generic_method_full_checked (result, klass, mono_class_get_context (klass), &error);
g_assert (mono_error_ok (&error)); /* FIXME don't swallow the error */
if (!mono_error_ok (&error)) {
buffer_add_string (buf, mono_error_get_message (&error));
mono_error_cleanup (&error);
return ERR_INVALID_ARGUMENT;
}
}
}
}
Expand Down Expand Up @@ -10981,7 +11014,12 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g
char *s;

s = mono_string_to_utf8_checked ((MonoString *)val, &error);
mono_error_assert_ok (&error);
if (!mono_error_ok (&error)) {
buffer_add_string (buf, mono_error_get_message (&error));
mono_error_cleanup (&error);
g_free (s);
return ERR_INVALID_ARGUMENT;
}
buffer_add_byte (buf, TOKEN_TYPE_STRING);
buffer_add_string (buf, s);
g_free (s);
Expand Down Expand Up @@ -11049,7 +11087,11 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g
tmp_context.method_inst = ginst;

inflated = mono_class_inflate_generic_method_checked (method, &tmp_context, &error);
g_assert (mono_error_ok (&error)); /* FIXME don't swallow the error */
if (!mono_error_ok (&error)) {
buffer_add_string (buf, mono_error_get_message (&error));
mono_error_cleanup (&error);
return ERR_INVALID_ARGUMENT;
}
if (!mono_verifier_is_method_valid_generic_instantiation (inflated))
return ERR_INVALID_ARGUMENT;
buffer_add_methodid (buf, domain, inflated);
Expand Down Expand Up @@ -11518,7 +11560,10 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
set_interp_var (&frame->actual_method->klass->this_arg, addr, val_buf);
} else {
var = jit->this_var;
g_assert (var);
if (!var) {
buffer_add_string (buf, "Invalid this object");
return ERR_INVALID_ARGUMENT;
}

set_var (&frame->actual_method->klass->this_arg, var, &frame->ctx, frame->domain, val_buf, frame->reg_locations, &tls->restore_state.ctx);
}
Expand Down Expand Up @@ -11561,9 +11606,11 @@ array_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
index = decode_int (p, &p, end);
len = decode_int (p, &p, end);

g_assert (index >= 0 && len >= 0);
if (index < 0 || len < 0)
return ERR_INVALID_ARGUMENT;
// Reordered to avoid integer overflow
g_assert (!(index > arr->max_length - len));
if (index > arr->max_length - len)
return ERR_INVALID_ARGUMENT;

esize = mono_array_element_size (mono_object_get_class (&arr->obj));
for (i = index; i < index + len; ++i) {
Expand All @@ -11575,9 +11622,11 @@ array_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
index = decode_int (p, &p, end);
len = decode_int (p, &p, end);

g_assert (index >= 0 && len >= 0);
if (index < 0 || len < 0)
return ERR_INVALID_ARGUMENT;
// Reordered to avoid integer overflow
g_assert (!(index > arr->max_length - len));
if (index > arr->max_length - len)
return ERR_INVALID_ARGUMENT;

esize = mono_array_element_size (mono_object_get_class (&arr->obj));
for (i = index; i < index + len; ++i) {
Expand Down

0 comments on commit 4fc45ee

Please sign in to comment.