labath created this revision.
labath added reviewers: teemperor, JDevlieghere.
Herald added a project: LLDB.
The Scalar class claims to follow the C type conversion rules. This is
true for the Promote function, but it is not true for the implicit
conversions done in the getter methods.
These functions had a subtle bug: when extending the type, they used the
signedness of the *target* type in order to determine whether to do
sign-extension or zero-extension. This is not how things work in C,
which uses the signedness of the *source* type. I.e., C does
(sign-)extension before it does signed->unsigned conversion, and not the
other way around.
This means that: (unsigned long)(int)-1
is equal to (unsigned long)0xffffffffffffffff
and not (unsigned long)0x00000000ffffffff
Unsurprisingly, we have accumulated code which depended on this
inconsistent behavior. It mainly manifested itself as code calling
"ULongLong/SLongLong" as a way to get the value of the Scalar object in
a primitive type that is "large enough". Previously, the ULongLong
conversion did not do sign-extension, but now it does.
This patch makes the Scalar getters consistent with the declared
semantics, and fixes the couple of call sites that were using it
incorrectly.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D82772
Files:
lldb/include/lldb/Utility/Scalar.h
lldb/source/Core/ValueObject.cpp
lldb/source/Expression/IRInterpreter.cpp
lldb/source/Utility/Scalar.cpp
lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
lldb/unittests/Utility/ScalarTest.cpp
Index: lldb/unittests/Utility/ScalarTest.cpp
===================================================================
--- lldb/unittests/Utility/ScalarTest.cpp
+++ lldb/unittests/Utility/ScalarTest.cpp
@@ -66,6 +66,44 @@
ASSERT_TRUE(s2 >= s1);
}
+template <typename T1, typename T2>
+static T2 ConvertHost(T1 val, T2 (Scalar::*)(T2) const) {
+ return T2(val);
+}
+
+template <typename T1, typename T2>
+static T2 ConvertScalar(T1 val, T2 (Scalar::*conv)(T2) const) {
+ return (Scalar(val).*conv)(T2());
+}
+
+template<typename T>
+static void CheckConversion(T val) {
+ SCOPED_TRACE("val = " + std::to_string(val));
+ EXPECT_EQ((signed char)val, Scalar(val).SChar());
+ EXPECT_EQ((unsigned char)val, Scalar(val).UChar());
+ EXPECT_EQ((short)val, Scalar(val).SShort());
+ EXPECT_EQ((unsigned short)val, Scalar(val).UShort());
+ EXPECT_EQ((int)val, Scalar(val).SInt());
+ EXPECT_EQ((unsigned)val, Scalar(val).UInt());
+ EXPECT_EQ((long)val, Scalar(val).SLong());
+ EXPECT_EQ((unsigned long)val, Scalar(val).ULong());
+ EXPECT_EQ((long long)val, Scalar(val).SLongLong());
+ EXPECT_EQ((unsigned long long)val, Scalar(val).ULongLong());
+ EXPECT_NEAR((float)val, Scalar(val).Float(), std::abs(val/1e6));
+ EXPECT_NEAR((double)val, Scalar(val).Double(), std::abs(val/1e12));
+}
+
+TEST(ScalarTest, Getters) {
+ CheckConversion((int)0x87654321);
+ CheckConversion((unsigned int)0x87654321u);
+ CheckConversion((long)0x87654321l);
+ CheckConversion((unsigned long)0x87654321ul);
+ CheckConversion((long long)0x8765432112345678ll);
+ CheckConversion((unsigned long long)0x8765432112345678ull);
+ CheckConversion((float)42.25f);
+ CheckConversion((double)42.25);
+}
+
TEST(ScalarTest, RightShiftOperator) {
int a = 0x00001000;
int b = 0xFFFFFFFF;
@@ -324,11 +362,11 @@
TEST(ScalarTest, TruncOrExtendTo) {
Scalar S(0xffff);
S.TruncOrExtendTo(12, true);
- EXPECT_EQ(S.ULong(), 0xfffu);
+ EXPECT_EQ(S.UInt128(APInt()), APInt(12, 0xfffu));
S.TruncOrExtendTo(20, true);
- EXPECT_EQ(S.ULong(), 0xfffffu);
+ EXPECT_EQ(S.UInt128(APInt()), APInt(20, 0xfffffu));
S.TruncOrExtendTo(24, false);
- EXPECT_EQ(S.ULong(), 0x0fffffu);
+ EXPECT_EQ(S.UInt128(APInt()), APInt(24, 0x0fffffu));
S.TruncOrExtendTo(16, false);
- EXPECT_EQ(S.ULong(), 0xffffu);
+ EXPECT_EQ(S.UInt128(APInt()), APInt(16, 0xffffu));
}
Index: lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
===================================================================
--- lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
+++ lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
@@ -51,7 +51,8 @@
options = lldb.SBExpressionOptions()
options.SetLanguage(lldb.eLanguageTypeC_plus_plus)
- set_up_expressions = ["int $i = 9", "int $j = 3", "int $k = 5"]
+ set_up_expressions = ["int $i = 9", "int $j = 3", "int $k = 5",
+ "unsigned long long $ull = -1", "unsigned $u = -1"]
expressions = ["$i + $j",
"$i - $j",
@@ -61,7 +62,8 @@
"$i << $j",
"$i & $j",
"$i | $j",
- "$i ^ $j"]
+ "$i ^ $j",
+ "($ull & -1) == $u"]
for expression in set_up_expressions:
self.frame().EvaluateExpression(expression, options)
Index: lldb/source/Utility/Scalar.cpp
===================================================================
--- lldb/source/Utility/Scalar.cpp
+++ lldb/source/Utility/Scalar.cpp
@@ -644,51 +644,23 @@
return success;
}
-template <typename T> T Scalar::GetAsSigned(T fail_value) const {
+template <typename T> T Scalar::GetAs(T fail_value) const {
switch (m_type) {
case e_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:
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());
- }
- return fail_value;
-}
-
-template <typename T> T Scalar::GetAsUnsigned(T fail_value) const {
- switch (m_type) {
- case e_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:
return m_integer.zextOrTrunc(sizeof(T) * 8).getZExtValue();
@@ -698,45 +670,45 @@
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());
+ return static_cast<T>((ldbl_val.sextOrTrunc(sizeof(T) * 8)).getSExtValue());
}
return fail_value;
}
signed char Scalar::SChar(signed char fail_value) const {
- return GetAsSigned<signed char>(fail_value);
+ return GetAs<signed char>(fail_value);
}
unsigned char Scalar::UChar(unsigned char fail_value) const {
- return GetAsUnsigned<unsigned char>(fail_value);
+ return GetAs<unsigned char>(fail_value);
}
short Scalar::SShort(short fail_value) const {
- return GetAsSigned<short>(fail_value);
+ return GetAs<short>(fail_value);
}
unsigned short Scalar::UShort(unsigned short fail_value) const {
- return GetAsUnsigned<unsigned short>(fail_value);
+ return GetAs<unsigned short>(fail_value);
}
-int Scalar::SInt(int fail_value) const { return GetAsSigned<int>(fail_value); }
+int Scalar::SInt(int fail_value) const { return GetAs<int>(fail_value); }
unsigned int Scalar::UInt(unsigned int fail_value) const {
- return GetAsUnsigned<unsigned int>(fail_value);
+ return GetAs<unsigned int>(fail_value);
}
-long Scalar::SLong(long fail_value) const { return GetAsSigned<long>(fail_value); }
+long Scalar::SLong(long fail_value) const { return GetAs<long>(fail_value); }
unsigned long Scalar::ULong(unsigned long fail_value) const {
- return GetAsUnsigned<unsigned long>(fail_value);
+ return GetAs<unsigned long>(fail_value);
}
long long Scalar::SLongLong(long long fail_value) const {
- return GetAsSigned<long long>(fail_value);
+ return GetAs<long long>(fail_value);
}
unsigned long long Scalar::ULongLong(unsigned long long fail_value) const {
- return GetAsUnsigned<unsigned long long>(fail_value);
+ return GetAs<unsigned long long>(fail_value);
}
llvm::APInt Scalar::SInt128(const llvm::APInt &fail_value) const {
@@ -794,18 +766,21 @@
case e_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:
+ return llvm::APIntOps::RoundSignedAPIntToFloat(m_integer);
+
+ case e_uint:
+ case e_ulong:
+ case e_ulonglong:
+ case e_uint128:
+ case e_uint256:
case e_uint512:
return llvm::APIntOps::RoundAPIntToFloat(m_integer);
+
case e_float:
return m_float.convertToFloat();
case e_double:
@@ -822,18 +797,21 @@
case e_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:
+ return llvm::APIntOps::RoundSignedAPIntToDouble(m_integer);
+
+ case e_uint:
+ case e_ulong:
+ case e_ulonglong:
+ case e_uint128:
+ case e_uint256:
case e_uint512:
return llvm::APIntOps::RoundAPIntToDouble(m_integer);
+
case e_float:
return static_cast<double_t>(m_float.convertToFloat());
case e_double:
@@ -850,19 +828,21 @@
case e_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:
+ return llvm::APIntOps::RoundSignedAPIntToDouble(m_integer);
+
+ case e_uint:
+ case e_ulong:
+ case e_ulonglong:
+ case e_uint128:
+ case e_uint256:
case e_uint512:
- return static_cast<long_double_t>(
- llvm::APIntOps::RoundAPIntToDouble(m_integer));
+ return llvm::APIntOps::RoundAPIntToDouble(m_integer);
+
case e_float:
return static_cast<long_double_t>(m_float.convertToFloat());
case e_double:
Index: lldb/source/Expression/IRInterpreter.cpp
===================================================================
--- lldb/source/Expression/IRInterpreter.cpp
+++ lldb/source/Expression/IRInterpreter.cpp
@@ -147,7 +147,7 @@
return std::string(ss.GetString());
}
- bool AssignToMatchType(lldb_private::Scalar &scalar, uint64_t u64value,
+ bool AssignToMatchType(lldb_private::Scalar &scalar, llvm::APInt value,
Type *type) {
size_t type_size = m_target_data.getTypeStoreSize(type);
@@ -157,7 +157,7 @@
if (type_size != 1)
type_size = PowerOf2Ceil(type_size);
- scalar = llvm::APInt(type_size*8, u64value);
+ scalar = value.zextOrTrunc(type_size * 8);
return true;
}
@@ -171,8 +171,7 @@
if (!ResolveConstantValue(value_apint, constant))
return false;
- return AssignToMatchType(scalar, value_apint.getLimitedValue(),
- value->getType());
+ return AssignToMatchType(scalar, value_apint, value->getType());
}
lldb::addr_t process_address = ResolveValue(value, module);
@@ -190,13 +189,14 @@
lldb::offset_t offset = 0;
if (value_size <= 8) {
uint64_t u64value = value_extractor.GetMaxU64(&offset, value_size);
- return AssignToMatchType(scalar, u64value, value->getType());
+ return AssignToMatchType(scalar, llvm::APInt(64, u64value),
+ value->getType());
}
return false;
}
- bool AssignValue(const Value *value, lldb_private::Scalar &scalar,
+ bool AssignValue(const Value *value, lldb_private::Scalar scalar,
Module &module) {
lldb::addr_t process_address = ResolveValue(value, module);
@@ -205,7 +205,9 @@
lldb_private::Scalar cast_scalar;
- if (!AssignToMatchType(cast_scalar, scalar.ULongLong(), value->getType()))
+ scalar.MakeUnsigned();
+ if (!AssignToMatchType(cast_scalar, scalar.UInt128(llvm::APInt()),
+ value->getType()))
return false;
size_t value_byte_size = m_target_data.getTypeStoreSize(value->getType());
Index: lldb/source/Core/ValueObject.cpp
===================================================================
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1203,6 +1203,7 @@
if (ResolveValue(scalar)) {
if (success)
*success = true;
+ scalar.MakeUnsigned();
return scalar.ULongLong(fail_value);
}
// fallthrough, otherwise...
@@ -1220,6 +1221,7 @@
if (ResolveValue(scalar)) {
if (success)
*success = true;
+ scalar.MakeSigned();
return scalar.SLongLong(fail_value);
}
// fallthrough, otherwise...
Index: lldb/include/lldb/Utility/Scalar.h
===================================================================
--- lldb/include/lldb/Utility/Scalar.h
+++ lldb/include/lldb/Utility/Scalar.h
@@ -267,8 +267,7 @@
llvm::APInt m_integer;
llvm::APFloat m_float;
- template <typename T> T GetAsSigned(T fail_value) const;
- template <typename T> T GetAsUnsigned(T fail_value) const;
+ template <typename T> T GetAs(T fail_value) const;
private:
friend const Scalar operator+(const Scalar &lhs, const Scalar &rhs);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
