From: Aleksey Kliger (λgeek) Date: Fri, 15 Sep 2017 22:32:04 +0000 (-0400) Subject: [loader] Init MonoClass:sizes.element_size lazily (Fixes #43563) (#5559) X-Git-Url: http://wien.tomnetworks.com/gitweb/?p=mono.git;a=commitdiff_plain;h=e88f9c11f1c432f82b2fb0ff5c319e247b50e2da [loader] Init MonoClass:sizes.element_size lazily (Fixes #43563) (#5559) * [loader] If signaling recursive type initialize, don't double free If we detect that a recursive type initialization is happening, we should leave without removing the class from the init_pending_tld_id list. Otherwise we get a double-free when the outer recursive call finally finishes and tries to remove the same element. Also re-load the init_list from TLS since a recursive call to mono_class_init could have modified it between the time the current mono_class_init call started and when it is finishing. * [loader] Improve error message when a field's type is a failed class Add the class failure to the field failure error message. * [tests] Check that runtime can represent recursive structs Regression tests for https://bugzilla.xamarin.com/show_bug.cgi?id=43563 * [loader] Init MonoClass:sizes.element_size lazily (Fixes #43563) Originally mono_bounded_array_class_get () would populate MonoClass:sizes.element_size as soon as the MonoClass for the array was needed. This is a problem because we cannot call mono_class_array_element_size () on the element class on a valuetype until mono_class_layout_fields () finishes initializing it. That means that structs which contain an array of themselves would hit the recursive initialization check in mono_class_setup_fields and the MonoClass for the element class would be marked as failed to load. Instead we rely on the MonoClass:size_inited bit to tell us whether the array class has been initialized and if not, we set the array element size in mono_class_layout_fields. This is possible because MonoClass:sizes.element_size is only really needed to know how to allocate space for an array. When laying out a class that contains an array we don't need the element size - they array is just a reference type. Example: ```csharp struct S { static S[][] foo; } ``` Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=43563 --- diff --git a/mono/metadata/class.c b/mono/metadata/class.c index 8df8cb531d4..f00d436281a 100644 --- a/mono/metadata/class.c +++ b/mono/metadata/class.c @@ -1513,6 +1513,7 @@ mono_class_set_type_load_failure_causedby_class (MonoClass *klass, const MonoCla * Sets the following fields in \p klass: * - all the fields initialized by mono_class_init_sizes () * - element_class/cast_class (for enums) + * - sizes:element_size (for arrays) * - field->type/offset for all fields * - fields_inited * @@ -1772,6 +1773,7 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_ MonoClassField *field; gboolean blittable; int instance_size = base_instance_size; + int element_size = -1; int class_size, min_align; int *field_offsets; gboolean *fields_has_references; @@ -2101,6 +2103,9 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_ else if (klass->byval_arg.type == MONO_TYPE_PTR) instance_size = sizeof (MonoObject) + sizeof (gpointer); + if (klass->byval_arg.type == MONO_TYPE_SZARRAY || klass->byval_arg.type == MONO_TYPE_ARRAY) + element_size = mono_class_array_element_size (klass->element_class); + /* Publish the data */ mono_loader_lock (); if (klass->instance_size && !klass->image->dynamic) { @@ -2131,6 +2136,9 @@ mono_class_layout_fields (MonoClass *klass, int base_instance_size, int packing_ klass->fields [i].offset = field_offsets [i]; } + if (klass->byval_arg.type == MONO_TYPE_SZARRAY || klass->byval_arg.type == MONO_TYPE_ARRAY) + klass->sizes.element_size = element_size; + mono_memory_barrier (); klass->size_inited = 1; mono_loader_unlock (); @@ -4880,7 +4888,7 @@ mono_class_init (MonoClass *klass) GSList *init_list = (GSList *)mono_native_tls_get_value (init_pending_tls_id); if (g_slist_find (init_list, klass)) { mono_class_set_type_load_failure (klass, "Recursive type definition detected"); - goto leave; + goto leave_no_init_pending; } init_list = g_slist_prepend (init_list, klass); mono_native_tls_set_value (init_pending_tls_id, init_list); @@ -5079,10 +5087,12 @@ mono_class_init (MonoClass *klass) goto leave; - leave: +leave: + init_list = mono_native_tls_get_value (init_pending_tls_id); init_list = g_slist_remove (init_list, klass); mono_native_tls_set_value (init_pending_tls_id, init_list); +leave_no_init_pending: if (locked) mono_loader_unlock (); @@ -6596,7 +6606,7 @@ mono_bounded_array_class_get (MonoClass *eclass, guint32 rank, gboolean bounded) /* element_size -1 is ok as this is not an instantitable type*/ klass->sizes.element_size = -1; } else - klass->sizes.element_size = mono_class_array_element_size (eclass); + klass->sizes.element_size = -1; mono_class_setup_supertypes (klass); @@ -8634,11 +8644,16 @@ handle_enum: * \param ac pointer to a \c MonoArrayClass * * \returns The size of single array element. + * + * LOCKING: Acquires the loader lock. */ gint32 mono_array_element_size (MonoClass *ac) { g_assert (ac->rank); + if (G_UNLIKELY (!ac->size_inited)) { + mono_class_setup_fields (ac); + } return ac->sizes.element_size; } diff --git a/mono/metadata/loader.c b/mono/metadata/loader.c index ea3a441b76b..5281a5d39b8 100644 --- a/mono/metadata/loader.c +++ b/mono/metadata/loader.c @@ -314,9 +314,17 @@ mono_field_from_token_checked (MonoImage *image, guint32 token, MonoClass **retk mono_class_init (k); if (retklass) *retklass = k; - field = mono_class_get_field (k, token); - if (!field) { - mono_error_set_bad_image (error, image, "Could not resolve field token 0x%08x", token); + if (mono_class_has_failure (k)) { + MonoError causedby_error; + error_init (&causedby_error); + mono_error_set_for_class_failure (&causedby_error, k); + mono_error_set_bad_image (error, image, "Could not resolve field token 0x%08x, due to: %s", token, mono_error_get_message (&causedby_error)); + mono_error_cleanup (&causedby_error); + } else { + field = mono_class_get_field (k, token); + if (!field) { + mono_error_set_bad_image (error, image, "Could not resolve field token 0x%08x", token); + } } } diff --git a/mono/metadata/sgen-mono.c b/mono/metadata/sgen-mono.c index 4cd30493fff..ac977c0e590 100644 --- a/mono/metadata/sgen-mono.c +++ b/mono/metadata/sgen-mono.c @@ -398,6 +398,7 @@ get_array_fill_vtable (void) klass.rank = 1; klass.instance_size = MONO_SIZEOF_MONO_ARRAY; klass.sizes.element_size = 1; + klass.size_inited = 1; klass.name = "array_filler_type"; vtable->klass = &klass; diff --git a/mono/tests/Makefile.am b/mono/tests/Makefile.am index a2e3287a244..49e22162884 100755 --- a/mono/tests/Makefile.am +++ b/mono/tests/Makefile.am @@ -517,7 +517,8 @@ TESTS_CS_SRC= \ runtime-invoke.gen.cs \ imt_big_iface_test.cs \ bug-58782-plain-throw.cs \ - bug-58782-capture-and-throw.cs + bug-58782-capture-and-throw.cs \ + recursive-struct-arrays.cs if AMD64 TESTS_CS_SRC += async-exc-compilation.cs finally_guard.cs finally_block_ending_in_dead_bb.cs diff --git a/mono/tests/recursive-struct-arrays.cs b/mono/tests/recursive-struct-arrays.cs new file mode 100644 index 00000000000..dd1c8b262f3 --- /dev/null +++ b/mono/tests/recursive-struct-arrays.cs @@ -0,0 +1,65 @@ +using System; + +/* Test that the runtime can represent value types that have array fields that + * recursively refer to the same value type */ + +struct S1 { + static S1[][] foo; +} + +struct S2 { + static S2[] foo; +} + +struct S3a { + static S3b[] foo; +} + +struct S3b { + static S3a[][] foo; +} + +struct P where X : struct { + static P[][] foo; +} + +public struct S4 +{ + private static S4[][] foo; + + public static readonly S4 West = new S4(-1, 0); + public static readonly S4 East = new S4(1, 0); + public static readonly S4 North = new S4(0, 1); + public static readonly S4 South = new S4(0, -1); + public static readonly S4[] Directions = { North, South, East, West }; + + public readonly int x; + public readonly int z; + + public S4(int x, int z) + { + this.x = x; + this.z = z; + } + + public override string ToString() + { + return string.Format("[{0}, {1}]", x, z); + } +} + + +class Program { + static int Main() { + Console.WriteLine (typeof (S1).Name); + Console.WriteLine (typeof (S2).Name); + Console.WriteLine (typeof (S3a).Name); + Console.WriteLine (typeof (S3b).Name); + foreach (var s4 in S4.Directions) { + Console.WriteLine (s4); + } + Console.WriteLine (typeof (P).Name); + Console.WriteLine (typeof (P).Name); + return 0; + } +}