Author: Pavel Labath Date: 2020-07-01T10:29:42+02:00 New Revision: 8270a903baf55122289499ba00a979e9c04dcd44
URL: https://github.com/llvm/llvm-project/commit/8270a903baf55122289499ba00a979e9c04dcd44 DIFF: https://github.com/llvm/llvm-project/commit/8270a903baf55122289499ba00a979e9c04dcd44.diff LOG: [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. Added: Modified: lldb/source/Utility/Scalar.cpp lldb/unittests/Utility/ScalarTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp index d275f6211e5c..7d0b7178ddd2 100644 --- 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 960161c01bac..64b3c9e2bc45 100644 --- 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) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
