[LLDB] Change EmulateInstriction::ReadRegister to return Optional

[DavidSpickett]

Summary

Making it easier to understand and harder to misuse.
This only applies to the ReadRegister(const RegisterInfo &reg_info) variant.

Depends on D135671

[DavidSpickett]

Fix some undeclared vars, somehow I forgot to actualy build this.

lldb/include/lldb/Core/EmulateInstruction.h

virtual bool SetInstruction(const Opcode &insn_opcode,
                               const Address &inst_addr, Target *target);
 
   virtual bool CreateFunctionEntryUnwind(UnwindPlan &unwind_plan);
 
   static const char *TranslateRegister(lldb::RegisterKind reg_kind,
                                        uint32_t reg_num, std::string &reg_name);
 
   // RegisterInfo variants
-  bool ReadRegister(const RegisterInfo &reg_info, RegisterValue &reg_value);
+  llvm::Optional<RegisterValue> ReadRegister(const RegisterInfo &reg_info);
 
   uint64_t ReadRegisterUnsigned(const RegisterInfo &reg_info,
                                 uint64_t fail_value, bool *success_ptr);
 
   bool WriteRegister(const Context &context, const RegisterInfo &ref_info,
                      const RegisterValue &reg_value);
 
   bool WriteRegisterUnsigned(const Context &context,

lldb/source/Core/EmulateInstruction.cpp

for (uint32_t idx = 0;
         return emulate_insn_ptr;
     }
   }
   return nullptr;
 }
 
 EmulateInstruction::EmulateInstruction(const ArchSpec &arch) : m_arch(arch) {}
 
-bool EmulateInstruction::ReadRegister(const RegisterInfo &reg_info,
-                                      RegisterValue &reg_value) {
-  if (m_read_reg_callback != nullptr)
-    return m_read_reg_callback(this, m_baton, &reg_info, reg_value);
-  return false;
+llvm::Optional<RegisterValue>
+EmulateInstruction::ReadRegister(const RegisterInfo &reg_info) {
+  if (m_read_reg_callback == nullptr)
+    return {};
+
+  RegisterValue reg_value;
+  bool success = m_read_reg_callback(this, m_baton, &reg_info, reg_value);
+  if (success)
+    return reg_value;
+  return {};
 }
 
 bool EmulateInstruction::ReadRegister(lldb::RegisterKind reg_kind,
                                       uint32_t reg_num,
                                       RegisterValue &reg_value) {
   llvm::Optional<RegisterInfo> reg_info = GetRegisterInfo(reg_kind, reg_num);
-  if (reg_info)
-    return ReadRegister(*reg_info, reg_value);
+  if (!reg_info)
     return false;
+
+  llvm::Optional<RegisterValue> value = ReadRegister(*reg_info);
+  if (value)
+    reg_value = *value;
+  return value.has_value();
 }
 
 uint64_t EmulateInstruction::ReadRegisterUnsigned(lldb::RegisterKind reg_kind,
                                                   uint32_t reg_num,
                                                   uint64_t fail_value,
                                                   bool *success_ptr) {
   RegisterValue reg_value;
   if (ReadRegister(reg_kind, reg_num, reg_value))
     return reg_value.GetAsUInt64(fail_value, success_ptr);
   if (success_ptr)
     *success_ptr = false;
   return fail_value;
 }
 
 uint64_t EmulateInstruction::ReadRegisterUnsigned(const RegisterInfo &reg_info,
                                                   uint64_t fail_value,
                                                   bool *success_ptr) {
-  RegisterValue reg_value;
-  if (ReadRegister(reg_info, reg_value))
-    return reg_value.GetAsUInt64(fail_value, success_ptr);
+  llvm::Optional<RegisterValue> reg_value = ReadRegister(reg_info);
+  if (!reg_value) {
     if (success_ptr)
       *success_ptr = false;
     return fail_value;
   }
 
+  return reg_value->GetAsUInt64(fail_value, success_ptr);
+}
+
 bool EmulateInstruction::WriteRegister(const Context &context,
                                        const RegisterInfo &reg_info,
                                        const RegisterValue &reg_value) {
   if (m_write_reg_callback != nullptr)
     return m_write_reg_callback(this, m_baton, context, &reg_info, reg_value);
   return false;
 }
 

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

bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t opcode) {
   }
 
   idx = LSL(llvm::SignExtend64<7>(imm7), scale);
   size = (integer)1 << scale;
   uint64_t datasize = size * 8;
   uint64_t address;
   uint64_t wb_address;
 
