Skip to content

Commit 4d76ea5

Browse files
jeanclaudegrenierBrzVladalhad-deshpande
authored
Fix/math operation error uum 85488 (#2086)
* [mono] Fix a few corner case overflow operations (#58114) * [mono][jit] Fix invalid uses of SHL instead of MUL optimization mono_is_power_of_two is meant to be used only on unsigned numbers. In some cases we pass a signed value instead. This is typically not a problem because negative numbers are not detected as a power of two, except for the special number -2147483648, which is coded as 0x80000000, therefore a power of two. * Enable tests * [interp] Fix a few overflow conversions Floating point numbers are truncated towards 0 when converted to integers, therefore we need to increase the overflow range. For example, (int8)-128.5 = -128, without overflows, even if -128.5 is smaller than the minimum integer. Stop relying on undefined behavior by casting from floating point to integral without range checks. * [interp] Extract some conversion logic in mono-math.h Later we should reuse these methods also with jit * Avoid transformation from multiplication to left shift in case of 64 bit value (#71189) * Cherry-picked 2 commits from the upstream repo to fix issue with math operations, this change removes a dependency on another commit. --------- Co-authored-by: Vlad Brezae <brezaevlad@gmail.com> Co-authored-by: Alhad Deshpande <97085048+alhad-deshpande@users.noreply.github.com>
1 parent 54d13da commit 4d76ea5

File tree

6 files changed

+74
-40
lines changed

6 files changed

+74
-40
lines changed

‎mono/mini/interp/interp.c

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5534,17 +5534,15 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs
55345534
}
55355535
MINT_IN_CASE(MINT_CONV_OVF_U8_R4) {
55365536
float val = LOCAL_VAR (ip [2], float);
5537-
if (mono_isnan (val) || mono_trunc (val) != (guint64)val)
5537+
if (!mono_try_trunc_u64 (val, (guint64*)(locals + ip [1])))
55385538
THROW_EX (mono_get_exception_overflow (), ip);
5539-
LOCAL_VAR (ip [1], guint64) = (guint64)val;
55405539
ip += 3;
55415540
MINT_IN_BREAK;
55425541
}
55435542
MINT_IN_CASE(MINT_CONV_OVF_U8_R8) {
55445543
double val = LOCAL_VAR (ip [2], double);
5545-
if (mono_isnan (val) || mono_trunc (val) != (guint64)val)
5544+
if (!mono_try_trunc_u64 (val, (guint64*)(locals + ip [1])))
55465545
THROW_EX (mono_get_exception_overflow (), ip);
5547-
LOCAL_VAR (ip [1], guint64) = (guint64)val;
55485546
ip += 3;
55495547
MINT_IN_BREAK;
55505548
}
@@ -5566,17 +5564,15 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs
55665564
}
55675565
MINT_IN_CASE(MINT_CONV_OVF_I8_R4) {
55685566
float val = LOCAL_VAR (ip [2], float);
5569-
if (mono_isnan (val) || mono_trunc (val) != (gint64)val)
5567+
if (!mono_try_trunc_i64 (val, (gint64*)(locals + ip [1])))
55705568
THROW_EX (mono_get_exception_overflow (), ip);
5571-
LOCAL_VAR (ip [1], gint64) = (gint64)val;
55725569
ip += 3;
55735570
MINT_IN_BREAK;
55745571
}
55755572
MINT_IN_CASE(MINT_CONV_OVF_I8_R8) {
55765573
double val = LOCAL_VAR (ip [2], double);
5577-
if (mono_isnan (val) || mono_trunc (val) != (gint64)val)
5574+
if (!mono_try_trunc_i64 (val, (gint64*)(locals + ip [1])))
55785575
THROW_EX (mono_get_exception_overflow (), ip);
5579-
LOCAL_VAR (ip [1], gint64) = (gint64)val;
55805576
ip += 3;
55815577
MINT_IN_BREAK;
55825578
}
@@ -5878,17 +5874,20 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs
58785874
}
58795875
MINT_IN_CASE(MINT_CONV_OVF_I4_R4) {
58805876
float val = LOCAL_VAR (ip [2], float);
5881-
if (mono_isnan (val) || mono_trunc (val) != (gint32)val)
5877+
double val_r8 = (double)val;
5878+
if (val_r8 > ((double)G_MININT32 - 1) && val_r8 < ((double)G_MAXINT32 + 1))
5879+
LOCAL_VAR (ip [1], gint32) = (gint32) val;
5880+
else
58825881
THROW_EX (mono_get_exception_overflow (), ip);
5883-
LOCAL_VAR (ip [1], gint32) = (gint32) val;
58845882
ip += 3;
58855883
MINT_IN_BREAK;
58865884
}
58875885
MINT_IN_CASE(MINT_CONV_OVF_I4_R8) {
58885886
double val = LOCAL_VAR (ip [2], double);
5889-
if (val < G_MININT32 || val > G_MAXINT32 || isnan (val))
5887+
if (val > ((double)G_MININT32 - 1) && val < ((double)G_MAXINT32 + 1))
5888+
LOCAL_VAR (ip [1], gint32) = (gint32) val;
5889+
else
58905890
THROW_EX (mono_get_exception_overflow (), ip);
5891-
LOCAL_VAR (ip [1], gint32) = (gint32)val;
58925891
ip += 3;
58935892
MINT_IN_BREAK;
58945893
}
@@ -5910,17 +5909,20 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs
59105909
}
59115910
MINT_IN_CASE(MINT_CONV_OVF_U4_R4) {
59125911
float val = LOCAL_VAR (ip [2], float);
5913-
if (mono_isnan (val) || mono_trunc (val) != (guint32)val)
5912+
double val_r8 = val;
5913+
if (val_r8 > -1.0 && val_r8 < ((double)G_MAXUINT32 + 1))
5914+
LOCAL_VAR (ip [1], gint32) = (guint32)val;
5915+
else
59145916
THROW_EX (mono_get_exception_overflow (), ip);
5915-
LOCAL_VAR (ip [1], gint32) = (guint32)val;
59165917
ip += 3;
59175918
MINT_IN_BREAK;
59185919
}
59195920
MINT_IN_CASE(MINT_CONV_OVF_U4_R8) {
59205921
double val = LOCAL_VAR (ip [2], double);
5921-
if (val < 0 || val > G_MAXUINT32 || isnan (val))
5922+
if (val > -1.0 && val < ((double)G_MAXUINT32 + 1))
5923+
LOCAL_VAR (ip [1], gint32) = (guint32)val;
5924+
else
59225925
THROW_EX (mono_get_exception_overflow (), ip);
5923-
LOCAL_VAR (ip [1], gint32) = (guint32) val;
59245926
ip += 3;
59255927
MINT_IN_BREAK;
59265928
}
@@ -5958,17 +5960,19 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs
59585960
}
59595961
MINT_IN_CASE(MINT_CONV_OVF_I2_R4) {
59605962
float val = LOCAL_VAR (ip [2], float);
5961-
if (val < G_MININT16 || val > G_MAXINT16 || isnan (val))
5963+
if (val > (G_MININT16 - 1) && val < (G_MAXINT16 + 1))
5964+
LOCAL_VAR (ip [1], gint32) = (gint16) val;
5965+
else
59625966
THROW_EX (mono_get_exception_overflow (), ip);
5963-
LOCAL_VAR (ip [1], gint32) = (gint16) val;
59645967
ip += 3;
59655968
MINT_IN_BREAK;
59665969
}
59675970
MINT_IN_CASE(MINT_CONV_OVF_I2_R8) {
59685971
double val = LOCAL_VAR (ip [2], double);
5969-
if (val < G_MININT16 || val > G_MAXINT16 || isnan (val))
5972+
if (val > (G_MININT16 - 1) && val < (G_MAXINT16 + 1))
5973+
LOCAL_VAR (ip [1], gint32) = (gint16) val;
5974+
else
59705975
THROW_EX (mono_get_exception_overflow (), ip);
5971-
LOCAL_VAR (ip [1], gint32) = (gint16) val;
59725976
ip += 3;
59735977
MINT_IN_BREAK;
59745978
}
@@ -6005,17 +6009,19 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs
60056009
}
60066010
MINT_IN_CASE(MINT_CONV_OVF_U2_R4) {
60076011
float val = LOCAL_VAR (ip [2], float);
6008-
if (val < 0 || val > G_MAXUINT16 || isnan (val))
6012+
if (val > -1.0f && val < (G_MAXUINT16 + 1))
6013+
LOCAL_VAR (ip [1], gint32) = (guint16) val;
6014+
else
60096015
THROW_EX (mono_get_exception_overflow (), ip);
6010-
LOCAL_VAR (ip [1], gint32) = (guint16) val;
60116016
ip += 3;
60126017
MINT_IN_BREAK;
60136018
}
60146019
MINT_IN_CASE(MINT_CONV_OVF_U2_R8) {
60156020
double val = LOCAL_VAR (ip [2], double);
6016-
if (val < 0 || val > G_MAXUINT16 || isnan (val))
6021+
if (val > -1.0 && val < (G_MAXUINT16 + 1))
6022+
LOCAL_VAR (ip [1], gint32) = (guint16) val;
6023+
else
60176024
THROW_EX (mono_get_exception_overflow (), ip);
6018-
LOCAL_VAR (ip [1], gint32) = (guint16) val;
60196025
ip += 3;
60206026
MINT_IN_BREAK;
60216027
}
@@ -6053,17 +6059,19 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs
60536059
}
60546060
MINT_IN_CASE(MINT_CONV_OVF_I1_R4) {
60556061
float val = LOCAL_VAR (ip [2], float);
6056-
if (val < G_MININT8 || val > G_MAXINT8 || isnan (val))
6062+
if (val > (G_MININT8 - 1) && val < (G_MAXINT8 + 1))
6063+
LOCAL_VAR (ip [1], gint32) = (gint8) val;
6064+
else
60576065
THROW_EX (mono_get_exception_overflow (), ip);
6058-
LOCAL_VAR (ip [1], gint32) = (gint8) val;
60596066
ip += 3;
60606067
MINT_IN_BREAK;
60616068
}
60626069
MINT_IN_CASE(MINT_CONV_OVF_I1_R8) {
60636070
double val = LOCAL_VAR (ip [2], double);
6064-
if (val < G_MININT8 || val > G_MAXINT8 || isnan (val))
6071+
if (val > (G_MININT8 - 1) && val < (G_MAXINT8 + 1))
6072+
LOCAL_VAR (ip [1], gint32) = (gint8) val;
6073+
else
60656074
THROW_EX (mono_get_exception_overflow (), ip);
6066-
LOCAL_VAR (ip [1], gint32) = (gint8) val;
60676075
ip += 3;
60686076
MINT_IN_BREAK;
60696077
}
@@ -6101,17 +6109,19 @@ interp_exec_method (InterpFrame *frame, ThreadContext *context, FrameClauseArgs
61016109
}
61026110
MINT_IN_CASE(MINT_CONV_OVF_U1_R4) {
61036111
float val = LOCAL_VAR (ip [2], float);
6104-
if (val < 0 || val > G_MAXUINT8 || isnan (val))
6112+
if (val > -1.0f && val < (G_MAXUINT8 + 1))
6113+
LOCAL_VAR (ip [1], gint32) = (guint8)val;
6114+
else
61056115
THROW_EX (mono_get_exception_overflow (), ip);
6106-
LOCAL_VAR (ip [1], gint32) = (guint8) val;
61076116
ip += 3;
61086117
MINT_IN_BREAK;
61096118
}
61106119
MINT_IN_CASE(MINT_CONV_OVF_U1_R8) {
61116120
double val = LOCAL_VAR (ip [2], double);
6112-
if (val < 0 || val > G_MAXUINT8 || isnan (val))
6121+
if (val > -1.0 && val < (G_MAXUINT8 + 1))
6122+
LOCAL_VAR (ip [1], gint32) = (guint8)val;
6123+
else
61136124
THROW_EX (mono_get_exception_overflow (), ip);
6114-
LOCAL_VAR (ip [1], gint32) = (guint8) val;
61156125
ip += 3;
61166126
MINT_IN_BREAK;
61176127
}

