[LLDB] Change RegisterValue::SetFromMemoryData to const RegisterInfo&

[DavidSpickett]

Summary

All callers were either assuming their pointer was not null before calling
this, or checking beforehand.

[DavidSpickett]

This set of 4 patches will be it for a while on this front, honest :)

Thanks for the reviews so far!

[DavidSpickett]

clang-format

lldb/include/lldb/Utility/RegisterValue.h

public:
   // "reg_info" and only copy the least significant bytes of the register
   // value, or pad the destination with zeroes if the register byte size is
   // shorter that "dst_len" (all while correctly abiding the "dst_byte_order").
   // Returns the number of bytes copied into "dst".
   uint32_t GetAsMemoryData(const RegisterInfo &reg_info, void *dst,
                            uint32_t dst_len, lldb::ByteOrder dst_byte_order,
                            Status &error) const;
 
-  uint32_t SetFromMemoryData(const RegisterInfo *reg_info, const void *src,
+  uint32_t SetFromMemoryData(const RegisterInfo &reg_info, const void *src,
                              uint32_t src_len, lldb::ByteOrder src_byte_order,
                              Status &error);
 
   bool GetScalarValue(Scalar &scalar) const;
 
   uint8_t GetAsUInt8(uint8_t fail_value = UINT8_MAX,
                      bool *success_ptr = nullptr) const {
     if (m_type == eTypeUInt8) {

lldb/source/Host/common/NativeRegisterContext.cpp

if (bytes_read != src_len) {
     return error;
   }
 
   // We now have a memory buffer that contains the part or all of the register
   // value. Set the register value using this memory data.
   // TODO: we might need to add a parameter to this function in case the byte
   // order of the memory data doesn't match the process. For now we are
   // assuming they are the same.
-  reg_value.SetFromMemoryData(reg_info, src, src_len, process.GetByteOrder(),
+  reg_value.SetFromMemoryData(*reg_info, src, src_len, process.GetByteOrder(),
                               error);
 
   return error;
 }
 
 Status NativeRegisterContext::WriteRegisterValueToMemory(
     const RegisterInfo *reg_info, lldb::addr_t dst_addr, size_t dst_len,
     const RegisterValue &reg_value) {

lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp

case MemOp_LOAD: {
 
     if (rt_unknown)
       memset(buffer, 'U', reg_info_Rt->byte_size);
     else {
       if (!ReadMemory(context_t, address, buffer, reg_info_Rt->byte_size))
         return false;
     }
 
-    if (data_Rt.SetFromMemoryData(&(*reg_info_Rt), buffer,
-                                  reg_info_Rt->byte_size, eByteOrderLittle,
-                                  error) == 0)
+    if (data_Rt.SetFromMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
+                                  eByteOrderLittle, error) == 0)
       return false;
 
     if (!vector && is_signed && !data_Rt.SignExtend(datasize))
       return false;
 
     if (!WriteRegister(context_t, *reg_info_Rt, data_Rt))
       return false;
 
     if (!rt_unknown) {
       if (!ReadMemory(context_t2, address + size, buffer,
                       reg_info_Rt2->byte_size))
         return false;
     }
 
-    if (data_Rt2.SetFromMemoryData(&(*reg_info_Rt2), buffer,
+    if (data_Rt2.SetFromMemoryData(*reg_info_Rt2, buffer,
                                    reg_info_Rt2->byte_size, eByteOrderLittle,
                                    error) == 0)
       return false;
 
     if (!vector && is_signed && !data_Rt2.SignExtend(datasize))
       return false;
 
     if (!WriteRegister(context_t2, *reg_info_Rt2, data_Rt2))
if (n == 31 || n == GetFramePointerRegisterNumber()) // if this store is
       context.type = eContextPopRegisterOffStack;
     else
       context.type = eContextRegisterLoad;
     context.SetAddress(address);
 
     if (!ReadMemory(context, address, buffer, reg_info_Rt->byte_size))
       return false;
 
-    if (data_Rt.SetFromMemoryData(&(*reg_info_Rt), buffer,
-                                  reg_info_Rt->byte_size, eByteOrderLittle,
-                                  error) == 0)
+    if (data_Rt.SetFromMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
+                                  eByteOrderLittle, error) == 0)
       return false;
 
     if (!WriteRegister(context, *reg_info_Rt, data_Rt))
       return false;
     break;
   default:
     return false;
   }

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp

if (IsGPR(reg)) {
 
     offset = reg_info->byte_offset - GetRegisterInfo().GetMTEOffset();
     assert(offset < GetMTEControlSize());
     src = (uint8_t *)GetMTEControl() + offset;
   } else
     return Status("failed - register wasn't recognized to be a GPR or an FPR, "
                   "write strategy unknown");
 
-  reg_value.SetFromMemoryData(reg_info, src, reg_info->byte_size,
+  reg_value.SetFromMemoryData(*reg_info, src, reg_info->byte_size,
                               eByteOrderLittle, error);
 
   return error;
 }
 
 Status NativeRegisterContextLinux_arm64::WriteRegister(
     const RegisterInfo *reg_info, const RegisterValue &reg_value) {
   Status error;

lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp

bool RegisterContextCorePOSIX_arm64::ReadRegister(const RegisterInfo *reg_info,
   if (reg == LLDB_INVALID_REGNUM)
     return false;
 
   if (IsFPR(reg)) {
     if (m_sve_state == SVEState::Disabled) {
       // SVE is disabled take legacy route for FPU register access
       offset -= GetGPRSize();
       if (offset < m_fpr_data.GetByteSize()) {
-        value.SetFromMemoryData(reg_info, m_fpr_data.GetDataStart() + offset,
+        value.SetFromMemoryData(*reg_info, m_fpr_data.GetDataStart() + offset,
                                 reg_info->byte_size, lldb::eByteOrderLittle,
                                 error);
         return error.Success();
       }
     } else {
       // FPSR and FPCR will be located right after Z registers in
       // SVEState::FPSIMD while in SVEState::Full they will be located at the
       // end of register data after an alignment correction based on currently
if (m_sve_state == SVEState::Disabled) {
         if (reg_info->value_regs &&
             reg_info->value_regs[0] != LLDB_INVALID_REGNUM)
           sve_reg_num = reg_info->value_regs[0];
         offset = CalculateSVEOffset(GetRegisterInfoAtIndex(sve_reg_num));
       }
 
       assert(sve_reg_num != LLDB_INVALID_REGNUM);
       assert(offset < m_sve_data.GetByteSize());
-      value.SetFromMemoryData(reg_info, GetSVEBuffer(offset),
+      value.SetFromMemoryData(*reg_info, GetSVEBuffer(offset),
                               reg_info->byte_size, lldb::eByteOrderLittle,
                               error);
     }
   } else if (IsSVE(reg)) {
     if (IsSVEVG(reg)) {
       value = GetSVERegVG();
       return true;
     }
 
     switch (m_sve_state) {
     case SVEState::FPSIMD: {
       // In FPSIMD state SVE payload mirrors legacy fpsimd struct and so just
       // copy 16 bytes of v register to the start of z register. All other
       // SVE register will be set to zero.
       uint64_t byte_size = 1;
       uint8_t zeros = 0;
       const uint8_t *src = &zeros;
       if (IsSVEZ(reg)) {
         byte_size = 16;
         offset = CalculateSVEOffset(reg_info);
         assert(offset < m_sve_data.GetByteSize());
         src = GetSVEBuffer(offset);
       }
-      value.SetFromMemoryData(reg_info, src, byte_size, lldb::eByteOrderLittle,
+      value.SetFromMemoryData(*reg_info, src, byte_size, lldb::eByteOrderLittle,
                               error);
     } break;
     case SVEState::Full:
       offset = CalculateSVEOffset(reg_info);
       assert(offset < m_sve_data.GetByteSize());
-      value.SetFromMemoryData(reg_info, GetSVEBuffer(offset),
+      value.SetFromMemoryData(*reg_info, GetSVEBuffer(offset),
                               reg_info->byte_size, lldb::eByteOrderLittle,
                               error);
       break;
     case SVEState::Disabled:
     default:
       return false;
     }
   } else if (IsPAuth(reg)) {
     offset = reg_info->byte_offset - m_register_info_up->GetPAuthOffset();
     assert(offset < m_pac_data.GetByteSize());
-    value.SetFromMemoryData(reg_info, m_pac_data.GetDataStart() + offset,
+    value.SetFromMemoryData(*reg_info, m_pac_data.GetDataStart() + offset,
                             reg_info->byte_size, lldb::eByteOrderLittle, error);
   } else
     return false;
 
   return error.Success();
 }
 
 bool RegisterContextCorePOSIX_arm64::ReadAllRegisterValues(

lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.cpp

bool RegisterContextCorePOSIX_x86_64::ReadRegister(const RegisterInfo *reg_info,
   } else if (m_fpregset && fxsave_offset < sizeof(FXSAVE)) {
     src = m_fpregset.get();
     offset = fxsave_offset;
   } else {
     return false;
   }
 
   Status error;
-  value.SetFromMemoryData(reg_info, src + offset, reg_info->byte_size,
+  value.SetFromMemoryData(*reg_info, src + offset, reg_info->byte_size,
                           lldb::eByteOrderLittle, error);
 
   return error.Success();
 }
 
 bool RegisterContextCorePOSIX_x86_64::ReadAllRegisterValues(
     lldb::WritableDataBufferSP &data_sp) {
   return false;

lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

if (reg_info->value_regs &&
                                 combined_data.data() + offset) !=
             parent_reg->byte_size)
           return false;
         offset += parent_reg->byte_size;
       }
 
       Status error;
       return value.SetFromMemoryData(
-                 reg_info, combined_data.data(), combined_data.size(),
+                 *reg_info, combined_data.data(), combined_data.size(),
                  m_reg_data.GetByteOrder(), error) == combined_data.size();
     } else {
       const bool partial_data_ok = false;
       Status error(value.SetValueFromData(
           reg_info, m_reg_data, reg_info->byte_offset, partial_data_ok));
       return error.Success();
     }
   }

lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp

if (reg < k_num_reg_infos)
     return g_reg_infos[reg].name;
   return nullptr;
 }
 
 bool RegisterContextMinidump_ARM::ReadRegister(const RegisterInfo *reg_info,
                                                RegisterValue &reg_value) {
   Status error;
   reg_value.SetFromMemoryData(
-      reg_info, (const uint8_t *)&m_regs + reg_info->byte_offset,
+      *reg_info, (const uint8_t *)&m_regs + reg_info->byte_offset,
       reg_info->byte_size, lldb::eByteOrderLittle, error);
   return error.Success();
 }
 
 bool RegisterContextMinidump_ARM::WriteRegister(const RegisterInfo *,
                                                 const RegisterValue &) {
   return false;
 }

lldb/source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp

if (reg < k_num_reg_infos)
     return g_reg_infos[reg].name;
   return nullptr;
 }
 
 bool RegisterContextMinidump_ARM64::ReadRegister(const RegisterInfo *reg_info,
                                                  RegisterValue &reg_value) {
   Status error;
   reg_value.SetFromMemoryData(
-      reg_info, (const uint8_t *)&m_regs + reg_info->byte_offset,
+      *reg_info, (const uint8_t *)&m_regs + reg_info->byte_offset,
       reg_info->byte_size, lldb::eByteOrderLittle, error);
   return error.Success();
 }
 
 bool RegisterContextMinidump_ARM64::WriteRegister(const RegisterInfo *,
                                                   const RegisterValue &) {
   return false;
 }

lldb/source/Target/RegisterContext.cpp

if (bytes_read != src_len) {
       return error;
     }
 
     // We now have a memory buffer that contains the part or all of the
     // register value. Set the register value using this memory data.
     // TODO: we might need to add a parameter to this function in case the byte
     // order of the memory data doesn't match the process. For now we are
     // assuming they are the same.
-    reg_value.SetFromMemoryData(reg_info, src, src_len,
+    reg_value.SetFromMemoryData(*reg_info, src, src_len,
                                 process_sp->GetByteOrder(), error);
   } else
     error.SetErrorString("invalid process");
 
   return error;
 }
 
 Status RegisterContext::WriteRegisterValueToMemory(

lldb/source/Utility/RegisterValue.cpp

const uint32_t bytes_copied =
                                    dst_byte_order); // dst byte order
   if (bytes_copied == 0)
     error.SetErrorStringWithFormat(
         "failed to copy data for register write of %s", reg_info.name);
 
   return bytes_copied;
 }
 
-uint32_t RegisterValue::SetFromMemoryData(const RegisterInfo *reg_info,
+uint32_t RegisterValue::SetFromMemoryData(const RegisterInfo &reg_info,
                                           const void *src, uint32_t src_len,
                                           lldb::ByteOrder src_byte_order,
                                           Status &error) {
-  if (reg_info == nullptr) {
-    error.SetErrorString("invalid register info argument.");
-    return 0;
-  }
-
   // Moving from addr into a register
   //
   // Case 1: src_len == dst_len
   //
   //   |AABBCCDD| Address contents
   //   |AABBCCDD| Register contents
   //
   // Case 2: src_len > dst_len
   //
   //   Status!  (The register should always be big enough to hold the data)
   //
   // Case 3: src_len < dst_len
   //
   //   |AABB| Address contents
   //   |AABB0000| Register contents [on little-endian hardware]
   //   |0000AABB| Register contents [on big-endian hardware]
   if (src_len > kMaxRegisterByteSize) {
     error.SetErrorStringWithFormat(
         "register buffer is too small to receive %u bytes of data.", src_len);
     return 0;
   }
 
-  const uint32_t dst_len = reg_info->byte_size;
+  const uint32_t dst_len = reg_info.byte_size;
 
   if (src_len > dst_len) {
     error.SetErrorStringWithFormat(
         "%u bytes is too big to store in register %s (%u bytes)", src_len,
-        reg_info->name, dst_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);
 
-  error = SetValueFromData(reg_info, src_data, 0, true);
+  error = SetValueFromData(&reg_info, src_data, 0, true);
   if (error.Fail())
     return 0;
 
   // If SetValueFromData succeeded, we must have copied all of src_len
   return src_len;
 }
 
 bool RegisterValue::GetScalarValue(Scalar &scalar) const {

lldb/unittests/tools/lldb-server/tests/MessageObjects.cpp

if (ZeroPad && HexValue.size() < Info.byte_size * 2) {
     HexValue = Storage;
   }
 
   SmallVector<uint8_t, 64> Bytes(HexValue.size() / 2);
   StringExtractor(HexValue).GetHexBytes(Bytes, '\xcc');
   RegisterValue Value;
   Status ST;
   Value.SetFromMemoryData(
-      &Info, Bytes.data(), Bytes.size(),
+      Info, Bytes.data(), Bytes.size(),
       Endian == support::little ? eByteOrderLittle : eByteOrderBig, ST);
   if (ST.Fail())
     return ST.ToError();
   return Value;
 }
 
 //====== StopReply =============================================================
 Expected<std::unique_ptr<StopReply>>