[2020-02][debugger] Fix NOT_IMPLEMENTED while debugging. (#19450)

* Backporting this, and after this I will backport the bump protocol for this PR.

* Bump API snapshot submodule

Co-authored-by: monojenkins <jo.shields+jenkins@xamarin.com>

6 files changed, 58 insertions, 8 deletions

diff --git a/external/api-snapshot b/external/api-snapshot
-Subproject a9a0857d638ee3bf573db3429180243ec7cdd76
+Subproject 95b3c72db797c679e59731d7d59b0be467c94f2
diff --git a/mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/Connection.cs b/mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/Connection.cs
index 03724b7f7bc..70de4ad7bea 100644
--- a/mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/Connection.cs
+++ b/mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/Connection.cs
@@ -409,6 +409,9 @@ namespace Mono.Debugger.Soft
 		public ErrorCode ErrorCode {
 			get; set;
 		}
+		public string ErrorMessage {
+			get; set;
+		}
 	}
 
 	/*
@@ -792,10 +795,12 @@ namespace Mono.Debugger.Soft
 
 				// For reply packets
 				offset = 0;
-				ReadInt (); // length
+				var len = ReadInt (); // length
 				ReadInt (); // id
 				ReadByte (); // flags
 				ErrorCode = ReadShort ();
+				if (ErrorCode == (int)Mono.Debugger.Soft.ErrorCode.INVALID_ARGUMENT && len > offset)
+					ErrorMsg = ReadString ();
 			}
 
 			public CommandSet CommandSet {
@@ -810,6 +815,10 @@ namespace Mono.Debugger.Soft
 				get; set;
 			}
 
+			public string ErrorMsg {
+				get; internal set;
+			}
+
 			public int Offset {
 				get {
 					return offset;
@@ -1792,7 +1801,7 @@ namespace Mono.Debugger.Soft
 							LogPacket (packetId, encoded_packet, reply, command_set, command, watch);
 						if (r.ErrorCode != 0) {
 							if (ErrorHandler != null)
-								ErrorHandler (this, new ErrorHandlerEventArgs () { ErrorCode = (ErrorCode)r.ErrorCode });
+								ErrorHandler (this, new ErrorHandlerEventArgs () { ErrorCode = (ErrorCode)r.ErrorCode, ErrorMessage = r.ErrorMsg});
 							throw new NotImplementedException ("No error handler set.");
 						} else {
 							return r;
diff --git a/mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/VirtualMachine.cs b/mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/VirtualMachine.cs
index 12274864d4c..f85e807201f 100644
--- a/mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/VirtualMachine.cs
+++ b/mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/VirtualMachine.cs
@@ -371,7 +371,7 @@ namespace Mono.Debugger.Soft
 			case ErrorCode.NO_SEQ_POINT_AT_IL_OFFSET:
 				throw new ArgumentException ("Cannot set breakpoint on the specified IL offset.");
 			default:
-				throw new CommandException (args.ErrorCode);
+				throw new CommandException (args.ErrorCode, args.ErrorMessage);
 			}
 		}
 
@@ -887,13 +887,18 @@ namespace Mono.Debugger.Soft
 
 	public class CommandException : Exception {
 
-		internal CommandException (ErrorCode error_code) : base ("Debuggee returned error code " + error_code + ".") {
+		internal CommandException (ErrorCode error_code, string error_message) : base ("Debuggee returned error code " + error_code + (error_message == null || error_message.Length == 0 ? "." : " - " + error_message + ".")) {
 			ErrorCode = error_code;
+			ErrorMessage = error_message;
 		}
 
 		public ErrorCode ErrorCode {
 			get; set;
 		}
+
+		public string ErrorMessage {
+			get; internal set;
+		}
 	}
 
 	public class VMNotSuspendedException : InvalidOperationException
diff --git a/mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs b/mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
index e6b79fdd406..2f77ac510c6 100644
--- a/mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
+++ b/mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs
@@ -617,6 +617,7 @@ public class Tests : TestsBase, ITest2
 			ss_multi_thread ();
 			return 0;
 		}
+		test_invalid_argument_assembly_get_type ();
 		return 3;
 	}
 
@@ -645,6 +646,10 @@ public class Tests : TestsBase, ITest2
 		LocalReflectClass.RunMe ();
 	}
 
+	public static void test_invalid_argument_assembly_get_type () {
+
+	}
+
 	public static void breakpoints () {
 		/* Call these early so it is JITted by the time a breakpoint is placed on it */
 		bp3 ();
diff --git a/mcs/class/Mono.Debugger.Soft/Test/dtest.cs b/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
index d6ca7dc3255..a8f12094bbd 100644
--- a/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
+++ b/mcs/class/Mono.Debugger.Soft/Test/dtest.cs
@@ -5202,6 +5202,18 @@ public class DebuggerTests
 	}
 	
 	[Test]
