[Lldb-commits] [PATCH] D24124: [LLDB][MIPS] Fix register read/write for big endian

[Pavel Labath via lldb-commits]

labath added inline comments.

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1844
@@ +1843,3 @@
+    
+    uint64_t value;
+    value = reg_size == 4 ? *(uint32_t *)reg_bytes : *(uint64_t *)reg_bytes;
----------------
nitesh.jain wrote:
> labath wrote:
> > This looks like a massive hack. The register value object already takes a byte order as a parameter, so the fact that you are doing some funny endian conversions here means that there is something wrong. Also, this probably will not work for registers whose sizes are not 4 or 8 (%ah, %ax, all SSE registers, etc.).
> > 
> > I think we'll need to find a different way to fix this.
> The problem is  with RegisterValue.SetBytes 
> 
> RegisterValue (uint8_t *bytes, size_t length, lldb::ByteOrder byte_order)
>         {
>             SetBytes (bytes, length, byte_order);
>         }
> 
> The RegisterValue.SetBytes use memcpy to perform copy . So for register whose size is 4 it will be copy to lower 32bit LSB and hence RegisterValue.GetAsUInt64 will give incorrect result for 32 bit big endian system. 
I see that, but I still don't think this is a good idea. I think the fix should be done in a different way although I have to say I don't have an idea how (I am not too familiar with the RegisterValue class).
A couple of thoughts come to mind:
- Why are you calling GetAsUint64 on a 32-bit register in the first place?
- GetAsUint64 (or GetAsUInt32 for that matter) doesn't seem to be very endian-aware. Maybe it should be?
- It could be that what you need to do is simply not possible to do sanely with the current RegisterValue interface. If that's the case, we need to change the interface.


https://reviews.llvm.org/D24124