nitesh.jain created this revision.
nitesh.jain added reviewers: clayborg, labath.
nitesh.jain added subscribers: jaydeep, bhushan, slthakur, lldb-commits.
Herald added a subscriber: sdardis.

The RegisterValue.SetBytes for 4 byte data followed by GetAsUInt64 for 32 bit 
big endian system will produce incorrect result. Instead use 
RegisterValue.SetUInt which will preserved endianess. This patch also add 
register read/write via PTRACE_PEEKUSER/PTRACE_POKEUSER for and fix floating 
point register read/write for MIPS.

https://reviews.llvm.org/D24124

Files:
  source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1840,8 +1840,12 @@
 
     // Build the reginfos response.
     StreamGDBRemote response;
+    
+    uint64_t value;
+    value = reg_size == 4 ? *(uint32_t *)reg_bytes : *(uint64_t *)reg_bytes;
 
-    RegisterValue reg_value (reg_bytes, reg_size, process_arch.GetByteOrder ());
+    RegisterValue reg_value;
+    reg_value.SetUInt (value, reg_size);
     Error error = reg_context_sp->WriteRegister (reg_info, reg_value);
     if (error.Fail ())
     {
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
===================================================================
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.h
@@ -94,15 +94,22 @@
 
     protected:
         Error
-        DoReadRegisterValue(uint32_t offset,
-                            const char* reg_name,
-                            uint32_t size,
-                            RegisterValue &value) override;
+        DoReadRegister_SR_Config (uint32_t offset,
+                                  const char* reg_name,
+                                  uint32_t size,
+                                  RegisterValue &value);
+
+        uint32_t
+        GetPtraceRegisterOffset (uint32_t reg_index,
+                                 const RegisterInfo *const reg_info);
+
+        Error
+        ReadRegisterRaw (uint32_t reg_index,
+                         RegisterValue &value) override;
 
         Error
-        DoWriteRegisterValue(uint32_t offset,
-                             const char* reg_name,
-                             const RegisterValue &value) override;
+        WriteRegisterRaw (uint32_t reg_index,
+                          const RegisterValue &value) override;
 
         Error
         DoReadWatchPointRegisterValue(lldb::tid_t tid, void* watch_readback);
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
@@ -28,9 +28,16 @@
 #include "Plugins/Process/Linux/Procfs.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_mips64.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_mips.h"
+
 #define NT_MIPS_MSA 0x600
 #define CONFIG5_FRE (1 << 8)
 #define SR_FR (1 << 26)
+#define FPR_BASE        32
+#define PC              64
+#define CAUSE           65
+#define BADVADDR        66
+#define MMHI            67
+#define MMLO            68
 #define NUM_REGISTERS 32
 
 #include <sys/ptrace.h>
@@ -649,7 +656,7 @@
 
     if (IsFPR(reg_index) || IsMSA(reg_index))
     {
-        uint8_t *dst;
+        uint8_t *dst, byte_size;
         uint64_t *src;
 
         // Initialise the FP and MSA buffers by reading all co-processor 1 registers
@@ -659,13 +666,14 @@
         {
             assert (reg_info->byte_offset < sizeof(UserArea));
             dst = (uint8_t *)&m_fpr + reg_info->byte_offset - (sizeof(m_gpr));
+            byte_size = IsFR0 () ? 4 : 8;
         }
         else
         {
             assert (reg_info->byte_offset < sizeof(UserArea));
             dst = (uint8_t *)&m_msa + reg_info->byte_offset - (sizeof(m_gpr) + sizeof(m_fpr));
         }
-        switch (reg_info->byte_size)
+        switch (byte_size)
         {
             case 4:
                 *(uint32_t *)dst = reg_value.GetAsUInt32();
@@ -801,7 +809,7 @@
 
     lldb::ByteOrder byte_order = GetByteOrder();
 
-    uint32_t IsBigEndian = (byte_order == lldb::eByteOrderBig);
+    bool IsBigEndian = (byte_order == lldb::eByteOrderBig);
 
     if (IsMSAAvailable())
     {
@@ -827,12 +835,20 @@
     // TODO: Add support for FRE
     if (IsFR0())
     {
-         src = (uint8_t *)&m_fpr + 4 + (IsBigEndian * 4);
-         dst = (uint8_t *)&m_fpr + 8 + (IsBigEndian * 4);
+         src = (uint8_t *)&m_fpr + (!IsBigEndian) * 4;
+         dst = (uint8_t *)&m_fpr + 8;
          for (int i = 0; i < (NUM_REGISTERS / 2); i++)
          {
               // copy odd single from top of neighbouring even double
-              *(uint32_t *) dst = *(uint32_t *) src;
+              // In case of little endian, 32 bit LSB contain even floating point register
+              // and 32 bit MSB contain odd floating point register
+              // vice-versa for big endian 
+              *(uint32_t *) dst = *(uint32_t *) src; 
+
+              if (IsBigEndian)
+                   // Copy 32 bit MSB to 32 bit LSB 
+                  *(uint32_t *) src = *(uint32_t *)(src + 4);
+
               src = src + 16;
               dst = dst + 16;
          }
@@ -850,19 +866,24 @@
 
     lldb::ByteOrder byte_order = GetByteOrder();
 
-    uint32_t IsBigEndian = (byte_order == lldb::eByteOrderBig);
+    bool IsBigEndian = (byte_order == lldb::eByteOrderBig);
 
     // TODO: Add support for FRE
     if (IsFR0())
     {
-        src = (uint8_t *)&m_fpr + 8 + (IsBigEndian * 4);
-        dst = (uint8_t *)&m_fpr + 4 + (IsBigEndian * 4);
+        dst = (uint8_t *)&m_fpr + (!IsBigEndian) * 4;
+        src = dst + 8 - (!IsBigEndian) * 4;
         for (int i = 0; i < (NUM_REGISTERS / 2); i++)
         {
              // copy odd single to top of neighbouring even double
-             *(uint32_t *) dst = *(uint32_t *) src;
+             if (IsBigEndian)
+                  // Copy 32 bit LSB to 32 bit MSB
+                 *(uint32_t *) (dst + 4) = *(uint32_t *) dst;
+
+             *(uint32_t *)dst = *(uint32_t *)src;
+             
              src = src + 16;
-             dst = dst + 16;
+             dst = src + 16;
         }
     }
 
@@ -1374,51 +1395,105 @@
     }
     return num_valid;
 }
-Error
-NativeRegisterContextLinux_mips64::DoReadRegisterValue(uint32_t offset,
-                                                       const char* reg_name,
-                                                       uint32_t size,
-                                                       RegisterValue &value)
-{
-    GPR_linux_mips regs;
-    ::memset(&regs, 0, sizeof(GPR_linux_mips));
 
-    // Clear all bits in RegisterValue before writing actual value read from ptrace to avoid garbage value in 32-bit MSB 
-    value.SetBytes((void *)(((unsigned char *)&regs) + offset), 8, GetByteOrder());
-    Error error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGS, m_thread.GetID(), NULL, &regs, sizeof regs);
-    if (error.Success())
+uint32_t
+NativeRegisterContextLinux_mips64::GetPtraceRegisterOffset (uint32_t reg_index, 
+                                                            const RegisterInfo *const reg_info)
+{
+    // In case of MIPS, the PTRACE_PEEKUSER/PTRACE_POKEUSER 
+    // take register number has an offset
+    // Apart from GPR registers , the offset for other registers are
+    // FPR_BASE        32
+    // PC              64
+    // CAUSE           65
+    // BADVADDR        66
+    // MMHI            67
+    // MMLO            68
+
+    static_assert (dwarf_zero_mips == dwarf_zero_mips64 &&
+                   dwarf_pc_mips == dwarf_pc_mips64,
+                   "MIPS and MIPS64 dwarf no. mismatch");
+
+    if (reg_index < dwarf_sr_mips)
+        return reg_index;
+
+    switch (reg_info->kinds[lldb::eRegisterKindDWARF])
     {
-        lldb_private::ArchSpec arch;
-        if (m_thread.GetProcess()->GetArchitecture(arch))
-        {
-            void* target_address = ((uint8_t*)&regs) + offset + 4 * (arch.GetMachine() == llvm::Triple::mips);
-            uint32_t target_size;
-            if ((::strcmp(reg_name, "sr") == 0) || (::strcmp(reg_name, "cause") == 0) || (::strcmp(reg_name, "config5") == 0))
-                target_size = 4;
-            else
-                target_size = arch.GetFlags() & lldb_private::ArchSpec::eMIPSABI_O32 ? 4 : 8;
-            value.SetBytes(target_address, target_size, arch.GetByteOrder());
-        }
-        else
-            error.SetErrorString("failed to get architecture");
+        case dwarf_pc_mips:
+            return PC;
+        case dwarf_sr_mips:
+        case dwarf_config5_mips:
+        case dwarf_config5_mips64:
+            return reg_info->byte_offset;
+        case dwarf_cause_mips:
+            return CAUSE;
+        case dwarf_bad_mips:
+            return BADVADDR;
+        case dwarf_hi_mips:
+            return MMHI;
+        case dwarf_lo_mips:
+            return MMLO;
+        default:
+            assert (false && "Invalid Register Kinds");
     }
-    return error;
+    return 0;
+}
+
+Error
+NativeRegisterContextLinux_mips64::ReadRegisterRaw (uint32_t reg_index,
+                                                    RegisterValue &value)
+{
+    const RegisterInfo *const reg_info = GetRegisterInfoAtIndex (reg_index);
+
+    if (!reg_info)
+        return Error ("register %" PRIu32 " not found", reg_index);
+
+    uint32_t offset = GetPtraceRegisterOffset (reg_index, reg_info);
+    
+    if ((reg_index == dwarf_sr_mips) || (strcmp(reg_info->name, "config5") == 0))
+        return DoReadRegister_SR_Config (offset, reg_info->name,
+                                         reg_info->byte_size, value);
+
+    return DoReadRegisterValue (offset, reg_info->name,
+                                reg_info->byte_size, value);
 }
 
 Error
-NativeRegisterContextLinux_mips64::DoWriteRegisterValue(uint32_t offset,
-                                                        const char* reg_name,
-                                                        const RegisterValue &value)
+NativeRegisterContextLinux_mips64::WriteRegisterRaw (uint32_t reg_index,
+                                                     const RegisterValue &value)
+{
+    const RegisterInfo *const reg_info = GetRegisterInfoAtIndex (reg_index);
+
+    if (!reg_info)
+        return Error ("register %" PRIu32 " not found", reg_index);
+
+    if (reg_info->invalidate_regs)
+        assert (false && "In MIPS, reg_info->invalidate_regs is unhandled");
+
+    uint32_t offset = GetPtraceRegisterOffset (reg_index, reg_info);
+    return DoWriteRegisterValue (offset, reg_info->name, value);
+}
+
+Error
+NativeRegisterContextLinux_mips64::DoReadRegister_SR_Config (uint32_t offset,
+                                                             const char* reg_name,
+                                                             uint32_t size,
+                                                             RegisterValue &value)
 {
     GPR_linux_mips regs;
-    Error error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGS, m_thread.GetID(), NULL, &regs, sizeof regs);
+    ::memset(&regs, 0, sizeof(GPR_linux_mips));
+
+    Error error = NativeProcessLinux::PtraceWrapper(PTRACE_GETREGS,
+                                                    m_thread.GetID(), NULL,
+                                                    &regs, sizeof regs);
     if (error.Success())
     {
         lldb_private::ArchSpec arch;
         if (m_thread.GetProcess()->GetArchitecture(arch))
         {
-            ::memcpy((void *)(((unsigned char *)(&regs)) + offset), value.GetBytes(), arch.GetFlags() & lldb_private::ArchSpec::eMIPSABI_O32 ? 4 : 8);
-            error = NativeProcessLinux::PtraceWrapper(PTRACE_SETREGS, m_thread.GetID(), NULL, &regs, sizeof regs);
+            void* target_address = ((uint8_t*)&regs) + offset +
+                                    4 * (arch.GetMachine() == llvm::Triple::mips);
+            value.SetUInt (*(uint32_t *)target_address, size);
         }
         else
             error.SetErrorString("failed to get architecture");
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
===================================================================
--- source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -181,7 +181,7 @@
 
     if (error.Success())
         // First cast to an unsigned of the same size to avoid sign extension.
-        value.SetUInt64(static_cast<unsigned long>(data));
+        value.SetUInt(static_cast<unsigned long>(data), size);
 
     if (log)
         log->Printf ("NativeRegisterContextLinux::%s() reg %s: 0x%lx", __FUNCTION__, reg_name, data);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to