From aa88ef35fb286310cff5cab8f9ecf1da101f16ed Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Mon, 23 Aug 2021 17:43:34 -0400 Subject: [PATCH 1/6] [Libraries][Android] Fix alignment for padding System.Decimal --- .../System/Runtime/InteropServices/Marshal/OffsetOfTests.cs | 1 - src/mono/mono/metadata/marshal.c | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs index 3781d521678c86..1ce00e225b67a6 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs @@ -34,7 +34,6 @@ public void OffsetOf_ClassWithSequentialLayout_ReturnsExpected() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/49872", typeof(PlatformDetection), nameof(PlatformDetection.IsAndroid), nameof(PlatformDetection.Is32BitProcess))] public void OffsetOf_ExplicitLayout_ReturnsExpected() { Type t = typeof(ExplicitLayoutTest); diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 0a9d8f8ff1abd0..ac9a6e2d429368 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5623,10 +5623,8 @@ mono_marshal_type_size (MonoType *type, MonoMarshalSpec *mspec, guint32 *align, return 16; } else if (strcmp (m_class_get_name_space (klass), "System") == 0 && strcmp (m_class_get_name (klass), "Decimal") == 0) { - - // Special case: Managed Decimal consists of 4 int32 fields, the alignment should be 8 on x64 to follow - // https://github.com/dotnet/coreclr/blob/4450e5ca663b9e66c20e6f9751c941efa3716fde/src/vm/methodtablebuilder.cpp#L9753 - *align = MONO_ABI_ALIGNOF (gpointer); + + *align = 8; return mono_class_native_size (klass, NULL); } padded_size = mono_class_native_size (klass, align); From 62415ee7774bcd1ca1557c25efb4cb445909285d Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Mon, 23 Aug 2021 18:03:03 -0400 Subject: [PATCH 2/6] Remove other active issue --- .../System/Runtime/InteropServices/Marshal/OffsetOfTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs index 1ce00e225b67a6..a873a9524ebb43 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs @@ -106,7 +106,6 @@ public void OffsetOf_ValidField_ReturnsExpected() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/49872", typeof(PlatformDetection), nameof(PlatformDetection.IsAndroid), nameof(PlatformDetection.Is32BitProcess))] public void OffsetOf_Decimal_ReturnsExpected() { Type t = typeof(FieldAlignmentTest_Decimal); From 588bb49215ed4ea75e733cf7b3c4d819285a4898 Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Tue, 24 Aug 2021 10:47:39 -0400 Subject: [PATCH 3/6] Remove trailing whitespaces --- src/mono/mono/metadata/marshal.c | 152 +++++++++++++++---------------- 1 file changed, 76 insertions(+), 76 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index ac9a6e2d429368..777f2fab392628 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -1,7 +1,7 @@ /** * \file * Routines for marshaling complex types in P/Invoke methods. - * + * * Author: * Paolo Molaro (lupus@ximian.com) * @@ -76,7 +76,7 @@ enum { }; #undef OPDEF -/* +/* * This mutex protects the various marshalling related caches in MonoImage * and a few other data structures static to this file. * @@ -145,7 +145,7 @@ mono_signature_no_pinvoke (MonoMethod *method) sig = mono_metadata_signature_dup_full (get_method_image (method), sig); sig->pinvoke = FALSE; } - + return sig; } @@ -348,7 +348,7 @@ mono_delegate_to_ftnptr_impl (MonoDelegateHandle delegate, MonoError *error) return result; } -/* +/* * this hash table maps from a delegate trampoline object to a weak reference * of the delegate. As an optimizations with a non-moving GC we store the * object pointer itself, otherwise we use a GC handle. @@ -360,7 +360,7 @@ delegate_hash_table_new (void) { return g_hash_table_new (NULL, NULL); } -static void +static void delegate_hash_table_remove (MonoDelegate *d) { MonoGCHandle gchandle = NULL; @@ -429,7 +429,7 @@ parse_unmanaged_function_pointer_attr (MonoClass *klass, MonoMethodPInvoke *piin /* The attribute is only available in Net 2.0 */ if (mono_class_try_get_unmanaged_function_pointer_attribute_class ()) { - /* + /* * The pinvoke attributes are stored in a real custom attribute so we have to * construct it. */ @@ -652,7 +652,7 @@ mono_array_to_lparray_impl (MonoArrayHandle array_handle, MonoError *error) break; case MONO_TYPE_GENERICINST: case MONO_TYPE_OBJECT: - case MONO_TYPE_ARRAY: + case MONO_TYPE_ARRAY: case MONO_TYPE_SZARRAY: case MONO_TYPE_STRING: default: @@ -1138,7 +1138,7 @@ mono_type_to_ldind (MonoType *type) case MONO_TYPE_STRING: case MONO_TYPE_OBJECT: case MONO_TYPE_SZARRAY: - case MONO_TYPE_ARRAY: + case MONO_TYPE_ARRAY: return CEE_LDIND_REF; case MONO_TYPE_I8: case MONO_TYPE_U8: @@ -1192,7 +1192,7 @@ mono_type_to_stind (MonoType *type) case MONO_TYPE_STRING: case MONO_TYPE_OBJECT: case MONO_TYPE_SZARRAY: - case MONO_TYPE_ARRAY: + case MONO_TYPE_ARRAY: return CEE_STIND_REF; case MONO_TYPE_I8: case MONO_TYPE_U8: @@ -1389,7 +1389,7 @@ mono_marshal_get_ptr_to_stringbuilder_conv (MonoMethodPInvoke *piinfo, MonoMarsh } /* - * Return whenever a field of a native structure or an array member needs to + * Return whenever a field of a native structure or an array member needs to * be freed. */ gboolean @@ -1426,7 +1426,7 @@ get_cache (GHashTable **var, GHashFunc hash_func, GCompareFunc equal_func) if (!(*var)) { mono_marshal_lock (); if (!(*var)) { - GHashTable *cache = + GHashTable *cache = g_hash_table_new (hash_func, equal_func); mono_memory_barrier (); *var = cache; @@ -1503,7 +1503,7 @@ mono_mb_create_and_cache_full (GHashTable *cache, gpointer key, } return res; -} +} MonoMethod* mono_mb_create_and_cache (GHashTable *cache, gpointer key, @@ -1616,14 +1616,14 @@ get_wrapper_target_class (MonoImage *image) /* * Notes: * - can't put all wrappers into an mscorlib class, because they reference - * metadata (signature) so they should be put into the same image as the + * metadata (signature) so they should be put into the same image as the * method they wrap, so they are unloaded together. - * - putting them into a class with a type initalizer could cause the - * initializer to be executed which can be a problem if the wrappers are + * - putting them into a class with a type initalizer could cause the + * initializer to be executed which can be a problem if the wrappers are * shared. - * - putting them into an inflated class can cause problems if the the + * - putting them into an inflated class can cause problems if the the * class is deleted because it references an image which is unloaded. - * To avoid these problems, we put the wrappers into the class of + * To avoid these problems, we put the wrappers into the class of * the image. */ if (image_is_dynamic (image)) { @@ -1818,7 +1818,7 @@ mono_marshal_get_delegate_begin_invoke (MonoMethod *method) return res; } else { cache = get_cache (&get_method_image (method)->wrapper_caches.delegate_begin_invoke_cache, - (GHashFunc)mono_signature_hash, + (GHashFunc)mono_signature_hash, (GCompareFunc)mono_metadata_signature_equal); if ((res = mono_marshal_find_in_cache (cache, sig))) return res; @@ -1896,7 +1896,7 @@ mono_marshal_get_delegate_end_invoke (MonoMethod *method) return res; } else { cache = get_cache (&get_method_image (method)->wrapper_caches.delegate_end_invoke_cache, - (GHashFunc)mono_signature_hash, + (GHashFunc)mono_signature_hash, (GCompareFunc)mono_metadata_signature_equal); if ((res = mono_marshal_find_in_cache (cache, sig))) return res; @@ -1991,8 +1991,8 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt invoke_sig = sig = mono_signature_no_pinvoke (method); /* - * If the delegate target is null, and the target method is not static, a virtual - * call is made to that method with the first delegate argument as this. This is + * If the delegate target is null, and the target method is not static, a virtual + * call is made to that method with the first delegate argument as this. This is * a non-documented .NET feature. */ if (callvirt) { @@ -2056,7 +2056,7 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt cache_ptr = &mono_method_get_wrapper_cache (target_method)->delegate_bound_static_invoke_cache; cache = get_cache (cache_ptr, - (GHashFunc)mono_signature_hash, + (GHashFunc)mono_signature_hash, (GCompareFunc)mono_metadata_signature_equal); /* * The wrapper is based on sig+invoke_sig, but sig can be derived from invoke_sig. @@ -2085,7 +2085,7 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt // Inflated methods should not be in this cache because it's not stored on the imageset. g_assert (!method->is_inflated); cache = get_cache (&get_method_image (method)->wrapper_caches.delegate_invoke_cache, - (GHashFunc)mono_signature_hash, + (GHashFunc)mono_signature_hash, (GCompareFunc)mono_metadata_signature_equal); res = mono_marshal_find_in_cache (cache, sig); if (res) @@ -2147,7 +2147,7 @@ mono_marshal_get_delegate_invoke_internal (MonoMethod *method, gboolean callvirt /* mono_method_print_code (res); */ - return res; + return res; } /** @@ -2377,7 +2377,7 @@ wrapper_cache_signature_key_equal (MonoWrapperSignatureCacheKey *key1, MonoWrapp * * We also catch exceptions if \p exc is not NULL. * If \p virtual is TRUE, then \p method is invoked virtually on \p this. This is useful since - * it means that the compiled code for \p method does not have to be looked up + * it means that the compiled code for \p method does not have to be looked up * before calling the runtime invoke wrapper. In this case, the wrapper ignores * its \p method argument. */ @@ -2559,7 +2559,7 @@ mono_marshal_get_runtime_invoke_full (MonoMethod *method, gboolean virtual_, gbo mono_mb_free (mb); - return res; + return res; } MonoMethod * @@ -2597,7 +2597,7 @@ mono_marshal_get_runtime_invoke (MonoMethod *method, gboolean virtual_) * dynamically. * The signature of the returned method is given by RuntimeInvokeDynamicFunction: * void runtime_invoke (void *args, MonoObject **exc, void *compiled_method) - * ARGS should point to an architecture specific structure containing + * ARGS should point to an architecture specific structure containing * the arguments and space for the return value. * The other arguments are the same as for runtime_invoke (), except that * ARGS should contain the this argument too. @@ -2759,7 +2759,7 @@ mono_marshal_get_icall_wrapper (MonoJitICallInfo *callinfo, gboolean check_excep WrapperInfo *info; gconstpointer const func = callinfo->func; - + GHashTable *cache = get_cache (& m_class_get_image (mono_defaults.object_class)->icall_wrapper_cache, mono_aligned_addr_hash, NULL); if ((res = mono_marshal_find_in_cache (cache, (gpointer) func))) return res; @@ -2961,8 +2961,8 @@ mono_marshal_boolean_managed_conv_in_get_conv_arg_class (MonoMarshalSpec *spec, } int -mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t, - MonoMarshalSpec *spec, int conv_arg, +mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t, + MonoMarshalSpec *spec, int conv_arg, MonoType **conv_arg_type, MarshalAction action) { /* Ensure that we have marshalling info for this param */ @@ -2973,12 +2973,12 @@ mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t, if (spec && spec->native == MONO_NATIVE_ASANY) return get_marshal_cb ()->emit_marshal_asany (m, argnum, t, spec, conv_arg, conv_arg_type, action); - + switch (t->type) { case MONO_TYPE_VALUETYPE: if (t->data.klass == mono_class_try_get_handleref_class ()) return get_marshal_cb ()->emit_marshal_handleref (m, argnum, t, spec, conv_arg, conv_arg_type, action); - + return get_marshal_cb ()->emit_marshal_vtype (m, argnum, t, spec, conv_arg, conv_arg_type, action); case MONO_TYPE_STRING: return get_marshal_cb ()->emit_marshal_string (m, argnum, t, spec, conv_arg, conv_arg_type, action); @@ -2995,8 +2995,8 @@ mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t, spec->native == MONO_NATIVE_INTERFACE)) || (t->type == MONO_TYPE_CLASS && mono_cominterop_is_interface(t->data.klass))) return mono_cominterop_emit_marshal_com_interface (m, argnum, t, spec, conv_arg, conv_arg_type, action); - if (spec && (spec->native == MONO_NATIVE_SAFEARRAY) && - (spec->data.safearray_data.elem_type == MONO_VARIANT_VARIANT) && + if (spec && (spec->native == MONO_NATIVE_SAFEARRAY) && + (spec->data.safearray_data.elem_type == MONO_VARIANT_VARIANT) && ((action == MARSHAL_ACTION_CONV_OUT) || (action == MARSHAL_ACTION_CONV_IN) || (action == MARSHAL_ACTION_PUSH))) return mono_cominterop_emit_marshal_safearray (m, argnum, t, spec, conv_arg, conv_arg_type, action); #endif @@ -3004,7 +3004,7 @@ mono_emit_marshal (EmitMarshalContext *m, int argnum, MonoType *t, if (mono_class_try_get_safehandle_class () != NULL && t->data.klass && mono_class_is_subclass_of_internal (t->data.klass, mono_class_try_get_safehandle_class (), FALSE)) return get_marshal_cb ()->emit_marshal_safehandle (m, argnum, t, spec, conv_arg, conv_arg_type, action); - + return get_marshal_cb ()->emit_marshal_object (m, argnum, t, spec, conv_arg, conv_arg_type, action); case MONO_TYPE_ARRAY: case MONO_TYPE_SZARRAY: @@ -3439,7 +3439,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions, * wrapper. */ MonoMethod * -mono_marshal_get_native_func_wrapper (MonoImage *image, MonoMethodSignature *sig, +mono_marshal_get_native_func_wrapper (MonoImage *image, MonoMethodSignature *sig, MonoMethodPInvoke *piinfo, MonoMarshalSpec **mspecs, gpointer func) { MonoMethodSignature *csig; @@ -3589,17 +3589,17 @@ mono_marshal_get_native_func_wrapper_indirect (MonoClass *caller_class, MonoMeth #endif GHashTable *cache = get_cache (&image->wrapper_caches.native_func_wrapper_indirect_cache, - (GHashFunc)mono_signature_hash, + (GHashFunc)mono_signature_hash, (GCompareFunc)mono_metadata_signature_equal); - + MonoMethod *res; if ((res = mono_marshal_find_in_cache (cache, sig))) return res; - + #if 0 fprintf (stderr, "generating wrapper for signature %s\n", mono_signature_full_name (sig)); #endif - + /* FIXME: better wrapper name */ char * name = g_strdup_printf ("wrapper_native_indirect_%p", sig); MonoMethodBuilder *mb = mono_mb_new (caller_class, name, MONO_WRAPPER_MANAGED_TO_NATIVE); @@ -3689,7 +3689,7 @@ method_signature_is_blittable (MonoMethodSignature *sig) /** * mono_marshal_get_managed_wrapper: - * Generates IL code to call managed methods from unmanaged code + * Generates IL code to call managed methods from unmanaged code * If \p target_handle is \c 0, the wrapper info will be a \c WrapperInfo structure. * * If \p delegate_klass is \c NULL, we're creating a wrapper for a function pointer to a method marked with @@ -3715,7 +3715,7 @@ mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, return NULL; } - /* + /* * FIXME: Should cache the method+delegate type pair, since the same method * could be called with different delegates, thus different marshalling * options. @@ -3785,7 +3785,7 @@ mono_marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, MonoCustomAttrInfo *cinfo; MonoCustomAttrEntry *attr; - /* + /* * The pinvoke attributes are stored in a real custom attribute. Obtain the * contents of the attribute without constructing it, as that might not be * possible when running in cross-compiling mode. @@ -4296,7 +4296,7 @@ mono_marshal_get_synchronized_wrapper (MonoMethod *method) } mono_mb_free (mb); - return res; + return res; } /** @@ -4320,7 +4320,7 @@ mono_marshal_get_unbox_wrapper (MonoMethod *method) mb = mono_mb_new (method->klass, method->name, MONO_WRAPPER_UNBOX); g_assert (sig->hasthis); - + get_marshal_cb ()->emit_unbox_wrapper (mb, method); info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_NONE); @@ -4332,7 +4332,7 @@ mono_marshal_get_unbox_wrapper (MonoMethod *method) /* mono_method_print_code (res); */ - return res; + return res; } static gboolean @@ -4493,7 +4493,7 @@ mono_marshal_get_stelemref (void) WrapperInfo *info; MONO_STATIC_POINTER_INIT (MonoMethod, ret) - + mb = mono_mb_new (mono_defaults.object_class, "stelemref", MONO_WRAPPER_STELEMREF); sig = mono_metadata_signature_alloc (mono_defaults.corlib, 3); @@ -4533,9 +4533,9 @@ mono_marshal_get_gsharedvt_in_wrapper (void) MonoMethodSignature *sig; MonoMethodBuilder *mb; WrapperInfo *info; - + mb = mono_mb_new (mono_defaults.object_class, "gsharedvt_in", MONO_WRAPPER_OTHER); - + sig = mono_metadata_signature_alloc (mono_defaults.corlib, 0); sig->ret = mono_get_void_type (); @@ -4566,9 +4566,9 @@ mono_marshal_get_gsharedvt_out_wrapper (void) MonoMethodSignature *sig; MonoMethodBuilder *mb; WrapperInfo *info; - + mb = mono_mb_new (mono_defaults.object_class, "gsharedvt_out", MONO_WRAPPER_OTHER); - + sig = mono_metadata_signature_alloc (mono_defaults.corlib, 0); sig->ret = mono_get_void_type (); @@ -4646,7 +4646,7 @@ mono_marshal_get_array_address (int rank, int elem_size) name = g_strdup_printf ("ElementAddr_%d", elem_size); mb = mono_mb_new (mono_defaults.object_class, name, MONO_WRAPPER_MANAGED_TO_MANAGED); g_free (name); - + get_marshal_cb ()->emit_array_address (mb, rank, elem_size); info = mono_wrapper_info_create (mb, WRAPPER_SUBTYPE_ELEMENT_ADDR); @@ -4738,7 +4738,7 @@ mono_marshal_get_array_accessor_wrapper (MonoMethod *method) } mono_mb_free (mb); - return res; + return res; } #ifndef HOST_WIN32 @@ -4802,7 +4802,7 @@ mono_marshal_free (gpointer ptr) * mono_marshal_free_array: */ void -mono_marshal_free_array (gpointer *ptr, int size) +mono_marshal_free_array (gpointer *ptr, int size) { int i; @@ -5070,9 +5070,9 @@ mono_struct_delete_old (MonoClass *klass, char *ptr) if (ftype->attrs & FIELD_ATTRIBUTE_STATIC) continue; - mono_type_to_unmanaged (ftype, info->fields [i].mspec, TRUE, + mono_type_to_unmanaged (ftype, info->fields [i].mspec, TRUE, m_class_is_unicode (klass), &conv); - + cpos = ptr + info->fields [i].offset; switch (conv) { @@ -5087,7 +5087,7 @@ mono_struct_delete_old (MonoClass *klass, char *ptr) break; case MONO_MARSHAL_CONV_STR_LPTSTR: #ifdef TARGET_WIN32 - /* We assume this field points inside a MonoString + /* We assume this field points inside a MonoString * on Win32 */ break; #endif @@ -5259,7 +5259,7 @@ ves_icall_System_Runtime_InteropServices_Marshal_IsPinnableType (MonoReflectionT if (m_class_get_rank (klass)) { MonoClass *eklass = m_class_get_element_class (klass); if (m_class_is_primitive (eklass)) - return TRUE; + return TRUE; return eklass != mono_defaults.object_class && m_class_is_blittable (eklass); } else return m_class_is_blittable (klass); @@ -5283,7 +5283,7 @@ mono_marshal_is_loading_type_info (MonoClass *klass) * mono_marshal_load_type_info: * * Initialize \c klass::marshal_info using information from metadata. This function can - * recursively call itself, and the caller is responsible to avoid that by calling + * recursively call itself, and the caller is responsible to avoid that by calling * \c mono_marshal_is_loading_type_info beforehand. * * LOCKING: Acquires the loader lock. @@ -5320,7 +5320,7 @@ mono_marshal_load_type_info (MonoClass* klass) loads_list = (GSList *)mono_native_tls_get_value (load_type_info_tls_id); loads_list = g_slist_prepend (loads_list, klass); mono_native_tls_set_value (load_type_info_tls_id, loads_list); - + iter = NULL; while ((field = mono_class_get_fields_internal (klass, &iter))) { if (field->type->attrs & FIELD_ATTRIBUTE_STATIC) @@ -5334,7 +5334,7 @@ mono_marshal_load_type_info (MonoClass* klass) info = (MonoMarshalType *)mono_image_alloc0 (m_class_get_image (klass), MONO_SIZEOF_MARSHAL_TYPE + sizeof (MonoMarshalField) * count); info->num_fields = count; - + /* Try to find a size for this type in metadata */ mono_metadata_packing_from_typedef (m_class_get_image (klass), m_class_get_type_token (klass), NULL, &native_size); @@ -5352,14 +5352,14 @@ mono_marshal_load_type_info (MonoClass* klass) while ((field = mono_class_get_fields_internal (klass, &iter))) { int size; guint32 align; - + if (field->type->attrs & FIELD_ATTRIBUTE_STATIC) continue; if (mono_field_is_deleted (field)) continue; if (field->type->attrs & FIELD_ATTRIBUTE_HAS_FIELD_MARSHAL) - mono_metadata_field_info_with_mempool (m_class_get_image (klass), mono_metadata_token_index (mono_class_get_field_token (field)) - 1, + mono_metadata_field_info_with_mempool (m_class_get_image (klass), mono_metadata_token_index (mono_class_get_field_token (field)) - 1, NULL, NULL, &info->fields [j].mspec); info->fields [j].field = field; @@ -5373,7 +5373,7 @@ mono_marshal_load_type_info (MonoClass* klass) switch (layout) { case TYPE_ATTRIBUTE_AUTO_LAYOUT: case TYPE_ATTRIBUTE_SEQUENTIAL_LAYOUT: - size = mono_marshal_type_size (field->type, info->fields [j].mspec, + size = mono_marshal_type_size (field->type, info->fields [j].mspec, &align, TRUE, m_class_is_unicode (klass)); align = m_class_get_packing_size (klass) ? MIN (m_class_get_packing_size (klass), align): align; min_align = MAX (align, min_align); @@ -5383,13 +5383,13 @@ mono_marshal_load_type_info (MonoClass* klass) info->native_size = info->fields [j].offset + size; break; case TYPE_ATTRIBUTE_EXPLICIT_LAYOUT: - size = mono_marshal_type_size (field->type, info->fields [j].mspec, + size = mono_marshal_type_size (field->type, info->fields [j].mspec, &align, TRUE, m_class_is_unicode (klass)); min_align = MAX (align, min_align); info->fields [j].offset = field->offset - MONO_ABI_SIZEOF (MonoObject); info->native_size = MAX (info->native_size, info->fields [j].offset + size); break; - } + } j++; } @@ -5448,9 +5448,9 @@ mono_marshal_load_type_info (MonoClass* klass) /** * mono_class_native_size: - * \param klass a class - * \returns the native size of an object instance (when marshaled - * to unmanaged code) + * \param klass a class + * \returns the native size of an object instance (when marshaled + * to unmanaged code) */ gint32 mono_class_native_size (MonoClass *klass, guint32 *align) @@ -5531,7 +5531,7 @@ mono_type_native_stack_size (MonoType *t, guint32 *align) if (!mono_type_generic_inst_is_valuetype (t)) { *align = TARGET_SIZEOF_VOID_P; return TARGET_SIZEOF_VOID_P; - } + } /* Fall through */ case MONO_TYPE_TYPEDBYREF: case MONO_TYPE_VALUETYPE: { @@ -5544,7 +5544,7 @@ mono_type_native_stack_size (MonoType *t, guint32 *align) size = mono_class_native_size (klass, align); *align = *align + 3; *align &= ~3; - + size += 3; size &= ~3; @@ -5615,13 +5615,13 @@ mono_marshal_type_size (MonoType *type, MonoMarshalSpec *mspec, guint32 *align, case MONO_NATIVE_LPSTRUCT: *align = MONO_ABI_ALIGNOF (gpointer); return TARGET_SIZEOF_VOID_P; - case MONO_NATIVE_STRUCT: + case MONO_NATIVE_STRUCT: klass = mono_class_from_mono_type_internal (type); if (klass == mono_defaults.object_class && (mspec && mspec->native == MONO_NATIVE_STRUCT)) { *align = 16; return 16; - } else if (strcmp (m_class_get_name_space (klass), "System") == 0 && + } else if (strcmp (m_class_get_name_space (klass), "System") == 0 && strcmp (m_class_get_name (klass), "Decimal") == 0) { *align = 8; @@ -5657,7 +5657,7 @@ mono_marshal_type_size (MonoType *type, MonoMarshalSpec *mspec, guint32 *align, case MONO_NATIVE_CURRENCY: case MONO_NATIVE_VBBYREFSTR: default: - g_error ("native type %02x not implemented", native_type); + g_error ("native type %02x not implemented", native_type); break; } g_assert_not_reached (); @@ -5730,7 +5730,7 @@ mono_marshal_asany_impl (MonoObjectHandle o, MonoMarshalNative string_encoding, return res; } case MONO_TYPE_SZARRAY: { - //TODO: Implement structs and in-params for all value types + //TODO: Implement structs and in-params for all value types MonoClass *klass = t->data.klass; MonoClass *eklass = m_class_get_element_class (klass); MonoArray *arr = (MonoArray *) MONO_HANDLE_RAW (o); @@ -6038,7 +6038,7 @@ mono_marshal_free_dynamic_wrappers (MonoMethod *method) /* This could be called during shutdown */ if (marshal_mutex_initialized) mono_marshal_lock (); - /* + /* * FIXME: We currently leak the wrappers. Freeing them would be tricky as * they could be shared with other methods ? */ From 228b823f926ab97bd4a8349a9f69c1813b4e6245 Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Tue, 24 Aug 2021 11:27:30 -0400 Subject: [PATCH 4/6] Modify tests instead of the align behavior --- .../InteropServices/Marshal/OffsetOfTests.cs | 22 +++++++++++++------ src/mono/mono/metadata/marshal.c | 4 +++- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs index a873a9524ebb43..2e08cda77a9e80 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs @@ -37,7 +37,7 @@ public void OffsetOf_ClassWithSequentialLayout_ReturnsExpected() public void OffsetOf_ExplicitLayout_ReturnsExpected() { Type t = typeof(ExplicitLayoutTest); - Assert.Equal(56, Marshal.SizeOf(t)); + Assert.Equal(OperatingSystem.IsAndroid() && RuntimeInformation.ProcessArchitecture == Architecture.X86 ? 52 : 56, Marshal.SizeOf(t)); Assert.Equal(new IntPtr(0), Marshal.OffsetOf(t, nameof(ExplicitLayoutTest.m_short1))); Assert.Equal(new IntPtr(2), Marshal.OffsetOf(t, nameof(ExplicitLayoutTest.m_short2))); @@ -114,18 +114,26 @@ public void OffsetOf_Decimal_ReturnsExpected() { Assert.Equal(96, Marshal.SizeOf(t)); } + else if (OperatingSystem.IsAndroid()) + { + Assert.Equal(72, Marshal.SizeOf(t)); + } else { Assert.Equal(88, Marshal.SizeOf(t)); } Assert.Equal(new IntPtr(0), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.b))); - Assert.Equal(new IntPtr(8), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.p))); + Assert.Equal(OperatingSystem.IsAndroid() && RuntimeInformation.ProcessArchitecture == Architecture.X86 ? new IntPtr(4) : new IntPtr(8), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.p))); if (OperatingSystem.IsWindows() || (RuntimeInformation.ProcessArchitecture != Architecture.X86 && RuntimeInformation.ProcessArchitecture != Architecture.Wasm)) { Assert.Equal(new IntPtr(88), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.s))); } + else if (OperatingSystem.IsAndroid()) + { + Assert.Equal(new IntPtr(68), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.s))); + } else { Assert.Equal(new IntPtr(80), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.s))); @@ -476,26 +484,26 @@ internal struct FieldAlignmentTest public double m_double2; // 8 bytes public byte m_byte3; // 1 byte public byte m_byte4; // 1 byte - // 6 bytes of padding + // 6 bytes of padding (2 bytes on Linux x86) public decimal m_decimal1; // 16 bytes public char m_char4; // 1 byte - // 7 bytes of padding + // 7 bytes of padding (3 bytes on Linux x86) } struct FieldAlignmentTest_Decimal { public byte b; // 1 byte - // 7 bytes of padding + // 7 bytes of padding (3 bytes on Linux x86) // The largest field in below struct is decimal (16 bytes wide). // However, alignment requirement for the below struct should be only 8 bytes (not 16). // This is because unlike fields of other types well known to mcg (like long, char etc.) // which need to be aligned according to their byte size, decimal is really a struct // with 8 byte alignment requirement. - public FieldAlignmentTest p; // 80 bytes (72 bytes on x86/Unix) + public FieldAlignmentTest p; // 80 bytes (72 bytes on Win x86/Unix) (64 bytes on Linux x86) public short s; // 2 bytes - // 6 bytes of padding + // 6 bytes of padding (2 bytes on Linux x86) } struct FieldAlignmentTest_Guid diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 777f2fab392628..68332a921137cc 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5624,7 +5624,9 @@ mono_marshal_type_size (MonoType *type, MonoMarshalSpec *mspec, guint32 *align, } else if (strcmp (m_class_get_name_space (klass), "System") == 0 && strcmp (m_class_get_name (klass), "Decimal") == 0) { - *align = 8; + // Special case: Managed Decimal consists of 4 int32 fields, the alignment should be 8 on x64 to follow + // https://github.com/dotnet/coreclr/blob/4450e5ca663b9e66c20e6f9751c941efa3716fde/src/vm/methodtablebuilder.cpp#L9753 + *align = MONO_ABI_ALIGNOF (gpointer); return mono_class_native_size (klass, NULL); } padded_size = mono_class_native_size (klass, align); From ddbe7c3e956a220b20363e6303445f87c31bc637 Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Tue, 24 Aug 2021 14:13:07 -0400 Subject: [PATCH 5/6] Remove Decimal struct special casing in marshal --- src/mono/mono/metadata/marshal.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/mono/mono/metadata/marshal.c b/src/mono/mono/metadata/marshal.c index 68332a921137cc..ede9551b57459d 100644 --- a/src/mono/mono/metadata/marshal.c +++ b/src/mono/mono/metadata/marshal.c @@ -5621,13 +5621,6 @@ mono_marshal_type_size (MonoType *type, MonoMarshalSpec *mspec, guint32 *align, (mspec && mspec->native == MONO_NATIVE_STRUCT)) { *align = 16; return 16; - } else if (strcmp (m_class_get_name_space (klass), "System") == 0 && - strcmp (m_class_get_name (klass), "Decimal") == 0) { - - // Special case: Managed Decimal consists of 4 int32 fields, the alignment should be 8 on x64 to follow - // https://github.com/dotnet/coreclr/blob/4450e5ca663b9e66c20e6f9751c941efa3716fde/src/vm/methodtablebuilder.cpp#L9753 - *align = MONO_ABI_ALIGNOF (gpointer); - return mono_class_native_size (klass, NULL); } padded_size = mono_class_native_size (klass, align); if (padded_size == 0) From babdf4eb214270fd1bf458fc507a76196ae70751 Mon Sep 17 00:00:00 2001 From: Mitchell Hwang Date: Wed, 25 Aug 2021 09:24:46 -0400 Subject: [PATCH 6/6] Remove wasm special case tests --- .../System/Runtime/InteropServices/Marshal/OffsetOfTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs index 2e08cda77a9e80..4d1353ffd6546b 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/Marshal/OffsetOfTests.cs @@ -110,7 +110,7 @@ public void OffsetOf_Decimal_ReturnsExpected() { Type t = typeof(FieldAlignmentTest_Decimal); - if (OperatingSystem.IsWindows() || (RuntimeInformation.ProcessArchitecture != Architecture.X86 && RuntimeInformation.ProcessArchitecture != Architecture.Wasm)) + if (OperatingSystem.IsWindows() || (RuntimeInformation.ProcessArchitecture != Architecture.X86)) { Assert.Equal(96, Marshal.SizeOf(t)); } @@ -126,7 +126,7 @@ public void OffsetOf_Decimal_ReturnsExpected() Assert.Equal(new IntPtr(0), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.b))); Assert.Equal(OperatingSystem.IsAndroid() && RuntimeInformation.ProcessArchitecture == Architecture.X86 ? new IntPtr(4) : new IntPtr(8), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.p))); - if (OperatingSystem.IsWindows() || (RuntimeInformation.ProcessArchitecture != Architecture.X86 && RuntimeInformation.ProcessArchitecture != Architecture.Wasm)) + if (OperatingSystem.IsWindows() || (RuntimeInformation.ProcessArchitecture != Architecture.X86)) { Assert.Equal(new IntPtr(88), Marshal.OffsetOf(t, nameof(FieldAlignmentTest_Decimal.s))); }