Merge pull request #1037 from Unity-Technologies/unity-master-debugger-ref-returns

Handle ref returns in runtime invoke wrappers (case 1076527)

Author: joncham

mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs

@@ -350,6 +350,7 @@ public static int Main (String[] args) {
 		set_ip ();
 		step_filters ();
 		pointers ();
+		ref_return ();
 		if (args.Length > 0 && args [0] == "local-reflect")
 			local_reflect ();
 		if (args.Length > 0 && args [0] == "domain-test")
@@ -1739,6 +1740,21 @@ public static unsafe void pointers () {
 		fixed (int* pa = a)
 			pointer_arguments (pa, &s);
 	}
+
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static void ref_return () {
+
+	}
+
+	static int ret_val = 1;
+	public static ref int get_ref_int() {
+		return ref ret_val;
+	}
+
+	static string ref_return_string = "byref";
+	public static ref string get_ref_string() {
+		return ref ref_return_string;
+	}
 }
 
 public class SentinelClass : MarshalByRefObject {

mcs/class/Mono.Debugger.Soft/Test/dtest.cs

@@ -4398,15 +4398,31 @@ public void Pointer_GetValue () {
 	}
 
 	[Test]
-    public void InvokeGenericMethod () {
-        Event e = run_until ("bp1");
-        StackFrame frame = e.Thread.GetFrames()[0];
-        TypeMirror t = frame.Method.DeclaringType;
-        MethodMirror m;
-        m = t.GetMethod ("generic_method");
-        AssertThrows<ArgumentException> (delegate {
-        	t.InvokeMethod (e.Thread, m, null);
-        	});      
-    }
+	public void InvokeGenericMethod () {
+		Event e = run_until ("bp1");
+		StackFrame frame = e.Thread.GetFrames()[0];
+		TypeMirror t = frame.Method.DeclaringType;
+		MethodMirror m;
+		m = t.GetMethod ("generic_method");
+		AssertThrows<ArgumentException> (delegate {
+				t.InvokeMethod (e.Thread, m, null);
+			});
+	}
+
+	[Test]
+	public void InvokeRefReturnMethod () {
+		Event e = run_until ("ref_return");
+		StackFrame frame = e.Thread.GetFrames()[0];
+		TypeMirror t = frame.Method.DeclaringType;
+		MethodMirror m;
+
+		m = t.GetMethod ("get_ref_int");
+		var v = t.InvokeMethod (e.Thread, m, null);
+		AssertValue (1, v);
+
+		m = t.GetMethod ("get_ref_string");
+		v = t.InvokeMethod (e.Thread, m, null);
+		AssertValue ("byref", v);
+	}
 } // class DebuggerTests
 } // namespace

mono/metadata/marshal.c

@@ -3936,6 +3936,11 @@ get_runtime_invoke_type (MonoType *t, gboolean ret)
 	if (t->byref) {
 		if (t->type == MONO_TYPE_GENERICINST && mono_class_is_nullable (mono_class_from_mono_type (t)))
 			return t;
+
+		/* The result needs loaded indirectly */
+		if (ret)
+			return t;
+
 		/* Can't share this with 'I' as that needs another indirection */
 		return &mono_defaults.int_class->this_arg;
 	}
@@ -4143,8 +4148,10 @@ emit_invoke_call (MonoMethodBuilder *mb, MonoMethod *method,
 	}
 
 	if (sig->ret->byref) {
-		/* fixme: */
-		g_assert_not_reached ();
+		/* perform indirect load and return by value */
+		MonoType* ret_byval = &mono_class_from_mono_type (sig->ret)->byval_arg;
+		g_assert (!ret_byval->byref);
+		mono_mb_emit_byte (mb, mono_type_to_ldind (ret_byval));
 	}
 
 	switch (sig->ret->type) {

mono/mini/debugger-agent.c

@@ -8366,7 +8366,12 @@ do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, guint8
 				buffer_add_value (buf, &VM_DEFAULTS_VOID_CLASS->byval_arg, NULL, domain);
 			}
 		} else if (MONO_TYPE_IS_REFERENCE (sig->ret)) {
-			buffer_add_value (buf, sig->ret, &res, domain);
+			if (sig->ret->byref) {
+				MonoType* ret_byval = &mono_class_from_mono_type (sig->ret)->byval_arg;
+				buffer_add_value (buf, ret_byval, &res, domain);
+			} else {
+				buffer_add_value (buf, sig->ret, &res, domain);
+			}
 		} else if (mono_class_from_mono_type (sig->ret)->valuetype || sig->ret->type == MONO_TYPE_PTR || sig->ret->type == MONO_TYPE_FNPTR) {
 			if (mono_class_is_nullable (mono_class_from_mono_type (sig->ret))) {
 				MonoClass *k = mono_class_from_mono_type (sig->ret);
@@ -8377,7 +8382,13 @@ do_invoke_method (DebuggerTlsData *tls, Buffer *buf, InvokeData *invoke, guint8
 				buffer_add_value (buf, sig->ret, nullable_buf, domain);
 			} else {
 				g_assert (res);
-				buffer_add_value (buf, sig->ret, mono_object_unbox (res), domain);
+
+				if (sig->ret->byref) {
+					MonoType* ret_byval = &mono_class_from_mono_type (sig->ret)->byval_arg;
+					buffer_add_value (buf, ret_byval, mono_object_unbox (res), domain);
+				} else {
+					buffer_add_value (buf, sig->ret, mono_object_unbox (res), domain);
+				}
 			}
 		} else {
 			NOT_IMPLEMENTED;