Avoid an assertion failure when a bit field is extracted from a value of the same size.

[bryanpkc]

Summary

One of the cases handled by ValueObjectChild::UpdateValue() uses the entire width of the parent's scalar value as the size of the child, and extracts the child by calling Scalar::ExtractBitfield(). This seems valid but APInt::trunc(), APInt::sext() and APInt::zext() assert that the bit field must not have the same size as the parent scalar. Replacing those calls with sextOrTrunc(), zextOrTrunc(), sextOrSelf() and zextOrSelf() fixes the assertion failures.

[labath]

This fixes the same issue as D19535, which is stalled at the moment waiting for a unit test. You can take over if you write a quick unit test for this (unittests/Core/ScalarTest.cpp). Thanks.

[bryanpkc]

Added a unit test in ScalarTest.cpp that catches this particular error.

[labath]

Looks good. Thank you for taking the time to add the tests. Please consider the comment below though.

Do you have commit access?

unittests/Core/ScalarTest.cpp

90

Is there a reason this couldn't be written as ASSERT_EQ(a1, s_scalar.SLongLong()) ?
If there isn't one, I think this would make the check more readable.

[bryanpkc]

In D20355#434117, @labath wrote:

Do you have commit access?

Yes I do. Thanks for your review!

unittests/Core/ScalarTest.cpp

90

SLongLong() invokes sextOrTrunc() and getSExtValue(), potentially further changing the contents of the underlying m_integer. I felt that checking with memcmp() immediately after ExtractBitfield() is a more fool-proof way to confirm the behaviour of ExtractBitfield(). There are precedents for using memcmp() in the file, and it is not that much more unreadable.

[labath]

unittests/Core/ScalarTest.cpp

90

Ok, I'll leave that up to you.

source/Core/Scalar.cpp

switch (m_type)
         case Scalar::e_long_double:
             break;
             
         case Scalar::e_sint:
         case Scalar::e_slong:
         case Scalar::e_slonglong:
         case Scalar::e_sint128:
         case Scalar::e_sint256:
-            m_integer = m_integer.ashr(bit_offset).trunc(bit_size).sext(8 * GetByteSize());
+            m_integer = m_integer.ashr(bit_offset).sextOrTrunc(bit_size).sextOrSelf(8 * GetByteSize());
             return true;
 
         case Scalar::e_uint:
         case Scalar::e_ulong:
         case Scalar::e_ulonglong:
         case Scalar::e_uint128:
         case Scalar::e_uint256:
-            m_integer = m_integer.lshr(bit_offset).trunc(bit_size).zext(8 * GetByteSize());
+            m_integer = m_integer.lshr(bit_offset).zextOrTrunc(bit_size).zextOrSelf(8 * GetByteSize());
             return true;
     }
     return false;
 }
 
 bool
 lldb_private::operator== (const Scalar& lhs, const Scalar& rhs)
 {

unittests/Core/ScalarTest.cpp

TEST(ScalarTest, CastOperations)
     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());
 }
 
+TEST(ScalarTest, ExtractBitfield)
+{
+    uint32_t len = sizeof(long long) * 8;
+
+    long long a1 = 0xf1f2f3f4f5f6f7f8LL;
+    long long b1 = 0xff1f2f3f4f5f6f7fLL;
+    Scalar s_scalar(a1);
+    ASSERT_TRUE(s_scalar.ExtractBitfield(0, 0));
+    ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+    ASSERT_TRUE(s_scalar.ExtractBitfield(len, 0));
+    ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1)));
+    ASSERT_TRUE(s_scalar.ExtractBitfield(len - 4, 4));
+    ASSERT_EQ(0, memcmp(&b1, s_scalar.GetBytes(), sizeof(b1)));
+
+    unsigned long long a2 = 0xf1f2f3f4f5f6f7f8ULL;
+    unsigned long long b2 = 0x0f1f2f3f4f5f6f7fULL;
+    Scalar u_scalar(a2);
+    ASSERT_TRUE(u_scalar.ExtractBitfield(0, 0));
+    ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2)));
+    ASSERT_TRUE(u_scalar.ExtractBitfield(len, 0));
+    ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2)));
+    ASSERT_TRUE(u_scalar.ExtractBitfield(len - 4, 4));
+    ASSERT_EQ(0, memcmp(&b2, u_scalar.GetBytes(), sizeof(b2)));
+}