[debugger] Removing some asserts (#36234)

Removing some asserts and returning err_invalid_argument with an error message when it's possible.

Fixes mono/mono#19651

Co-authored-by: thaystg <[email protected]>

Author: monojenkins

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) {