From 96a4671bc52e70024da409f5f48b0abaa30cb901 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 17 Jun 2021 11:40:36 +0300 Subject: [PATCH] [mono] Fix comparisons between non runtime types (#54062) * [interp] Fix comparisons between non runtime types We know that the result of object.GetType and ldftn + GetTypeFromHandle are runtime types and we track it on the compilation stack. If type equality operator is applied on two runtime types we use reference equality comparison, otherwise we use the managed implementation which uses Type.Equals. * [mini] Fix comparisons between non runtime types We know that the result of object.GetType and ldftn + GetTypeFromHandle are runtime types and we track it on the compilation stack. If type equality operator is applied on two runtime types we use reference equality comparison, otherwise we use the managed implementation which uses Type.Equals. * [interp] Resolve GetType on constrained valuetype to the actual type --- .../System.Private.CoreLib/src/System/Type.Mono.cs | 17 +++++++- src/mono/mono/mini/interp/transform.c | 48 +++++++++++++++++++--- src/mono/mono/mini/intrinsics.c | 11 +++-- src/mono/mono/mini/method-to-ir.c | 8 ++-- src/mono/mono/mini/mini.h | 2 +- 5 files changed, 73 insertions(+), 13 deletions(-) diff --git a/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs b/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs index 7fa48ec..4179127 100644 --- a/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs +++ b/src/mono/System.Private.CoreLib/src/System/Type.Mono.cs @@ -135,7 +135,22 @@ namespace System private static extern Type internal_from_handle(IntPtr handle); [Intrinsic] - public static bool operator ==(Type? left, Type? right) => left == right; + public static bool operator ==(Type? left, Type? right) + { + if (object.ReferenceEquals(left, right)) + return true; + + if (left is null || right is null) + return false; + + // CLR-compat: runtime types are never equal to non-runtime types + // If `left` is a non-runtime type with a weird Equals implementation + // this is where operator `==` would differ from `Equals` call. + if (left.IsRuntimeImplemented() || right.IsRuntimeImplemented()) + return false; + + return left.Equals(right); + } public static bool operator !=(Type? left, Type? right) { diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 29d1dec..e465125 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -2424,13 +2424,51 @@ interp_handle_intrinsics (TransformData *td, MonoMethod *target_method, MonoClas } else if (in_corlib && !strcmp (klass_name_space, "System") && !strcmp (klass_name, "RuntimeMethodHandle") && !strcmp (tm, "GetFunctionPointer") && csignature->param_count == 1) { // We must intrinsify this method on interp so we don't return a pointer to native code entering interpreter *op = MINT_LDFTN_DYNAMIC; - } else if (in_corlib && target_method->klass == mono_defaults.systemtype_class && !strcmp (target_method->name, "op_Equality")) { + } else if (in_corlib && target_method->klass == mono_defaults.systemtype_class && !strcmp (target_method->name, "op_Equality") && + td->sp [-1].klass == mono_defaults.runtimetype_class && td->sp [-2].klass == mono_defaults.runtimetype_class) { + // We do a reference comparison only if we know both operands are runtime type + // (they originate from object.GetType or ldftn + GetTypeFromHandle) *op = MINT_CEQ_P; } else if (in_corlib && target_method->klass == mono_defaults.object_class) { - if (!strcmp (tm, "InternalGetHashCode")) + if (!strcmp (tm, "InternalGetHashCode")) { *op = MINT_INTRINS_GET_HASHCODE; - else if (!strcmp (tm, "GetType")) - *op = MINT_INTRINS_GET_TYPE; + } else if (!strcmp (tm, "GetType")) { + if (constrained_class && m_class_is_valuetype (constrained_class)) { + // If constrained_class is valuetype we already know its type. + // Resolve GetType to a constant so we can fold type comparisons + ERROR_DECL(error); + gpointer systype = mono_type_get_object_checked (m_class_get_byval_arg (constrained_class), error); + return_val_if_nok (error, FALSE); + + td->sp--; + interp_add_ins (td, MINT_MONO_LDPTR); + push_type (td, STACK_TYPE_O, mono_defaults.runtimetype_class); + interp_ins_set_dreg (td->last_ins, td->sp [-1].local); + td->last_ins->data [0] = get_data_item_index (td, systype); + + td->ip += 5; + return TRUE; + } else { + if (constrained_class) { + // deref the managed pointer to get the object + interp_add_ins (td, MINT_LDIND_I); + td->sp--; + interp_ins_set_sreg (td->last_ins, td->sp [0].local); + push_simple_type (td, STACK_TYPE_O); + interp_ins_set_dreg (td->last_ins, td->sp [-1].local); + } + interp_add_ins (td, MINT_INTRINS_GET_TYPE); + td->sp--; + interp_ins_set_sreg (td->last_ins, td->sp [0].local); + push_type (td, STACK_TYPE_O, mono_defaults.runtimetype_class); + interp_ins_set_dreg (td->last_ins, td->sp [-1].local); + + mono_class_init_internal (target_method->klass); + + td->ip += 5; + return TRUE; + } + } } else if (in_corlib && target_method->klass == mono_defaults.enum_class && !strcmp (tm, "HasFlag")) { gboolean intrinsify = FALSE; MonoClass *base_klass = NULL; @@ -6733,7 +6771,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header, interp_add_ins (td, MINT_MONO_LDPTR); gpointer systype = mono_type_get_object_checked ((MonoType*)handle, error); goto_if_nok (error, exit); - push_simple_type (td, STACK_TYPE_MP); + push_type (td, STACK_TYPE_O, mono_defaults.runtimetype_class); interp_ins_set_dreg (td->last_ins, td->sp [-1].local); td->last_ins->data [0] = get_data_item_index (td, systype); td->ip = next_ip + 5; diff --git a/src/mono/mono/mini/intrinsics.c b/src/mono/mono/mini/intrinsics.c index 948fd5d..5ad1eb4 100644 --- a/src/mono/mono/mini/intrinsics.c +++ b/src/mono/mono/mini/intrinsics.c @@ -665,11 +665,13 @@ byref_arg_is_reference (MonoType *t) } MonoInst* -mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig, MonoInst **args) +mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig, MonoInst **args, gboolean *ins_type_initialized) { MonoInst *ins = NULL; MonoClass *runtime_helpers_class = mono_class_get_runtime_helpers_class (); + *ins_type_initialized = FALSE; + const char* cmethod_klass_name_space; if (m_class_get_nested_in (cmethod->klass)) cmethod_klass_name_space = m_class_get_name_space (m_class_get_nested_in (cmethod->klass)); @@ -746,7 +748,9 @@ mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSign MONO_EMIT_NEW_LOAD_MEMBASE_FAULT (cfg, vt_reg, args [0]->dreg, MONO_STRUCT_OFFSET (MonoObject, vtable)); EMIT_NEW_LOAD_MEMBASE (cfg, ins, OP_LOAD_MEMBASE, dreg, vt_reg, MONO_STRUCT_OFFSET (MonoVTable, type)); mini_type_from_op (cfg, ins, NULL, NULL); - + mini_type_to_eval_stack_type (cfg, fsig->ret, ins); + ins->klass = mono_defaults.runtimetype_class; + *ins_type_initialized = TRUE; return ins; } else if (!cfg->backend->emulate_mul_div && strcmp (cmethod->name, "InternalGetHashCode") == 0 && fsig->param_count == 1 && !mono_gc_is_moving ()) { int dreg = alloc_ireg (cfg); @@ -1801,7 +1805,8 @@ mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSign return ins; } } - } else if (cmethod->klass == mono_defaults.systemtype_class && !strcmp (cmethod->name, "op_Equality")) { + } else if (cmethod->klass == mono_defaults.systemtype_class && !strcmp (cmethod->name, "op_Equality") && + args [0]->klass == mono_defaults.runtimetype_class && args [1]->klass == mono_defaults.runtimetype_class) { EMIT_NEW_BIALU (cfg, ins, OP_COMPARE, -1, args [0]->dreg, args [1]->dreg); MONO_INST_NEW (cfg, ins, OP_PCEQ); ins->dreg = alloc_preg (cfg); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 02ca3ba..964d31d 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -7392,9 +7392,11 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b } /* Conversion to a JIT intrinsic */ - if ((ins = mini_emit_inst_for_method (cfg, cmethod, fsig, sp))) { + gboolean ins_type_initialized; + if ((ins = mini_emit_inst_for_method (cfg, cmethod, fsig, sp, &ins_type_initialized))) { if (!MONO_TYPE_IS_VOID (fsig->ret)) { - mini_type_to_eval_stack_type ((cfg), fsig->ret, ins); + if (!ins_type_initialized) + mini_type_to_eval_stack_type ((cfg), fsig->ret, ins); emit_widen = FALSE; } // FIXME This is only missed if in fact the intrinsic involves a call. @@ -10258,7 +10260,7 @@ field_access_end: EMIT_NEW_PCONST (cfg, ins, rt); } ins->type = STACK_OBJ; - ins->klass = cmethod->klass; + ins->klass = mono_defaults.runtimetype_class; il_op = (MonoOpcodeEnum)next_ip [0]; next_ip += 5; } else { diff --git a/src/mono/mono/mini/mini.h b/src/mono/mono/mini/mini.h index f953cd2..8d55a61 100644 --- a/src/mono/mono/mini/mini.h +++ b/src/mono/mono/mini/mini.h @@ -2301,7 +2301,7 @@ void mini_emit_memory_copy_bytes (MonoCompile *cfg, MonoInst *dest, void mini_emit_memory_init_bytes (MonoCompile *cfg, MonoInst *dest, MonoInst *value, MonoInst *size, int ins_flag); void mini_emit_memory_copy (MonoCompile *cfg, MonoInst *dest, MonoInst *src, MonoClass *klass, gboolean native, int ins_flag); MonoInst* mini_emit_array_store (MonoCompile *cfg, MonoClass *klass, MonoInst **sp, gboolean safety_checks); -MonoInst* mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig, MonoInst **args); +MonoInst* mini_emit_inst_for_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig, MonoInst **args, gboolean *ins_type_initialized); MonoInst* mini_emit_inst_for_ctor (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig, MonoInst **args); MonoInst* mini_emit_inst_for_field_load (MonoCompile *cfg, MonoClassField *field); MonoInst* mini_handle_enum_has_flag (MonoCompile *cfg, MonoClass *klass, MonoInst *enum_this, int enum_val_reg, MonoInst *enum_flag); -- 2.7.4