From 0d488117f0413dac7ec1aedada6194d26a4ddbaf Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Wed, 15 Jan 2025 12:19:49 +0200 Subject: [PATCH] [mono][interp] Fix execution of delegate invoke wrapper with interpreter (#111310) 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. --- src/mono/mono/cil/opcode.def | 2 ++ src/mono/mono/metadata/marshal-lightweight.c | 3 +- src/mono/mono/mini/interp/interp.c | 37 ++++++++++++++++++++ src/mono/mono/mini/interp/mintops.def | 1 + src/mono/mono/mini/interp/transform.c | 10 ++++++ src/mono/mono/mini/method-to-ir.c | 8 +++++ 6 files changed, 60 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/cil/opcode.def b/src/mono/mono/cil/opcode.def index 47bccb295e99b2..049c0500b66bd1 100644 --- a/src/mono/mono/cil/opcode.def +++ b/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) diff --git a/src/mono/mono/metadata/marshal-lightweight.c b/src/mono/mono/metadata/marshal-lightweight.c index 3c5b0de4c67bc2..b3499ca0b86ef9 100644 --- a/src/mono/mono/metadata/marshal-lightweight.c +++ b/src/mono/mono/metadata/marshal-lightweight.c @@ -2102,7 +2102,8 @@ emit_delegate_invoke_internal_ilgen (MonoMethodBuilder *mb, MonoMethodSignature else mono_mb_emit_ldarg (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); diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 5328d982af6f03..fd8a6560f19e87 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -3744,6 +3744,34 @@ max_d (double lhs, double rhs) static JiterpreterCallInfo jiterpreter_call_info = { 0 }; #endif +// 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 @@ -7566,6 +7594,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)); \ diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index 5352b65a4ae8df..4c4f85a30fcfc7 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -716,6 +716,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 diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index da1d0c31e086d8..66e9f4373f4f24 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -7604,6 +7604,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].local); 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].local, td->sp [1].local); + push_simple_type (td, STACK_TYPE_I); + interp_ins_set_dreg (td->last_ins, td->sp [-1].local); + break; + case CEE_MONO_CALLI_EXTRA_ARG: { int saved_local = td->sp [-1].local; /* Same as CEE_CALLI, except that we drop the extra arg required for llvm specific behaviour */ diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 0b1706dd3c61be..3faf125cd0af4b 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -11342,6 +11342,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;