Hi, Rodrigo Kumpera <kump...@gmail.com> writes:
> The attached patch changes class.c/inflate_generic_type to not abort the > runtime when facing a bad instantiation. > > My only issue is that I'm not sure if mono_class_inflate_generic_type* and > mono_class_inflate_generic_class should > set a loader error when returning null. I don't think this is necessary as > this is something their callers should do. But maybe > mono_class_inflate_generic_method_full should. > > This patch doesn't fix the whole thing as an audit of all callers of > mono_class_inflate_generic_type(_with_mempool | _no_copy) > and mono_class_inflate_generic_class are required to property handle them > returning NULL. > > I got some local tests for these errors, but they are not in good shape for > been part of this patch. [snip] > +static gboolean > +inflate_generic_type (MonoImage *image, MonoType *type, MonoGenericContext > *context, MonoType **res) > { > +#define SUCCESS(VAL) do { *res = VAL; return TRUE; } while (0) > +#define ERROR() do { *res = NULL; return FALSE; } while (0) > switch (type->type) { > case MONO_TYPE_MVAR: { > MonoType *nt; > int num = mono_type_get_generic_param_num (type); > MonoGenericInst *inst = context->method_inst; > if (!inst || !inst->type_argv) > - return NULL; > + SUCCESS (NULL); > if (num >= inst->type_argc) > - g_error ("MVAR %d (%s) cannot be expanded in this > context with %d instantiations", > - num, mono_generic_param_info > (type->data.generic_param)->name, inst->type_argc); > - > + ERROR (); Hmm, so much machinery for the one use of ERROR! I think the issue is that we're forced to intertwine this particular error check in the middle of code dealing with the mechanics of inflating. It'd be much nicer for inflate_generic_type to have the precondition that no VAR or MVAR in 'type' will be out-of-bounds WRT 'context'. The problem is that this precondition check is currently expensive, as it would duplicate the same recursive traversal. However, we _can_ and _should_ make it non-recursive -- we can replace the field and computation of MonoGenericInst::is_open with something like MonoGenericInst::min_context_size (yeah that name is horribly bad. I've been putting of writing that code since I couldn't get a better name). The reason I think this is better is that we already have code in JIT and the verifier that need only that information (i.e. are equivalent to that precondition check), but are forced to walk the MonoType tree because 'is_open' is too limited. - Hari _______________________________________________ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list