[LLDB][LLGS Test] Check length of register, only when its available

[mohit.bhakkad]

Summary

Some machines of an arch may not have all the regs listed for it in RegInfo, and we get 'E15' as error while reading from such register, i.e. its not available.

For example MSA registers listed for MIPS are not present in all MIPS boards.

[labath]

If the registers are not present, wouldn't it be better to *not* include them in qRegisterInfo response in the first place?

[clayborg]

We can't rely on any specific error codes since different GDB remote servers will hand back different errors for any packet. Do you have control over the MIPS debugserver? Is this just lldb-server? If so, we should have it not tell LLDB about a register it doesn't have. So I agree with labath that the GDB server should not tell us about this register in the first place.

[mohit.bhakkad]

This patch makes server send an End of register(E45) response if a register present in reginfo is not available on actual machine.

[labath]

I think this should go in a little bit deeper, i.e., inside the NativeRegisterContextLinux_mips, or whatever is the right class for you. I think this is better for several reasons:

• if the register is physically not present, the register context should not report it (through GetUserRegisterCount, GetRegisterInfoAtIndex, etc.) - someone else may be iterating through the registers in the context and doing stuff - he will get errors when encountering your registers
• this can cause us to silently lose some registers if we fail to read them for any reason (due to an unrelated bug somewhere)

[clayborg]

Is there a list of error codes and what they mean for the GDB remote protocol? debugserver returns random errors and doesn't abide by any specific error codes. Quote from some GDB remote protocol docs I found:

The error response returned for some packets includes a two character error number. That number is not well defined.

So we can't rely on specific responses unless you verify that the error is coming from a specific GDB server. In order to do this, we would need to add a qGDBServerInfo packet that could return a name, version info, etc:

$qGDBServerInfo#00
$name:debugserver;version:123.2.3;

The real fix is to fix the GDB server to not have it return this register if it is not available.

[mohit.bhakkad]

Changes in this revision:

• Fixed GetUserRegisterCount () to get count registers which are actually present.

This will make llgs send E45 when reg_index is of some unavailable reg.

[clayborg]

Much better.

lldb/trunk/source/Plugins/Process/Utility/RegisterContextLinux_mips.h

 
 #include "lldb/lldb-private.h"
 #include "RegisterInfoInterface.h"
 
 class RegisterContextLinux_mips
     : public lldb_private::RegisterInfoInterface
 {
 public:
-    RegisterContextLinux_mips(const lldb_private::ArchSpec &target_arch);
+    RegisterContextLinux_mips(const lldb_private::ArchSpec &target_arch, bool msa_present = true);
 
     size_t
     GetGPRSize() const override;
 
     const lldb_private::RegisterInfo *
     GetRegisterInfo() const override;
 
     uint32_t
     GetRegisterCount () const override;
 
     uint32_t
     GetUserRegisterCount () const override;
+
+private:
+    uint32_t m_user_register_count;
 };
 
 #endif

lldb/trunk/source/Plugins/Process/Utility/RegisterContextLinux_mips.cpp

 
 //---------------------------------------------------------------------------
 // Include RegisterInfos_mips to declare our g_register_infos_mips structure.
 //---------------------------------------------------------------------------
 #define DECLARE_REGISTER_INFOS_MIPS_STRUCT
 #include "RegisterInfos_mips.h"
 #undef DECLARE_REGISTER_INFOS_MIPS_STRUCT
 
-RegisterContextLinux_mips::RegisterContextLinux_mips(const ArchSpec &target_arch) :
-    RegisterInfoInterface(target_arch)
+uint32_t
+GetUserRegisterInfoCount (bool msa_present)
+{
+    if (msa_present)
+        return static_cast<uint32_t> (k_num_user_registers_mips);
+    return static_cast<uint32_t> (k_num_user_registers_mips - k_num_msa_registers_mips);
+}
+
+RegisterContextLinux_mips::RegisterContextLinux_mips(const ArchSpec &target_arch, bool msa_present) :
+    RegisterInfoInterface(target_arch),
+    m_user_register_count (GetUserRegisterInfoCount (msa_present))
 {
 }
 
 size_t
 RegisterContextLinux_mips::GetGPRSize() const
 {
     return sizeof(GPR_linux_mips);
 }

 RegisterContextLinux_mips::GetRegisterCount () const
 {
     return static_cast<uint32_t> (sizeof (g_register_infos_mips) / sizeof (g_register_infos_mips [0]));
 }
 
 uint32_t
 RegisterContextLinux_mips::GetUserRegisterCount () const
 {
-    return static_cast<uint32_t> (k_num_user_registers_mips);
+    return static_cast<uint32_t> (m_user_register_count);
 }

lldb/trunk/source/Plugins/Process/Utility/RegisterContextLinux_mips64.h

 
 #include "lldb/lldb-private.h"
 #include "RegisterInfoInterface.h"
 
 class RegisterContextLinux_mips64
     : public lldb_private::RegisterInfoInterface
 {
 public:
-    RegisterContextLinux_mips64(const lldb_private::ArchSpec &target_arch);
+    RegisterContextLinux_mips64(const lldb_private::ArchSpec &target_arch, bool msa_present = true);
 
     size_t
     GetGPRSize() const override;
 
     const lldb_private::RegisterInfo *
     GetRegisterInfo() const override;
 
     uint32_t

lldb/trunk/source/Plugins/Process/Utility/RegisterContextLinux_mips64.cpp

switch (target_arch.GetMachine())
             return static_cast<uint32_t> (sizeof (g_register_infos_mips) / sizeof (g_register_infos_mips [0]));
         default:
             assert(false && "Unhandled target architecture.");
             return 0;
     }
 }
 
 uint32_t
-GetUserRegisterInfoCount (const ArchSpec &target_arch)
+GetUserRegisterInfoCount (const ArchSpec &target_arch, bool msa_present)
 {
     switch (target_arch.GetMachine())
     {
         case llvm::Triple::mips:
         case llvm::Triple::mipsel:
+            if (msa_present)
                 return static_cast<uint32_t> (k_num_user_registers_mips);
+            return static_cast<uint32_t> (k_num_user_registers_mips - k_num_msa_registers_mips); 
         case llvm::Triple::mips64el:
         case llvm::Triple::mips64:
+            if (msa_present)
                 return static_cast<uint32_t> (k_num_user_registers_mips64);
+            return static_cast<uint32_t> (k_num_user_registers_mips64 - k_num_msa_registers_mips64);
         default:
             assert(false && "Unhandled target architecture.");
             return 0;
     }
 }
 
-RegisterContextLinux_mips64::RegisterContextLinux_mips64(const ArchSpec &target_arch) :
+RegisterContextLinux_mips64::RegisterContextLinux_mips64(const ArchSpec &target_arch, bool msa_present) :
     lldb_private::RegisterInfoInterface(target_arch),
     m_register_info_p (GetRegisterInfoPtr (target_arch)),
     m_register_info_count (GetRegisterInfoCount (target_arch)),
-    m_user_register_count (GetUserRegisterInfoCount (target_arch))
+    m_user_register_count (GetUserRegisterInfoCount (target_arch, msa_present))
 {
 }
 
 size_t
 RegisterContextLinux_mips64::GetGPRSize() const
 {
     return sizeof(GPR_linux_mips);
 }