Skip to content

Commit

Permalink
[runtime] Copy signature return type when copying MonoMethodSignature
Browse files Browse the repository at this point in the history 
This commit also removed an extra signature duplicator living in
marshal.c, so that this fix is used consistently
  • Loading branch information
@alexanderkyte @kumpera
alexanderkyte authored and kumpera committed Jul 28, 2015
1 parent 46fae9b commit c905bcb
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 64 deletions.
81 changes: 23 additions & 58 deletions mono/metadata/marshal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,25 +160,12 @@ register_icall (gpointer func, const char *name, const char *sigstr, gboolean sa
mono_register_jit_icall (func, name, sig, save);
}

static MonoMethodSignature*
signature_dup (MonoImage *image, MonoMethodSignature *sig)
{
MonoMethodSignature *res;
int sigsize;

res = mono_metadata_signature_alloc (image, sig->param_count);
sigsize = MONO_SIZEOF_METHOD_SIGNATURE + sig->param_count * sizeof (MonoType *);
memcpy (res, sig, sigsize);

return res;
}

MonoMethodSignature*
mono_signature_no_pinvoke (MonoMethod *method)
{
MonoMethodSignature *sig = mono_method_signature (method);
if (sig->pinvoke) {
sig = signature_dup (method->klass->image, sig);
sig = mono_metadata_signature_dup_full (method->klass->image, sig);
sig->pinvoke = FALSE;
}

Expand Down Expand Up @@ -3138,7 +3125,7 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt
cache_key = sig;
}

static_sig = signature_dup (method->klass->image, sig);
static_sig = mono_metadata_signature_dup_full (method->klass->image, sig);
static_sig->hasthis = 0;
if (!static_method_with_first_arg_bound)
invoke_sig = static_sig;
Expand Down Expand Up @@ -3372,28 +3359,6 @@ mono_marshal_get_delegate_invoke (MonoMethod *method, MonoDelegate *del)
return mono_marshal_get_delegate_invoke_internal (method, callvirt, static_method_with_first_arg_bound, target_method);
}

/*
* signature_dup_add_this:
*
* Make a copy of @sig, adding an explicit this argument.
*/
static MonoMethodSignature*
signature_dup_add_this (MonoImage *image, MonoMethodSignature *sig, MonoClass *klass)
{
MonoMethodSignature *res;
int i;

res = mono_metadata_signature_alloc (image, sig->param_count + 1);
memcpy (res, sig, MONO_SIZEOF_METHOD_SIGNATURE);
res->param_count = sig->param_count + 1;
res->hasthis = FALSE;
for (i = sig->param_count - 1; i >= 0; i --)
res->params [i + 1] = sig->params [i];
res->params [0] = klass->valuetype ? &klass->this_arg : &klass->byval_arg;

return res;
}

