QVariant: fix assert in FP->int conversions

The newly introduced asserts in qRound make QVariant FP->int conversions
crash. The crash is legitimate, because the conversion has UB. However
we've historically supported these conversions with a somehow specific
(albeit undocumented) behavior; I'm trying to keep it as accurately as
possible, based on testing on x86-64. (I have no idea if the behavior
was actually different elsewhere. Formally there's no "behavior" to
talk about here.)

Specifically: use "saturation rounding" towards qint64, and then convert
the result to the target type. This can be done by using the private
qSaturateRound function, which however needs to be extended to 64 bits.

This extension revealed an implementation problem: using qBound to clamp
the result of the rounding is inaccurate for certain combination of
input and output types (e.g. double to qint64), because the maximal
value can't be correctly represented in floating point.  Use a better
algorithm instead.

Finally, I'm making NaN *fail* the conversion (and return 0). I don't
think there's any other sensible alternative here. I'm not sure of the
pre-existing behavior of the code across different architectures and
using different encodings for NaN, and it sounds extremely brittle to do
something else instead.

Fixes: QTBUG-135285
Change-Id: Ia5f54094807210d0dbbdbb9a0a4f3a0f18ea3d80
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

src/corelib/kernel/qmetatype.cpp

@@ -16,6 +16,7 @@
 #include "qlist.h"
 #include "qlocale.h"
 #include "qdebug.h"
+#include "private/qnumeric_p.h"
 #if QT_CONFIG(easingcurve)
 #include "qeasingcurve.h"
 #endif
@@ -972,6 +973,28 @@ static const struct { const char * typeName; int typeNameLength; int type; } typ
     {nullptr, 0, QMetaType::UnknownType}
 };
 
+template <typename From, typename To>
+static bool qIntegerConversionFromFPHelper(From from, To *to)
+{
+#ifndef Q_CC_GHS
+    // actually is_floating_point, but include qfloat16:
+    static_assert(std::numeric_limits<From>::is_iec559);
+#endif
+    static_assert(std::is_integral_v<To>);
+    static_assert(sizeof(From) <= sizeof(double));
+    const double fromD = static_cast<double>(from);
+
+    if (qt_is_nan(fromD)) {
+        *to = To(0);
+        return false;
+    }
+
+    qint64 result;
+    convertDoubleTo(std::round(fromD), &result);
+    *to = To(result);
+    return true;
+}
+
 // NOLINTNEXTLINE(cppcoreguidelines-virtual-class-destructor): this is not a base class
 static constexpr struct : QMetaTypeModuleHelper
 {
@@ -1074,9 +1097,9 @@ static constexpr struct : QMetaTypeModuleHelper
     QMETATYPE_CONVERTER_ASSIGN(To, ULong); \
     QMETATYPE_CONVERTER_ASSIGN(To, LongLong); \
     QMETATYPE_CONVERTER_ASSIGN(To, ULongLong); \
-    QMETATYPE_CONVERTER(To, Float16, result = qRound64(source); return true;); \
-    QMETATYPE_CONVERTER(To, Float, result = qRound64(source); return true;); \
-    QMETATYPE_CONVERTER(To, Double, result = qRound64(source); return true;); \
+    QMETATYPE_CONVERTER(To, Float16, return qIntegerConversionFromFPHelper(source, &result);); \
+    QMETATYPE_CONVERTER(To, Float, return qIntegerConversionFromFPHelper(source, &result);); \
+    QMETATYPE_CONVERTER(To, Double, return qIntegerConversionFromFPHelper(source, &result);); \
     QMETATYPE_CONVERTER(To, QChar, result = source.unicode(); return true;); \
     QMETATYPE_CONVERTER(To, QString, \
         bool ok = false; \

tests/auto/corelib/kernel/qvariant/tst_qvariant.cpp

@@ -914,8 +914,20 @@ template <typename To> static void addNumberConversions()
         }
     }
 
-    if constexpr (std::is_integral_v<To>)
+    if constexpr (std::is_integral_v<To>) {
         QTest::newRow("QChar") << QVariant(QChar('a')) << To('a') << true;
+
+        QTest::newRow("NaN") << QVariant::fromValue(qQNaN()) << To(0) << false;
+
+        constexpr qint64 maximal = std::numeric_limits<qint64>::max();
+        constexpr qint64 minimal = std::numeric_limits<qint64>::min();
+
+        QTest::newRow("positive overflow") << QVariant(1.0e200) << To(maximal) << true;
+        QTest::newRow("positive inf") << QVariant(qInf()) << To(maximal) << true;
+        QTest::newRow("negative overflow") << QVariant(-1.0e200) << To(minimal) << true;
+        QTest::newRow("negative inf") << QVariant(-qInf()) << To(minimal) << true;
+    }
+
     QTest::newRow("nonint-QByteArray") << QVariant(QByteArray("zzzz")) << To{} << false;
     QTest::newRow("nonint-QString") << QVariant(QString("zzzz")) << To{} << false;
     QTest::newRow("undefined-QCborValue") << QVariant::fromValue<QCborValue>({}) << To{} << false;