‎mono/mini/local-propagation.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ mono_strength_reduction_division (MonoCompile *cfg, MonoInst *ins)
223223
guint32 tmp_regi;
224224
#endif
225225
struct magic_signed mag;
226-
int power2 = mono_is_power_of_two (ins->inst_imm);
226+
int power2 = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
227227
/* The decomposition doesn't handle exception throwing */
228228
/* Optimization with MUL does not apply for -1, 0 and 1 divisors */
229229
if (ins->inst_imm == 0 || ins->inst_imm == -1) {
@@ -350,8 +350,8 @@ mono_strength_reduction_ins (MonoCompile *cfg, MonoInst *ins, const char **spec)
350350
ins->opcode = OP_INEG;
351351
} else if ((ins->opcode == OP_LMUL_IMM) && (ins->inst_imm == -1)) {
352352
ins->opcode = OP_LNEG;
353-
} else {
354-
int power2 = mono_is_power_of_two (ins->inst_imm);
353+
} else if (ins->inst_imm > 0 && ins->inst_imm <= UINT32_MAX) {
354+
int power2 = mono_is_power_of_two ( ((guint32)(ins->inst_imm)));
355355
if (power2 >= 0) {
356356
ins->opcode = (ins->opcode == OP_MUL_IMM) ? OP_SHL_IMM : ((ins->opcode == OP_LMUL_IMM) ? OP_LSHL_IMM : OP_ISHL_IMM);
357357
ins->inst_imm = power2;

‎mono/mini/mini-arm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3462,7 +3462,7 @@ mono_arch_lowering_pass (MonoCompile *cfg, MonoBasicBlock *bb)
34623462
ins->inst_c0 = 0;
34633463
break;
34643464
}
3465-
imm8 = mono_is_power_of_two (ins->inst_imm);
3465+
imm8 = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
34663466
if (imm8 > 0) {
34673467
ins->opcode = OP_SHL_IMM;
34683468
ins->inst_imm = imm8;

‎mono/mini/mini-mips.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1987,7 +1987,7 @@ mono_arch_peephole_pass_2 (MonoCompile *cfg, MonoBasicBlock *bb)
19871987
MONO_DELETE_INS (bb, ins);
19881988
continue;
19891989
}
1990-
} else {
1990+
} else if (ins->inst_imm > 0) {
19911991
int power2 = mono_is_power_of_two (ins->inst_imm);
19921992
if (power2 > 0) {
19931993
ins->opcode = OP_SHL_IMM;
@@ -2666,7 +2666,7 @@ mono_arch_lowering_pass (MonoCompile *cfg, MonoBasicBlock *bb)
26662666
ins->inst_c0 = 0;
26672667
break;
26682668
}
2669-
imm = mono_is_power_of_two (ins->inst_imm);
2669+
imm = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
26702670
if (imm > 0) {
26712671
ins->opcode = OP_SHL_IMM;
26722672
ins->inst_imm = imm;

‎mono/mini/mini-ppc.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1963,7 +1963,7 @@ mono_arch_peephole_pass_2 (MonoCompile *cfg, MonoBasicBlock *bb)
19631963
MONO_DELETE_INS (bb, ins);
19641964
continue;
19651965
}
1966-
} else {
1966+
} else if (inst->inst_imm > 0) {
19671967
int power2 = mono_is_power_of_two (ins->inst_imm);
19681968
if (power2 > 0) {
19691969
ins->opcode = OP_SHL_IMM;
@@ -2537,7 +2537,7 @@ mono_arch_lowering_pass (MonoCompile *cfg, MonoBasicBlock *bb)
25372537
ins->inst_c0 = 0;
25382538
break;
25392539
}
2540-
imm = mono_is_power_of_two (ins->inst_imm);
2540+
imm = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
25412541
if (imm > 0) {
25422542
ins->opcode = OP_SHL_IMM;
25432543
ins->inst_imm = imm;

‎mono/utils/mono-math.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <math.h>
99
#include <mono/utils/mono-publib.h>
10+
#include <glib.h>
1011

1112
// Instead of isfinite, isinf, isnan, etc.,
1213
// use mono_isfininite, mono_isinf, mono_isnan, etc.
@@ -99,4 +100,27 @@ mono_round_to_even (double x)
99100
return copysign (floor_tmp, x);
100101
}
101102

103+
static inline gboolean
104+
mono_try_trunc_i64 (double val, gint64 *out)
105+
{
106+
const double two63 = 2147483648.0 * 4294967296.0;
107+
// 0x402 is epsilon used to get us to the next value
108+
if (val > (-two63 - 0x402) && val < two63) {
109+
*out = (gint64)val;
110+
return TRUE;
111+
}
112+
return FALSE;
113+
}
114+
115+
static inline gboolean
116+
mono_try_trunc_u64 (double val, guint64 *out)
117+
{
118+
const double two64 = 4294967296.0 * 4294967296.0;
119+
if (val > -1.0 && val < two64) {
120+
*out = (guint64)val;
121+
return TRUE;
122+
}
123+
return FALSE;
124+
}
125+
102126
#endif

0 commit comments

Comments
 (0)