[Lldb-commits] [lldb] 0e91241 - [LLDB] Add an llvm::Optional version of GetRegisterInfo

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Mon Sep 26 03:54:45 PDT 2022


Author: David Spickett
Date: 2022-09-26T10:54:34Z
New Revision: 0e912417c67db2dc32d32c98213dba42b9b607a6

URL: https://github.com/llvm/llvm-project/commit/0e912417c67db2dc32d32c98213dba42b9b607a6
DIFF: https://github.com/llvm/llvm-project/commit/0e912417c67db2dc32d32c98213dba42b9b607a6.diff

LOG: [LLDB] Add an llvm::Optional version of GetRegisterInfo

We have some 500 ish uses of the bool plus ref version
so changing them all at once isn't a great idea.

This adds an overload that doesn't take a RegisterInfo&
and returns an optional.

Once I'm done switching all the existing callers I'll
remove the original function.

Benefits of optional over bool plus ref:
* The intent of the function is clear from the prototype.
* It's harder to forget to check if the return is valid,
  and if you do you'll get an assert.
* You don't hide ununsed variables, which happens because
  passing by ref marks a variable used.
* You can't forget to reset the RegisterInfo in between
  calls.

Reviewed By: clayborg

Differential Revision: https://reviews.llvm.org/D134536

Added: 
    

Modified: 
    lldb/include/lldb/Core/EmulateInstruction.h
    lldb/source/Core/EmulateInstruction.cpp
    lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
    lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
    lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
    lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
    lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
    lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
    lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
    lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
    lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
    lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
    lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
    lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Core/EmulateInstruction.h b/lldb/include/lldb/Core/EmulateInstruction.h
index a710c866d9803..fa049d4180fbf 100644
--- a/lldb/include/lldb/Core/EmulateInstruction.h
+++ b/lldb/include/lldb/Core/EmulateInstruction.h
@@ -375,8 +375,11 @@ class EmulateInstruction : public PluginInterface {
   virtual bool TestEmulation(Stream *out_stream, ArchSpec &arch,
                              OptionValueDictionary *test_data) = 0;
 
-  virtual bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-                               RegisterInfo &reg_info) = 0;
+  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
+                       RegisterInfo &reg_info);
+
+  virtual llvm::Optional<RegisterInfo>
+  GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num) = 0;
 
   // Optional overrides
   virtual bool SetInstruction(const Opcode &insn_opcode,

diff  --git a/lldb/source/Core/EmulateInstruction.cpp b/lldb/source/Core/EmulateInstruction.cpp
index 1320e8925553e..271301b9d3831 100644
--- a/lldb/source/Core/EmulateInstruction.cpp
+++ b/lldb/source/Core/EmulateInstruction.cpp
@@ -582,3 +582,12 @@ bool EmulateInstruction::CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) {
   unwind_plan.Clear();
   return false;
 }
+
+bool EmulateInstruction::GetRegisterInfo(lldb::RegisterKind reg_kind,
+                                         uint32_t reg_num,
+                                         RegisterInfo &reg_info) {
+  llvm::Optional<RegisterInfo> info = GetRegisterInfo(reg_kind, reg_num);
+  if (info)
+    reg_info = *info;
+  return info.has_value();
+}
\ No newline at end of file

diff  --git a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
index 0abfefa43e099..54aec79d24773 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -42,7 +42,8 @@ LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionARM, InstructionARM)
 // ITSession implementation
 //
 
