Improve PowerPC unwind support

[jhibbits]

Summary

Taking advantage of the new 'CFAIsRegisterDereferenced' CFA register type, add
full stack unwind support to the PowerPC/PowerPC64 ABI. Also, add a new
register set for powerpc32-on-64, so the register sizes are correct. This also
requires modifying the ProcessMonitor to add support for non-uintptr_t-sized
register values.

[emaste]

I'd like to think about the 32-on-64 implementation some more. Can you submit the unwind changes separately first?

source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp

1015 ↗

(On Diff #15958)

I'd specifically call out this fix in the commit message

[jhibbits]

Unfortunately I can't. The unwind code depends on the 32-on-64 code in
order to give a valid back trace so becomes pointless without it. I guess I
could separate them and there would be no regression from the current
state, there just would be no gain.

[jasonmolenda]

source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp

1015 ↗

(On Diff #15958)

If you need this to be dynamically determined, when the ABISysV_ppc::CreateInstance method is called, it is passed an ArchSpec - that object has a GetAddressByteSize() method. I think it would be safe to cache the value in an ivar, or copy the ArchSpec. Greg might know of some reason why that's a bad idea -- but I think if a Target changes architecture, the ABI should be updated as well.

1016 ↗

(On Diff #15958)

You should be calling SetCFARegisterNumberToDereference() here, shouldn't you?

source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp

1016 ↗

(On Diff #15958)

Same thihng, this should be SetCFARegisterNumberToDereference().

[emaste]

source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp

236 ↗

(On Diff #15958)

This seems like it ought to have a comment.

[jhibbits]

Update per comments from Ed.

[emaste]

source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

320–323 ↗

(On Diff #15966)

This will be dead code for one of the two cases (whichever has the same size as uintptr_t) no?

[jhibbits]

Remove uintptr_t usage, it's handled by uint32_t and uint64_t.

[emaste]

LGTM

I'd like to revisit ReadRegOperation::Execute later on for endianness considerations (for i386 on x86_64 and ppc on ppc64), but am happy with this for now.

[jhibbits]

Closed by commit rL221789 (authored by @jhibbits).

lldb/trunk/source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp

ABISysV_ppc::CreateDefaultUnwindPlan (UnwindPlan &unwind_plan)
     unwind_plan.Clear();
     unwind_plan.SetRegisterKind (eRegisterKindDWARF);
 
     uint32_t sp_reg_num = gcc_dwarf_r1;
     uint32_t pc_reg_num = gcc_dwarf_lr;
 
     UnwindPlan::RowSP row(new UnwindPlan::Row);
 
-    const int32_t ptr_size = 8;
-    row->SetCFARegister (LLDB_REGNUM_GENERIC_SP);
-    row->SetCFAOffset (8);
-    row->SetOffset (0);
+    const int32_t ptr_size = 4;
+    row->SetCFARegister (sp_reg_num);
+    row->SetCFAType(lldb_private::UnwindPlan::Row::CFAIsRegisterDereferenced);
 
-    row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -1, true);
-    row->SetRegisterLocationToAtCFAPlusOffset(sp_reg_num, ptr_size * -2, true);
+    row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * 1, true);
+    row->SetRegisterLocationToIsCFAPlusOffset(sp_reg_num, 0, true);
 
     unwind_plan.AppendRow (row);
     unwind_plan.SetSourceName ("ppc default unwind plan");
     unwind_plan.SetSourcedFromCompiler (eLazyBoolNo);
     unwind_plan.SetUnwindPlanValidAtAllInstructions (eLazyBoolNo);
     unwind_plan.SetReturnAddressRegister(gcc_dwarf_lr);
     return true;
 }

lldb/trunk/source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp

ABISysV_ppc64::CreateDefaultUnwindPlan (UnwindPlan &unwind_plan)
     unwind_plan.SetRegisterKind (eRegisterKindDWARF);
 
     uint32_t sp_reg_num = gcc_dwarf_r1;
     uint32_t pc_reg_num = gcc_dwarf_lr;
 
     UnwindPlan::RowSP row(new UnwindPlan::Row);
 
     const int32_t ptr_size = 8;
-    row->SetCFARegister (LLDB_REGNUM_GENERIC_SP);
-    row->SetCFAOffset (48);
-    row->SetOffset (0);
-
-    row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * -4, true);
-    row->SetRegisterLocationToAtCFAPlusOffset(sp_reg_num, ptr_size * -6, true);
-    row->SetRegisterLocationToAtCFAPlusOffset(gcc_dwarf_cr, ptr_size * -5, true);
+    row->SetCFARegister (sp_reg_num);
+    row->SetCFAType(lldb_private::UnwindPlan::Row::CFAIsRegisterDereferenced);
+
+    row->SetRegisterLocationToAtCFAPlusOffset(pc_reg_num, ptr_size * 2, true);
+    row->SetRegisterLocationToIsCFAPlusOffset(sp_reg_num, 0, true);
+    row->SetRegisterLocationToAtCFAPlusOffset(gcc_dwarf_cr, ptr_size, true);
 
     unwind_plan.AppendRow (row);
     unwind_plan.SetSourceName ("ppc64 default unwind plan");
     unwind_plan.SetSourcedFromCompiler (eLazyBoolNo);
     unwind_plan.SetUnwindPlanValidAtAllInstructions (eLazyBoolNo);
     unwind_plan.SetReturnAddressRegister(gcc_dwarf_lr);
     return true;
 }

lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp

 ReadRegOperation::Execute(ProcessMonitor *monitor)
 {
     struct reg regs;
     int rc;
 
     if ((rc = PTRACE(PT_GETREGS, m_tid, (caddr_t)&regs, 0)) < 0) {
         m_result = false;
     } else {
-        if (m_size == sizeof(uintptr_t))
-            m_value = *(uintptr_t *)(((caddr_t)&regs) + m_offset);
-        else 
+        // 'struct reg' contains only 32- or 64-bit register values.  Punt on
+        // others.  Also, not all entries may be uintptr_t sized, such as 32-bit
+        // processes on powerpc64 (probably the same for i386 on amd64)
+        if (m_size == sizeof(uint32_t))
+            m_value = *(uint32_t *)(((caddr_t)&regs) + m_offset);
+        else if (m_size == sizeof(uint64_t))
+            m_value = *(uint64_t *)(((caddr_t)&regs) + m_offset);
+        else
             memcpy(&m_value, (((caddr_t)&regs) + m_offset), m_size);
         m_result = true;
     }
 }
 
 //------------------------------------------------------------------------------
 /// @class WriteRegOperation
 /// @brief Implements ProcessMonitor::WriteRegisterValue.

lldb/trunk/source/Plugins/Process/POSIX/RegisterContextPOSIXProcessMonitor_powerpc.cpp

if (reg_info->invalidate_regs && (reg_info->invalidate_regs[0] != LLDB_INVALID_REGNUM))
                     value_to_write.SetType(full_reg_info);
                     reg_to_write = full_reg;
                 }
             }
         }
     }
 
     ProcessMonitor &monitor = GetMonitor();
