Skip to content

Commit

Permalink
[mono][mini] Fix conversion from r4/r8 to native u (#85964)
Browse files Browse the repository at this point in the history
* [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.
  • Loading branch information
BrzVlad authored Jun 20, 2023
1 parent 0c77cbe commit 6352c39
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<nuint>.CreateChecked<double>(+0.0));
Expand Down Expand Up @@ -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<nuint>.CreateChecked<NFloat>(0.0f));
Expand Down Expand Up @@ -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<nuint>.CreateChecked<float>(+0.0f));
Expand Down
37 changes: 6 additions & 31 deletions src/mono/mono/mini/jit-icalls.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand All @@ -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);
Expand All @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion src/mono/mono/mini/method-to-ir.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 6352c39

Please sign in to comment.