Skip to content

Commit 3c75627

Browse files
committed
Fix invalid check on value types (UUM-79115).
The previously inserted null check on 'this' assumed, at least on amd64, that the pointer could be dereferenced as a pointer sized value. This is not true for value types which can be smaller than pointer sized, especially if the memory adjacent to the value type is protected. Rather than improving the null check, we can avoid it entirely for value type methods as they should never have a NULL 'this' pointer.
1 parent 2466afe commit 3c75627

File tree

1 file changed

+12
-7
lines changed

1 file changed

+12
-7
lines changed

mono/mini/method-to-ir.c

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4601,7 +4601,7 @@ inline_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig,
46014601
guint32 prev_cil_offset_to_bb_len;
46024602
MonoMethod *prev_current_method;
46034603
MonoGenericContext *prev_generic_context;
4604-
gboolean ret_var_set, prev_ret_var_set, prev_disable_inline, virtual_ = FALSE;
4604+
gboolean ret_var_set, prev_ret_var_set, prev_disable_inline, emit_check_this = FALSE;
46054605
MonoProfilerCoverageInfo *prev_coverage_info;
46064606

46074607
g_assert (cfg->exception_type == MONO_EXCEPTION_NONE);
@@ -4681,10 +4681,15 @@ inline_method (MonoCompile *cfg, MonoMethod *cmethod, MonoMethodSignature *fsig,
46814681
cfg->ret_var_set = FALSE;
46824682
cfg->inline_depth ++;
46834683

4684-
if (ip && *ip == CEE_CALLVIRT && !(cmethod->flags & METHOD_ATTRIBUTE_STATIC))
4685-
virtual_ = TRUE;
4684+
if (ip && *ip == CEE_CALLVIRT && !(cmethod->flags & METHOD_ATTRIBUTE_STATIC) &&
4685+
// Only perform a null check on 'this' arguments for methods on:
4686+
// 1. Reference types
4687+
// 2. System.ValueType and System.Enum as those will have a boxed 'this' argument.
4688+
// NOTE: klass->valuetype is FALSE for System.ValueType so we only need to check System.Enum.
4689+
(!m_class_is_valuetype(cmethod->klass) || m_class_is_enumtype(cmethod->klass)))
4690+
emit_check_this = TRUE;
46864691

4687-
costs = mono_method_to_ir (cfg, cmethod, sbblock, ebblock, rvar, sp, real_offset, virtual_);
4692+
costs = mono_method_to_ir (cfg, cmethod, sbblock, ebblock, rvar, sp, real_offset, emit_check_this);
46884693

46894694
ret_var_set = cfg->ret_var_set;
46904695

@@ -6116,7 +6121,7 @@ mono_opcode_decode (guchar *ip, guint op_size, MonoOpcodeEnum il_op, MonoOpcodeP
61166121
* @return_var: if not NULL, the place where the return value is stored, used during inlining.
61176122
* @inline_args: if not NULL, contains the arguments to the inline call
61186123
* @inline_offset: if not zero, the real offset from the inline call, or zero otherwise.
6119-
* @is_virtual_call: whether this method is being called as a result of a call to callvirt
6124+
* @emit_check_this: whether this method needs to perform an explicit this null check.
61206125
*
61216126
* This method is used to turn ECMA IL into Mono's internal Linear IR
61226127
* reprensetation. It is used both for entire methods, as well as
@@ -6129,7 +6134,7 @@ mono_opcode_decode (guchar *ip, guint op_size, MonoOpcodeEnum il_op, MonoOpcodeP
61296134
int
61306135
mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_bblock, MonoBasicBlock *end_bblock,
61316136
MonoInst *return_var, MonoInst **inline_args,
6132-
guint inline_offset, gboolean is_virtual_call)
6137+
guint inline_offset, gboolean emit_check_this)
61336138
{
61346139
ERROR_DECL (error);
61356140
// Buffer to hold parameters to mono_new_array, instead of varargs.
@@ -6603,7 +6608,7 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b
66036608
}
66046609

66056610
/* add a check for this != NULL to inlined methods */
6606-
if (is_virtual_call) {
6611+
if (emit_check_this) {
66076612
MonoInst *arg_ins;
66086613

66096614
NEW_ARGLOAD (cfg, arg_ins, 0);

0 commit comments

Comments
 (0)