-  RegisterValue data_Rt;
-  RegisterValue data_Rt2;
   llvm::Optional<RegisterInfo> reg_info_base =
       GetRegisterInfo(eRegisterKindLLDB, gpr_x0_arm64 + n);
   if (!reg_info_base)
     return false;
 
   llvm::Optional<RegisterInfo> reg_info_Rt;
   llvm::Optional<RegisterInfo> reg_info_Rt2;
 
case MemOp_STORE: {
     } else {
       context_t.type = eContextRegisterStore;
       context_t2.type = eContextRegisterStore;
     }
     context_t.SetRegisterToRegisterPlusOffset(*reg_info_Rt, *reg_info_base, 0);
     context_t2.SetRegisterToRegisterPlusOffset(*reg_info_Rt2, *reg_info_base,
                                                size);
 
-    if (!ReadRegister(*reg_info_Rt, data_Rt))
+    llvm::Optional<RegisterValue> data_Rt = ReadRegister(*reg_info_Rt);
+    if (!data_Rt)
       return false;
 
-    if (data_Rt.GetAsMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
+    if (data_Rt->GetAsMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
                                  eByteOrderLittle, error) == 0)
       return false;
 
     if (!WriteMemory(context_t, address + 0, buffer, reg_info_Rt->byte_size))
       return false;
 
-    if (!ReadRegister(*reg_info_Rt2, data_Rt2))
+    llvm::Optional<RegisterValue> data_Rt2 = ReadRegister(*reg_info_Rt2);
+    if (!data_Rt2)
       return false;
 
-    if (data_Rt2.GetAsMemoryData(*reg_info_Rt2, buffer, reg_info_Rt2->byte_size,
-                                 eByteOrderLittle, error) == 0)
+    if (data_Rt2->GetAsMemoryData(*reg_info_Rt2, buffer,
+                                  reg_info_Rt2->byte_size, eByteOrderLittle,
+                                  error) == 0)
       return false;
 
     if (!WriteMemory(context_t2, address + size, buffer,
                      reg_info_Rt2->byte_size))
       return false;
   } break;
 
   case MemOp_LOAD: {
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;
     }
 
+    RegisterValue data_Rt;
     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;
     }
 
+    RegisterValue data_Rt2;
     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 (Bit32(opc, 1) == 0) {
     if (size == 2 && Bit32(opc, 0) == 1)
       return false;
   }
 
   Status error;
   bool success = false;
   uint64_t address;
   uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
-  RegisterValue data_Rt;
 
   if (n == 31)
     address =
         ReadRegisterUnsigned(eRegisterKindLLDB, gpr_sp_arm64, 0, &success);
   else
     address =
         ReadRegisterUnsigned(eRegisterKindLLDB, gpr_x0_arm64 + n, 0, &success);
 
bool EmulateInstructionARM64::EmulateLDRSTRImm(const uint32_t opcode) {
 
   llvm::Optional<RegisterInfo> reg_info_Rt =
       GetRegisterInfo(eRegisterKindLLDB, gpr_x0_arm64 + t);
   if (!reg_info_Rt)
     return false;
 
   Context context;
   switch (memop) {
-  case MemOp_STORE:
+  case MemOp_STORE: {
     if (n == 31 || n == GetFramePointerRegisterNumber()) // if this store is
                                                          // based off of the sp
                                                          // or fp register
       context.type = eContextPushRegisterOnStack;
     else
       context.type = eContextRegisterStore;
     context.SetRegisterToRegisterPlusOffset(*reg_info_Rt, *reg_info_base,
                                             postindex ? 0 : offset);
 
-    if (!ReadRegister(*reg_info_Rt, data_Rt))
+    llvm::Optional<RegisterValue> data_Rt = ReadRegister(*reg_info_Rt);
+    if (!data_Rt)
       return false;
 
-    if (data_Rt.GetAsMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
+    if (data_Rt->GetAsMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
                                  eByteOrderLittle, error) == 0)
       return false;
 
     if (!WriteMemory(context, address, buffer, reg_info_Rt->byte_size))
       return false;
-    break;
+  } break;
 
-  case MemOp_LOAD:
+  case MemOp_LOAD: {
     if (n == 31 || n == GetFramePointerRegisterNumber()) // if this store is
                                                          // based off of the sp
                                                          // or fp register
       context.type = eContextPopRegisterOffStack;
     else
       context.type = eContextRegisterLoad;
     context.SetAddress(address);
 
     if (!ReadMemory(context, address, buffer, reg_info_Rt->byte_size))
       return false;
 
+    RegisterValue data_Rt;
     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;
+  } break;
   default:
     return false;
   }
 
   if (wback) {
     if (postindex)
       address += offset;
 

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

bool EmulateInstructionMIPS::Emulate_SW(llvm::MCInst &insn) {
   /* We look for sp based non-volatile register stores */
   if (nonvolatile_reg_p(src)) {
     llvm::Optional<RegisterInfo> reg_info_src =
         GetRegisterInfo(eRegisterKindDWARF, dwarf_zero_mips + src);
     if (!reg_info_src)
       return false;
 
     Context context;
-    RegisterValue data_src;
     context.type = eContextPushRegisterOnStack;
     context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
     uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
     Status error;
 
-    if (!ReadRegister(*reg_info_base, data_src))
+    llvm::Optional<RegisterValue> data_src = ReadRegister(*reg_info_base);
+    if (!data_src)
       return false;
 
-    if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
-                                 eByteOrderLittle, error) == 0)
+    if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+                                  reg_info_src->byte_size, eByteOrderLittle,
+                                  error) == 0)
       return false;
 
     if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
       return false;
 
     return true;
   }
 
