Make Scalar::GetBytes and RegisterValue::GetBytes const

[uweigand]

Summary

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.

[clayborg]

Looks good.

[tberghammer]

Looks good

lldb/trunk/include/lldb/Core/RegisterValue.h

public:
         bool
         Dump (Stream *s, 
               const RegisterInfo *reg_info, 
               bool prefix_with_name,
               bool prefix_with_alt_name,
               lldb::Format format,
               uint32_t reg_name_right_align_at = 0) const;
 
-        void *
-        GetBytes ();
-        
         const void *
         GetBytes () const;
 
         lldb::ByteOrder
         GetByteOrder () const
         {
             if (m_type == eTypeBytes)
                 return buffer.byte_order;

lldb/trunk/include/lldb/Core/Scalar.h

ExtractBitfield (uint32_t bit_size,
                      uint32_t bit_offset);
 
     bool
     SetBit(uint32_t bit);
 
     bool
     ClearBit(uint32_t bit);
 
-    void *
+    const void *
     GetBytes() const;
 
     size_t
     GetByteSize() const;
 
     bool
     GetData (DataExtractor &data, size_t limit_byte_size = UINT32_MAX) const;
 
public:
     OnesComplement();                       // Returns true on success
 
     //----------------------------------------------------------------------
     // Access the type of the current value.
     //----------------------------------------------------------------------
     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
     // of the value is void or invalid.
     //----------------------------------------------------------------------
     int
     SInt(int fail_value = 0) const;
 

lldb/trunk/source/Core/RegisterValue.cpp

RegisterValue::SetFromMemoryData (const RegisterInfo *reg_info,
     {
         error.SetErrorStringWithFormat("%u bytes is too big to store in register %s (%u bytes)", src_len, reg_info->name, dst_len);
         return 0;
     }
 
     // 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
 RegisterValue::GetScalarValue (Scalar &scalar) const
 {
     switch (m_type)
     {
         case eTypeInvalid:      break;

 RegisterValue::Clear()
 {
     m_type = eTypeInvalid;
 }
 
 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;
-
-        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;
+    // 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 eEncodingVector:
-            m_type = eTypeBytes;
-            break;
-    }
-    m_scalar.SetType(reg_info);
     return m_type;
 }
 
 Error
 RegisterValue::SetValueFromData (const RegisterInfo *reg_info, DataExtractor &src, lldb::offset_t src_offset, bool partial_data_ok)
 {
     Error error;
     
RegisterValue::SetValueFromData (const RegisterInfo *reg_info, DataExtractor &src, lldb::offset_t src_offset, bool partial_data_ok)
     // value
     if (src_len > reg_info->byte_size)
         src_len = reg_info->byte_size;
 
     // Zero out the value in case we get partial data...
     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);
                 if (src.GetByteSize() == eByteOrderBig)
                 {
                     int128.x[0] = data1;
                     int128.x[1] = data2;
                 }
                 else
                 {
                     int128.x[0] = data2;
                     int128.x[1] = data1;
                 }
                 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;
             buffer.byte_order = src.GetByteOrder();
             assert (buffer.length <= kMaxRegisterByteSize);
             if (buffer.length > kMaxRegisterByteSize)
                 buffer.length = kMaxRegisterByteSize;
             if (src.CopyByteOrderedData (src_offset,                    // offset within "src" to start extracting data
                                          src_len,                       // src length
                                          buffer.bytes,           // dst buffer
                                          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;
 }
 
 static inline void StripSpaces(llvm::StringRef &Str)
 {
     while (!Str.empty() && isspace(Str[0]))
         Str = Str.substr(1);
     while (!Str.empty() && isspace(Str.back()))
switch (m_type)
         case eTypeFloat:
         case eTypeDouble:
         case eTypeLongDouble:   return m_scalar.GetBytes();
         case eTypeBytes:        return buffer.bytes;
     }
     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
 {
     switch (m_type)
     {
         case eTypeInvalid: break;
         case eTypeUInt8:   return 1;
         case eTypeUInt16:  return 2;

lldb/trunk/source/Core/Scalar.cpp

if (byte_size > 0)
             }
         }
         return true;
     }
     data.Clear();
     return false;
 }
 
-void *
+const void *
 Scalar::GetBytes() const
 {
     static float_t flt_val;
     static double_t dbl_val;
     switch (m_type)
     {
     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:
-        return const_cast<void *>(reinterpret_cast<const void *>(m_integer.getRawData()));
+        return reinterpret_cast<const void *>(m_integer.getRawData());
     case e_float:
         flt_val = m_float.convertToFloat();
-        return (void *)&flt_val;
+        return reinterpret_cast<const void *>(&flt_val);
     case e_double:
         dbl_val = m_float.convertToDouble();
-        return (void *)&dbl_val;
+        return reinterpret_cast<const void *>(&dbl_val);
     case e_long_double:
         llvm::APInt ldbl_val = m_float.bitcastToAPInt();
-        return const_cast<void *>(reinterpret_cast<const void *>(ldbl_val.getRawData()));
+        return reinterpret_cast<const void *>(ldbl_val.getRawData());
     }
     return nullptr;
 }
 
 size_t
 Scalar::GetByteSize() const
 {
     switch (m_type)
Scalar::SetBit (uint32_t bit)
     case e_uint256: m_integer.setBit(bit); return true;
     case e_float:
     case e_double:
     case e_long_double: break;
     }
     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;
-    }
-}

lldb/trunk/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp

 }
 
 bool
 EmulateInstructionMIPS::Emulate_MSA_Branch_DF (llvm::MCInst& insn, int element_byte_size, bool bnz)
 {
     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();
 
     int32_t pc = ReadRegisterUnsigned (eRegisterKindDWARF, dwarf_pc_mips, 0, &success);
     if (!success)
         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;
 
     for(int i = 0; i < 16 / element_byte_size; i++)
     {
         switch(element_byte_size)
         {
             case 1:
                 if((*ptr == 0 && bnz) || (*ptr != 0 && !bnz) )
                     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;
         }
         if(!branch_hit)
            break;
         ptr = ptr + element_byte_size;
     }
 

lldb/trunk/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp

 }
 
 bool
 EmulateInstructionMIPS64::Emulate_MSA_Branch_DF (llvm::MCInst& insn, int element_byte_size, bool bnz)
 {
     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();
 
     int64_t pc = ReadRegisterUnsigned (eRegisterKindDWARF, dwarf_pc_mips64, 0, &success);
     if (!success)
         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;
 
     for(int i = 0; i < 16 / element_byte_size; i++)
     {
         switch(element_byte_size)
         {
             case 1:
                 if((*ptr == 0 && bnz) || (*ptr != 0 && !bnz) )
                     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;
         }
         if(!branch_hit)
            break;
         ptr = ptr + element_byte_size;
     }