[lldb] Scalar re-fix UB in float->int conversions

The refactor in 48ca15592f1 reintroduced UB when converting out-of-bounds
floating point numbers to integers -- the behavior for ULongLong() was
originally fixed in r341685, but did not survive my refactor because I
based my template code on one of the methods which did not have this
fix.

This time, I apply the fix to all float->int conversions, instead of
just the "double->unsigned long long" case. I also use a slightly
simpler version of the code, with fewer round-trips
(APFloat->APSInt->native_int vs
APFloat->native_float->APInt->native_int).

I also add some unit tests for the conversions.

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index d275f62..7d0b717 100644 (file)
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -14,7 +14,7 @@
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/lldb-types.h"
-
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallString.h"
 
 #include <cinttypes>
@@ -645,60 +645,34 @@ bool Scalar::MakeUnsigned() {
 }
 
 template <typename T> T Scalar::GetAsSigned(T fail_value) const {
-  switch (m_type) {
-  case e_void:
+  switch (GetCategory(m_type)) {
+  case Category::Void:
     break;
-  case e_sint:
-  case e_uint:
-  case e_slong:
-  case e_ulong:
-  case e_slonglong:
-  case e_ulonglong:
-  case e_sint128:
-  case e_uint128:
-  case e_sint256:
-  case e_uint256:
-  case e_sint512:
-  case e_uint512:
+  case Category::Integral:
     return m_integer.sextOrTrunc(sizeof(T) * 8).getSExtValue();
 
-  case e_float:
-    return static_cast<T>(m_float.convertToFloat());
-  case e_double:
-    return static_cast<T>(m_float.convertToDouble());
-  case e_long_double:
-    llvm::APInt ldbl_val = m_float.bitcastToAPInt();
-    return static_cast<T>(
-        (ldbl_val.sextOrTrunc(sizeof(schar_t) * 8)).getSExtValue());
+  case Category::Float: {
+    llvm::APSInt result(sizeof(T) * 8, /*isUnsigned=*/false);
+    bool isExact;
+    m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact);
+    return result.getSExtValue();
+  }
   }
   return fail_value;
 }
 
 template <typename T> T Scalar::GetAsUnsigned(T fail_value) const {
-  switch (m_type) {
-  case e_void:
+  switch (GetCategory(m_type)) {
+  case Category::Void:
     break;
-  case e_sint:
-  case e_uint:
-  case e_slong:
-  case e_ulong:
-  case e_slonglong:
-  case e_ulonglong:
-  case e_sint128:
-  case e_uint128:
-  case e_sint256:
-  case e_uint256:
-  case e_sint512:
-  case e_uint512:
+  case Category::Integral:
     return m_integer.zextOrTrunc(sizeof(T) * 8).getZExtValue();
-
-  case e_float:
-    return static_cast<T>(m_float.convertToFloat());
-  case e_double:
-    return static_cast<T>(m_float.convertToDouble());
-  case e_long_double:
-    llvm::APInt ldbl_val = m_float.bitcastToAPInt();
-    return static_cast<T>((ldbl_val.zextOrTrunc(sizeof(T) * 8)).getZExtValue());
+  case Category::Float: {
+    llvm::APSInt result(sizeof(T) * 8, /*isUnsigned=*/true);
+    bool isExact;
+    m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact);
+    return result.getZExtValue();
+  }
   }
   return fail_value;
 }
@@ -736,17 +710,7 @@ long long Scalar::SLongLong(long long fail_value) const {
 }
 
 unsigned long long Scalar::ULongLong(unsigned long long fail_value) const {
-  switch (m_type) {
-  case e_double: {
-    double d_val = m_float.convertToDouble();
-    llvm::APInt rounded_double =
-        llvm::APIntOps::RoundDoubleToAPInt(d_val, sizeof(ulonglong_t) * 8);
-    return static_cast<ulonglong_t>(
-        (rounded_double.zextOrTrunc(sizeof(ulonglong_t) * 8)).getZExtValue());
-  }
-  default:
-    return GetAsUnsigned<unsigned long long>(fail_value);
-  }
+  return GetAsUnsigned<unsigned long long>(fail_value);
 }
 
 llvm::APInt Scalar::SInt128(const llvm::APInt &fail_value) const {

diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp
index 960161c..64b3c9e 100644 (file)
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -119,22 +119,34 @@ TEST(ScalarTest, GetBytes) {
 TEST(ScalarTest, CastOperations) {
   long long a = 0xf1f2f3f4f5f6f7f8LL;
   Scalar a_scalar(a);
-  ASSERT_EQ((signed char)a, a_scalar.SChar());
-  ASSERT_EQ((unsigned char)a, a_scalar.UChar());
-  ASSERT_EQ((signed short)a, a_scalar.SShort());
-  ASSERT_EQ((unsigned short)a, a_scalar.UShort());
-  ASSERT_EQ((signed int)a, a_scalar.SInt());
-  ASSERT_EQ((unsigned int)a, a_scalar.UInt());
-  ASSERT_EQ((signed long)a, a_scalar.SLong());
-  ASSERT_EQ((unsigned long)a, a_scalar.ULong());
-  ASSERT_EQ((signed long long)a, a_scalar.SLongLong());
-  ASSERT_EQ((unsigned long long)a, a_scalar.ULongLong());
+  EXPECT_EQ((signed char)a, a_scalar.SChar());
+  EXPECT_EQ((unsigned char)a, a_scalar.UChar());
+  EXPECT_EQ((signed short)a, a_scalar.SShort());
+  EXPECT_EQ((unsigned short)a, a_scalar.UShort());
+  EXPECT_EQ((signed int)a, a_scalar.SInt());
+  EXPECT_EQ((unsigned int)a, a_scalar.UInt());
+  EXPECT_EQ((signed long)a, a_scalar.SLong());
+  EXPECT_EQ((unsigned long)a, a_scalar.ULong());
+  EXPECT_EQ((signed long long)a, a_scalar.SLongLong());
+  EXPECT_EQ((unsigned long long)a, a_scalar.ULongLong());
 
   int a2 = 23;
   Scalar a2_scalar(a2);
-  ASSERT_EQ((float)a2, a2_scalar.Float());
-  ASSERT_EQ((double)a2, a2_scalar.Double());
-  ASSERT_EQ((long double)a2, a2_scalar.LongDouble());
+  EXPECT_EQ((float)a2, a2_scalar.Float());
+  EXPECT_EQ((double)a2, a2_scalar.Double());
+  EXPECT_EQ((long double)a2, a2_scalar.LongDouble());
+
+  EXPECT_EQ(std::numeric_limits<unsigned int>::min(), Scalar(-1.0f).UInt());
+  EXPECT_EQ(std::numeric_limits<unsigned int>::max(), Scalar(1e11f).UInt());
+  EXPECT_EQ(std::numeric_limits<unsigned long long>::min(),
+            Scalar(-1.0).ULongLong());
+  EXPECT_EQ(std::numeric_limits<unsigned long long>::max(),
+            Scalar(1e22).ULongLong());
+
+  EXPECT_EQ(std::numeric_limits<int>::min(), Scalar(-1e11f).SInt());
+  EXPECT_EQ(std::numeric_limits<int>::max(), Scalar(1e11f).SInt());
+  EXPECT_EQ(std::numeric_limits<long long>::min(), Scalar(-1e22).SLongLong());
+  EXPECT_EQ(std::numeric_limits<long long>::max(), Scalar(1e22).SLongLong());
 }
 
 TEST(ScalarTest, ExtractBitfield) {