[mono][interp] Fix execution of delegate invoke wrapper with interpreter (#111700)

The wrapper was relatively recently changed to icall into mono_get_addr_compiled_method in order to obtain a native function pointer to call using calli. This is incorrect on interpreter where we expect an `InterpMethod*`. This commit adds a new opcode instead, that on jit it goes through the same icall path, while on interpeter in similarly computes the appropiate method to call.

On a separate track, it might be useful to investigate whether the necessary delegate invoke wrapper should have been present in the aot image and not be executed with the interpreter in the first place.

Co-authored-by: Vlad Brezae <[email protected]>
Co-authored-by: Steve Pfister <[email protected]>

Author: 3 people

src/mono/mono/cil/opcode.def

@@ -328,6 +328,8 @@ OPDEF(CEE_MONO_GET_SP, "mono_get_sp", Pop0, PushI, InlineNone, 0, 2, 0xF0, 0x20,
 OPDEF(CEE_MONO_METHODCONST, "mono_methodconst", Pop0, PushI, InlineI, 0, 2, 0xF0, 0x21, NEXT)
 OPDEF(CEE_MONO_PINVOKE_ADDR_CACHE, "mono_pinvoke_addr_cache", Pop0, PushI, InlineI, 0, 2, 0xF0, 0x22, NEXT)
 OPDEF(CEE_MONO_REMAP_OVF_EXC, "mono_remap_ovf_exc", Pop0, Push0, InlineI, 0, 2, 0xF0, 0x23, NEXT)
+OPDEF(CEE_MONO_LDVIRTFTN_DELEGATE, "mono_ldvirtftn_delegate", PopI+PopI, PushI, InlineNone, 0, 2, 0xF0, 0x24, NEXT)
+
 #ifndef OPALIAS
 #define _MONO_CIL_OPALIAS_DEFINED_
 #define OPALIAS(a,s,r)

src/mono/mono/metadata/marshal-lightweight.c

@@ -2052,7 +2052,8 @@ emit_delegate_invoke_internal_ilgen (MonoMethodBuilder *mb, MonoMethodSignature
 			}
 			mono_mb_emit_ldarg_addr (mb, 1);
 			mono_mb_emit_ldarg (mb, 0);
-			mono_mb_emit_icall (mb, mono_get_addr_compiled_method);
+			mono_mb_emit_byte (mb, MONO_CUSTOM_PREFIX);
+			mono_mb_emit_byte (mb, CEE_MONO_LDVIRTFTN_DELEGATE);
 			mono_mb_emit_op (mb, CEE_CALLI, target_method_sig);
 		} else {
 			mono_mb_emit_byte (mb, CEE_LDNULL);

src/mono/mono/mini/interp/interp.c

@@ -3862,6 +3862,34 @@ max_d (double lhs, double rhs)
 		return fmax (lhs, rhs);
 }
 
+// Equivalent of mono_get_addr_compiled_method
+static gpointer
+interp_ldvirtftn_delegate (gpointer arg, MonoDelegate *del)
+{
+	MonoMethod *virtual_method = del->method;
+	ERROR_DECL(error);
+
+	MonoClass *klass = del->object.vtable->klass;
+	MonoMethod *invoke = mono_get_delegate_invoke_internal (klass);
+	MonoMethodSignature *invoke_sig = mono_method_signature_internal (invoke);
+
+	MonoClass *arg_class = NULL;
+	if (m_type_is_byref (invoke_sig->params [0])) {
+		arg_class = mono_class_from_mono_type_internal (invoke_sig->params [0]);
+	} else {
+		MonoObject *object = (MonoObject*)arg;
+		arg_class = object->vtable->klass;
+	}
+
+	MonoMethod *res = mono_class_get_virtual_method (arg_class, virtual_method, error);
+	mono_error_assert_ok (error);
+
+	gboolean need_unbox = m_class_is_valuetype (res->klass) && !m_class_is_valuetype (virtual_method->klass);
+
+	InterpMethod *imethod = mono_interp_get_imethod (res);
+	return imethod_to_ftnptr (imethod, need_unbox);
+}
+
 /*
  * If CLAUSE_ARGS is non-null, start executing from it.
  * The ERROR argument is used to avoid declaring an error object for every interp frame, its not used
@@ -7790,6 +7818,15 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 			ip += 3;
 			MINT_IN_BREAK;
 		}
+		MINT_IN_CASE(MINT_LDVIRTFTN_DELEGATE) {
+			gpointer arg = LOCAL_VAR (ip [2], gpointer);
+			MonoDelegate *del = LOCAL_VAR (ip [3], MonoDelegate*);
+			NULL_CHECK (arg);
+
+			LOCAL_VAR (ip [1], gpointer) = interp_ldvirtftn_delegate (arg, del);
+			ip += 4;
+			MINT_IN_BREAK;
+		}
 
 #define MATH_UNOP(mathfunc) \
 	LOCAL_VAR (ip [1], double) = mathfunc (LOCAL_VAR (ip [2], double)); \

src/mono/mono/mini/interp/mintops.def

@@ -728,6 +728,7 @@ OPDEF(MINT_SDB_INTR_LOC, "sdb_intr_loc", 1, 0, 0, MintOpNoArgs)
 OPDEF(MINT_SDB_SEQ_POINT, "sdb_seq_point", 1, 0, 0, MintOpNoArgs)
 OPDEF(MINT_SDB_BREAKPOINT, "sdb_breakpoint", 1, 0, 0, MintOpNoArgs)
 OPDEF(MINT_LD_DELEGATE_METHOD_PTR, "ld_delegate_method_ptr", 3, 1, 1, MintOpNoArgs)
+OPDEF(MINT_LDVIRTFTN_DELEGATE, "ldvirtftn_delegate", 4, 1, 2, MintOpNoArgs)
 
 // Math intrinsics
 // double

src/mono/mono/mini/interp/transform.c

@@ -8011,6 +8011,16 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
 					push_simple_type (td, STACK_TYPE_I);
 					interp_ins_set_dreg (td->last_ins, td->sp [-1].var);
 					break;
+				case CEE_MONO_LDVIRTFTN_DELEGATE:
+					CHECK_STACK (td, 2);
+					td->sp -= 2;
+					td->ip += 1;
+					interp_add_ins (td, MINT_LDVIRTFTN_DELEGATE);
+					interp_ins_set_sregs2 (td->last_ins, td->sp [0].var, td->sp [1].var);
+					push_simple_type (td, STACK_TYPE_I);
+					interp_ins_set_dreg (td->last_ins, td->sp [-1].var);
+					break;
+
 				case CEE_MONO_CALLI_EXTRA_ARG: {
 					int saved_local = td->sp [-1].var;
 					/* Same as CEE_CALLI, except that we drop the extra arg required for llvm specific behaviour */

src/mono/mono/mini/method-to-ir.c

@@ -11597,6 +11597,14 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
 			*sp++ = ins;
 			break;
 		}
+		case MONO_CEE_MONO_LDVIRTFTN_DELEGATE: {
+			CHECK_STACK (2);
+			sp -= 2;
+
+			ins = mono_emit_jit_icall (cfg, mono_get_addr_compiled_method, sp);
+			*sp++ = ins;
+			break;
+		}
 		case MONO_CEE_MONO_CALLI_EXTRA_ARG: {
 			MonoInst *addr;
 			MonoInst *arg;