-static bool GetARMDWARFRegisterInfo(unsigned reg_num, RegisterInfo &reg_info) {
+static llvm::Optional<RegisterInfo> GetARMDWARFRegisterInfo(unsigned reg_num) {
+  RegisterInfo reg_info;
   ::memset(&reg_info, 0, sizeof(RegisterInfo));
   ::memset(reg_info.kinds, LLDB_INVALID_REGNUM, sizeof(reg_info.kinds));
 
@@ -594,9 +595,9 @@ static bool GetARMDWARFRegisterInfo(unsigned reg_num, RegisterInfo &reg_info) {
     break;
 
   default:
-    return false;
+    return {};
   }
-  return true;
+  return reg_info;
 }
 
 // A8.6.50
@@ -782,9 +783,9 @@ bool EmulateInstructionARM::WriteBits32Unknown(int n) {
   return true;
 }
 
-bool EmulateInstructionARM::GetRegisterInfo(lldb::RegisterKind reg_kind,
-                                            uint32_t reg_num,
-                                            RegisterInfo &reg_info) {
+llvm::Optional<RegisterInfo>
+EmulateInstructionARM::GetRegisterInfo(lldb::RegisterKind reg_kind,
+                                       uint32_t reg_num) {
   if (reg_kind == eRegisterKindGeneric) {
     switch (reg_num) {
     case LLDB_REGNUM_GENERIC_PC:
@@ -808,13 +809,13 @@ bool EmulateInstructionARM::GetRegisterInfo(lldb::RegisterKind reg_kind,
       reg_num = dwarf_cpsr;
       break;
     default:
-      return false;
+      return {};
     }
   }
 
   if (reg_kind == eRegisterKindDWARF)
-    return GetARMDWARFRegisterInfo(reg_num, reg_info);
-  return false;
+    return GetARMDWARFRegisterInfo(reg_num);
+  return {};
 }
 
 uint32_t EmulateInstructionARM::GetFramePointerRegisterNumber() const {

diff  --git a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
index c877724a9d305..9a51445f9c1a9 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
+++ b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
@@ -135,8 +135,9 @@ class EmulateInstructionARM : public EmulateInstruction {
   bool TestEmulation(Stream *out_stream, ArchSpec &arch,
                      OptionValueDictionary *test_data) override;
 
-  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-                       RegisterInfo &reg_info) override;
+  using EmulateInstruction::GetRegisterInfo;
+  llvm::Optional<RegisterInfo> GetRegisterInfo(lldb::RegisterKind reg_kind,
+                                               uint32_t reg_num) override;
 
   bool CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) override;
 

diff  --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index 6ab77d30564b9..96a7caa29981a 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -51,11 +51,10 @@ using namespace lldb_private;
 
 LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionARM64, InstructionARM64)
 
-static bool LLDBTableGetRegisterInfo(uint32_t reg_num, RegisterInfo &reg_info) {
+static llvm::Optional<RegisterInfo> LLDBTableGetRegisterInfo(uint32_t reg_num) {
   if (reg_num >= std::size(g_register_infos_arm64_le))
-    return false;
-  reg_info = g_register_infos_arm64_le[reg_num];
-  return true;
+    return {};
+  return g_register_infos_arm64_le[reg_num];
 }
 
 #define No_VFP 0
@@ -144,9 +143,9 @@ bool EmulateInstructionARM64::SetTargetTriple(const ArchSpec &arch) {
   return false;
 }
 
-bool EmulateInstructionARM64::GetRegisterInfo(RegisterKind reg_kind,
-                                              uint32_t reg_num,
-                                              RegisterInfo &reg_info) {
+llvm::Optional<RegisterInfo>
+EmulateInstructionARM64::GetRegisterInfo(RegisterKind reg_kind,
+                                         uint32_t reg_num) {
   if (reg_kind == eRegisterKindGeneric) {
     switch (reg_num) {
     case LLDB_REGNUM_GENERIC_PC:
@@ -171,13 +170,13 @@ bool EmulateInstructionARM64::GetRegisterInfo(RegisterKind reg_kind,
       break;
 
     default:
-      return false;
+      return {};
     }
   }
 
   if (reg_kind == eRegisterKindLLDB)
-    return LLDBTableGetRegisterInfo(reg_num, reg_info);
-  return false;
+    return LLDBTableGetRegisterInfo(reg_num);
+  return {};
 }
 
 EmulateInstructionARM64::Opcode *

diff  --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
index 4f11f7387a2ec..20b1c33c66cda 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
@@ -65,8 +65,10 @@ class EmulateInstructionARM64 : public lldb_private::EmulateInstruction {
     return false;
   }
 
-  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-                       lldb_private::RegisterInfo &reg_info) override;
+  using EmulateInstruction::GetRegisterInfo;
+
+  llvm::Optional<lldb_private::RegisterInfo>
+  GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num) override;
 
   bool
   CreateFunctionEntryUnwind(lldb_private::UnwindPlan &unwind_plan) override;

diff  --git a/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp b/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
index 7aff11ede400d..37096a5cc6704 100644
--- a/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
+++ b/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
@@ -585,9 +585,9 @@ const char *EmulateInstructionMIPS::GetRegisterName(unsigned reg_num,
   return nullptr;
 }
 
-bool EmulateInstructionMIPS::GetRegisterInfo(RegisterKind reg_kind,
-                                             uint32_t reg_num,
-                                             RegisterInfo &reg_info) {
+llvm::Optional<RegisterInfo>
+EmulateInstructionMIPS::GetRegisterInfo(RegisterKind reg_kind,
+                                        uint32_t reg_num) {
   if (reg_kind == eRegisterKindGeneric) {
     switch (reg_num) {
     case LLDB_REGNUM_GENERIC_PC:
@@ -611,11 +611,12 @@ bool EmulateInstructionMIPS::GetRegisterInfo(RegisterKind reg_kind,
       reg_num = dwarf_sr_mips;
       break;
     default:
-      return false;
+      return {};
     }
   }
 
   if (reg_kind == eRegisterKindDWARF) {
+    RegisterInfo reg_info;
     ::memset(&reg_info, 0, sizeof(RegisterInfo));
     ::memset(reg_info.kinds, LLDB_INVALID_REGNUM, sizeof(reg_info.kinds));
 
@@ -636,7 +637,7 @@ bool EmulateInstructionMIPS::GetRegisterInfo(RegisterKind reg_kind,
       reg_info.format = eFormatVectorOfUInt8;
       reg_info.encoding = eEncodingVector;
     } else {
-      return false;
+      return {};
     }
 
     reg_info.name = GetRegisterName(reg_num, false);
@@ -662,9 +663,9 @@ bool EmulateInstructionMIPS::GetRegisterInfo(RegisterKind reg_kind,
     default:
       break;
     }
-    return true;
+    return reg_info;
   }
-  return false;
+  return {};
 }
 
 EmulateInstructionMIPS::MipsOpcode *

diff  --git a/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h b/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
index 4862f6c7e0dc5..e771bda2e1dea 100644
--- a/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
+++ b/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
@@ -80,8 +80,10 @@ class EmulateInstructionMIPS : public lldb_private::EmulateInstruction {
     return false;
   }
 
-  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-                       lldb_private::RegisterInfo &reg_info) override;
+  using EmulateInstruction::GetRegisterInfo;
+
+  llvm::Optional<lldb_private::RegisterInfo>
+  GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num) override;
 
   bool
   CreateFunctionEntryUnwind(lldb_private::UnwindPlan &unwind_plan) override;

diff  --git a/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp b/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
index b4a860af54bd9..341d954e74be6 100644
--- a/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
+++ b/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
@@ -572,9 +572,9 @@ const char *EmulateInstructionMIPS64::GetRegisterName(unsigned reg_num,
   return nullptr;
 }
 
-bool EmulateInstructionMIPS64::GetRegisterInfo(RegisterKind reg_kind,
-                                               uint32_t reg_num,
-                                               RegisterInfo &reg_info) {
+llvm::Optional<RegisterInfo>
+EmulateInstructionMIPS64::GetRegisterInfo(RegisterKind reg_kind,
+                                          uint32_t reg_num) {
   if (reg_kind == eRegisterKindGeneric) {
     switch (reg_num) {
     case LLDB_REGNUM_GENERIC_PC:
@@ -598,11 +598,12 @@ bool EmulateInstructionMIPS64::GetRegisterInfo(RegisterKind reg_kind,
       reg_num = dwarf_sr_mips64;
       break;
     default:
-      return false;
+      return {};
     }
   }
 
   if (reg_kind == eRegisterKindDWARF) {
+    RegisterInfo reg_info;
     ::memset(&reg_info, 0, sizeof(RegisterInfo));
     ::memset(reg_info.kinds, LLDB_INVALID_REGNUM, sizeof(reg_info.kinds));
 
@@ -623,7 +624,7 @@ bool EmulateInstructionMIPS64::GetRegisterInfo(RegisterKind reg_kind,
       reg_info.format = eFormatVectorOfUInt8;
       reg_info.encoding = eEncodingVector;
     } else {
-      return false;
+      return {};
     }
 
     reg_info.name = GetRegisterName(reg_num, false);
@@ -649,9 +650,9 @@ bool EmulateInstructionMIPS64::GetRegisterInfo(RegisterKind reg_kind,
     default:
       break;
     }
-    return true;
+    return reg_info;
   }
-  return false;
+  return {};
 }
 
 EmulateInstructionMIPS64::MipsOpcode *

diff  --git a/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h b/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
index 3f56bc658c16e..9c8a95a64f942 100644
--- a/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
+++ b/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
@@ -72,8 +72,10 @@ class EmulateInstructionMIPS64 : public lldb_private::EmulateInstruction {
     return false;
   }
 
-  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-                       lldb_private::RegisterInfo &reg_info) override;
+  using EmulateInstruction::GetRegisterInfo;
+
+  llvm::Optional<lldb_private::RegisterInfo>
+  GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num) override;
 
   bool
   CreateFunctionEntryUnwind(lldb_private::UnwindPlan &unwind_plan) override;

diff  --git a/lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp b/lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
index 4b56a9b6b8c51..19598ebfd4c30 100644
--- a/lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
+++ b/lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
@@ -58,16 +58,15 @@ bool EmulateInstructionPPC64::SetTargetTriple(const ArchSpec &arch) {
   return arch.GetTriple().isPPC64();
 }
 
-static bool LLDBTableGetRegisterInfo(uint32_t reg_num, RegisterInfo &reg_info) {
+static llvm::Optional<RegisterInfo> LLDBTableGetRegisterInfo(uint32_t reg_num) {
   if (reg_num >= std::size(g_register_infos_ppc64le))
-    return false;
-  reg_info = g_register_infos_ppc64le[reg_num];
-  return true;
+    return {};
+  return g_register_infos_ppc64le[reg_num];
 }
 
-bool EmulateInstructionPPC64::GetRegisterInfo(RegisterKind reg_kind,
-                                              uint32_t reg_num,
-                                              RegisterInfo &reg_info) {
+llvm::Optional<RegisterInfo>
+EmulateInstructionPPC64::GetRegisterInfo(RegisterKind reg_kind,
+                                         uint32_t reg_num) {
   if (reg_kind == eRegisterKindGeneric) {
     switch (reg_num) {
     case LLDB_REGNUM_GENERIC_PC:
@@ -88,13 +87,13 @@ bool EmulateInstructionPPC64::GetRegisterInfo(RegisterKind reg_kind,
       break;
 
     default:
-      return false;
+      return {};
     }
   }
 
   if (reg_kind == eRegisterKindLLDB)
-    return LLDBTableGetRegisterInfo(reg_num, reg_info);
-  return false;
+    return LLDBTableGetRegisterInfo(reg_num);
+  return {};
 }
 
 bool EmulateInstructionPPC64::ReadInstruction() {

diff  --git a/lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h b/lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
index 117ff8965eb5c..b0d9130bfb068 100644
--- a/lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
+++ b/lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
@@ -61,8 +61,10 @@ class EmulateInstructionPPC64 : public EmulateInstruction {
     return false;
   }
 
-  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-                       RegisterInfo &reg_info) override;
+  using EmulateInstruction::GetRegisterInfo;
+
+  llvm::Optional<RegisterInfo> GetRegisterInfo(lldb::RegisterKind reg_kind,
+                                               uint32_t reg_num) override;
 
   bool CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) override;
 

diff  --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
index bcd18ff63d11b..f84c1159f254d 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -1286,9 +1286,9 @@ bool EmulateInstructionRISCV::WritePC(lldb::addr_t pc) {
                                LLDB_REGNUM_GENERIC_PC, pc);
 }
 
-bool EmulateInstructionRISCV::GetRegisterInfo(lldb::RegisterKind reg_kind,
-                                              uint32_t reg_index,
-                                              RegisterInfo &reg_info) {
+llvm::Optional<RegisterInfo>
+EmulateInstructionRISCV::GetRegisterInfo(lldb::RegisterKind reg_kind,
+                                         uint32_t reg_index) {
   if (reg_kind == eRegisterKindGeneric) {
     switch (reg_index) {
     case LLDB_REGNUM_GENERIC_PC:
@@ -1320,10 +1320,9 @@ bool EmulateInstructionRISCV::GetRegisterInfo(lldb::RegisterKind reg_kind,
       RegisterInfoPOSIX_riscv64::GetRegisterInfoCount(m_arch);
 
   if (reg_index >= length || reg_kind != eRegisterKindLLDB)
-    return false;
+    return {};
 
-  reg_info = array[reg_index];
-  return true;
+  return array[reg_index];
 }
 
 bool EmulateInstructionRISCV::SetTargetTriple(const ArchSpec &arch) {

diff  --git a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
index 1c7cf6cb08d66..92f5c950c26ad 100644
--- a/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ b/lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
@@ -76,8 +76,10 @@ class EmulateInstructionRISCV : public EmulateInstruction {
   bool EvaluateInstruction(uint32_t options) override;
   bool TestEmulation(Stream *out_stream, ArchSpec &arch,
                      OptionValueDictionary *test_data) override;
-  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-                       RegisterInfo &reg_info) override;
+  using EmulateInstruction::GetRegisterInfo;
+
+  llvm::Optional<RegisterInfo> GetRegisterInfo(lldb::RegisterKind reg_kind,
+                                               uint32_t reg_num) override;
 
   lldb::addr_t ReadPC(bool &success);
   bool WritePC(lldb::addr_t pc);


        


More information about the lldb-commits mailing list