From 9c19ac664747b78adc4a9a1f01519f75dff978bc Mon Sep 17 00:00:00 2001 From: thaystg Date: Wed, 13 May 2020 14:44:19 +0000 Subject: [PATCH] [debugger] Removing some asserts Removing some asserts and returning err_invalid_argument with an error message when it's possible. Fixes https://github.com/mono/mono/issues/19651 --- src/mono/mono/mini/debugger-agent.c | 83 +++++++++++++++++++++++------ 1 file changed, 66 insertions(+), 17 deletions(-) diff --git a/src/mono/mono/mini/debugger-agent.c b/src/mono/mono/mini/debugger-agent.c index 9e619923281895..f76e887e6db885 100644 --- a/src/mono/mono/mini/debugger-agent.c +++ b/src/mono/mono/mini/debugger-agent.c @@ -4589,7 +4589,16 @@ get_object_id_for_debugger_method (MonoClass* async_builder_class) ERROR_DECL (error); GPtrArray *array = mono_class_get_methods_by_name (async_builder_class, "get_ObjectIdForDebugger", 0x24, 1, 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_internal (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; @@ -4607,7 +4616,9 @@ get_class_to_get_builder_field(DbgEngineStackFrame *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 (m_class_get_byval_arg (original_class), &context, error); mono_error_assert_ok (error); /* FIXME don't swallow the error */ @@ -4632,7 +4643,8 @@ get_async_method_builder (DbgEngineStackFrame *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) @@ -4671,7 +4683,8 @@ get_this_async_id (DbgEngineStackFrame *frame) return 0; builder_field = mono_class_get_field_from_name_full (get_class_to_get_builder_field(frame), "<>t__builder", NULL); - g_assert (builder_field); + if (!builder_field) + return 0; tls = (DebuggerTlsData *)mono_native_tls_get_value (debugger_tls_id); if (tls) { @@ -4680,6 +4693,11 @@ get_this_async_id (DbgEngineStackFrame *frame) } method = get_object_id_for_debugger_method (mono_class_from_mono_type_internal (builder_field->type)); + 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); @@ -4695,9 +4713,11 @@ static gboolean set_set_notification_for_wait_completion_flag (DbgEngineStackFrame *frame) { MonoClassField *builder_field = mono_class_get_field_from_name_full (get_class_to_get_builder_field(frame), "<>t__builder", NULL); - g_assert (builder_field); + if (!builder_field) + return FALSE; gpointer builder = get_async_method_builder (frame); - g_assert (builder); + if (!builder) + return FALSE; MonoMethod* method = get_set_notification_method (mono_class_from_mono_type_internal (builder_field->type)); if (method == NULL) @@ -5071,7 +5091,10 @@ ss_create_init_args (SingleStepReq *ss_req, SingleStepArgs *args) * 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 @@ -5080,7 +5103,10 @@ ss_create_init_args (SingleStepReq *ss_req, SingleStepArgs *args) found_sp = mono_find_next_seq_point_for_native_offset (frame.domain, frame.method, frame.native_offset, &info, &args->sp); if (!found_sp) no_seq_points_found (frame.method, frame.native_offset); - g_assert (found_sp); + if (!found_sp) { + DEBUG_PRINTF (1, "Could not find next sequence point.\n"); + return ERR_INVALID_ARGUMENT; + } method = frame.method; @@ -5125,7 +5151,10 @@ ss_create_init_args (SingleStepReq *ss_req, SingleStepArgs *args) found_sp = mono_find_prev_seq_point_for_native_offset (frame->de.domain, frame->de.method, frame->de.native_offset, &info, &args->sp); if (!found_sp) no_seq_points_found (frame->de.method, frame->de.native_offset); - g_assert (found_sp); + if (!found_sp) { + DEBUG_PRINTF (1, "Could not find next sequence point.\n"); + return ERR_INVALID_ARGUMENT; + } method = frame->de.method; } } @@ -8861,7 +8890,11 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g if (mono_class_get_context (klass)) { ERROR_DECL (error); result = mono_class_inflate_generic_method_full_checked (result, klass, mono_class_get_context (klass), error); - g_assert (is_ok (error)); /* FIXME don't swallow the error */ + if (!is_ok (error)) { + add_error_string (buf, mono_error_get_message (error)); + mono_error_cleanup (error); + return ERR_INVALID_ARGUMENT; + } } } } @@ -8999,7 +9032,12 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g char *s; s = mono_string_to_utf8_checked_internal ((MonoString *)val, error); - mono_error_assert_ok (error); + if (!is_ok (error)) { + add_error_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); @@ -9062,7 +9100,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 (is_ok (error)); /* FIXME don't swallow the error */ + if (!is_ok (error)) { + add_error_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); @@ -9489,7 +9531,10 @@ frame_commands (int command, guint8 *p, guint8 *end, Buffer *buf) set_interp_var (m_class_get_this_arg (frame->actual_method->klass), addr, val_buf); } else { var = jit->this_var; - g_assert (var); + if (!var) { + add_error_string (buf, "Invalid this object"); + return ERR_INVALID_ARGUMENT; + } set_var (m_class_get_this_arg (frame->actual_method->klass), var, &frame->ctx, frame->de.domain, val_buf, frame->reg_locations, &tls->restore_state.ctx); } @@ -9532,9 +9577,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 (arr->obj.vtable->klass); for (i = index; i < index + len; ++i) { @@ -9546,9 +9593,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 (arr->obj.vtable->klass); for (i = index; i < index + len; ++i) {