[Lldb-commits] [PATCH] D25864: Fix arm64 floating point register spill recording in UnwindPlan analysis

[Jason Molenda via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: clayborg, tberghammer.
jasonmolenda added a subscriber: lldb-commits.
jasonmolenda set the repository for this revision to rL LLVM.
Herald added subscribers: modocache, mgorny, beanz, emaste, rengolin, aemerson.

When we're creating an UnwindPlan for arm64 functions that spill a floating point register, we don't decode the vector register numbers correctly and we don't have a way to represent the fact that the lower 64-bits of the registers are being saved to the stack (as the AAPCS64 ABI dictates).  The first fix is easiest, the second is a bit more involved.  The patch I have here may be a bit controversial, I'd like feedback from Greg and Tamas.

RegisterInfos_arm64.h defines the eRegisterKindLLDB register numbers used on Linux, Darwin, and FreeBSD.  It also defines the layout of these registers in the register context buffer.

I add the "w" pseudo registers to represent the low 32 bits of the "x" registers, and "s" and "d" registers to represent the low 32/64 bits of the "v" registers.  I add definitions for how to find those pseudo registers in the register context in RegisterInfos_arm64.h.  It's not possible to specify this in a bi-endian correct way, so I've renamed the table from "g_register_infos_arm64" to "g_register_infos_arm64_le".  I used defines to pick the offset of the w, s, and d registers so it will be easy (by duplicating the table) to have a correct definition for big-endian if we add support for a target like that in the future.

I hoisted the register number definitions (the eRegisterKindLLDB register numbers) out of RegisterInfos_arm64.h into a separate file so they aren't co-mingled with the register context definitions.  I have them in Utility/ARM64_LLDB_Registers.h currently.

I switched EmulateInstructionARM64 and TestArm64InstEmulation to create / test against unwind plans using the eRegisterKindLLDB register numbers because DWARF doesn't have codepoints defined for the "s" or "d" registers.

I fixed the STP register decoding in EmulateInstructionARM64 for the vector registers.

I added a unit test to test that these spill and restore instructions are included in the UnwindPlan correctly.

Here are some reasons why this patch isn't usable as-is.

First, Greg's objection is along the lines that it is not correct to have a generic eRegisterKindLLDB register table shared by multiple targets.  Today all arm64 targets have the same reg set, but it's possible to imagine an arm64  Cortex-M core that might not have the vector registers, for instance.  If such a target used a generic table that defines v0, it might try to display that register as zeros or give an error about how that register is unavailable or something.

Second, Tamas is going to notice that there's a fourth arm64 RegisterContext file - RegisterContextPOSIX_arm64 - and a header file already defining eRegisterKindLLDB register numbers for arm64 in lldb-arm64-register-enums.h (similar tables also exist for arm, s390x, x86, and mips with different ones for different targets).  I'm duplicating that file.  To be honest, I only just noticed that file right before I was writing up this patch and I didn't want to wait an extra day to roll the register definitions into one.

I don't have a strong preference for whether I keep the Utility/ARM64_LLDB_Registers.h or whether I add the w, s, and d register numbers to lldb-arm64-register-enums.h and use that.


Repository:
  rL LLVM

https://reviews.llvm.org/D25864

Files:
  lldb.xcodeproj/project.pbxproj
  source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp
  source/Plugins/Process/Utility/RegisterContextFreeBSD_arm64.cpp
  source/Plugins/Process/Utility/RegisterContextLinux_arm64.cpp
  source/Plugins/Process/Utility/RegisterInfos_arm64.h
  source/Utility/ARM64_LLDB_Registers.cpp
  source/Utility/ARM64_LLDB_Registers.h
  source/Utility/CMakeLists.txt
  unittests/UnwindAssembly/InstEmulation/TestArm64InstEmulation.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25864.75389.patch
Type: text/x-patch
Size: 162504 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161021/ef07d309/attachment-0001.bin>