From 6352c39f666676b87246f650fe5163068c9c7de6 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Tue, 20 Jun 2023 09:08:35 +0300 Subject: [PATCH] [mono][mini] Fix conversion from r4/r8 to native u (#85964) * [mono][mini] Fix conversion from r4/r8 to native u When converting from float point to unsigned integer, we always do a conversion to u8, then do a simple integer conversion with range checks. The problem is that the first conversion to u8 pushes a STACK_I8 and doing a further conversion to u8 is not actually a nop since it will throw if the result of the previous conversion is negative. * Enable tests * [mono][jit] Reuse conv ovf methods used by interpreter These follow CoreCLR implementation and don't rely on undefined behavior from out of range conversions. --- .../tests/System/UIntPtrTests.GenericMath.cs | 3 -- src/mono/mono/mini/jit-icalls.c | 37 +++---------------- src/mono/mono/mini/method-to-ir.c | 5 ++- 3 files changed, 10 insertions(+), 35 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System/UIntPtrTests.GenericMath.cs b/src/libraries/System.Runtime/tests/System/UIntPtrTests.GenericMath.cs index 2e752a91af21f..10b1f760d9362 100644 --- a/src/libraries/System.Runtime/tests/System/UIntPtrTests.GenericMath.cs +++ b/src/libraries/System.Runtime/tests/System/UIntPtrTests.GenericMath.cs @@ -1876,7 +1876,6 @@ public static void CreateCheckedFromDecimalTest() } [Fact] - [SkipOnMono("https://github.com/dotnet/runtime/issues/69794")] public static void CreateCheckedFromDoubleTest() { Assert.Equal((nuint)0x0000_0000, NumberBaseHelper.CreateChecked(+0.0)); @@ -2006,7 +2005,6 @@ public static void CreateCheckedFromIntPtrTest() } [Fact] - [SkipOnMono("https://github.com/dotnet/runtime/issues/69794")] public static void CreateCheckedFromNFloatTest() { Assert.Equal((nuint)0x0000_0000, NumberBaseHelper.CreateChecked(0.0f)); @@ -2052,7 +2050,6 @@ public static void CreateCheckedFromSByteTest() } [Fact] - [SkipOnMono("https://github.com/dotnet/runtime/issues/69794")] public static void CreateCheckedFromSingleTest() { Assert.Equal((nuint)0x0000_0000, NumberBaseHelper.CreateChecked(+0.0f)); diff --git a/src/mono/mono/mini/jit-icalls.c b/src/mono/mono/mini/jit-icalls.c index fee5d3718cd78..13c8b92e0b605 100644 --- a/src/mono/mono/mini/jit-icalls.c +++ b/src/mono/mono/mini/jit-icalls.c @@ -990,9 +990,8 @@ mono_rconv_u4 (float v) gint64 mono_fconv_ovf_i8 (double v) { - const gint64 res = (gint64)v; - - if (mono_isnan (v) || mono_trunc (v) != res) { + gint64 res; + if (!mono_try_trunc_i64 (v, &res)) { ERROR_DECL (error); mono_error_set_overflow (error); mono_error_set_pending_exception (error); @@ -1005,33 +1004,12 @@ guint64 mono_fconv_ovf_u8 (double v) { guint64 res; - -/* - * The soft-float implementation of some ARM devices have a buggy guin64 to double - * conversion that it looses precision even when the integer if fully representable - * as a double. - * - * This was found with 4294967295ull, converting to double and back looses one bit of precision. - * - * To work around this issue we test for value boundaries instead. - */ -#if defined(__arm__) && defined(MONO_ARCH_SOFT_FLOAT_FALLBACK) - if (mono_isnan (v) || !(v >= -0.5 && v <= ULLONG_MAX+0.5)) { + if (!mono_try_trunc_u64 (v, &res)) { ERROR_DECL (error); mono_error_set_overflow (error); mono_error_set_pending_exception (error); return 0; } - res = (guint64)v; -#else - res = (guint64)v; - if (mono_isnan (v) || mono_trunc (v) != res) { - ERROR_DECL (error); - mono_error_set_overflow (error); - mono_error_set_pending_exception (error); - return 0; - } -#endif return res; } @@ -1046,9 +1024,8 @@ mono_rconv_i8 (float v) gint64 mono_rconv_ovf_i8 (float v) { - const gint64 res = (gint64)v; - - if (mono_isnan (v) || mono_trunc (v) != res) { + gint64 res; + if (!mono_try_trunc_i64 (v, &res)) { ERROR_DECL (error); mono_error_set_overflow (error); mono_error_set_pending_exception (error); @@ -1061,9 +1038,7 @@ guint64 mono_rconv_ovf_u8 (float v) { guint64 res; - - res = (guint64)v; - if (mono_isnan (v) || mono_trunc (v) != res) { + if (!mono_try_trunc_u64 (v, &res)) { ERROR_DECL (error); mono_error_set_overflow (error); mono_error_set_pending_exception (error); diff --git a/src/mono/mono/mini/method-to-ir.c b/src/mono/mono/mini/method-to-ir.c index 7c0ebbb7bfb1a..5a98a40a2d437 100644 --- a/src/mono/mono/mini/method-to-ir.c +++ b/src/mono/mono/mini/method-to-ir.c @@ -8943,7 +8943,10 @@ mono_method_to_ir (MonoCompile *cfg, MonoMethod *method, MonoBasicBlock *start_b if (sp [-1]->type == STACK_R8 || sp [-1]->type == STACK_R4) { /* floats are always signed, _UN has no effect */ ADD_UNOP (CEE_CONV_OVF_U8); - ADD_UNOP (il_op); + if (TARGET_SIZEOF_VOID_P == 8 && il_op == MONO_CEE_CONV_OVF_U) + sp [-1]->type = STACK_PTR; // no additional conversion needed + else + ADD_UNOP (il_op); } else { ADD_UNOP (il_op); }