+    // Account for the fact that 32-bit targets on powerpc64 really use 64-bit
+    // registers in ptrace, but expose here 32-bit registers with a higher
+    // offset.
+    uint64_t offset = GetRegisterOffset(reg_to_write);
+    offset &= ~(sizeof(uintptr_t) - 1);
     return monitor.WriteRegisterValue(m_thread.GetID(),
-                                      GetRegisterOffset(reg_to_write),
+                                      offset,
                                       GetRegisterName(reg_to_write),
                                       value_to_write);
 }
 
 bool
 RegisterContextPOSIXProcessMonitor_powerpc::ReadRegister(const RegisterInfo *reg_info, RegisterValue &value)
 {
     if (!reg_info)

lldb/trunk/source/Plugins/Process/Utility/RegisterContextFreeBSD_powerpc.cpp

 
 RegisterContextFreeBSD_powerpc::~RegisterContextFreeBSD_powerpc()
 {
 }
 
 size_t
 RegisterContextFreeBSD_powerpc::GetGPRSize() const
 {
-    return sizeof(GPR64);
+    // This is an 'abstract' base, so no GPR struct.
+    return 0;
 }
 
 const RegisterInfo *
 RegisterContextFreeBSD_powerpc::GetRegisterInfo() const
 {
     //assert (m_target_arch.GetCore() == ArchSpec::eCore_powerpc);
     llvm_unreachable("Abstract class!");
     return NULL;

 {
     return sizeof(GPR64);
 }
 
 const RegisterInfo *
 RegisterContextFreeBSD_powerpc64::GetRegisterInfo() const
 {
     //assert (m_target_arch.GetCore() == ArchSpec::eCore_powerpc);
+    if (m_target_arch.GetMachine() == llvm::Triple::ppc)
+        return g_register_infos_powerpc64_32;
     return g_register_infos_powerpc64;
 }
 
 uint32_t
 RegisterContextFreeBSD_powerpc64::GetRegisterCount () const
 {
     return static_cast<uint32_t> (sizeof (g_register_infos_powerpc64) / sizeof (g_register_infos_powerpc64 [0]));
 }

lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_powerpc.h

 //===-- RegisterInfos_powerpc.h ---------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===---------------------------------------------------------------------===//
 
 #include <stddef.h>
 
 // Computes the offset of the given GPR in the user data area.
 #define GPR_OFFSET(regname)                                                 \
     (offsetof(GPR, regname))
 #define FPR_OFFSET(regname)                                                 \
     (offsetof(FPR, regname))
+#define GPR_SIZE(regname)                                                   \
+    (sizeof(((GPR*)NULL)->regname))
 
 #ifdef DECLARE_REGISTER_INFOS_POWERPC_STRUCT
 
 // Note that the size and offset will be updated by platform-specific classes.
 #define DEFINE_GPR(reg, alt, lldb_kind)           \
-    { #reg, alt, sizeof(((GPR*)NULL)->reg), GPR_OFFSET(reg), eEncodingUint, \
+    { #reg, alt, GPR_SIZE(reg), GPR_OFFSET(reg), eEncodingUint, \
       eFormatHex, { gcc_dwarf_##reg##_powerpc, gcc_dwarf_##reg##_powerpc, lldb_kind, gdb_##reg##_powerpc, gpr_##reg##_powerpc }, NULL, NULL }
 #define DEFINE_FPR(reg, lldb_kind)           \
     { #reg, NULL, 8, FPR_OFFSET(reg), eEncodingIEEE754, \
       eFormatFloat, { gcc_dwarf_##reg##_powerpc, gcc_dwarf_##reg##_powerpc, lldb_kind, gdb_##reg##_powerpc, fpr_##reg##_powerpc }, NULL, NULL }
 
     // General purpose registers.                 GCC,                  DWARF,              Generic,                GDB
 #define POWERPC_REGS \
     DEFINE_GPR(r0,       NULL,  LLDB_INVALID_REGNUM), \
#define POWERPC_REGS \
     DEFINE_FPR(f25,      LLDB_INVALID_REGNUM), \
     DEFINE_FPR(f26,      LLDB_INVALID_REGNUM), \
     DEFINE_FPR(f27,      LLDB_INVALID_REGNUM), \
     DEFINE_FPR(f28,      LLDB_INVALID_REGNUM), \
     DEFINE_FPR(f29,      LLDB_INVALID_REGNUM), \
     DEFINE_FPR(f30,      LLDB_INVALID_REGNUM), \
     DEFINE_FPR(f31,      LLDB_INVALID_REGNUM), \
     { "fpscr", NULL, 8, FPR_OFFSET(fpscr), eEncodingUint, eFormatHex, { gcc_dwarf_fpscr_powerpc, gcc_dwarf_fpscr_powerpc, LLDB_INVALID_REGNUM, gdb_fpscr_powerpc, fpr_fpscr_powerpc }, NULL, NULL },
-    //{ NULL, NULL, sizeof(((GPR*)NULL)->r0), 0, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, gpr_cfa_powerpc}, NULL, NULL}
 static RegisterInfo
 g_register_infos_powerpc64[] =
 {
 #define GPR GPR64
     POWERPC_REGS
 #undef GPR
 };
 
 static RegisterInfo
 g_register_infos_powerpc32[] =
 {
 #define GPR GPR32
     POWERPC_REGS
 #undef GPR
 };
+
+static RegisterInfo
+g_register_infos_powerpc64_32[] =
+{
+#define GPR GPR64
+#undef GPR_SIZE
+#define GPR_SIZE(reg)   (sizeof(uint32_t))
+#undef GPR_OFFSET
+#define GPR_OFFSET(regname)                                                 \
+    (offsetof(GPR, regname) + (sizeof(((GPR *)NULL)->regname) - GPR_SIZE(reg)))
+    POWERPC_REGS
+#undef GPR
+};
+
 static_assert((sizeof(g_register_infos_powerpc32) / sizeof(g_register_infos_powerpc32[0])) == k_num_registers_powerpc,
     "g_register_infos_powerpc32 has wrong number of register infos");
 static_assert((sizeof(g_register_infos_powerpc64) / sizeof(g_register_infos_powerpc64[0])) == k_num_registers_powerpc,
     "g_register_infos_powerpc64 has wrong number of register infos");
+static_assert(sizeof(g_register_infos_powerpc64_32) == sizeof(g_register_infos_powerpc64),
+    "g_register_infos_powerpc64_32 doesn't match size of g_register_infos_powerpc64");
 
 #undef DEFINE_FPR
 #undef DEFINE_GPR
 
 #endif // DECLARE_REGISTER_INFOS_POWERPC_STRUCT
 
 #undef GPR_OFFSET