[mono] Fix a few corner case overflow operations (#57407)

* [interp] Fix a few overflow conversions Comparison for overflow was incorrect because for example, (int8)-128.5 = -128, without overflows, even if -128.5 is smaller than the minimum integer. We use instead the float equality comparison between trunc (val) and (int)val. * [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
dotnet · Aug 16, 2021 · e81efc8 · e81efc8
1 parent 86562e1
commit e81efc8
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 38 deletions.
diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c
@@ -5808,7 +5808,7 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_I4_R8) {
 			double val = LOCAL_VAR (ip [2], double);
-			if (val < G_MININT32 || val > G_MAXINT32 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (gint32)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (gint32)val;
 			ip += 3;
@@ -5840,7 +5840,7 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_U4_R8) {
 			double val = LOCAL_VAR (ip [2], double);
-			if (val < 0 || val > G_MAXUINT32 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (guint32)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (guint32) val;
 			ip += 3;
@@ -5880,15 +5880,15 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_I2_R4) {
 			float val = LOCAL_VAR (ip [2], float);
-			if (val < G_MININT16 || val > G_MAXINT16 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (gint16)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (gint16) val;
 			ip += 3;
 			MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_I2_R8) {
 			double val = LOCAL_VAR (ip [2], double);
-			if (val < G_MININT16 || val > G_MAXINT16 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (gint16)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (gint16) val;
 			ip += 3;
@@ -5912,15 +5912,15 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_U2_R4) {
 			float val = LOCAL_VAR (ip [2], float);
-			if (val < 0 || val > G_MAXUINT16 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (guint16)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (guint16) val;
 			ip += 3;
 			MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_U2_R8) {
 			double val = LOCAL_VAR (ip [2], double);
-			if (val < 0 || val > G_MAXUINT16 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (guint16)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (guint16) val;
 			ip += 3;
@@ -5960,15 +5960,15 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_I1_R4) {
 			float val = LOCAL_VAR (ip [2], float);
-			if (val < G_MININT8 || val > G_MAXINT8 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (gint8)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (gint8) val;
 			ip += 3;
 			MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_I1_R8) {
 			double val = LOCAL_VAR (ip [2], double);
-			if (val < G_MININT8 || val > G_MAXINT8 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (gint8)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (gint8) val;
 			ip += 3;
@@ -5992,15 +5992,15 @@ MINT_IN_CASE(MINT_BRTRUE_I8_SP) ZEROP_SP(gint64, !=); MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_U1_R4) {
 			float val = LOCAL_VAR (ip [2], float);
-			if (val < 0 || val > G_MAXUINT8 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (guint8)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (guint8) val;
 			ip += 3;
 			MINT_IN_BREAK;
 		}
 		MINT_IN_CASE(MINT_CONV_OVF_U1_R8) {
 			double val = LOCAL_VAR (ip [2], double);
-			if (val < 0 || val > G_MAXUINT8 || isnan (val))
+			if (mono_isnan (val) || mono_trunc (val) != (guint8)val)
 				THROW_EX (mono_get_exception_overflow (), ip);
 			LOCAL_VAR (ip [1], gint32) = (guint8) val;
 			ip += 3;

diff --git a/src/mono/mono/mini/local-propagation.c b/src/mono/mono/mini/local-propagation.c
@@ -223,7 +223,7 @@ mono_strength_reduction_division (MonoCompile *cfg, MonoInst *ins)
 			guint32 tmp_regi;
 #endif
 			struct magic_signed mag;
-			int power2 = mono_is_power_of_two (ins->inst_imm);
+			int power2 = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
 			/* The decomposition doesn't handle exception throwing */
 			/* Optimization with MUL does not apply for -1, 0 and 1 divisors */
 			if (ins->inst_imm == 0 || ins->inst_imm == -1) {
@@ -350,7 +350,7 @@ mono_strength_reduction_ins (MonoCompile *cfg, MonoInst *ins, const char **spec)
 			ins->opcode = OP_INEG;
 		} else if ((ins->opcode == OP_LMUL_IMM) && (ins->inst_imm == -1)) {
 			ins->opcode = OP_LNEG;
-		} else {
+		} else if (ins->inst_imm > 0) {
 			int power2 = mono_is_power_of_two (ins->inst_imm);
 			if (power2 >= 0) {
 				ins->opcode = (ins->opcode == OP_MUL_IMM) ? OP_SHL_IMM : ((ins->opcode == OP_LMUL_IMM) ? OP_LSHL_IMM : OP_ISHL_IMM);

diff --git a/src/mono/mono/mini/mini-arm.c b/src/mono/mono/mini/mini-arm.c
@@ -3466,7 +3466,7 @@ mono_arch_lowering_pass (MonoCompile *cfg, MonoBasicBlock *bb)
 				ins->inst_c0 = 0;
 				break;
 			}
-			imm8 = mono_is_power_of_two (ins->inst_imm);
+			imm8 = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
 			if (imm8 > 0) {
 				ins->opcode = OP_SHL_IMM;
 				ins->inst_imm = imm8;

diff --git a/src/mono/mono/mini/mini-mips.c b/src/mono/mono/mini/mini-mips.c
@@ -1987,7 +1987,7 @@ mono_arch_peephole_pass_2 (MonoCompile *cfg, MonoBasicBlock *bb)
 					MONO_DELETE_INS (bb, ins);
 					continue;
 				}
-			} else {
+			} else if (ins->inst_imm > 0) {
 				int power2 = mono_is_power_of_two (ins->inst_imm);
 				if (power2 > 0) {
 					ins->opcode = OP_SHL_IMM;
@@ -2666,7 +2666,7 @@ mono_arch_lowering_pass (MonoCompile *cfg, MonoBasicBlock *bb)
 				ins->inst_c0 = 0;
 				break;
 			}
-			imm = mono_is_power_of_two (ins->inst_imm);
+			imm = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
 			if (imm > 0) {
 				ins->opcode = OP_SHL_IMM;
 				ins->inst_imm = imm;

diff --git a/src/mono/mono/mini/mini-ppc.c b/src/mono/mono/mini/mini-ppc.c
@@ -1963,7 +1963,7 @@ mono_arch_peephole_pass_2 (MonoCompile *cfg, MonoBasicBlock *bb)
 					MONO_DELETE_INS (bb, ins);
 					continue;
 				}
-			} else {
+			} else if (inst->inst_imm > 0) {
 				int power2 = mono_is_power_of_two (ins->inst_imm);
 				if (power2 > 0) {
 					ins->opcode = OP_SHL_IMM;
@@ -2537,7 +2537,7 @@ mono_arch_lowering_pass (MonoCompile *cfg, MonoBasicBlock *bb)
 				ins->inst_c0 = 0;
 				break;
 			}
-			imm = mono_is_power_of_two (ins->inst_imm);
+			imm = (ins->inst_imm > 0) ? mono_is_power_of_two (ins->inst_imm) : -1;
 			if (imm > 0) {
 				ins->opcode = OP_SHL_IMM;
 				ins->inst_imm = imm;

diff --git a/src/tests/issues.targets b/src/tests/issues.targets
@@ -965,27 +965,9 @@
         </ExcludeList>
 
 
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/Convert/value_numbering_checked_casts_of_constants/*">
-            <Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
-        </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/int64/misc/longmul/*">
-            <Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/Convert/out_of_range_fp_to_int_conversions/*">
             <Issue>Mono does not define out of range fp to int conversions</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Overflow/FloatOvfToInt2_r/*">
-            <Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
-        </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Overflow/FloatOvfToInt2_ro/*">
-            <Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
-        </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Overflow/FloatOvfToInt2_d/*">
-            <Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
-        </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Methodical/Overflow/FloatOvfToInt2_do/*">
-            <Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/JIT/opt/Devirtualization/Comparer_get_Default/*">
             <Issue>https://github.com/dotnet/runtime/issues/48190</Issue>
         </ExcludeList>
@@ -1239,9 +1221,6 @@
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/Convert/ldind_conv/**">
             <Issue>needs triage</Issue>
         </ExcludeList>
-        <ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/Convert/signed_overflow_conversions_are_not_treated_as_unsigned/**">
-            <Issue>https://github.com/dotnet/runtime/issues/51323</Issue>
-        </ExcludeList>
         <ExcludeList Include="$(XunitTestBinBase)/JIT/Directed/coverage/compiler/FilterToHandler/**">
             <Issue>needs triage</Issue>
         </ExcludeList>