From: Pavel Labath Date: Thu, 25 Jun 2020 13:18:02 +0000 (+0200) Subject: [lldb] Rewrite Scalar::GetBytes X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d0fa52cc3797fd8805d24a04e6b8198154cd7b53;p=platform%2Fupstream%2Fllvm.git [lldb] Rewrite Scalar::GetBytes This function was modifying and returning pointers to static storage, which meant that any two accesses to different Scalar objects could potentially race (depending on which types the objects were storing and the host endianness). In the new version the user is responsible for providing a buffer into which this method will store its binary representation. The main caller (RegisterValue::GetBytes) already has one such buffer handy, so this did not require any major rewrites. To make that work, I've needed to mark the RegisterValue value buffer mutable -- not an ideal solution, but definitely better than modifying global storage. This could be further improved by changing RegisterValue::GetBytes to take a buffer too. --- diff --git a/lldb/include/lldb/Utility/RegisterValue.h b/lldb/include/lldb/Utility/RegisterValue.h index 494f8be..c9f295a 100644 --- a/lldb/include/lldb/Utility/RegisterValue.h +++ b/lldb/include/lldb/Utility/RegisterValue.h @@ -260,8 +260,9 @@ protected: Scalar m_scalar; struct { - uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any - // register for any supported target. + mutable uint8_t + bytes[kMaxRegisterByteSize]; // This must be big enough to hold any + // register for any supported target. uint16_t length; lldb::ByteOrder byte_order; } buffer; diff --git a/lldb/include/lldb/Utility/Scalar.h b/lldb/include/lldb/Utility/Scalar.h index 203ba56..eda7528 100644 --- a/lldb/include/lldb/Utility/Scalar.h +++ b/lldb/include/lldb/Utility/Scalar.h @@ -106,7 +106,10 @@ public: bool ClearBit(uint32_t bit); - const void *GetBytes() const; + /// Store the binary representation of this value into the given storage. + /// Exactly GetByteSize() bytes will be stored, and the buffer must be large + /// enough to hold this data. + void GetBytes(llvm::MutableArrayRef storage) const; size_t GetByteSize() const; diff --git a/lldb/source/Utility/RegisterValue.cpp b/lldb/source/Utility/RegisterValue.cpp index 91f4025..7f545c2 100644 --- a/lldb/source/Utility/RegisterValue.cpp +++ b/lldb/source/Utility/RegisterValue.cpp @@ -728,7 +728,8 @@ const void *RegisterValue::GetBytes() const { case eTypeFloat: case eTypeDouble: case eTypeLongDouble: - return m_scalar.GetBytes(); + m_scalar.GetBytes(buffer.bytes); + return buffer.bytes; case eTypeBytes: return buffer.bytes; } diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp index 748dd55..b34311b 100644 --- a/lldb/source/Utility/Scalar.cpp +++ b/lldb/source/Utility/Scalar.cpp @@ -7,7 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/Utility/Scalar.h" - +#include "lldb/Utility/DataBufferHeap.h" #include "lldb/Utility/DataExtractor.h" #include "lldb/Utility/Endian.h" #include "lldb/Utility/Status.h" @@ -73,36 +73,36 @@ Scalar::Scalar() : m_type(e_void), m_float(static_cast(0)) {} bool Scalar::GetData(DataExtractor &data, size_t limit_byte_size) const { size_t byte_size = GetByteSize(); - if (byte_size > 0) { - const uint8_t *bytes = static_cast(GetBytes()); - - if (limit_byte_size < byte_size) { - if (endian::InlHostByteOrder() == eByteOrderLittle) { - // On little endian systems if we want fewer bytes from the current - // type we just specify fewer bytes since the LSByte is first... - byte_size = limit_byte_size; - } else if (endian::InlHostByteOrder() == eByteOrderBig) { - // On big endian systems if we want fewer bytes from the current type - // have to advance our initial byte pointer and trim down the number of - // bytes since the MSByte is first - bytes += byte_size - limit_byte_size; - byte_size = limit_byte_size; - } + if (byte_size == 0) { + data.Clear(); + return false; + } + auto buffer_up = std::make_unique(byte_size, 0); + GetBytes(buffer_up->GetData()); + lldb::offset_t offset = 0; + + if (limit_byte_size < byte_size) { + if (endian::InlHostByteOrder() == eByteOrderLittle) { + // On little endian systems if we want fewer bytes from the current + // type we just specify fewer bytes since the LSByte is first... + byte_size = limit_byte_size; + } else if (endian::InlHostByteOrder() == eByteOrderBig) { + // On big endian systems if we want fewer bytes from the current type + // have to advance our initial byte pointer and trim down the number of + // bytes since the MSByte is first + offset = byte_size - limit_byte_size; + byte_size = limit_byte_size; } - - data.SetData(bytes, byte_size, endian::InlHostByteOrder()); - return true; } - data.Clear(); - return false; + + data.SetData(std::move(buffer_up), offset, byte_size); + data.SetByteOrder(endian::InlHostByteOrder()); + return true; } -const void *Scalar::GetBytes() const { - const uint64_t *apint_words; - const uint8_t *bytes; - static float_t flt_val; - static double_t dbl_val; - static uint64_t swapped_words[8]; +void Scalar::GetBytes(llvm::MutableArrayRef storage) const { + assert(storage.size() >= GetByteSize()); + switch (m_type) { case e_void: break; @@ -112,73 +112,31 @@ const void *Scalar::GetBytes() const { case e_ulong: case e_slonglong: case e_ulonglong: - bytes = reinterpret_cast(m_integer.getRawData()); - // getRawData always returns a pointer to an uint64_t. If we have a - // smaller type, we need to update the pointer on big-endian systems. - if (endian::InlHostByteOrder() == eByteOrderBig) { - size_t byte_size = m_integer.getBitWidth() / 8; - if (byte_size < 8) - bytes += 8 - byte_size; - } - return bytes; - // getRawData always returns a pointer to an array of uint64_t values, - // where the least-significant word always comes first. On big-endian - // systems we need to swap the words. case e_sint128: case e_uint128: - apint_words = m_integer.getRawData(); - if (endian::InlHostByteOrder() == eByteOrderBig) { - swapped_words[0] = apint_words[1]; - swapped_words[1] = apint_words[0]; - apint_words = swapped_words; - } - return static_cast(apint_words); case e_sint256: case e_uint256: - apint_words = m_integer.getRawData(); - if (endian::InlHostByteOrder() == eByteOrderBig) { - swapped_words[0] = apint_words[3]; - swapped_words[1] = apint_words[2]; - swapped_words[2] = apint_words[1]; - swapped_words[3] = apint_words[0]; - apint_words = swapped_words; - } - return static_cast(apint_words); case e_sint512: case e_uint512: - apint_words = m_integer.getRawData(); - if (endian::InlHostByteOrder() == eByteOrderBig) { - swapped_words[0] = apint_words[7]; - swapped_words[1] = apint_words[6]; - swapped_words[2] = apint_words[5]; - swapped_words[3] = apint_words[4]; - swapped_words[4] = apint_words[3]; - swapped_words[5] = apint_words[2]; - swapped_words[6] = apint_words[1]; - swapped_words[7] = apint_words[0]; - apint_words = swapped_words; - } - return static_cast(apint_words); - case e_float: - flt_val = m_float.convertToFloat(); - return static_cast(&flt_val); - case e_double: - dbl_val = m_float.convertToDouble(); - return static_cast(&dbl_val); - case e_long_double: - llvm::APInt ldbl_val = m_float.bitcastToAPInt(); - apint_words = ldbl_val.getRawData(); - // getRawData always returns a pointer to an array of two uint64_t values, - // where the least-significant word always comes first. On big-endian - // systems we need to swap the two words. - if (endian::InlHostByteOrder() == eByteOrderBig) { - swapped_words[0] = apint_words[1]; - swapped_words[1] = apint_words[0]; - apint_words = swapped_words; - } - return static_cast(apint_words); + StoreIntToMemory(m_integer, storage.data(), + (m_integer.getBitWidth() + 7) / 8); + break; + case e_float: { + float val = m_float.convertToFloat(); + memcpy(storage.data(), &val, sizeof(val)); + break; + } + case e_double: { + double val = m_float.convertToDouble(); + memcpy(storage.data(), &val, sizeof(double)); + break; + } + case e_long_double: { + llvm::APInt val = m_float.bitcastToAPInt(); + StoreIntToMemory(val, storage.data(), storage.size()); + break; + } } - return nullptr; } size_t Scalar::GetByteSize() const { diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp index be23320..960161c 100644 --- a/lldb/unittests/Utility/ScalarTest.cpp +++ b/lldb/unittests/Utility/ScalarTest.cpp @@ -78,6 +78,7 @@ TEST(ScalarTest, RightShiftOperator) { } TEST(ScalarTest, GetBytes) { + uint8_t Storage[256]; int a = 0x01020304; long long b = 0x0102030405060708LL; float c = 1234567.89e32f; @@ -99,14 +100,20 @@ TEST(ScalarTest, GetBytes) { sizeof(void *)); Status f_error = f_scalar.SetValueFromData(f_data, lldb::eEncodingUint, sizeof(f)); - ASSERT_EQ(0, memcmp(&a, a_scalar.GetBytes(), sizeof(a))); - ASSERT_EQ(0, memcmp(&b, b_scalar.GetBytes(), sizeof(b))); - ASSERT_EQ(0, memcmp(&c, c_scalar.GetBytes(), sizeof(c))); - ASSERT_EQ(0, memcmp(&d, d_scalar.GetBytes(), sizeof(d))); + a_scalar.GetBytes(Storage); + ASSERT_EQ(0, memcmp(&a, Storage, sizeof(a))); + b_scalar.GetBytes(Storage); + ASSERT_EQ(0, memcmp(&b, Storage, sizeof(b))); + c_scalar.GetBytes(Storage); + ASSERT_EQ(0, memcmp(&c, Storage, sizeof(c))); + d_scalar.GetBytes(Storage); + ASSERT_EQ(0, memcmp(&d, Storage, sizeof(d))); ASSERT_EQ(0, e_error.Fail()); - ASSERT_EQ(0, memcmp(e, e_scalar.GetBytes(), sizeof(e))); + e_scalar.GetBytes(Storage); + ASSERT_EQ(0, memcmp(e, Storage, sizeof(e))); ASSERT_EQ(0, f_error.Fail()); - ASSERT_EQ(0, memcmp(f, f_scalar.GetBytes(), sizeof(f))); + f_scalar.GetBytes(Storage); + ASSERT_EQ(0, memcmp(f, Storage, sizeof(f))); } TEST(ScalarTest, CastOperations) { @@ -137,21 +144,21 @@ TEST(ScalarTest, ExtractBitfield) { 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))); + EXPECT_EQ(s_scalar, a1); ASSERT_TRUE(s_scalar.ExtractBitfield(len, 0)); - ASSERT_EQ(0, memcmp(&a1, s_scalar.GetBytes(), sizeof(a1))); + EXPECT_EQ(s_scalar, a1); ASSERT_TRUE(s_scalar.ExtractBitfield(len - 4, 4)); - ASSERT_EQ(0, memcmp(&b1, s_scalar.GetBytes(), sizeof(b1))); + EXPECT_EQ(s_scalar, 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))); + EXPECT_EQ(u_scalar, a2); ASSERT_TRUE(u_scalar.ExtractBitfield(len, 0)); - ASSERT_EQ(0, memcmp(&a2, u_scalar.GetBytes(), sizeof(a2))); + EXPECT_EQ(u_scalar, a2); ASSERT_TRUE(u_scalar.ExtractBitfield(len - 4, 4)); - ASSERT_EQ(0, memcmp(&b2, u_scalar.GetBytes(), sizeof(b2))); + EXPECT_EQ(u_scalar, b2); } template static std::string ScalarGetValue(T value) {