From da1f8ebc45515402dcab14edce2167192aaa32ad Mon Sep 17 00:00:00 2001 From: Brezae Vlad Date: Wed, 29 Sep 2021 12:40:42 +0300 Subject: [PATCH] [interp] Fix unboxing during CALLI On interp we were storing function pointers as a simple InterpMethod. The problem is that, when loading an InterpMethod with a ldvirtftn opcode, the resulting method might need to have its this pointer unboxed, so this information needs to be embedded into the function pointer. We achieve this by passing function pointers in the interpreter via an InterpFtnDesc, which contains the need_unbox information. Each InterpMethod caches the function descriptors for both unboxing scenarios. In order to interop with llvmonly we must use a MonoFtnDesc as a function pointer so this must point to point to an InterpFtnDesc in order to resolve normal interp calls. --- src/mono/mono/mini/interp/interp-internals.h | 12 ++- src/mono/mono/mini/interp/interp.c | 95 +++++++++++++------- src/mono/mono/mini/interp/mintops.def | 2 +- src/mono/mono/mini/interp/transform.c | 1 - src/mono/mono/mini/mini.h | 4 +- 5 files changed, 75 insertions(+), 39 deletions(-) diff --git a/src/mono/mono/mini/interp/interp-internals.h b/src/mono/mono/mini/interp/interp-internals.h index 465fd20240fa4..ab8c17e25d502 100644 --- a/src/mono/mono/mini/interp/interp-internals.h +++ b/src/mono/mono/mini/interp/interp-internals.h @@ -88,7 +88,6 @@ typedef struct InterpFrame InterpFrame; typedef void (*MonoFuncV) (void); typedef void (*MonoPIFunc) (void *callme, void *margs); - typedef enum { IMETHOD_CODE_INTERP, IMETHOD_CODE_COMPILED, @@ -97,6 +96,7 @@ typedef enum { #define PROFILE_INTERP 0 +typedef struct InterpFtnDesc InterpFtnDesc; /* * Structure representing a method transformed for the interpreter */ @@ -124,7 +124,8 @@ struct InterpMethod { MonoType *rtype; MonoType **param_types; MonoJitInfo *jinfo; - MonoFtnDesc *ftndesc; + InterpFtnDesc *ftndesc; + InterpFtnDesc *ftndesc_unbox; guint32 locals_size; guint32 alloca_size; @@ -146,6 +147,13 @@ struct InterpMethod { #endif }; +struct InterpFtnDesc { + InterpMethod *imethod; + gboolean need_unbox; + + MonoFtnDesc *desc; +}; + /* Used for localloc memory allocation */ typedef struct _FrameDataFragment FrameDataFragment; struct _FrameDataFragment { diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index c2c423b5018a9..ac254a40ec045 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -508,6 +508,38 @@ mono_interp_get_imethod (MonoMethod *method, MonoError *error) return imethod; } +static InterpFtnDesc* +interp_get_ftndesc (InterpMethod *imethod, gboolean need_unbox) +{ + InterpFtnDesc **ftndesc_p; + InterpFtnDesc *ftndesc; + if (need_unbox) + ftndesc_p = &imethod->ftndesc_unbox; + else + ftndesc_p = &imethod->ftndesc; + + ftndesc = *ftndesc_p; + if (ftndesc) + return ftndesc; + + MonoJitMemoryManager *jit_mm = jit_mm_for_method (imethod->method); + jit_mm_lock (jit_mm); + ftndesc = *ftndesc_p; + if (ftndesc) { + jit_mm_unlock (jit_mm); + return ftndesc; + } + + ftndesc = (InterpFtnDesc*)m_method_alloc0 (imethod->method, sizeof (InterpFtnDesc)); + ftndesc->imethod = imethod; + ftndesc->need_unbox = need_unbox; + *ftndesc_p = ftndesc; + jit_mm_unlock (jit_mm); + + return ftndesc; +} + + #if defined (MONO_CROSS_COMPILE) || defined (HOST_WASM) #define INTERP_PUSH_LMF_WITH_CTX_BODY(ext, exit_label) \ (ext).kind = MONO_LMFEXT_INTERP_EXIT; @@ -1678,10 +1710,10 @@ interp_init_delegate (MonoDelegate *del, MonoError *error) } /* Convert a function pointer for a managed method to an InterpMethod* */ -static InterpMethod* -ftnptr_to_imethod (gpointer addr) +static InterpFtnDesc* +ftnptr_to_interp_ftndesc (gpointer addr) { - InterpMethod *imethod; + InterpFtnDesc *interp_ftndesc; if (mono_llvm_only) { ERROR_DECL (error); @@ -1690,36 +1722,37 @@ ftnptr_to_imethod (gpointer addr) g_assert (ftndesc); g_assert (ftndesc->method); - imethod = ftndesc->interp_method; - if (!imethod) { - imethod = mono_interp_get_imethod (ftndesc->method, error); + interp_ftndesc = ftndesc->interp_ftndesc; + if (!interp_ftndesc) { + InterpMethod *imethod = mono_interp_get_imethod (ftndesc->method, error); + interp_ftndesc = interp_get_ftndesc (imethod, FALSE); mono_error_assert_ok (error); mono_memory_barrier (); - ftndesc->interp_method = imethod; + ftndesc->interp_ftndesc = interp_ftndesc; } } else { - /* Function pointers are represented by their InterpMethod */ - imethod = (InterpMethod*)addr; + /* Function pointers are represented by a InterpFtnDesc structure */ + interp_ftndesc = (InterpFtnDesc*)addr; } - return imethod; + return interp_ftndesc; } static gpointer -imethod_to_ftnptr (InterpMethod *imethod) +interp_ftndesc_to_ftnptr (InterpFtnDesc *interp_ftndesc) { if (mono_llvm_only) { ERROR_DECL (error); /* Function pointers are represented by a MonoFtnDesc structure */ - MonoFtnDesc *ftndesc = imethod->ftndesc; + MonoFtnDesc *ftndesc = interp_ftndesc->desc; if (!ftndesc) { - ftndesc = mini_llvmonly_load_method_ftndesc (imethod->method, FALSE, FALSE, error); + ftndesc = mini_llvmonly_load_method_ftndesc (interp_ftndesc->imethod->method, FALSE, FALSE, error); mono_error_assert_ok (error); mono_memory_barrier (); - imethod->ftndesc = ftndesc; + interp_ftndesc->desc = ftndesc; } return ftndesc; } else { - return imethod; + return interp_ftndesc; } } @@ -1727,7 +1760,7 @@ static void interp_delegate_ctor (MonoObjectHandle this_obj, MonoObjectHandle target, gpointer addr, MonoError *error) { /* addr is the result of an LDFTN opcode */ - InterpMethod *imethod = ftnptr_to_imethod (addr); + InterpMethod *imethod = ftnptr_to_interp_ftndesc (addr)->imethod; if (!(imethod->method->flags & METHOD_ATTRIBUTE_STATIC)) { MonoMethod *invoke = mono_get_delegate_invoke_internal (mono_handle_class (this_obj)); @@ -3484,12 +3517,11 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs goto call; } MINT_IN_CASE(MINT_CALLI) { - MonoMethodSignature *csignature; - - csignature = (MonoMethodSignature*)frame->imethod->data_items [ip [4]]; + InterpFtnDesc *ftndesc; /* In mixed mode, stay in the interpreter for simplicity even if there is an AOT version of the callee */ - cmethod = ftnptr_to_imethod (LOCAL_VAR (ip [2], gpointer)); + ftndesc = ftnptr_to_interp_ftndesc (LOCAL_VAR (ip [2], gpointer)); + cmethod = ftndesc->imethod; if (cmethod->method->flags & METHOD_ATTRIBUTE_PINVOKE_IMPL) { cmethod = mono_interp_get_imethod (mono_marshal_get_native_wrapper (cmethod->method, FALSE, FALSE), error); @@ -3499,15 +3531,11 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs return_offset = ip [1]; call_args_offset = ip [3]; - if (csignature->hasthis) { + if (ftndesc->need_unbox) { MonoObject *this_arg = LOCAL_VAR (call_args_offset, MonoObject*); - - if (m_class_is_valuetype (this_arg->vtable->klass)) { - gpointer unboxed = mono_object_unbox_internal (this_arg); - LOCAL_VAR (call_args_offset, gpointer) = unboxed; - } + LOCAL_VAR (call_args_offset, gpointer) = mono_object_unbox_internal (this_arg); } - ip += 5; + ip += 4; goto call; } @@ -6508,17 +6536,18 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; MINT_IN_CASE(MINT_LDFTN) { InterpMethod *m = (InterpMethod*)frame->imethod->data_items [ip [2]]; - LOCAL_VAR (ip [1], gpointer) = imethod_to_ftnptr (m); + LOCAL_VAR (ip [1], gpointer) = interp_ftndesc_to_ftnptr (interp_get_ftndesc (m, FALSE)); ip += 3; MINT_IN_BREAK; } MINT_IN_CASE(MINT_LDVIRTFTN) { - InterpMethod *m = (InterpMethod*)frame->imethod->data_items [ip [3]]; + InterpMethod *virtual_method = (InterpMethod*)frame->imethod->data_items [ip [3]]; MonoObject *o = LOCAL_VAR (ip [2], MonoObject*); NULL_CHECK (o); - m = get_virtual_method (m, o->vtable); - LOCAL_VAR (ip [1], gpointer) = imethod_to_ftnptr (m); + InterpMethod *res_method = get_virtual_method (virtual_method, o->vtable); + gboolean need_unbox = m_class_is_valuetype (res_method->method->klass) && !m_class_is_valuetype (virtual_method->method->klass); + LOCAL_VAR (ip [1], gpointer) = interp_ftndesc_to_ftnptr (interp_get_ftndesc (res_method, need_unbox)); ip += 4; MINT_IN_BREAK; } @@ -6529,7 +6558,7 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; InterpMethod *m = mono_interp_get_imethod (cmethod, error); mono_error_assert_ok (error); - LOCAL_VAR (ip [1], gpointer) = imethod_to_ftnptr (m); + LOCAL_VAR (ip [1], gpointer) = interp_ftndesc_to_ftnptr (interp_get_ftndesc (m, FALSE)); ip += 3; MINT_IN_BREAK; } @@ -6711,7 +6740,7 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK; mono_error_assert_ok (error); } g_assert (del->interp_method); - LOCAL_VAR (ip [1], gpointer) = imethod_to_ftnptr (del->interp_method); + LOCAL_VAR (ip [1], gpointer) = interp_ftndesc_to_ftnptr (interp_get_ftndesc (del->interp_method, FALSE)); ip += 3; MINT_IN_BREAK; } diff --git a/src/mono/mono/mini/interp/mintops.def b/src/mono/mono/mini/interp/mintops.def index 9ef942e9d1b25..a274ff2c1b4ad 100644 --- a/src/mono/mono/mini/interp/mintops.def +++ b/src/mono/mono/mini/interp/mintops.def @@ -664,7 +664,7 @@ OPDEF(MINT_CALL, "call", 4, 1, 1, MintOpMethodToken) OPDEF(MINT_CALLVIRT, "callvirt", 4, 1, 1, MintOpMethodToken) OPDEF(MINT_CALLVIRT_FAST, "callvirt.fast", 5, 1, 1, MintOpMethodToken) OPDEF(MINT_CALL_DELEGATE, "call.delegate", 5, 1, 1, MintOpTwoShorts) -OPDEF(MINT_CALLI, "calli", 5, 1, 2, MintOpMethodToken) +OPDEF(MINT_CALLI, "calli", 4, 1, 2, MintOpNoArgs) OPDEF(MINT_CALLI_NAT, "calli.nat", 8, 1, 2, MintOpMethodToken) OPDEF(MINT_CALLI_NAT_DYNAMIC, "calli.nat.dynamic", 5, 1, 2, MintOpMethodToken) OPDEF(MINT_CALLI_NAT_FAST, "calli.nat.fast", 7, 1, 2, MintOpMethodToken) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 074290bb7bfe8..4ffa083840ba3 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3511,7 +3511,6 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target interp_add_ins (td, MINT_CALLI); interp_ins_set_dreg (td->last_ins, dreg); interp_ins_set_sregs2 (td->last_ins, fp_sreg, MINT_CALL_ARGS_SREG); - td->last_ins->data [0] = get_data_item_index (td, (void *)csignature); } } else { InterpMethod *imethod = mono_interp_get_imethod (target_method, error); diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index 0a7a157ac7c29..99f35d8def191 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -1146,8 +1146,8 @@ typedef struct gpointer addr; gpointer arg; MonoMethod *method; - /* InterpMethod* */ - gpointer interp_method; + /* InterpFtnDesc* */ + gpointer interp_ftndesc; } MonoFtnDesc; typedef enum {