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

Jason Molenda jmolenda at apple.com
Tue Sep 2 12:50:27 PDT 2014

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

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.

> On Aug 31, 2014, at 2:12 AM, Paul Osmialowski <pawelo at king.net.pl> wrote:
> Corrected version, overhead 'if (success)' removed - it consisted of two if's, first was anded with (reg_info->byte_offset & 0x1) which can never be true on ARM64, second 'if' can never be true because first 'if' action would never happen.
> Note that my RegisterContextPOSIXProcessMonitor_arm64.cpp file was based on RegistertContextPOSIXProcessMonitor_mips64.cpp which contains the same mistake and the same comment with x86 register names, I guess it was copy-pasted from x86 implementation.
> http://reviews.llvm.org/D5089
> Files:
>  source/Plugins/Process/POSIX/CMakeLists.txt
>  source/Plugins/Process/POSIX/POSIXThread.cpp
>  source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_arm64.cpp
>  source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_arm64.h
>  source/Plugins/Process/Utility/CMakeLists.txt
>  source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
>  source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
> <D5089.13127.patch>_______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

More information about the lldb-commits mailing list