Fix the handling of FPR offsets in Linux arm/aarch64 register contexts

[tberghammer]

Summary

Fix the handling of FPR offsets in Linux arm/aarch64 register contexts

This should fix the expression evaluation on the Android-aarch64 build bot, but I haven't managed to reproduce the issue locally

[labath]

I would rather see the register variables laid out in a way which makes the offsets meaningful. I.e., instead of having m_fpr, m_gpr, ... we put something like

struct Regs {
  uint32_t gpr[...];
  FPU fpu;
  DREG hbr_regs;
  ...
};
Regs m_registers;

Then the byte offset field in Register Info structure will truly correspond to the offset in this structure, and we can index it simply with:

assert(offset < sizeof m_register);
address =  (char *)&m_registers + offset;

But if you want to keep this change small then it's ok for now.

[tberghammer]

I prefer to keep it this way because there is no canonical register layout we can rely on so putting them just next to each other would be just an arbitrary layout what might break in the future. If we want the offsets to make sense, then on arm/aarch64 the best approach would be to store the offset from the beginning of the given register set because it will make sense on its own and with the data returned by ptrace.

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.h

private:
         bool
         IsFPR(unsigned reg) const;
 
         Error
         ReadHardwareDebugInfo();
 
         Error
         WriteHardwareDebugRegs(int hwbType, int hwb_index);
+
+        uint32_t
+        CalculateFprOffset(const RegisterInfo* reg_info) const;
     };
 
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextLinux_arm_h
 
 #endif // defined(__arm__)

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm.cpp

else
             // use the type specified by reg_info rather than the uint64_t default
             if (reg_value.GetByteSize() > reg_info->byte_size)
                 reg_value.SetType(reg_info);
         }
         return error;
     }
 
     // Get pointer to m_fpr variable and set the data from it.
-    assert (reg_info->byte_offset < sizeof m_fpr);
-    uint8_t *src = (uint8_t *)&m_fpr + reg_info->byte_offset;
+    uint32_t fpr_offset = CalculateFprOffset(reg_info);
+    assert (fpr_offset < sizeof m_fpr);
+    uint8_t *src = (uint8_t *)&m_fpr + fpr_offset;
     switch (reg_info->byte_size)
     {
         case 2:
             reg_value.SetUInt16(*(uint16_t *)src);
             break;
         case 4:
             reg_value.SetUInt32(*(uint32_t *)src);
             break;
if (reg_index == LLDB_INVALID_REGNUM)
         return Error ("no lldb regnum for %s", reg_info && reg_info->name ? reg_info->name : "<unknown register>");
 
     if (IsGPR(reg_index))
         return WriteRegisterRaw(reg_index, reg_value);
 
     if (IsFPR(reg_index))
     {
         // Get pointer to m_fpr variable and set the data to it.
-        assert (reg_info->byte_offset < sizeof(m_fpr));
-        uint8_t *dst = (uint8_t *)&m_fpr + reg_info->byte_offset;
+        uint32_t fpr_offset = CalculateFprOffset(reg_info);
+        assert (fpr_offset < sizeof m_fpr);
+        uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
         switch (reg_info->byte_size)
         {
             case 2:
                 *(uint16_t *)dst = reg_value.GetAsUInt16();
                 break;
             case 4:
                 *(uint32_t *)dst = reg_value.GetAsUInt32();
                 break;
else
         error = NativeProcessLinux::PtraceWrapper(PTRACE_SETHBPREGS,
                 m_thread.GetID(), (PTRACE_TYPE_ARG3) ((hwb_index << 1) + 2),
                 ctrl_buf, sizeof(unsigned int));
 
     }
 
     return error;
 }
+
+uint32_t
+NativeRegisterContextLinux_arm::CalculateFprOffset(const RegisterInfo* reg_info) const
+{
+    return reg_info->byte_offset - GetRegisterInfoAtIndex(m_reg_info.first_fpr)->byte_offset;
+}
+
 #endif // defined(__arm__)

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h

private:
         bool
         IsFPR(unsigned reg) const;
 
         Error
         ReadHardwareDebugInfo();
 
         Error
         WriteHardwareDebugRegs(int hwbType);
+
+        uint32_t
+        CalculateFprOffset(const RegisterInfo* reg_info) const;
     };
 
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextLinux_arm64_h
 
 #endif // defined (__arm64__) || defined (__aarch64__)

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

else
             // use the type specified by reg_info rather than the uint64_t default
             if (reg_value.GetByteSize() > reg_info->byte_size)
                 reg_value.SetType(reg_info);
         }
         return error;
     }
 
     // Get pointer to m_fpr variable and set the data from it.
