[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
This commit is contained in:
Pavel Labath 2020-06-29 16:17:29 +02:00
parent 8b7b0ad24c
commit b725142c8d
6 changed files with 98 additions and 60 deletions

View File

@ -267,8 +267,7 @@ protected:
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);

View File

@ -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...

View File

@ -147,7 +147,7 @@ public:
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 @@ public:
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 @@ public:
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 @@ public:
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 @@ public:
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());

View File

@ -644,15 +644,16 @@ bool Scalar::MakeUnsigned() {
return success;
}
template <typename T> T Scalar::GetAsSigned(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:
return m_integer.sextOrTrunc(sizeof(T) * 8).getSExtValue();
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=*/false);
llvm::APSInt result(sizeof(T) * 8, std::is_unsigned<T>::value);
bool isExact;
m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact);
return result.getSExtValue();
@ -661,56 +662,40 @@ template <typename T> T Scalar::GetAsSigned(T fail_value) const {
return fail_value;
}
template <typename T> T Scalar::GetAsUnsigned(T fail_value) const {
switch (GetCategory(m_type)) {
case Category::Void:
break;
case Category::Integral:
return m_integer.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;
}
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:

View File

@ -51,7 +51,8 @@ class IRInterpreterTestCase(TestBase):
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 @@ class IRInterpreterTestCase(TestBase):
"$i << $j",
"$i & $j",
"$i | $j",
"$i ^ $j"]
"$i ^ $j",
"($ull & -1) == $u"]
for expression in set_up_expressions:
self.frame().EvaluateExpression(expression, options)

View File

@ -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));
}