bool EmulateInstructionMIPS::Emulate_SWSP(llvm::MCInst &insn) {
   bad_vaddr_context.type = eContextInvalid;
   WriteRegisterUnsigned(bad_vaddr_context, eRegisterKindDWARF, dwarf_bad_mips,
                         address);
 
   // We look for sp based non-volatile register stores.
   if (base == dwarf_sp_mips && nonvolatile_reg_p(src)) {
     RegisterInfo reg_info_src = {};
     Context context;
-    RegisterValue data_src;
     context.type = eContextPushRegisterOnStack;
     context.SetRegisterToRegisterPlusOffset(reg_info_src, *reg_info_base, 0);
 
     uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
     Status error;
 
-    if (!ReadRegister(*reg_info_base, data_src))
+    llvm::Optional<RegisterValue> data_src = ReadRegister(*reg_info_base);
+    if (!data_src)
       return false;
 
-    if (data_src.GetAsMemoryData(reg_info_src, buffer, reg_info_src.byte_size,
+    if (data_src->GetAsMemoryData(reg_info_src, buffer, reg_info_src.byte_size,
                                   eByteOrderLittle, error) == 0)
       return false;
 
     if (!WriteMemory(context, address, buffer, reg_info_src.byte_size))
       return false;
 
     return true;
   }
 
if (!nonvolatile_reg_p(src))
       return false;
 
     llvm::Optional<RegisterInfo> reg_info_src =
         GetRegisterInfo(eRegisterKindDWARF, dwarf_zero_mips + src);
     if (!reg_info_src)
       return false;
 
     Context context;
-    RegisterValue data_src;
     context.type = eContextPushRegisterOnStack;
     context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
     uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
     Status error;
 
-    if (!ReadRegister(*reg_info_base, data_src))
+    llvm::Optional<RegisterValue> data_src = ReadRegister(*reg_info_base);
+    if (!data_src)
       return false;
 
-    if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
-                                 eByteOrderLittle, error) == 0)
+    if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+                                  reg_info_src->byte_size, eByteOrderLittle,
+                                  error) == 0)
       return false;
 
     if (!WriteMemory(context, base_address, buffer, reg_info_src->byte_size))
       return false;
 
     // Stack address for next register
     base_address = base_address + reg_info_src->byte_size;
   }

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

if (!success)
     return false;
 
   /* destination address */
   address = address + imm;
 
   /* We look for sp based non-volatile register stores */
   if (nonvolatile_reg_p(src)) {
     Context context;
-    RegisterValue data_src;
     context.type = eContextPushRegisterOnStack;
     context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
 
     uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
     Status error;
 
-    if (!ReadRegister(*reg_info_base, data_src))
+    llvm::Optional<RegisterValue> data_src = ReadRegister(*reg_info_base);
+    if (!data_src)
       return false;
 
-    if (data_src.GetAsMemoryData(*reg_info_src, buffer, reg_info_src->byte_size,
-                                 eByteOrderLittle, error) == 0)
+    if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+                                  reg_info_src->byte_size, eByteOrderLittle,
+                                  error) == 0)
       return false;
 
     if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
       return false;
   }
 
   /* Set the bad_vaddr register with base address used in the instruction */
   bad_vaddr_context.type = eContextInvalid;