+	public void InvalidArgumentAssemblyGetType () {
+		Event e = run_until ("test_invalid_argument_assembly_get_type");
+		var assembly = entry_point.DeclaringType.Assembly;
+		try {
+			var type = assembly.GetType ("System.Collections.Generic.Dictionary<double, float>.Main");
+		}
+		catch (CommandException ex) {
+			Assert.AreEqual(ex.ErrorMessage, "Unexpected assembly-qualified type \"System.Collections.Generic.Dictionary<double, float>.Main\" was provided");
+		}
+	}
+	
+	[Test]
 	public void CheckSuspendPolicySentWhenLaunchSuspendYes () {
 		vm.Exit (0);
 		var port = GetFreePort ();
diff --git a/mono/mini/debugger-agent.c b/mono/mini/debugger-agent.c
index 96c1558b2e4..21e823e1790 100644
--- a/mono/mini/debugger-agent.c
+++ b/mono/mini/debugger-agent.c
@@ -7269,6 +7269,7 @@ vm_commands (int command, int id, guint8 *p, guint8 *end, Buffer *buf)
 		ignore_case = decode_byte (p, &p, end);
 
 		if (!mono_reflection_parse_type_checked (name, &info, error)) {
+			buffer_add_string (buf, mono_error_get_message (error));
 			mono_error_cleanup (error);
 			g_free (name);
 			mono_reflection_free_type_info (&info);
@@ -7757,6 +7758,8 @@ assembly_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 	case CMD_ASSEMBLY_GET_TYPE: {
 		ERROR_DECL (error);
 		char *s = decode_string (p, &p, end);
+		char* original_s = g_strdup_printf ("\"%s\"", s);
+
 		gboolean ignorecase = decode_byte (p, &p, end);
 		MonoTypeNameParse info;
 		MonoType *t;
@@ -7772,20 +7775,33 @@ assembly_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 			mono_error_cleanup (error);
 			t = NULL;
 		} else {
-			if (info.assembly.name)
-				NOT_IMPLEMENTED;
+			if (info.assembly.name) {
+				mono_reflection_free_type_info (&info);
+				g_free (s);
+				mono_domain_set_fast (d, TRUE);
+				char* error_msg =  g_strdup_printf ("Unexpected assembly-qualified type %s was provided", original_s);
+				buffer_add_string (buf, error_msg);
+				g_free (error_msg);
+				g_free (original_s);
+				return ERR_INVALID_ARGUMENT;
+			}
 			t = mono_reflection_get_type_checked (alc, ass->image, ass->image, &info, ignorecase, TRUE, &type_resolve, error);
 			if (!is_ok (error)) {
 				mono_error_cleanup (error); /* FIXME don't swallow the error */
 				mono_reflection_free_type_info (&info);
 				g_free (s);
+				mono_domain_set_fast (d, TRUE);
+				char* error_msg =  g_strdup_printf ("Invalid type name %s", original_s);
+				buffer_add_string (buf, error_msg);
+				g_free (error_msg);
+				g_free (original_s);
 				return ERR_INVALID_ARGUMENT;
 			}
 		}
 		buffer_add_typeid (buf, domain, t ? mono_class_from_mono_type_internal (t) : NULL);
 		mono_reflection_free_type_info (&info);
 		g_free (s);
-
+		g_free (original_s);
 		mono_domain_set_fast (d, TRUE);
 
 		break;
@@ -7842,6 +7858,7 @@ assembly_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
         error_init (error);
         MonoClass* mono_class = mono_class_get_checked (ass->image, token, error);
         if (!is_ok (error)) {
+            buffer_add_string (buf, mono_error_get_message (error));
             mono_error_cleanup (error);
             return ERR_INVALID_ARGUMENT;
         }
@@ -7858,6 +7875,7 @@ assembly_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
         error_init (error);
         MonoMethod* mono_method = mono_get_method_checked (ass->image, token, NULL, NULL, error);
         if (!is_ok (error)) {
+            buffer_add_string (buf, mono_error_get_message (error));
             mono_error_cleanup (error);
             return ERR_INVALID_ARGUMENT;
         }
@@ -8710,6 +8728,7 @@ method_commands_internal (int command, MonoMethod *method, MonoDomain *domain, g
 
 		header = mono_method_get_header_checked (method, error);
 		if (!header) {
+			buffer_add_string (buf, mono_error_get_message (error));
 			mono_error_cleanup (error); /* FIXME don't swallow the error */
 			return ERR_INVALID_ARGUMENT;
 		}
@@ -9541,7 +9560,7 @@ string_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 			if (!is_ok (error)) {
 				if (s)
 					g_free (s);
-
+				buffer_add_string (buf, mono_error_get_message (error));
 				return ERR_INVALID_ARGUMENT;
 			}
 			buffer_add_string (buf, s);