From b00ef10b70b1939e7ef3a499eedbeac6860fa3fc Mon Sep 17 00:00:00 2001 From: Ulrich Weigand Date: Thu, 14 Apr 2016 14:31:08 +0000 Subject: [PATCH] Make Scalar::GetBytes and RegisterValue::GetBytes const Scalar::GetBytes provides a non-const access to the underlying bytes of the scalar value, supposedly allowing for modification of those bytes. However, even with the current implementation, this is not really possible. For floating-point scalars, the pointer returned by GetBytes refers to a temporary copy; modifications to that copy will be simply ignored. For integer scalars, the pointer refers to internal memory of the APInt implementation, which isn't supposed to be directly modifyable; GetBytes simply casts aways the const-ness of the pointer ... With my upcoming patch to fix Scalar::GetBytes for big-endian systems, this problem is going to get worse, since there we need temporary copies even for some integer scalars. Therefore, this patch makes Scalar::GetBytes const, fixing all those problems. As a follow-on change, RegisterValues::GetBytes must be made const as well. This in turn means that the way of initializing a RegisterValue by doing a SetType followed by writing to GetBytes no longer works. Instead, I've changed SetValueFromData to do the equivalent of SetType itself, and then re-implemented SetFromMemoryData to work on top of SetValueFromData. There is still a need for RegisterValue::SetType, since some platform-specific code uses it to reinterpret the contents of an already filled RegisterValue. To make this usage work in all cases (even changing from a type implemented via Scalar to a type implemented as a byte buffer), SetType now simply copies the old contents out, and then reloads the RegisterValue from this data using the new type via SetValueFromData. This in turn means that there is no remaining caller of Scalar::SetType, so it can be removed. The only other follow-on change was in MIPS EmulateInstruction code, where some uses of RegisterValue::GetBytes could be made const trivially. Differential Revision: http://reviews.llvm.org/D18980 llvm-svn: 266310 --- lldb/include/lldb/Core/RegisterValue.h | 3 - lldb/include/lldb/Core/Scalar.h | 5 +- lldb/source/Core/RegisterValue.cpp | 131 +++++------------- lldb/source/Core/Scalar.cpp | 89 +----------- .../MIPS/EmulateInstructionMIPS.cpp | 10 +- .../MIPS64/EmulateInstructionMIPS64.cpp | 10 +- 6 files changed, 54 insertions(+), 194 deletions(-) diff --git a/lldb/include/lldb/Core/RegisterValue.h b/lldb/include/lldb/Core/RegisterValue.h index 030b849212c4..0626b5f2c857 100644 --- a/lldb/include/lldb/Core/RegisterValue.h +++ b/lldb/include/lldb/Core/RegisterValue.h @@ -354,9 +354,6 @@ namespace lldb_private { lldb::Format format, uint32_t reg_name_right_align_at = 0) const; - void * - GetBytes (); - const void * GetBytes () const; diff --git a/lldb/include/lldb/Core/Scalar.h b/lldb/include/lldb/Core/Scalar.h index 545b91fd6109..db103ffc9850 100644 --- a/lldb/include/lldb/Core/Scalar.h +++ b/lldb/include/lldb/Core/Scalar.h @@ -120,7 +120,7 @@ public: bool ClearBit(uint32_t bit); - void * + const void * GetBytes() const; size_t @@ -231,9 +231,6 @@ public: Scalar::Type GetType() const { return m_type; } - void - SetType(const RegisterInfo*); - //---------------------------------------------------------------------- // Returns a casted value of the current contained data without // modifying the current value. FAIL_VALUE will be returned if the type diff --git a/lldb/source/Core/RegisterValue.cpp b/lldb/source/Core/RegisterValue.cpp index b715ca9a0980..91ef5b9efb96 100644 --- a/lldb/source/Core/RegisterValue.cpp +++ b/lldb/source/Core/RegisterValue.cpp @@ -203,34 +203,13 @@ RegisterValue::SetFromMemoryData (const RegisterInfo *reg_info, // Use a data extractor to correctly copy and pad the bytes read into the // register value DataExtractor src_data (src, src_len, src_byte_order, 4); - - // Given the register info, set the value type of this RegisterValue object - SetType (reg_info); - // And make sure we were able to figure out what that register value was - RegisterValue::Type value_type = GetType(); - if (value_type == eTypeInvalid) - { - // No value has been read into this object... - error.SetErrorStringWithFormat("invalid register value type for register %s", reg_info->name); + + error = SetValueFromData(reg_info, src_data, 0, true); + if (error.Fail()) return 0; - } - else if (value_type == eTypeBytes) - { - buffer.byte_order = src_byte_order; - // Make sure to set the buffer length of the destination buffer to avoid - // problems due to uninitialized variables. - buffer.length = src_len; - } - const uint32_t bytes_copied = src_data.CopyByteOrderedData (0, // src offset - src_len, // src length - GetBytes(), // dst buffer - GetByteSize(), // dst length - GetByteOrder()); // dst byte order - if (bytes_copied == 0) - error.SetErrorStringWithFormat("failed to copy data for register write of %s", reg_info->name); - - return bytes_copied; + // If SetValueFromData succeeded, we must have copied all of src_len + return src_len; } bool @@ -282,41 +261,12 @@ RegisterValue::Clear() RegisterValue::Type RegisterValue::SetType (const RegisterInfo *reg_info) { - m_type = eTypeInvalid; - const uint32_t byte_size = reg_info->byte_size; - switch (reg_info->encoding) - { - case eEncodingInvalid: - break; - - case eEncodingUint: - case eEncodingSint: - if (byte_size == 1) - m_type = eTypeUInt8; - else if (byte_size <= 2) - m_type = eTypeUInt16; - else if (byte_size <= 4) - m_type = eTypeUInt32; - else if (byte_size <= 8) - m_type = eTypeUInt64; - else if (byte_size <= 16) - m_type = eTypeUInt128; - break; + // To change the type, we simply copy the data in again, using the new format + RegisterValue copy; + DataExtractor copy_data; + if (copy.CopyValue(*this) && copy.GetData(copy_data)) + SetValueFromData(reg_info, copy_data, 0, true); - case eEncodingIEEE754: - if (byte_size == sizeof(float)) - m_type = eTypeFloat; - else if (byte_size == sizeof(double)) - m_type = eTypeDouble; - else if (byte_size == sizeof(long double)) - m_type = eTypeLongDouble; - break; - - case eEncodingVector: - m_type = eTypeBytes; - break; - } - m_scalar.SetType(reg_info); return m_type; } @@ -354,16 +304,23 @@ RegisterValue::SetValueFromData (const RegisterInfo *reg_info, DataExtractor &sr memset (buffer.bytes, 0, sizeof (buffer.bytes)); type128 int128; - switch (SetType (reg_info)) + + m_type = eTypeInvalid; + switch (reg_info->encoding) { - case eTypeInvalid: - error.SetErrorString(""); + case eEncodingInvalid: break; - case eTypeUInt8: SetUInt8 (src.GetMaxU32 (&src_offset, src_len)); break; - case eTypeUInt16: SetUInt16 (src.GetMaxU32 (&src_offset, src_len)); break; - case eTypeUInt32: SetUInt32 (src.GetMaxU32 (&src_offset, src_len)); break; - case eTypeUInt64: SetUInt64 (src.GetMaxU64 (&src_offset, src_len)); break; - case eTypeUInt128: + case eEncodingUint: + case eEncodingSint: + if (reg_info->byte_size == 1) + SetUInt8(src.GetMaxU32(&src_offset, src_len)); + else if (reg_info->byte_size <= 2) + SetUInt16(src.GetMaxU32(&src_offset, src_len)); + else if (reg_info->byte_size <= 4) + SetUInt32(src.GetMaxU32(&src_offset, src_len)); + else if (reg_info->byte_size <= 8) + SetUInt64(src.GetMaxU64(&src_offset, src_len)); + else if (reg_info->byte_size <= 16) { uint64_t data1 = src.GetU64 (&src_offset); uint64_t data2 = src.GetU64 (&src_offset); @@ -380,10 +337,15 @@ RegisterValue::SetValueFromData (const RegisterInfo *reg_info, DataExtractor &sr SetUInt128 (llvm::APInt(128, 2, int128.x)); } break; - case eTypeFloat: SetFloat (src.GetFloat (&src_offset)); break; - case eTypeDouble: SetDouble(src.GetDouble (&src_offset)); break; - case eTypeLongDouble: SetFloat (src.GetLongDouble (&src_offset)); break; - case eTypeBytes: + case eEncodingIEEE754: + if (reg_info->byte_size == sizeof(float)) + SetFloat(src.GetFloat(&src_offset)); + else if (reg_info->byte_size == sizeof(double)) + SetDouble(src.GetDouble(&src_offset)); + else if (reg_info->byte_size == sizeof(long double)) + SetLongDouble(src.GetLongDouble(&src_offset)); + break; + case eEncodingVector: { m_type = eTypeBytes; buffer.length = reg_info->byte_size; @@ -397,12 +359,14 @@ RegisterValue::SetValueFromData (const RegisterInfo *reg_info, DataExtractor &sr buffer.length, // dst length buffer.byte_order) == 0)// dst byte order { - error.SetErrorString ("data copy failed data."); + error.SetErrorStringWithFormat("failed to copy data for register write of %s", reg_info->name); return error; } } } - + + if (m_type == eTypeInvalid) + error.SetErrorStringWithFormat("invalid register value type for register %s", reg_info->name); return error; } @@ -840,25 +804,6 @@ RegisterValue::GetBytes () const return nullptr; } -void * -RegisterValue::GetBytes () -{ - switch (m_type) - { - case eTypeInvalid: break; - case eTypeUInt8: - case eTypeUInt16: - case eTypeUInt32: - case eTypeUInt64: - case eTypeUInt128: - case eTypeFloat: - case eTypeDouble: - case eTypeLongDouble: return m_scalar.GetBytes(); - case eTypeBytes: return buffer.bytes; - } - return nullptr; -} - uint32_t RegisterValue::GetByteSize () const { diff --git a/lldb/source/Core/Scalar.cpp b/lldb/source/Core/Scalar.cpp index d2987440eed3..85c37bc24c08 100644 --- a/lldb/source/Core/Scalar.cpp +++ b/lldb/source/Core/Scalar.cpp @@ -250,7 +250,7 @@ Scalar::GetData (DataExtractor &data, size_t limit_byte_size) const return false; } -void * +const void * Scalar::GetBytes() const { static float_t flt_val; @@ -269,16 +269,16 @@ Scalar::GetBytes() const case e_uint128: case e_sint256: case e_uint256: - return const_cast(reinterpret_cast(m_integer.getRawData())); + return reinterpret_cast(m_integer.getRawData()); case e_float: flt_val = m_float.convertToFloat(); - return (void *)&flt_val; + return reinterpret_cast(&flt_val); case e_double: dbl_val = m_float.convertToDouble(); - return (void *)&dbl_val; + return reinterpret_cast(&dbl_val); case e_long_double: llvm::APInt ldbl_val = m_float.bitcastToAPInt(); - return const_cast(reinterpret_cast(ldbl_val.getRawData())); + return reinterpret_cast(ldbl_val.getRawData()); } return nullptr; } @@ -3130,82 +3130,3 @@ Scalar::SetBit (uint32_t bit) return false; } -void -Scalar::SetType (const RegisterInfo *reg_info) -{ - const uint32_t byte_size = reg_info->byte_size; - switch (reg_info->encoding) - { - case eEncodingInvalid: - break; - case eEncodingUint: - if (byte_size == 1 || byte_size == 2 || byte_size == 4) - { - m_integer = llvm::APInt(sizeof(uint_t) * 8, *(const uint64_t *)m_integer.getRawData(), false); - m_type = e_uint; - } - if (byte_size == 8) - { - m_integer = llvm::APInt(sizeof(ulonglong_t) * 8, *(const uint64_t *)m_integer.getRawData(), false); - m_type = e_ulonglong; - } - if (byte_size == 16) - { - m_integer = llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128, ((const type128 *)m_integer.getRawData())->x); - m_type = e_uint128; - } - if (byte_size == 32) - { - m_integer = llvm::APInt(BITWIDTH_INT256, NUM_OF_WORDS_INT256, ((const type256 *)m_integer.getRawData())->x); - m_type = e_uint256; - } - break; - case eEncodingSint: - if (byte_size == 1 || byte_size == 2 || byte_size == 4) - { - m_integer = llvm::APInt(sizeof(sint_t) * 8, *(const uint64_t *)m_integer.getRawData(), true); - m_type = e_sint; - } - if (byte_size == 8) - { - m_integer = llvm::APInt(sizeof(slonglong_t) * 8, *(const uint64_t *)m_integer.getRawData(), true); - m_type = e_slonglong; - } - if (byte_size == 16) - { - m_integer = llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128, ((const type128 *)m_integer.getRawData())->x); - m_type = e_sint128; - } - if (byte_size == 32) - { - m_integer = llvm::APInt(BITWIDTH_INT256, NUM_OF_WORDS_INT256, ((const type256 *)m_integer.getRawData())->x); - m_type = e_sint256; - } - break; - case eEncodingIEEE754: - if (byte_size == sizeof(float)) - { - bool losesInfo = false; - m_float.convert(llvm::APFloat::IEEEsingle, llvm::APFloat::rmTowardZero, &losesInfo); - m_type = e_float; - } - else if (byte_size == sizeof(double)) - { - bool losesInfo = false; - m_float.convert(llvm::APFloat::IEEEdouble, llvm::APFloat::rmTowardZero, &losesInfo); - m_type = e_double; - } - else if (byte_size == sizeof(long double)) - { - if(m_ieee_quad) - m_float = llvm::APFloat(llvm::APFloat::IEEEquad, m_float.bitcastToAPInt()); - else - m_float = llvm::APFloat(llvm::APFloat::x87DoubleExtended, m_float.bitcastToAPInt()); - m_type = e_long_double; - } - break; - case eEncodingVector: - m_type = e_void; - break; - } -} diff --git a/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp b/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp index c4cda3482193..21766f722b8d 100644 --- a/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp +++ b/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp @@ -2603,7 +2603,7 @@ EmulateInstructionMIPS::Emulate_MSA_Branch_DF (llvm::MCInst& insn, int element_b bool success = false, branch_hit = true; int32_t target = 0; RegisterValue reg_value; - uint8_t * ptr = NULL; + const uint8_t *ptr = NULL; uint32_t wt = m_reg_info->getEncodingValue (insn.getOperand(0).getReg()); int32_t offset = insn.getOperand(1).getImm(); @@ -2613,7 +2613,7 @@ EmulateInstructionMIPS::Emulate_MSA_Branch_DF (llvm::MCInst& insn, int element_b return false; if (ReadRegister (eRegisterKindDWARF, dwarf_w0_mips + wt, reg_value)) - ptr = (uint8_t *)reg_value.GetBytes(); + ptr = (const uint8_t *)reg_value.GetBytes(); else return false; @@ -2626,15 +2626,15 @@ EmulateInstructionMIPS::Emulate_MSA_Branch_DF (llvm::MCInst& insn, int element_b branch_hit = false; break; case 2: - if((*(uint16_t *)ptr == 0 && bnz) || (*(uint16_t *)ptr != 0 && !bnz)) + if ((*(const uint16_t *)ptr == 0 && bnz) || (*(const uint16_t *)ptr != 0 && !bnz)) branch_hit = false; break; case 4: - if((*(uint32_t *)ptr == 0 && bnz) || (*(uint32_t *)ptr != 0 && !bnz)) + if ((*(const uint32_t *)ptr == 0 && bnz) || (*(const uint32_t *)ptr != 0 && !bnz)) branch_hit = false; break; case 8: - if((*(uint64_t *)ptr == 0 && bnz) || (*(uint64_t *)ptr != 0 && !bnz)) + if ((*(const uint64_t *)ptr == 0 && bnz) || (*(const uint64_t *)ptr != 0 && !bnz)) branch_hit = false; break; } diff --git a/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp b/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp index 4d4b582c7ec8..7ce14727b347 100644 --- a/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp +++ b/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp @@ -1874,7 +1874,7 @@ EmulateInstructionMIPS64::Emulate_MSA_Branch_DF (llvm::MCInst& insn, int element bool success = false, branch_hit = true; int64_t target = 0; RegisterValue reg_value; - uint8_t * ptr = NULL; + const uint8_t *ptr = NULL; uint32_t wt = m_reg_info->getEncodingValue (insn.getOperand(0).getReg()); int64_t offset = insn.getOperand(1).getImm(); @@ -1884,7 +1884,7 @@ EmulateInstructionMIPS64::Emulate_MSA_Branch_DF (llvm::MCInst& insn, int element return false; if (ReadRegister (eRegisterKindDWARF, dwarf_w0_mips64 + wt, reg_value)) - ptr = (uint8_t *)reg_value.GetBytes(); + ptr = (const uint8_t *)reg_value.GetBytes(); else return false; @@ -1897,15 +1897,15 @@ EmulateInstructionMIPS64::Emulate_MSA_Branch_DF (llvm::MCInst& insn, int element branch_hit = false; break; case 2: - if((*(uint16_t *)ptr == 0 && bnz) || (*(uint16_t *)ptr != 0 && !bnz)) + if ((*(const uint16_t *)ptr == 0 && bnz) || (*(const uint16_t *)ptr != 0 && !bnz)) branch_hit = false; break; case 4: - if((*(uint32_t *)ptr == 0 && bnz) || (*(uint32_t *)ptr != 0 && !bnz)) + if ((*(const uint32_t *)ptr == 0 && bnz) || (*(const uint32_t *)ptr != 0 && !bnz)) branch_hit = false; break; case 8: - if((*(uint64_t *)ptr == 0 && bnz) || (*(uint64_t *)ptr != 0 && !bnz)) + if ((*(const uint64_t *)ptr == 0 && bnz) || (*(const uint64_t *)ptr != 0 && !bnz)) branch_hit = false; break; }