typedef struct {
MonoMethodSignature *ctor_sig;
MonoMethodSignature *sig;
Expand Down Expand Up @@ -3431,7 +3396,7 @@ add_string_ctor_signature (MonoMethod *method)
MonoMethodSignature *callsig;
CtorSigPair *cs;

callsig = signature_dup (method->klass->image, mono_method_signature (method));
callsig = mono_metadata_signature_dup_full (method->klass->image, mono_method_signature (method));
callsig->ret = &mono_defaults.string_class->byval_arg;
cs = g_new (CtorSigPair, 1);
cs->sig = callsig;
Expand Down Expand Up @@ -3888,7 +3853,7 @@ mono_marshal_get_runtime_invoke (MonoMethod *method, gboolean virtual)
need_direct_wrapper = TRUE;
} else {
if (method_is_dynamic (method))
callsig = signature_dup (method->klass->image, mono_method_signature (method));
callsig = mono_metadata_signature_dup_full (method->klass->image, mono_method_signature (method));
else
callsig = mono_method_signature (method);
}
Expand Down Expand Up @@ -4171,9 +4136,9 @@ mono_marshal_get_icall_wrapper (MonoMethodSignature *sig, const char *name, gcon

/* Add an explicit this argument */
if (sig->hasthis)
csig2 = signature_dup_add_this (mono_defaults.corlib, sig, mono_defaults.object_class);
csig2 = mono_metadata_signature_dup_add_this (mono_defaults.corlib, sig, mono_defaults.object_class);
else
csig2 = signature_dup (mono_defaults.corlib, sig);
csig2 = mono_metadata_signature_dup_full (mono_defaults.corlib, sig);

#ifndef DISABLE_JIT
if (sig->hasthis)
Expand All @@ -4190,7 +4155,7 @@ mono_marshal_get_icall_wrapper (MonoMethodSignature *sig, const char *name, gcon
mono_mb_emit_byte (mb, CEE_RET);
#endif

csig = signature_dup (mono_defaults.corlib, sig);
csig = mono_metadata_signature_dup_full (mono_defaults.corlib, sig);
csig->pinvoke = 0;
if (csig->call_convention == MONO_CALL_VARARG)
csig->call_convention = 0;
Expand Down Expand Up @@ -7023,7 +6988,7 @@ mono_marshal_emit_native_wrapper (MonoImage *image, MonoMethodBuilder *mb, MonoM
g_assert (!sig->hasthis);
param_shift += 1;
}
csig = signature_dup (mb->method->klass->image, sig);
csig = mono_metadata_signature_dup_full (mb->method->klass->image, sig);
csig->pinvoke = 1;
m.csig = csig;
m.image = image;
Expand Down Expand Up @@ -7306,7 +7271,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,
g_assert (sig->hasthis);

/* CreateString returns a value */
csig = signature_dup (method->klass->image, sig);
csig = mono_metadata_signature_dup_full (method->klass->image, sig);
csig->ret = &mono_defaults.string_class->byval_arg;
csig->pinvoke = 0;

Expand Down Expand Up @@ -7362,7 +7327,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,
info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_NONE);
info->d.managed_to_native.method = method;

csig = signature_dup (method->klass->image, sig);
csig = mono_metadata_signature_dup_full (method->klass->image, sig);
csig->pinvoke = 0;
res = mono_mb_create_and_cache_full (cache, method, mb, csig,
csig->param_count + 16, info, NULL);
Expand All @@ -7374,9 +7339,9 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,
/* internal calls: we simply push all arguments and call the method (no conversions) */
if (method->iflags & (METHOD_IMPL_ATTRIBUTE_INTERNAL_CALL | METHOD_IMPL_ATTRIBUTE_RUNTIME)) {
if (sig->hasthis)
csig = signature_dup_add_this (method->klass->image, sig, method->klass);
csig = mono_metadata_signature_dup_add_this (method->klass->image, sig, method->klass);
else
csig = signature_dup (method->klass->image, sig);
csig = mono_metadata_signature_dup_full (method->klass->image, sig);

/* hack - string constructors returns a value */
if (method->string_ctor)
Expand Down Expand Up @@ -7416,7 +7381,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,
info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_NONE);
info->d.managed_to_native.method = method;

csig = signature_dup (method->klass->image, csig);
csig = mono_metadata_signature_dup_full (method->klass->image, csig);
csig->pinvoke = 0;
res = mono_mb_create_and_cache_full (cache, method, mb, csig, csig->param_count + 16,
info, NULL);
Expand All @@ -7438,7 +7403,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,
info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_PINVOKE);
info->d.managed_to_native.method = method;

