diff options
author | Rodrigo Kumpera <kumpera@gmail.com> | 2012-12-01 02:42:39 +0400 |
---|---|---|
committer | Rodrigo Kumpera <kumpera@gmail.com> | 2012-12-01 02:48:07 +0400 |
commit | b392a01a851570a8cdc60c340be098147da74421 (patch) | |
tree | 88a4a8cc5f5a6a779b01293ad17aa7db68be3a3e | |
parent | 41fe9f361ce3e59c612fe5eaebbe6f521b4d3a67 (diff) |
Handle a loader generic type hierarchy corner case. Fixes bxc #8549
* class.c: Handle the case when the parent of a GTD needs a generic
instance of the child type leading to a generic instance with
inconsistent values.
Say we have this set of types:
class TextBoxBase<T> : TextInput<TextBox> where T : TextBoxBase<T> {}
class TextBox : TextBoxBase<TextBox> {}
class TextInput<T> : Input<T> where T: TextInput<T> {}
class Input<T> {}
Now the runtime tries to load TextBoxBase<>, this is the sequence of events:
1)Load type TextBoxBase<>
2.1)Type has parent, find it
2.2)Lookup parent TextInput<TextBox>
3.1)Lookup TextInput<>
4)Load type TextInput<> //OK
3.2)Lookup TextBok
5)Load type TextBox
6.1)Type has parent, find it
6.2)Lookup TextBoxBase<TextBox>
7.1)Lookup TextBoxBase<> //OK
7.2)Lookup TextBox //OK
7.3)Create generic instance TextBoxBase<TextBox> //BOOM
Let's examine what happens in (7.3), we try to create the generic class instance TextBoxBase<TextBox>
which means we must inflate all of its MonoClass fields. One field in particular, parent, will be set to null
because the lookup for TextBoxBase parent in (2.2) still running which means the current value is null.
What does it mean? It means that TextBoxBase<TextBox> extends object and our program will misbehave.
Fixes bxc #8549
-rw-r--r-- | mono/metadata/class.c | 134 |
1 files changed, 116 insertions, 18 deletions
diff --git a/mono/metadata/class.c b/mono/metadata/class.c index bba6afbff9c..6378847ba3a 100644 --- a/mono/metadata/class.c +++ b/mono/metadata/class.c @@ -67,11 +67,77 @@ static char* mono_assembly_name_from_token (MonoImage *image, guint32 type_token static void mono_field_resolve_type (MonoClassField *field, MonoError *error); static guint32 mono_field_resolve_flags (MonoClassField *field); static void mono_class_setup_vtable_full (MonoClass *class, GList *in_setup); +static void mono_generic_class_setup_parent (MonoClass *klass, MonoClass *gklass); void (*mono_debugger_class_init_func) (MonoClass *klass) = NULL; void (*mono_debugger_class_loaded_methods_func) (MonoClass *klass) = NULL; + +/* +We use gclass recording to allow recursive system f types to be referenced by a parent. + +Given the following type hierarchy: + +class TextBox : TextBoxBase<TextBox> {} +class TextBoxBase<T> : TextInput<TextBox> where T : TextBoxBase<T> {} +class TextInput<T> : Input<T> where T: TextInput<T> {} +class Input<T> {} + +The runtime tries to load TextBoxBase<>. +To load TextBoxBase<> to do so it must resolve the parent which is TextInput<TextBox>. +To instantiate TextInput<TextBox> it must resolve TextInput<> and TextBox. +To load TextBox it must resolve the parent which is TextBoxBase<TextBox>. + +At this point the runtime must instantiate TextBoxBase<TextBox>. Both types are partially loaded +at this point, iow, both are registered in the type map and both and a NULL parent. This means +that the resulting generic instance will have a NULL parent, which is wrong and will cause breakage. + +To fix that what we do is to record all generic instantes created while resolving the parent of +any generic type definition and, after resolved, correct the parent field if needed. + +*/ +static int record_gclass_instantiation; +static GSList *gclass_recorded_list; +typedef gboolean (*gclass_record_func) (MonoClass*, void*); + +/* + * LOCKING: loader lock must be held until pairing disable_gclass_recording is called. +*/ +static void +enable_gclass_recording (void) +{ + ++record_gclass_instantiation; +} + +/* + * LOCKING: loader lock must be held since pairing enable_gclass_recording was called. +*/ +static void +disable_gclass_recording (gclass_record_func func, void *user_data) +{ + GSList **head = &gclass_recorded_list; + + g_assert (record_gclass_instantiation > 0); + --record_gclass_instantiation; + + while (*head) { + GSList *node = *head; + if (func ((MonoClass*)node->data, user_data)) { + *head = node->next; + g_slist_free_1 (node); + } else { + head = &node->next; + } + } + + /* We automatically discard all recorded gclasses when disabled. */ + if (!record_gclass_instantiation && gclass_recorded_list) { + g_slist_free (gclass_recorded_list); + gclass_recorded_list = NULL; + } +} + /* * mono_class_from_typeref: * @image: a MonoImage @@ -5343,6 +5409,21 @@ mono_class_setup_supertypes (MonoClass *class) mono_atomic_store_release (&class->supertypes, supertypes); } +static gboolean +fix_gclass_incomplete_instantiation (MonoClass *gclass, void *user_data) +{ + MonoClass *gtd = (MonoClass*)user_data; + /* Only try to fix generic instances of @gtd */ + if (gclass->generic_class->container_class != gtd) + return FALSE; + + /* Check if the generic instance has no parent. */ + if (gtd->parent && !gclass->parent) + mono_generic_class_setup_parent (gclass, gtd); + + return TRUE; +} + /** * mono_class_create_from_typedef: * @image: image where the token is valid @@ -5408,6 +5489,9 @@ mono_class_create_from_typedef (MonoImage *image, guint32 type_token) context = &class->generic_container->context; } + if (class->generic_container) + enable_gclass_recording (); + if (cols [MONO_TYPEDEF_EXTENDS]) { MonoClass *tmp; guint32 parent_token = mono_metadata_token_from_dor (cols [MONO_TYPEDEF_EXTENDS]); @@ -5445,6 +5529,9 @@ mono_class_create_from_typedef (MonoImage *image, guint32 type_token) /* uses ->valuetype, which is initialized by mono_class_setup_parent above */ mono_class_setup_mono_type (class); + if (class->generic_container) + disable_gclass_recording (fix_gclass_incomplete_instantiation, class); + /* * This might access class->byval_arg for recursion generated by generic constraints, * so it has to come after setup_mono_type (). @@ -5581,6 +5668,31 @@ mono_class_get_nullable_param (MonoClass *klass) return mono_class_from_mono_type (klass->generic_class->context.class_inst->type_argv [0]); } +static void +mono_generic_class_setup_parent (MonoClass *klass, MonoClass *gtd) +{ + if (gtd->parent) { + MonoError error; + MonoGenericClass *gclass = klass->generic_class; + + klass->parent = mono_class_inflate_generic_class_checked (gtd->parent, mono_generic_class_get_context (gclass), &error); + if (!mono_error_ok (&error)) { + /*Set parent to something safe as the runtime doesn't handle well this kind of failure.*/ + klass->parent = mono_defaults.object_class; + mono_class_set_failure (klass, MONO_EXCEPTION_TYPE_LOAD, NULL); + mono_error_cleanup (&error); + } + } + if (klass->parent) + mono_class_setup_parent (klass, klass->parent); + + if (klass->enumtype) { + klass->cast_class = gtd->cast_class; + klass->element_class = gtd->element_class; + } +} + + /* * Create the `MonoClass' for an instantiation of a generic type. * We only do this if we actually need it. @@ -5603,6 +5715,9 @@ mono_generic_class_get_class (MonoGenericClass *gclass) gklass = gclass->container_class; + if (record_gclass_instantiation > 0) + gclass_recorded_list = g_slist_append (gclass_recorded_list, klass); + if (gklass->nested_in) { /* The nested_in type should not be inflated since it's possible to produce a nested type with less generic arguments*/ klass->nested_in = gklass->nested_in; @@ -5637,24 +5752,7 @@ mono_generic_class_get_class (MonoGenericClass *gclass) * We use the generic type definition to look for nested classes. */ - if (gklass->parent) { - MonoError error; - klass->parent = mono_class_inflate_generic_class_checked (gklass->parent, mono_generic_class_get_context (gclass), &error); - if (!mono_error_ok (&error)) { - /*Set parent to something safe as the runtime doesn't handle well this kind of failure.*/ - klass->parent = mono_defaults.object_class; - mono_class_set_failure (klass, MONO_EXCEPTION_TYPE_LOAD, NULL); - mono_error_cleanup (&error); - } - } - - if (klass->parent) - mono_class_setup_parent (klass, klass->parent); - - if (klass->enumtype) { - klass->cast_class = gklass->cast_class; - klass->element_class = gklass->element_class; - } + mono_generic_class_setup_parent (klass, gklass); if (gclass->is_dynamic) { klass->inited = 1; |