Skip to content

Commit 7de1d1a

Browse files
BrzVladBrzVlad
authored
[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
1 parent a026b1d commit 7de1d1a

7 files changed

Lines changed: 73 additions & 60 deletions

File tree

‎src/mono/mono/mini/interp/interp.c‎

Lines changed: 42 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5476,33 +5476,29 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
54765476
}
54775477
MINT_IN_CASE(MINT_CONV_OVF_U8_R4) {
54785478
float val = LOCAL_VAR (ip [2], float);
5479-
if (mono_isnan (val) || mono_trunc (val) != (guint64)val)
5479+
if (!mono_try_trunc_u64 (val, (guint64*)(locals + ip [1])))
54805480
THROW_EX (mono_get_exception_overflow (), ip);
5481-
LOCAL_VAR (ip [1], guint64) = (guint64)val;
54825481
ip += 3;
54835482
MINT_IN_BREAK;
54845483
}
54855484
MINT_IN_CASE(MINT_CONV_OVF_U8_R8) {
54865485
double val = LOCAL_VAR (ip [2], double);
5487-
if (mono_isnan (val) || mono_trunc (val) != (guint64)val)
5486+
if (!mono_try_trunc_u64 (val, (guint64*)(locals + ip [1])))
54885487
THROW_EX (mono_get_exception_overflow (), ip);
5489-
LOCAL_VAR (ip [1], guint64) = (guint64)val;
54905488
ip += 3;
54915489
MINT_IN_BREAK;
54925490
}
54935491
MINT_IN_CASE(MINT_CONV_OVF_I8_R4) {
54945492
float val = LOCAL_VAR (ip [2], float);
5495-
if (mono_isnan (val) || mono_trunc (val) != (gint64)val)
5493+
if (!mono_try_trunc_i64 (val, (gint64*)(locals + ip [1])))
54965494
THROW_EX (mono_get_exception_overflow (), ip);
5497-
LOCAL_VAR (ip [1], gint64) = (gint64)val;
54985495
ip += 3;
54995496
MINT_IN_BREAK;
55005497
}
55015498
MINT_IN_CASE(MINT_CONV_OVF_I8_R8) {
55025499
double val = LOCAL_VAR (ip [2], double);
5503-
if (mono_isnan (val) || mono_trunc (val) != (gint64)val)
5500+
if (!mono_try_trunc_i64 (val, (gint64*)(locals + ip [1])))
55045501
THROW_EX (mono_get_exception_overflow (), ip);
5505-
LOCAL_VAR (ip [1], gint64) = (gint64)val;
55065502
ip += 3;
55075503
MINT_IN_BREAK;
55085504
}
@@ -5804,17 +5800,20 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
58045800
}
58055801
MINT_IN_CASE(MINT_CONV_OVF_I4_R4) {
58065802
float val = LOCAL_VAR (ip [2], float);
5807-
if (mono_isnan (val) || mono_trunc (val) != (gint32)val)
5803+
double val_r8 = (double)val;
5804+
if (val_r8 > ((double)G_MININT32 - 1) && val_r8 < ((double)G_MAXINT32 + 1))
5805+
LOCAL_VAR (ip [1], gint32) = (gint32) val;
5806+
else
58085807
THROW_EX (mono_get_exception_overflow (), ip);
5809-
LOCAL_VAR (ip [1], gint32) = (gint32) val;
58105808
ip += 3;
58115809
MINT_IN_BREAK;
58125810
}
58135811
MINT_IN_CASE(MINT_CONV_OVF_I4_R8) {
58145812
double val = LOCAL_VAR (ip [2], double);
5815-
if (val < G_MININT32 || val > G_MAXINT32 || isnan (val))
5813+
if (val > ((double)G_MININT32 - 1) && val < ((double)G_MAXINT32 + 1))
5814+
LOCAL_VAR (ip [1], gint32) = (gint32) val;
5815+
else
58165816
THROW_EX (mono_get_exception_overflow (), ip);
5817-
LOCAL_VAR (ip [1], gint32) = (gint32)val;
58185817
ip += 3;
58195818
MINT_IN_BREAK;
58205819
}
@@ -5836,17 +5835,20 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
58365835
}
58375836
MINT_IN_CASE(MINT_CONV_OVF_U4_R4) {
58385837
float val = LOCAL_VAR (ip [2], float);
5839-
if (mono_isnan (val) || mono_trunc (val) != (guint32)val)
5838+
double val_r8 = val;
5839+
if (val_r8 > -1.0 && val_r8 < ((double)G_MAXUINT32 + 1))
5840+
LOCAL_VAR (ip [1], gint32) = (guint32)val;
5841+
else
58405842
THROW_EX (mono_get_exception_overflow (), ip);
5841-
LOCAL_VAR (ip [1], gint32) = (guint32)val;
58425843
ip += 3;
58435844
MINT_IN_BREAK;
58445845
}
58455846
MINT_IN_CASE(MINT_CONV_OVF_U4_R8) {
58465847
double val = LOCAL_VAR (ip [2], double);
5847-
if (val < 0 || val > G_MAXUINT32 || isnan (val))
5848+
if (val > -1.0 && val < ((double)G_MAXUINT32 + 1))
5849+
LOCAL_VAR (ip [1], gint32) = (guint32)val;
5850+
else
58485851
THROW_EX (mono_get_exception_overflow (), ip);
5849-
LOCAL_VAR (ip [1], gint32) = (guint32) val;
58505852
ip += 3;
58515853
MINT_IN_BREAK;
58525854
}
@@ -5884,17 +5886,19 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
58845886
}
58855887
MINT_IN_CASE(MINT_CONV_OVF_I2_R4) {
58865888
float val = LOCAL_VAR (ip [2], float);
5887-
if (val < G_MININT16 || val > G_MAXINT16 || isnan (val))
5889+
if (val > (G_MININT16 - 1) && val < (G_MAXINT16 + 1))
5890+
LOCAL_VAR (ip [1], gint32) = (gint16) val;
5891+
else
58885892
THROW_EX (mono_get_exception_overflow (), ip);
5889-
LOCAL_VAR (ip [1], gint32) = (gint16) val;
58905893
ip += 3;
58915894
MINT_IN_BREAK;
58925895
}
58935896
MINT_IN_CASE(MINT_CONV_OVF_I2_R8) {
58945897
double val = LOCAL_VAR (ip [2], double);
5895-
if (val < G_MININT16 || val > G_MAXINT16 || isnan (val))
5898+
if (val > (G_MININT16 - 1) && val < (G_MAXINT16 + 1))
5899+
LOCAL_VAR (ip [1], gint32) = (gint16) val;
5900+
else
58965901
THROW_EX (mono_get_exception_overflow (), ip);
5897-
LOCAL_VAR (ip [1], gint32) = (gint16) val;
58985902
ip += 3;
58995903
MINT_IN_BREAK;
59005904
}
@@ -5916,17 +5920,19 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
59165920
}
59175921
MINT_IN_CASE(MINT_CONV_OVF_U2_R4) {
59185922
float val = LOCAL_VAR (ip [2], float);
5919-
if (val < 0 || val > G_MAXUINT16 || isnan (val))
5923+
if (val > -1.0f && val < (G_MAXUINT16 + 1))
5924+
LOCAL_VAR (ip [1], gint32) = (guint16) val;
5925+
else
59205926
THROW_EX (mono_get_exception_overflow (), ip);
5921-
LOCAL_VAR (ip [1], gint32) = (guint16) val;
59225927
ip += 3;
59235928
MINT_IN_BREAK;
59245929
}
59255930
MINT_IN_CASE(MINT_CONV_OVF_U2_R8) {
59265931
double val = LOCAL_VAR (ip [2], double);
5927-
if (val < 0 || val > G_MAXUINT16 || isnan (val))
5932+
if (val > -1.0 && val < (G_MAXUINT16 + 1))
5933+
LOCAL_VAR (ip [1], gint32) = (guint16) val;
5934+
else
59285935
THROW_EX (mono_get_exception_overflow (), ip);
5929-
LOCAL_VAR (ip [1], gint32) = (guint16) val;
59305936
ip += 3;
59315937
MINT_IN_BREAK;
59325938
}
@@ -5964,17 +5970,19 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
59645970
}
59655971
MINT_IN_CASE(MINT_CONV_OVF_I1_R4) {
59665972
float val = LOCAL_VAR (ip [2], float);
5967-
if (val < G_MININT8 || val > G_MAXINT8 || isnan (val))
5973+
if (val > (G_MININT8 - 1) && val < (G_MAXINT8 + 1))
5974+
LOCAL_VAR (ip [1], gint32) = (gint8) val;
5975+
else
59685976
THROW_EX (mono_get_exception_overflow (), ip);
5969-
LOCAL_VAR (ip [1], gint32) = (gint8) val;
59705977
ip += 3;
59715978
MINT_IN_BREAK;
59725979
}
59735980
MINT_IN_CASE(MINT_CONV_OVF_I1_R8) {
59745981
double val = LOCAL_VAR (ip [2], double);
5975-
if (val < G_MININT8 || val > G_MAXINT8 || isnan (val))
5982+
if (val > (G_MININT8 - 1) && val < (G_MAXINT8 + 1))
5983+
LOCAL_VAR (ip [1], gint32) = (gint8) val;
5984+
else
59765985
THROW_EX (mono_get_exception_overflow (), ip);
5977-
LOCAL_VAR (ip [1], gint32) = (gint8) val;
59785986
ip += 3;
59795987
MINT_IN_BREAK;
59805988
}
@@ -5996,17 +6004,19 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
59966004
}
59976005
MINT_IN_CASE(MINT_CONV_OVF_U1_R4) {
59986006
float val = LOCAL_VAR (ip [2], float);
5999-
if (val < 0 || val > G_MAXUINT8 || isnan (val))
6007+
if (val > -1.0f && val < (G_MAXUINT8 + 1))
6008+
LOCAL_VAR (ip [1], gint32) = (guint8)val;
6009+
else
60006010
THROW_EX (mono_get_exception_overflow (), ip);
6001-
LOCAL_VAR (ip [1], gint32) = (guint8) val;
60026011
ip += 3;
60036012
MINT_IN_BREAK;
60046013
}
60056014
MINT_IN_CASE(MINT_CONV_OVF_U1_R8) {
60066015
double val = LOCAL_VAR (ip [2], double);
6007-
if (val < 0 || val > G_MAXUINT8 || isnan (val))
6016+
if (val > -1.0 && val < (G_MAXUINT8 + 1))
6017+
LOCAL_VAR (ip [1], gint32) = (guint8)val;
6018+
else
60086019
THROW_EX (mono_get_exception_overflow (), ip);
6009-
LOCAL_VAR (ip [1], gint32) = (guint8) val;
60106020
ip += 3;
60116021
MINT_IN_BREAK;
60126022
}

