diff options
author | Aleksey Kliger (λgeek) <akliger@gmail.com> | 2016-08-16 17:13:35 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2016-08-16 17:13:35 +0300 |
commit | d0fc1a66e21eddba20ade505d6880238a0253d9e (patch) | |
tree | 7535732fcfccb17df5200ef1306cec7a3ba57957 | |
parent | d9e045251d57474528dbae0a284a5cd6314f06a3 (diff) |
[reflection] Don't crash when building dynamic custom attributes on dynamic types (Fixes #43291) (#3400)mono-4.6.0.150
* [corlib] Regression test for non-visible custom attributes.
See https://bugzilla.xamarin.com/show_bug.cgi?id=43291
* [reflection] Don't crash when building dynamic custom attributes on dynamic types (Fixes #43291)
1. Don't dereference a NULL custom attribute ctor, throw a type load
exception.
2. When building MonoCustomAttrInfo from an array of
MonoReflectinoCustomAttr*, compute the number of
non-visible (non-public) attributes correctly, and make sure to
iterate over all the attributes when populating the result array.
Fixes [#43291](https://bugzilla.xamarin.com/show_bug.cgi?id=43291)
* [reflection] Marginally better TLE for custom attrs
At least include a message about what went wrong when trying to
construct a custom attribute from a type that isn't finished yet.
-rw-r--r-- | mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs | 63 | ||||
-rw-r--r-- | mono/metadata/reflection.c | 47 |
2 files changed, 92 insertions, 18 deletions
diff --git a/mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs b/mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs index d2a00c76909..eb6eff866c6 100644 --- a/mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs +++ b/mcs/class/corlib/Test/System.Reflection.Emit/CustomAttributeBuilderTest.cs @@ -727,6 +727,69 @@ namespace MonoTests.System.Reflection.Emit Assert.AreEqual ("foo", res [0][0]); Assert.AreEqual ("bar", res [1][0]); } + + [AttributeUsage(AttributeTargets.Class, AllowMultiple = true)] + internal class NonVisibleCustomAttribute : Attribute + { + public NonVisibleCustomAttribute () {} + } + + [AttributeUsage(AttributeTargets.Class, AllowMultiple = true)] + public class PublicVisibleCustomAttribute : Attribute + { + public PublicVisibleCustomAttribute () {} + } + + private static void AddCustomClassAttribute (TypeBuilder typeBuilder, Type customAttrType) + { + var attribCtorParams = new Type[] {}; + var attribCtorInfo = customAttrType.GetConstructor(attribCtorParams); + var attribBuilder = new CustomAttributeBuilder(attribCtorInfo, new object[] { }); + typeBuilder.SetCustomAttribute(attribBuilder); + } + + [Test] + public void NonvisibleCustomAttribute () { + // + // We build: + // [VisiblePublicCustom] + // [VisiblePublicCustom] + // [NonVisibleCustom] + // [VisiblePublicCustom] + // class BuiltType { public BuiltType () { } } + // + // And then we try to get all the attributes. + // + // Regression test for https://bugzilla.xamarin.com/show_bug.cgi?id=43291 + var assemblyName = new AssemblyName("Repro43291Asm"); + var assemblyBuilder = AssemblyBuilder.DefineDynamicAssembly(assemblyName, AssemblyBuilderAccess.Run); + var moduleBuilder = assemblyBuilder.DefineDynamicModule("Repro43291Mod"); + + var typeBuilder = moduleBuilder.DefineType("BuiltType", + TypeAttributes.Public | TypeAttributes.Class | TypeAttributes.BeforeFieldInit); + + AddCustomClassAttribute (typeBuilder, typeof (PublicVisibleCustomAttribute)); + AddCustomClassAttribute (typeBuilder, typeof (PublicVisibleCustomAttribute)); + AddCustomClassAttribute (typeBuilder, typeof (NonVisibleCustomAttribute)); + AddCustomClassAttribute (typeBuilder, typeof (PublicVisibleCustomAttribute)); + + var createdType = typeBuilder.CreateType (); + + Assert.IsNotNull (createdType); + + var obj = Activator.CreateInstance (createdType); + + Assert.IsNotNull (obj); + + var attrs = obj.GetType ().GetCustomAttributes (typeof (Attribute), true); + + Assert.IsNotNull (attrs); + + Assert.AreEqual (3, attrs.Length); + Assert.IsInstanceOfType (typeof (PublicVisibleCustomAttribute), attrs[0]); + Assert.IsInstanceOfType (typeof (PublicVisibleCustomAttribute), attrs[1]); + Assert.IsInstanceOfType (typeof (PublicVisibleCustomAttribute), attrs[2]); + } } } diff --git a/mono/metadata/reflection.c b/mono/metadata/reflection.c index 86ddead705b..9fbac024ad6 100644 --- a/mono/metadata/reflection.c +++ b/mono/metadata/reflection.c @@ -1478,12 +1478,12 @@ mono_custom_attrs_from_builders (MonoImage *alloc_img, MonoImage *image, MonoArr if (!custom_attr_visible (image, cattr)) not_visible ++; } - count -= not_visible; - ainfo = (MonoCustomAttrInfo *)image_g_malloc0 (alloc_img, MONO_SIZEOF_CUSTOM_ATTR_INFO + sizeof (MonoCustomAttrEntry) * count); + int num_attrs = count - not_visible; + ainfo = (MonoCustomAttrInfo *)image_g_malloc0 (alloc_img, MONO_SIZEOF_CUSTOM_ATTR_INFO + sizeof (MonoCustomAttrEntry) * num_attrs); ainfo->image = image; - ainfo->num_attrs = count; + ainfo->num_attrs = num_attrs; ainfo->cached = alloc_img != NULL; index = 0; for (i = 0; i < count; ++i) { @@ -1492,11 +1492,13 @@ mono_custom_attrs_from_builders (MonoImage *alloc_img, MonoImage *image, MonoArr unsigned char *saved = (unsigned char *)mono_image_alloc (image, mono_array_length (cattr->data)); memcpy (saved, mono_array_addr (cattr->data, char, 0), mono_array_length (cattr->data)); ainfo->attrs [index].ctor = cattr->ctor->method; + g_assert (cattr->ctor->method); ainfo->attrs [index].data = saved; ainfo->attrs [index].data_size = mono_array_length (cattr->data); index ++; } } + g_assert (index == num_attrs && count == num_attrs + not_visible); return ainfo; } @@ -9877,24 +9879,35 @@ mono_custom_attrs_construct_by_type (MonoCustomAttrInfo *cinfo, MonoClass *attr_ mono_error_init (error); - n = 0; for (i = 0; i < cinfo->num_attrs; ++i) { - if (!attr_klass || mono_class_is_assignable_from (attr_klass, cinfo->attrs [i].ctor->klass)) - n ++; + MonoCustomAttrEntry *centry = &cinfo->attrs[i]; + if (!centry->ctor) { + /* The cattr type is not finished yet */ + /* We should include the type name but cinfo doesn't contain it */ + mono_error_set_type_load_name (error, NULL, NULL, "Custom attribute constructor is null because the custom attribute type is not finished yet."); + return NULL; + } + } + + n = 0; + if (attr_klass) { + for (i = 0; i < cinfo->num_attrs; ++i) { + MonoMethod *ctor = cinfo->attrs[i].ctor; + g_assert (ctor); + if (mono_class_is_assignable_from (attr_klass, ctor->klass)) + n++; + } + } else { + n = cinfo->num_attrs; } result = mono_array_new_cached (mono_domain_get (), mono_defaults.attribute_class, n, error); return_val_if_nok (error, NULL); n = 0; for (i = 0; i < cinfo->num_attrs; ++i) { - if (!cinfo->attrs [i].ctor) { - /* The cattr type is not finished yet */ - /* We should include the type name but cinfo doesn't contain it */ - mono_error_set_type_load_name (error, NULL, NULL, ""); - return NULL; - } - if (!attr_klass || mono_class_is_assignable_from (attr_klass, cinfo->attrs [i].ctor->klass)) { - attr = create_custom_attr (cinfo->image, cinfo->attrs [i].ctor, cinfo->attrs [i].data, cinfo->attrs [i].data_size, error); + MonoCustomAttrEntry *centry = &cinfo->attrs [i]; + if (!attr_klass || mono_class_is_assignable_from (attr_klass, centry->ctor->klass)) { + attr = create_custom_attr (cinfo->image, centry->ctor, centry->data, centry->data_size, error); if (!mono_error_ok (error)) return result; mono_array_setref (result, n, attr); @@ -10302,9 +10315,8 @@ gboolean mono_custom_attrs_has_attr (MonoCustomAttrInfo *ainfo, MonoClass *attr_klass) { int i; - MonoClass *klass; for (i = 0; i < ainfo->num_attrs; ++i) { - klass = ainfo->attrs [i].ctor->klass; + MonoClass *klass = ainfo->attrs [i].ctor->klass; if (mono_class_has_parent (klass, attr_klass) || (MONO_CLASS_IS_INTERFACE (attr_klass) && mono_class_is_assignable_from (attr_klass, klass))) return TRUE; } @@ -10324,14 +10336,13 @@ MonoObject* mono_custom_attrs_get_attr_checked (MonoCustomAttrInfo *ainfo, MonoClass *attr_klass, MonoError *error) { int i, attr_index; - MonoClass *klass; MonoArray *attrs; mono_error_init (error); attr_index = -1; for (i = 0; i < ainfo->num_attrs; ++i) { - klass = ainfo->attrs [i].ctor->klass; + MonoClass *klass = ainfo->attrs [i].ctor->klass; if (mono_class_has_parent (klass, attr_klass)) { attr_index = i; break; |