[Lldb-commits] [PATCH] D24603: [LLDB][MIPS] fix Floating point register read/write for big endian

[Pavel Labath via lldb-commits]

labath added inline comments.

================
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp:1161
@@ +1160,3 @@
+  // 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
----------------
Ok I think I understand this now.
`s/has/as/`


================
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp:1183
@@ +1182,3 @@
+  case dwarf_config5_mips64:
+    return reg_info->byte_offset;
+  case dwarf_cause_mips:
----------------
nitesh.jain wrote:
> labath wrote:
> > Why do we need to do this remapping? Couldn't we structure the register infos in a way that reg_info->byte_offset is exactly the offset that ptrace expects?
> > 
> > Or are you saying that ptrace does not accept register offsets, but some random register numbers instead? (I cannot tell, as the comment above is confusing.)
> In case of MIPS, ptrace request PTRACE_PEEKUSER/PTRACE_POKEUSER accept register number as an offset.  We used reg_info->byte_offset to find register value in the struct GPR_linux_mips. The struct GPR_linux_mips  is same for 32 and 64 bit since ptrace always return 64 bit value irrespective of Arch (32 and 64) . Hence we can't modify reg_info->byte_offset to match exactly the offset that ptrace expects.
Ok, I see what you mean. byte_offset is out in that case.
However, I am wondering if we couldn't tweak one of the existing numbering schemes to fit that one. For example, the LLDB numbering scheme is completely arbitrary, and under our control, so what if we just reordered that a bit to match what ptrace expects? I feel that we have enough numbering schemes already, so it should be possible to find one that works without introducing a new one.

This is not that bad, as the "scheme" is confined to the single file, but still you have to admit that this function looks very odd: sometimes you return a register index, sometimes a register offset and sometimes a completely made up number...

================
Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp:1207
@@ +1206,3 @@
+
+  if ((reg_index == dwarf_sr_mips) || (strcmp(reg_info->name, "config5") == 0))
+    return Read_SR_Config(offset, reg_info->name, reg_info->byte_size, value);
----------------
Isn't it possible to match the "config5" register by one of the numbering schemes, instead of by name?


https://reviews.llvm.org/D24603