‎src/mono/mono/mini/local-propagation.c‎

Lines changed: 2 additions & 2 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,7 +350,7 @@ 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 {
353+
} else if (ins->inst_imm > 0) {
354354
int power2 = mono_is_power_of_two (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);

‎src/mono/mono/mini/mini-arm.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3467,7 +3467,7 @@ mono_arch_lowering_pass (MonoCompile *cfg, MonoBasicBlock *bb)
34673467
ins->inst_c0 = 0;
34683468
break;
34693469
}
3470-
imm8 = mono_is_power_of_two (ins->inst_imm);
3470+
imm8 = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
34713471
if (imm8 > 0) {
34723472
ins->opcode = OP_SHL_IMM;
34733473
ins->inst_imm = imm8;

‎src/mono/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;

‎src/mono/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;

‎src/mono/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

‎src/tests/issues.targets‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -986,27 +986,9 @@
986986
</ExcludeList>
987987

988988

989-
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/Convert/value_numbering_checked_casts_of_constants/*">
990-
<Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
991-
</ExcludeList>
992-
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/int64/misc/longmul/*">
993-
<Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
994-
</ExcludeList>
995989
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/Convert/out_of_range_fp_to_int_conversions/*">
996990
<Issue>Mono does not define out of range fp to int conversions</Issue>
997991
</ExcludeList>
998-
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Overflow/FloatOvfToInt2_r/*">
999-
<Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
1000-
</ExcludeList>
1001-
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Overflow/FloatOvfToInt2_ro/*">
1002-
<Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
1003-
</ExcludeList>
1004-
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Overflow/FloatOvfToInt2_d/*">
1005-
<Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
1006-
</ExcludeList>
1007-
<ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Overflow/FloatOvfToInt2_do/*">
1008-
<Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
1009-
</ExcludeList>
1010992
<ExcludeList Include="$(XunitTestBinBase)/JIT/opt/Devirtualization/Comparer_get_Default/*">
1011993
<Issue>https://github.com/dotnet/runtime/issues/48190</Issue>
1012994
</ExcludeList>
@@ -1260,9 +1242,6 @@
12601242
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/Convert/ldind_conv/**">
12611243
<Issue>needs triage</Issue>
12621244
</ExcludeList>
1263-
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/Convert/signed_overflow_conversions_are_not_treated_as_unsigned/**">
1264-
<Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
1265-
</ExcludeList>
12661245
<ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/coverage/compiler/FilterToHandler/**">
12671246
<Issue>needs triage</Issue>
12681247
</ExcludeList>

0 commit comments

Comments
 (0)