[Lldb-commits] [lldb] b725142 - [lldb] Fix type conversion in the Scalar getters
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 2 09:03:06 PDT 2020
Author: Pavel Labath
Date: 2020-07-02T18:02:57+02:00
New Revision: b725142c8db8584007cb1cd9149e8bcecaa88547
URL: https://github.com/llvm/llvm-project/commit/b725142c8db8584007cb1cd9149e8bcecaa88547
DIFF: https://github.com/llvm/llvm-project/commit/b725142c8db8584007cb1cd9149e8bcecaa88547.diff
LOG: [lldb] Fix type conversion in the Scalar getters
Summary:
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.
Reviewers: teemperor, JDevlieghere
Subscribers: lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D82772
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/Scalar.h b/lldb/include/lldb/Utility/Scalar.h
index 566c52c14a8d..275df4d63b0d 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -267,8 +267,7 @@ class Scalar {
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);
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index ba3d2c509be8..dfadb3c5233f 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -1203,6 +1203,7 @@ uint64_t ValueObject::GetValueAsUnsigned(uint64_t fail_value, bool *success) {
if (ResolveValue(scalar)) {
if (success)
*success = true;
+ scalar.MakeUnsigned();
return scalar.ULongLong(fail_value);
}
// fallthrough, otherwise...
@@ -1220,6 +1221,7 @@ int64_t ValueObject::GetValueAsSigned(int64_t fail_value, bool *success) {
if (ResolveValue(scalar)) {
if (success)
*success = true;
+ scalar.MakeSigned();
return scalar.SLongLong(fail_value);
}
// fallthrough, otherwise...
diff --git a/lldb/source/Expression/IRInterpreter.cpp b/lldb/source/Expression/IRInterpreter.cpp
index 04d7cb3e74b6..4c7a65626598 100644
--- a/lldb/source/Expression/IRInterpreter.cpp
+++ b/lldb/source/Expression/IRInterpreter.cpp
@@ -147,7 +147,7 @@ class InterpreterStackFrame {
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 @@ class InterpreterStackFrame {
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 @@ class InterpreterStackFrame {
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 @@ class InterpreterStackFrame {
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 @@ class InterpreterStackFrame {
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());
diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 7d0b7178ddd2..7bd42f3b9535 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -644,73 +644,58 @@ bool Scalar::MakeUnsigned() {
return success;
}
-template <typename T> T Scalar::GetAsSigned(T fail_value) const {
- switch (GetCategory(m_type)) {
- case Category::Void:
- break;
- case Category::Integral:
- return m_integer.sextOrTrunc(sizeof(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 {
+template <typename T> T Scalar::GetAs(T fail_value) const {
switch (GetCategory(m_type)) {
case Category::Void:
break;
case Category::Integral:
+ if (IsSigned(m_type))
+ return m_integer.sextOrTrunc(sizeof(T) * 8).getSExtValue();
return m_integer.zextOrTrunc(sizeof(T) * 8).getZExtValue();
case Category::Float: {
- llvm::APSInt result(sizeof(T) * 8, /*isUnsigned=*/true);
+ llvm::APSInt result(sizeof(T) * 8, std::is_unsigned<T>::value);
bool isExact;
m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact);
- return result.getZExtValue();
+ return result.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 {
@@ -768,18 +753,21 @@ float Scalar::Float(float fail_value) const {
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:
@@ -796,18 +784,21 @@ double Scalar::Double(double fail_value) const {
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:
@@ -824,19 +815,23 @@ long double Scalar::LongDouble(long double fail_value) const {
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 static_cast<long_double_t>(
+ 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));
+
case e_float:
return static_cast<long_double_t>(m_float.convertToFloat());
case e_double:
diff --git a/lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py b/lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
index 033f5bc5fa1d..5efbba90ccba 100644
--- a/lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
+++ b/lldb/test/API/commands/expression/ir-interpreter/TestIRInterpreter.py
@@ -51,7 +51,8 @@ def test_ir_interpreter(self):
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 @@ def test_ir_interpreter(self):
"$i << $j",
"$i & $j",
"$i | $j",
- "$i ^ $j"]
+ "$i ^ $j",
+ "($ull & -1) == $u"]
for expression in set_up_expressions:
self.frame().EvaluateExpression(expression, options)
diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp
index 64b3c9e2bc45..afbb76103ca6 100644
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -66,6 +66,44 @@ TEST(ScalarTest, ComparisonFloat) {
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));
+ EXPECT_NEAR((long double)val, Scalar(val).LongDouble(), 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;
@@ -336,11 +374,11 @@ TEST(ScalarTest, Scalar_512) {
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));
}
More information about the lldb-commits
mailing list