From a5a26a2f92d00ab174d794f077007faaab5d3843 Mon Sep 17 00:00:00 2001 From: Ryan Lucia Date: Thu, 6 Jun 2019 15:38:57 -0400 Subject: [PATCH] Add logic to object array typecheck to handle arrays of unmanaged pointers (#14733) Fixes #14555 The NULL check alone is not enough, because interfaces are also parented to NULL for some reason, and so I've moved `class_kind` out from the bitfield section to make it accessible from IR. The beginning of the struct is going to be padded to 32 bits anyway at the least, so this move should save space if anything. Our type checking overall is not very robust, and in particular the supertype comparison doesn't work well for anything that isn't an object, so it's possible there are some other weird corner cases. As far as I can tell looking through the array of objects stuff though, we should have reasonable coverage with the existing tests + the stuff I added. --- mono/metadata/class-getters.h | 3 ++- mono/metadata/class-internals.h | 2 +- mono/metadata/class-private-definition.h | 6 ++++-- mono/metadata/class.c | 5 ++++- mono/metadata/object-offsets.h | 1 + mono/mini/objects.cs | 19 +++++++++++++++++++ mono/mini/type-checking.c | 21 +++++++++++++++++++-- mono/tests/assignable-tests.cs | 16 ++++++++++++++++ 8 files changed, 66 insertions(+), 7 deletions(-) diff --git a/mono/metadata/class-getters.h b/mono/metadata/class-getters.h index 77845188475..cf358dd9b13 100644 --- a/mono/metadata/class-getters.h +++ b/mono/metadata/class-getters.h @@ -44,7 +44,7 @@ MONO_CLASS_GETTER(m_class_has_static_refs, gboolean, , MonoClass, has_static_ref MONO_CLASS_GETTER(m_class_has_no_special_static_fields, gboolean, , MonoClass, no_special_static_fields) MONO_CLASS_GETTER(m_class_is_com_object, gboolean, , MonoClass, is_com_object) MONO_CLASS_GETTER(m_class_is_nested_classes_inited, gboolean, , MonoClass, nested_classes_inited) -MONO_CLASS_GETTER(m_class_get_class_kind, guint, , MonoClass, class_kind) +MONO_CLASS_GETTER(m_class_get_class_kind, guint8, , MonoClass, class_kind) MONO_CLASS_GETTER(m_class_is_interfaces_inited, gboolean, , MonoClass, interfaces_inited) MONO_CLASS_GETTER(m_class_is_simd_type, gboolean, , MonoClass, simd_type) MONO_CLASS_GETTER(m_class_is_has_finalize_inited, gboolean, , MonoClass, has_finalize_inited) @@ -117,3 +117,4 @@ MONO_CLASS_OFFSET(m_class_offsetof_parent, MonoClass, parent) MONO_CLASS_OFFSET(m_class_offsetof_rank, MonoClass, rank) MONO_CLASS_OFFSET(m_class_offsetof_sizes, MonoClass, sizes) MONO_CLASS_OFFSET(m_class_offsetof_supertypes, MonoClass, supertypes) +MONO_CLASS_OFFSET(m_class_offsetof_class_kind, MonoClass, class_kind) diff --git a/mono/metadata/class-internals.h b/mono/metadata/class-internals.h index 02192ddc32b..82aee73bc02 100644 --- a/mono/metadata/class-internals.h +++ b/mono/metadata/class-internals.h @@ -248,7 +248,7 @@ typedef enum { MONO_CLASS_GINST, /* generic instantiation */ MONO_CLASS_GPARAM, /* generic parameter */ MONO_CLASS_ARRAY, /* vector or array, bounded or not */ - MONO_CLASS_POINTER, /* pointer of function pointer*/ + MONO_CLASS_POINTER, /* pointer or function pointer*/ } MonoTypeKind; typedef struct _MonoClassDef MonoClassDef; diff --git a/mono/metadata/class-private-definition.h b/mono/metadata/class-private-definition.h index 534300035c3..9191bd0353f 100644 --- a/mono/metadata/class-private-definition.h +++ b/mono/metadata/class-private-definition.h @@ -22,7 +22,10 @@ struct _MonoClass { guint16 idepth; /* array dimension */ - guint8 rank; + guint8 rank; + + /* One of the values from MonoTypeKind */ + guint8 class_kind; int instance_size; /* object instance size */ @@ -74,7 +77,6 @@ struct _MonoClass { guint nested_classes_inited : 1; /* Whenever nested_class is initialized */ /* next byte*/ - guint class_kind : 3; /* One of the values from MonoTypeKind */ guint interfaces_inited : 1; /* interfaces is initialized */ guint simd_type : 1; /* class is a simd intrinsic type */ guint has_finalize_inited : 1; /* has_finalize is initialized */ diff --git a/mono/metadata/class.c b/mono/metadata/class.c index 8eae50bebd4..fe53774791f 100644 --- a/mono/metadata/class.c +++ b/mono/metadata/class.c @@ -3835,7 +3835,10 @@ mono_class_is_assignable_from_checked (MonoClass *klass, MonoClass *oklass, gboo mono_class_is_assignable_from_checked (m_class_get_cast_class (klass), oklass, result, error); return; } else if (klass == mono_defaults.object_class) { - *result = TRUE; + if (m_class_get_class_kind (oklass) == MONO_CLASS_POINTER) + *result = FALSE; + else + *result = TRUE; return; } diff --git a/mono/metadata/object-offsets.h b/mono/metadata/object-offsets.h index ec4e2cd15ea..4a00df05bd8 100644 --- a/mono/metadata/object-offsets.h +++ b/mono/metadata/object-offsets.h @@ -67,6 +67,7 @@ DECL_OFFSET(MonoClass, parent) DECL_OFFSET(MonoClass, rank) DECL_OFFSET(MonoClass, sizes) DECL_OFFSET(MonoClass, supertypes) +DECL_OFFSET(MonoClass, class_kind) DECL_OFFSET(MonoVTable, klass) DECL_OFFSET(MonoVTable, max_interface_id) diff --git a/mono/mini/objects.cs b/mono/mini/objects.cs index ed72ec7fe0c..44568ba0e88 100644 --- a/mono/mini/objects.cs +++ b/mono/mini/objects.cs @@ -841,6 +841,25 @@ class Tests { return 0; } + public static unsafe int test_0_pointer_array() { + int*[] ipa = new int* [0]; + Array a = ipa; + + + if (a is object[]) + return 1; + if (!(a is int*[])) + return 2; + if (a is ValueType[]) + return 3; + if (a is Enum[]) + return 4; + if (a is char*[]) + return 5; + + return 0; + } + private static int[] daysmonthleap = { 0, 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 }; private static int AbsoluteDays (int year, int month, int day) diff --git a/mono/mini/type-checking.c b/mono/mini/type-checking.c index e950d573bcc..33638d8d6b2 100644 --- a/mono/mini/type-checking.c +++ b/mono/mini/type-checking.c @@ -682,11 +682,28 @@ handle_isinst (MonoCompile *cfg, MonoClass *klass, MonoInst *src, int context_us EMIT_NEW_UNALU (cfg, move, OP_MOVE, res_reg, res_inst->dreg); MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_BR, end_bb); } else if (m_class_get_cast_class (klass) == mono_defaults.object_class) { - int parent_reg = alloc_preg (cfg); + int parent_reg, class_kind_reg; + MonoBasicBlock *pointer_check_bb; + + NEW_BBLOCK (cfg, pointer_check_bb); + + parent_reg = alloc_preg (cfg); + class_kind_reg = alloc_preg (cfg); MONO_EMIT_NEW_LOAD_MEMBASE (cfg, parent_reg, eclass_reg, m_class_offsetof_parent ()); - mini_emit_class_check_branch (cfg, parent_reg, m_class_get_parent (mono_defaults.enum_class), OP_PBNE_UN, is_null_bb); + MONO_EMIT_NEW_LOAD_MEMBASE_OP (cfg, OP_LOADU1_MEMBASE, class_kind_reg, eclass_reg, m_class_offsetof_class_kind ()); + + // Check if the parent class of the element is not System.ValueType + mini_emit_class_check_branch (cfg, parent_reg, m_class_get_parent (mono_defaults.enum_class), OP_PBNE_UN, pointer_check_bb); mini_emit_class_check_branch (cfg, eclass_reg, mono_defaults.enum_class, OP_PBEQ, is_null_bb); MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_BR, false_bb); + + MONO_START_BB (cfg, pointer_check_bb); + // Check if the parent class of the element is non-null, else manually check the type + MONO_EMIT_NEW_BIALU_IMM (cfg, OP_COMPARE_IMM, -1, parent_reg, NULL); + MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBNE_UN, is_null_bb); + MONO_EMIT_NEW_BIALU_IMM (cfg, OP_COMPARE_IMM, -1, class_kind_reg, MONO_CLASS_POINTER); + MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_PBEQ, false_bb); + MONO_EMIT_NEW_BRANCH_BLOCK (cfg, OP_BR, is_null_bb); } else if (m_class_get_cast_class (klass) == m_class_get_parent (mono_defaults.enum_class)) { mini_emit_class_check_branch (cfg, eclass_reg, m_class_get_parent (mono_defaults.enum_class), OP_PBEQ, is_null_bb); mini_emit_class_check_branch (cfg, eclass_reg, mono_defaults.enum_class, OP_PBEQ, is_null_bb); diff --git a/mono/tests/assignable-tests.cs b/mono/tests/assignable-tests.cs index f1513829a16..91f15ddc05a 100644 --- a/mono/tests/assignable-tests.cs +++ b/mono/tests/assignable-tests.cs @@ -12,10 +12,13 @@ namespace Test { public static int Main () { Type int_type = typeof (int); Type obj_type = typeof (object); + Type obj_arr_type = typeof (object[]); Type vt_type = typeof (System.ValueType); Type comp_type = typeof (System.IComparable); Type a_type = typeof (A); Type b_type = typeof (B); + Type ptr_type = typeof (int*); + Type ptr_arr_type = typeof (int*[]); int error = 1; if (!int_type.IsSubclassOf(vt_type)) @@ -63,6 +66,19 @@ namespace Test { if (!a_type.IsAssignableFrom(b_type)) return error; ++error; + + if (obj_type.IsAssignableFrom(ptr_type)) + return error; + ++error; + if (ptr_type.IsAssignableFrom(obj_type)) + return error; + ++error; + if (obj_arr_type.IsAssignableFrom(ptr_arr_type)) + return error; + ++error; + if (ptr_arr_type.IsAssignableFrom(obj_arr_type)) + return error; + ++error; return 0; } } -- 2.11.4.GIT