csig = signature_dup (method->klass->image, sig);
csig = mono_metadata_signature_dup_full (method->klass->image, sig);
csig->pinvoke = 0;
res = mono_mb_create_and_cache_full (cache, method, mb, csig, csig->param_count + 16,
info, NULL);
Expand Down Expand Up @@ -7493,7 +7458,7 @@ mono_marshal_get_native_func_wrapper (MonoImage *image, MonoMethodSignature *sig
mono_marshal_emit_native_wrapper (image, mb, sig, piinfo, mspecs, func, FALSE, TRUE, FALSE);
#endif

csig = signature_dup (image, sig);
csig = mono_metadata_signature_dup_full (image, sig);
csig->pinvoke = 0;

new_key = g_new (SignaturePointerPair,1);
Expand Down Expand Up @@ -7561,7 +7526,7 @@ mono_marshal_get_native_func_wrapper_aot (MonoClass *klass)
info->d.managed_to_native.method = invoke;

g_assert (!sig->hasthis);
csig = signature_dup_add_this (image, sig, mono_defaults.object_class);
csig = mono_metadata_signature_dup_add_this (image, sig, mono_defaults.object_class);
csig->pinvoke = 0;
res = mono_mb_create_and_cache_full (cache, invoke,
mb, csig, csig->param_count + 16,
Expand Down Expand Up @@ -7911,7 +7876,7 @@ mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass,
/* Need to free this later */
csig = mono_metadata_signature_dup (invoke_sig);
else
csig = signature_dup (method->klass->image, invoke_sig);
csig = mono_metadata_signature_dup_full (method->klass->image, invoke_sig);
csig->hasthis = 0;
csig->pinvoke = 1;

Expand Down Expand Up @@ -8056,7 +8021,7 @@ mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16 type)
mono_method_get_marshal_info (method, mspecs);

mb = mono_mb_new (method->klass, method->name, MONO_WRAPPER_NATIVE_TO_MANAGED);
csig = signature_dup (image, sig);
csig = mono_metadata_signature_dup_full (image, sig);
csig->hasthis = 0;
csig->pinvoke = 1;

Expand Down Expand Up @@ -8613,7 +8578,7 @@ mono_marshal_get_ptr_to_struct (MonoClass *klass)
static void PtrToStructure (IntPtr ptr, object structure);
defined in class/corlib/System.Runtime.InteropServices/Marshal.cs */
sig = mono_create_icall_signature ("void ptr object");
sig = signature_dup (mono_defaults.corlib, sig);
sig = mono_metadata_signature_dup_full (mono_defaults.corlib, sig);
sig->pinvoke = 0;
mono_memory_barrier ();
ptostr = sig;
Expand Down Expand Up @@ -8689,7 +8654,7 @@ mono_marshal_get_synchronized_inner_wrapper (MonoMethod *method)
mono_mb_emit_exception_full (mb, "System", "ExecutionEngineException", "Shouldn't be called.");
mono_mb_emit_byte (mb, CEE_RET);
#endif
sig = signature_dup (method->klass->image, mono_method_signature (method));
sig = mono_metadata_signature_dup_full (method->klass->image, mono_method_signature (method));

info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_SYNCHRONIZED_INNER);
info->d.synchronized_inner.method = method;
Expand Down Expand Up @@ -8751,7 +8716,7 @@ mono_marshal_get_synchronized_wrapper (MonoMethod *method)
return res;
}

sig = signature_dup (method->klass->image, mono_method_signature (method));
sig = mono_metadata_signature_dup_full (method->klass->image, mono_method_signature (method));
sig->pinvoke = 0;

mb = mono_mb_new (method->klass, method->name, MONO_WRAPPER_SYNCHRONIZED);
Expand Down Expand Up @@ -9876,7 +9841,7 @@ mono_marshal_get_array_accessor_wrapper (MonoMethod *method)
return res;
}

sig = signature_dup (method->klass->image, mono_method_signature (method));
sig = mono_metadata_signature_dup_full (method->klass->image, mono_method_signature (method));
sig->pinvoke = 0;

mb = mono_mb_new (method->klass, method->name, MONO_WRAPPER_UNKNOWN);
Expand Down Expand Up @@ -11031,7 +10996,7 @@ mono_marshal_get_generic_array_helper (MonoClass *class, MonoClass *iface, gchar
METHOD_ATTRIBUTE_NEW_SLOT | METHOD_ATTRIBUTE_HIDE_BY_SIG | METHOD_ATTRIBUTE_FINAL;

sig = mono_method_signature (method);
csig = signature_dup (method->klass->image, sig);
csig = mono_metadata_signature_dup_full (method->klass->image, sig);
csig->generic_param_count = 0;

#ifndef DISABLE_JIT
Expand Down
1 change: 1 addition & 0 deletions mono/metadata/metadata-internals.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,7 @@ void mono_unload_interface_ids (MonoBitSet *bitset);
MonoType *mono_metadata_type_dup (MonoImage *image, const MonoType *original);
MonoMethodSignature *mono_metadata_signature_dup_full (MonoImage *image,MonoMethodSignature *sig);
MonoMethodSignature *mono_metadata_signature_dup_mempool (MonoMemPool *mp, MonoMethodSignature *sig);
MonoMethodSignature *mono_metadata_signature_dup_add_this (MonoImage *image, MonoMethodSignature *sig, MonoClass *klass);

MonoGenericInst *
mono_get_shared_generic_inst (MonoGenericContainer *container);
Expand Down
60 changes: 55 additions & 5 deletions mono/metadata/metadata.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1858,11 +1858,13 @@ mono_metadata_signature_alloc (MonoImage *m, guint32 nparams)
}

