Skip to content

Commit

Permalink
[mono][interp] Fix execution of delegate invoke wrapper with interpre…
Browse files Browse the repository at this point in the history
…ter (#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.
  • Loading branch information
BrzVlad authored Jan 15, 2025
1 parent 44ad4db commit 5a395ed
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 1 deletion.
2 changes: 2 additions & 0 deletions src/mono/mono/cil/opcode.def
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion src/mono/mono/metadata/marshal-lightweight.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
37 changes: 37 additions & 0 deletions src/mono/mono/mini/interp/interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -3863,6 +3863,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
Expand Down Expand Up @@ -7794,6 +7822,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)); \
Expand Down
1 change: 1 addition & 0 deletions src/mono/mono/mini/interp/mintops.def
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,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
Expand Down
10 changes: 10 additions & 0 deletions src/mono/mono/mini/interp/transform.c
Original file line number Diff line number Diff line change
Expand Up @@ -8056,6 +8056,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 */
Expand Down
8 changes: 8 additions & 0 deletions src/mono/mono/mini/method-to-ir.c
Original file line number Diff line number Diff line change
Expand Up @@ -11594,6 +11594,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;
Expand Down

0 comments on commit 5a395ed

Please sign in to comment.