-    assert (reg_info->byte_offset < sizeof m_fpr);
-    uint8_t *src = (uint8_t *)&m_fpr + reg_info->byte_offset;
+    uint32_t fpr_offset = CalculateFprOffset(reg_info);
+    assert (fpr_offset < sizeof m_fpr);
+    uint8_t *src = (uint8_t *)&m_fpr + fpr_offset;
     reg_value.SetFromMemoryData(reg_info, src, reg_info->byte_size, eByteOrderLittle, error);
 
     return error;
 }
 
 Error
 NativeRegisterContextLinux_arm64::WriteRegister (const RegisterInfo *reg_info, const RegisterValue &reg_value)
 {
     if (!reg_info)
         return Error ("reg_info NULL");
 
     const uint32_t reg_index = reg_info->kinds[lldb::eRegisterKindLLDB];
     if (reg_index == LLDB_INVALID_REGNUM)
         return Error ("no lldb regnum for %s", reg_info && reg_info->name ? reg_info->name : "<unknown register>");
 
     if (IsGPR(reg_index))
         return WriteRegisterRaw(reg_index, reg_value);
 
     if (IsFPR(reg_index))
     {
         // Get pointer to m_fpr variable and set the data to it.
-        assert (reg_info->byte_offset < sizeof(m_fpr));
-        uint8_t *dst = (uint8_t *)&m_fpr + reg_info->byte_offset;
+        uint32_t fpr_offset = CalculateFprOffset(reg_info);
+        assert (fpr_offset < sizeof m_fpr);
+        uint8_t *dst = (uint8_t *)&m_fpr + fpr_offset;
         switch (reg_info->byte_size)
         {
             case 2:
                 *(uint16_t *)dst = reg_value.GetAsUInt16();
                 break;
             case 4:
                 *(uint32_t *)dst = reg_value.GetAsUInt32();
                 break;
NativeRegisterContextLinux_arm64::DoWriteFPR(void *buf, size_t buf_size)
     struct iovec ioVec;
     Error error;
 
     ioVec.iov_base = buf;
     ioVec.iov_len = buf_size;
     return NativeProcessLinux::PtraceWrapper(PTRACE_SETREGSET, m_thread.GetID(), &regset, &ioVec, buf_size);
 }
 
+uint32_t
+NativeRegisterContextLinux_arm64::CalculateFprOffset(const RegisterInfo* reg_info) const
+{
+    return reg_info->byte_offset - GetRegisterInfoAtIndex(m_reg_info.first_fpr)->byte_offset;
+}
+
 #endif // defined (__arm64__) || defined (__aarch64__)

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

 
 #include "RegisterContextLinux_arm64.h"
 
 // Based on RegisterContextDarwin_arm64.cpp
 #define GPR_OFFSET(idx) ((idx) * 8)
 #define GPR_OFFSET_NAME(reg) (LLVM_EXTENSION offsetof (RegisterContextLinux_arm64::GPR, reg))
 
 #define FPU_OFFSET(idx) ((idx) * 16 + sizeof (RegisterContextLinux_arm64::GPR))
-#define FPU_OFFSET_NAME(reg) (LLVM_EXTENSION offsetof (RegisterContextLinux_arm64::FPU, reg))
+#define FPU_OFFSET_NAME(reg) (LLVM_EXTENSION offsetof (RegisterContextLinux_arm64::FPU, reg) + sizeof (RegisterContextLinux_arm64::GPR))
 
 #define EXC_OFFSET_NAME(reg) (LLVM_EXTENSION offsetof (RegisterContextLinux_arm64::EXC, reg) + sizeof (RegisterContextLinux_arm64::GPR) + sizeof (RegisterContextLinux_arm64::FPU))
 #define DBG_OFFSET_NAME(reg) (LLVM_EXTENSION offsetof (RegisterContextLinux_arm64::DBG, reg) + sizeof (RegisterContextLinux_arm64::GPR) + sizeof (RegisterContextLinux_arm64::FPU) + sizeof (RegisterContextLinux_arm64::EXC))
 
 #define DEFINE_DBG(reg, i)  #reg, NULL, sizeof(((RegisterContextLinux_arm64::DBG *)NULL)->reg[i]), DBG_OFFSET_NAME(reg[i]), lldb::eEncodingUint, lldb::eFormatHex, { LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, dbg_##reg##i }, NULL, NULL
 #define REG_CONTEXT_SIZE (sizeof (RegisterContextLinux_arm64::GPR) + sizeof (RegisterContextLinux_arm64::FPU) + sizeof (RegisterContextLinux_arm64::EXC))