[Lldb-commits] [PATCH] lldb more on register context arm64

[Jason Molenda]

Thanks Paul.

Does the ReadRegister method really do the correct thing for the x0/w0 or d0/s0 registers?  If we ask for w0, it's going to realize this is a subreg of x0, and call it self again asking for x0:

        if (is_subreg)
        {
            // Read the full aligned 64-bit register.
            full_reg = reg_info->invalidate_regs[0];
        }

so we're going to call RegisterValue::SetUInt64 on that -- I don't see where we'd recognize we only want the lower 32 bits and return those.  Do you have this running on a live device?  Does this work?

(lldb) reg write x0 0x12345678abcdef12
(lldb) reg read x0
      x0 = 0x12345678abcdef12
(lldb) reg read w0
      w0 = 0xabcdef12

Or this?

(lldb) reg write d0 1.3
(lldb) reg read -f x d0
      d0 = 0x3ff4cccccccccccd
(lldb) reg read -f x s0
      s0 = 0xcccccccd
(lldb) 

When you go to read the bytes out of the register context:

    // 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;
    switch (reg_info->byte_size)

you're offsetting unconditionally into the m_fpr -- the floating point part of the register context, aren't you?  Don't you want m_gpr_arm64 if this is a GPR and m_fpr if it is a fp/vector reg?

If you look at RegisterContextPOSIXProcessMonitor_x86_64::ReadRegister, it has a separate section for floating point registers and for gpr's (it checks if the register encoding is eEncodingVector which doesn't seem like the most straightforward way of indicating this).  I think you'll need to do the same thing in your ReadRegister method - have separate blocks of code for floating point registers (it will be the full 16-byte v0-v31 registers by the time it gets here) or gpr's.


In the WriteRegister method, there's a big block of code to handle writing a sub-register value (e.g. w0) by only overwriting the lower 32 bits of the register.  It includes

                    // Copy the src bytes to the destination.
                    ::memcpy (dst + (reg_info->byte_offset & 0x1), src, src_size);

The bitwise & of 1 against the byte_offset doesn't make sense to me.  First, RegisterInfo::byte_offset is the offset into the full register context -- not the offset into the containing register.  But by bit-anding it with 1 this is always zero -- this code is always memcpy (dst, src, src_size) -- and as long as the bytes are stored in little endian order, this will work out.  The arm64 subset registers are easy, they're always the lower 32/64 bits of the containing register.  armv7 would require more care to do correctly.

http://reviews.llvm.org/D5089