static MonoMethodSignature*
mono_metadata_signature_dup_internal (MonoImage *image, MonoMemPool *mp, MonoMethodSignature *sig)
mono_metadata_signature_dup_internal_with_padding (MonoImage *image, MonoMemPool *mp, MonoMethodSignature *sig, size_t padding)
{
int sigsize;
int sigsize, sig_header_size;
MonoMethodSignature *ret;
sigsize = MONO_SIZEOF_METHOD_SIGNATURE + sig->param_count * sizeof (MonoType *);
sigsize = sig_header_size = MONO_SIZEOF_METHOD_SIGNATURE + sig->param_count * sizeof (MonoType *) + padding;
if (sig->ret)
sigsize += sizeof (MonoType);

if (image) {
ret = mono_image_alloc (image, sigsize);
Expand All @@ -1871,14 +1873,62 @@ mono_metadata_signature_dup_internal (MonoImage *image, MonoMemPool *mp, MonoMet
} else {
ret = g_malloc (sigsize);
}
memcpy (ret, sig, sigsize);

memcpy (ret, sig, sig_header_size - padding);

// Copy return value because of ownership semantics.
if (sig->ret) {
// Danger! Do not alter padding use without changing the dup_add_this below
intptr_t end_of_header = (intptr_t)( (char*)(ret) + sig_header_size);
ret->ret = (MonoType *)end_of_header;
*ret->ret = *sig->ret;
}

return ret;
}

static MonoMethodSignature*
mono_metadata_signature_dup_internal (MonoImage *image, MonoMemPool *mp, MonoMethodSignature *sig)
{
return mono_metadata_signature_dup_internal_with_padding (image, mp, sig, 0);
}
/*
* signature_dup_add_this:
*
* Make a copy of @sig, adding an explicit this argument.
*/
MonoMethodSignature*
mono_metadata_signature_dup_add_this (MonoImage *image, MonoMethodSignature *sig, MonoClass *klass)
{
MonoMethodSignature *ret;
ret = mono_metadata_signature_dup_internal_with_padding (image, NULL, sig, sizeof (MonoType *));

ret->param_count = sig->param_count + 1;
ret->hasthis = FALSE;

for (int i = sig->param_count - 1; i >= 0; i --)
ret->params [i + 1] = sig->params [i];
ret->params [0] = klass->valuetype ? &klass->this_arg : &klass->byval_arg;

for (int i = sig->param_count - 1; i >= 0; i --)
g_assert(ret->params [i + 1]->type == sig->params [i]->type && ret->params [i+1]->type != MONO_TYPE_END);
g_assert (ret->ret->type == sig->ret->type && ret->ret->type != MONO_TYPE_END);

return ret;
}



MonoMethodSignature*
mono_metadata_signature_dup_full (MonoImage *image, MonoMethodSignature *sig)
{
return mono_metadata_signature_dup_internal (image, NULL, sig);
MonoMethodSignature *ret = mono_metadata_signature_dup_internal (image, NULL, sig);

for (int i = 0 ; i < sig->param_count; i ++)
g_assert(ret->params [i]->type == sig->params [i]->type);
g_assert (ret->ret->type == sig->ret->type);

return ret;
}

/*The mempool is accessed without synchronization*/
Expand Down
2 changes: 1 addition & 1 deletion mono/metadata/method-builder.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ mono_mb_create_method (MonoMethodBuilder *mb, MonoMethodSignature *signature, in
memcpy ((char*)header->code, mb->code, mb->pos);

for (i = 0, l = mb->locals_list; l; l = l->next, i++) {
header->locals [i] = (MonoType *)l->data;
header->locals [i] = mono_metadata_type_dup (NULL, (MonoType*)l->data);
}
#endif
}
Expand Down

0 comments on commit c905bcb

Please sign in to comment.