Add an alternative CFA type.

[jhibbits]

Summary

PowerPC handles the stack chain with the current stack pointer being a pointer
to the backchain (CFA). LLDB currently has no way of handling this, so this
adds a "CFA is dereferenced from a register" type.

Discussed with Jason Molenda, who also provided the initial patch for this.

[jasonmolenda]

Looks good, some very minor comments inlined, fine to commit after those are tweaked. Have you tried exercising the code? Either with an architecture default unwindplan in your ABI or with your eh_frame parser? I'd like to know that it actually works for you. :)

source/Plugins/Process/Utility/RegisterContextLLDB.cpp

1422 ↗

(On Diff #15955)

I don't see cfa_regnum being used in this code block.

1426 ↗

(On Diff #15955)

I think ReadCFAValueForRow should add active_row->GetCFAOffset to the cfa register value itself (and the same thing up at line 575 for the previous call to ReadCFAValueForRow. I bet you can drop the cfa_regval variable entirely and just have m_cfa. This will break some UnwindLogMsg calls -- but I think ReadCFAValueForRow() should UnwindLogMsg the CFA register value and offset if it's a reg+offset CFA.

1461 ↗

(On Diff #15955)

When printing an addr_t, you should use PRIx64. e.g. "dereferenced address: 0x%" PRIx64 " yields: 0x%" PRIx64, tmp, value);". also, no \n at the end of UnwindLogMsgs.

source/Symbol/UnwindPlan.cpp

23 ↗

(On Diff #15955)

I don't have any problem with the changes to UnwindPlan::Row::RegisterLocation::operator == - but what motivated this change? It seems like it's functionally the same. Am I missing something?

[jhibbits]

In D6182#5, @jasonmolenda wrote:

Looks good, some very minor comments inlined, fine to commit after those are tweaked. Have you tried exercising the code? Either with an architecture default unwindplan in your ABI or with your eh_frame parser? I'd like to know that it actually works for you. :)

I have both powerpc64 and powerpc32 unwinding with the default unwindplan now (to be committed separately). Past experience has taught me infrastructure first and separate when reviewing.

source/Plugins/Process/Utility/RegisterContextLLDB.cpp

1422 ↗

(On Diff #15955)

Oops, missed this in my cleanup. In my first iteration I was passing discrete information, but changed to passing the row itself instead.

1426 ↗

(On Diff #15955)

I debated doing this and went with the path of least code change, but wanted your opinion. Trivial for me to change it.

1461 ↗

(On Diff #15955)

Gotcha. This started out as a debug printf for me that I changed to log and didn't update.

source/Symbol/UnwindPlan.cpp

23 ↗

(On Diff #15955)

This was part of the patch you sent me. I ripped out a chunk that didn't compile, and moved it down to UnwindPlan::Row::operator==(), and was left with this. I'll revert this since it makes no sense to keep it as changed.

[jhibbits]

Address Jason's comments

[jasonmolenda]

Thanks, please commit.

[jhibbits]

Closed by commit rL221788 (authored by @jhibbits).

lldb/trunk/include/lldb/Symbol/UnwindPlan.h

public:
                     undefined,          // reg is not available, e.g. volatile reg
                     same,               // reg is unchanged
                     atCFAPlusOffset,    // reg = deref(CFA + offset)
                     isCFAPlusOffset,    // reg = CFA + offset
                     inOtherRegister,    // reg = other reg
                     atDWARFExpression,  // reg = deref(eval(dwarf_expr))
                     isDWARFExpression   // reg = eval(dwarf_expr)
                 };
-    
             RegisterLocation() : 
                 m_type(unspecified), 
                 m_location() 
             {
             }
-    
             bool
             operator == (const RegisterLocation& rhs) const;
     
             bool
             operator != (const RegisterLocation &rhs) const
             {
                 return !(*this == rhs);
             }
public:
             } m_location;
         };
     
     public:
         Row ();
     
         Row (const UnwindPlan::Row& rhs) : 
             m_offset             (rhs.m_offset),
+            m_cfa_type           (rhs.m_cfa_type),
             m_cfa_reg_num        (rhs.m_cfa_reg_num),
             m_cfa_offset         (rhs.m_cfa_offset),
             m_register_locations (rhs.m_register_locations)
         {
         }
 
         bool
         operator == (const Row &rhs) const;
public:
         }
 
         void
         SlideOffset(lldb::addr_t offset)
         {
             m_offset += offset;
         }
 
+        // How we can reconstruct the CFA address for this stack frame, at this location
+        enum CFAType
+        {
+            CFAIsRegisterPlusOffset,   // the CFA value in a register plus (or minus) an offset
+            CFAIsRegisterDereferenced  // the address in a register is dereferenced to get CFA value
+        };
+
+        CFAType
+        GetCFAType () const
+        {
+            return m_cfa_type;
+        }
+
+        void
+        SetCFAType (CFAType cfa_type)
+        {
+            m_cfa_type = cfa_type;
+        }
+
+        // This should be used when GetCFAType() returns CFAIsRegisterPlusOffset
         uint32_t
         GetCFARegister () const
         {
             return m_cfa_reg_num;
         }
         
+        // This should be used when GetCFAType() is set to CFAIsRegisterPlusOffset
+        void
+        SetCFARegister (uint32_t reg_num);
+
+        // This should be used when GetCFAType() returns CFAIsRegisterPlusOffset
+        int32_t
+        GetCFAOffset () const
+        {
+            return m_cfa_offset;
+        }
+
+        // This should be used when GetCFAType() is set to CFAIsRegisterPlusOffset
+        void
+        SetCFAOffset (int32_t offset)
+        {
+            m_cfa_offset = offset;
+        }
+
         bool
         SetRegisterLocationToAtCFAPlusOffset (uint32_t reg_num, 
                                               int32_t offset, 
                                               bool can_replace);
 
         bool
         SetRegisterLocationToIsCFAPlusOffset (uint32_t reg_num, 
                                               int32_t offset, 
public:
         SetRegisterLocationToRegister (uint32_t reg_num, 
                                        uint32_t other_reg_num,
                                        bool can_replace);
 
         bool
         SetRegisterLocationToSame (uint32_t reg_num, 
                                    bool must_replace);
 
-
-
-        void
-        SetCFARegister (uint32_t reg_num);
-
-        int32_t
-        GetCFAOffset () const
-        {
-            return m_cfa_offset;
-        }
-
-        void
-        SetCFAOffset (int32_t offset)
-        {
-            m_cfa_offset = offset;
-        }
-
         void
         Clear ();
 
         void
         Dump (Stream& s, const UnwindPlan* unwind_plan, Thread* thread, lldb::addr_t base_addr) const;
 
     protected:
         typedef std::map<uint32_t, RegisterLocation> collection;
         lldb::addr_t m_offset;      // Offset into the function for this row
+
+        CFAType m_cfa_type;
+
+        // If m_cfa_type == CFAIsRegisterPlusOffset, the following ivars are used
         uint32_t m_cfa_reg_num;     // The Call Frame Address register number
         int32_t  m_cfa_offset;      // The offset from the CFA for this row
+
         collection m_register_locations;
     }; // class Row
 
 public:
 
     typedef std::shared_ptr<Row> RowSP;
 
     UnwindPlan (lldb::RegisterKind reg_kind) : 

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

private:
     bool
     TryFallbackUnwindPlan ();
 
     // Get the contents of a general purpose (address-size) register for this frame
     // (usually retrieved from the next frame)
     bool
     ReadGPRValue (lldb::RegisterKind register_kind, uint32_t regnum, lldb::addr_t &value);
 
+    // Get the CFA register for a given frame.
+    bool
+    ReadCFAValueForRow (lldb::RegisterKind register_kind, const UnwindPlan::RowSP &row, lldb::addr_t &value);
+
     lldb::UnwindPlanSP
     GetFastUnwindPlanForFrame ();
 
     lldb::UnwindPlanSP
     GetFullUnwindPlanForFrame ();
 
     void
     UnwindLogMsg (const char *fmt, ...) __attribute__ ((format (printf, 2, 3)));

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

RegisterContextLLDB::InitializeZerothFrame()
     }
 
     // We've set m_frame_type and m_sym_ctx before these calls.
 
     m_fast_unwind_plan_sp = GetFastUnwindPlanForFrame ();
     m_full_unwind_plan_sp = GetFullUnwindPlanForFrame ();
 
     UnwindPlan::RowSP active_row;
-    int cfa_offset = 0;
     lldb::RegisterKind row_register_kind = eRegisterKindGeneric;
     if (m_full_unwind_plan_sp && m_full_unwind_plan_sp->PlanValidAtAddress (m_current_pc))
     {
         active_row = m_full_unwind_plan_sp->GetRowForFunctionOffset (m_current_offset);
         row_register_kind = m_full_unwind_plan_sp->GetRegisterKind ();
         if (active_row.get() && log)
         {
             StreamString active_row_strm;
             active_row->Dump(active_row_strm, m_full_unwind_plan_sp.get(), &m_thread, m_start_pc.GetLoadAddress(exe_ctx.GetTargetPtr()));
             UnwindLogMsg ("%s", active_row_strm.GetString().c_str());
         }
     }
 
     if (!active_row.get())
     {
         UnwindLogMsg ("could not find an unwindplan row for this frame's pc");
         m_frame_type = eNotAValidFrame;
         return;
     }
 
 
-    addr_t cfa_regval = LLDB_INVALID_ADDRESS;
-    if (!ReadGPRValue (row_register_kind, active_row->GetCFARegister(), cfa_regval))
+    if (!ReadCFAValueForRow (row_register_kind, active_row, m_cfa))
     {
         UnwindLogMsg ("could not read CFA register for this frame.");
         m_frame_type = eNotAValidFrame;
         return;
     }
 
-    cfa_offset = active_row->GetCFAOffset ();
-    m_cfa = cfa_regval + cfa_offset;
-
-    UnwindLogMsg ("cfa_regval = 0x%16.16" PRIx64 " (cfa_regval = 0x%16.16" PRIx64 ", cfa_offset = %i)", m_cfa, cfa_regval, cfa_offset);
     UnwindLogMsg ("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" PRIx64 " using %s UnwindPlan",
             (uint64_t) m_current_pc.GetLoadAddress (exe_ctx.GetTargetPtr()),
             (uint64_t) m_cfa,
             m_full_unwind_plan_sp->GetSourceName().GetCString());
 }
 
 // Initialize a RegisterContextLLDB for the non-zeroth frame -- rely on the RegisterContextLLDB "below" it
 // to provide things like its current pc value.
if (!m_current_pc.IsValid() || !pc_module_sp)
             abi->CreateDefaultUnwindPlan(*m_full_unwind_plan_sp);
             if (m_frame_type != eSkipFrame)  // don't override eSkipFrame
             {
                 m_frame_type = eNormalFrame;
             }
             m_all_registers_available = false;
             m_current_offset = -1;
             m_current_offset_backed_up_one = -1;
-            addr_t cfa_regval = LLDB_INVALID_ADDRESS;
             RegisterKind row_register_kind = m_full_unwind_plan_sp->GetRegisterKind ();
             UnwindPlan::RowSP row = m_full_unwind_plan_sp->GetRowForFunctionOffset(0);
             if (row.get())
             {
-                uint32_t cfa_regnum = row->GetCFARegister();
-                int cfa_offset = row->GetCFAOffset();
-                if (!ReadGPRValue (row_register_kind, cfa_regnum, cfa_regval))
+                if (!ReadCFAValueForRow (row_register_kind, row, m_cfa))
                 {
                     UnwindLogMsg ("failed to get cfa value");
                     if (m_frame_type != eSkipFrame)   // don't override eSkipFrame
                     {
                         m_frame_type = eNormalFrame;
                     }
                     return;
                 }
-                m_cfa = cfa_regval + cfa_offset;
 
                 // A couple of sanity checks..
-                if (cfa_regval == LLDB_INVALID_ADDRESS || cfa_regval == 0 || cfa_regval == 1)
+                if (m_cfa == LLDB_INVALID_ADDRESS || m_cfa == 0 || m_cfa == 1)
                 {
                     UnwindLogMsg ("could not find a valid cfa address");
                     m_frame_type = eNotAValidFrame;
                     return;
                 }
 
-                // cfa_regval should point into the stack memory; if we can query memory region permissions,
+                // m_cfa should point into the stack memory; if we can query memory region permissions,
                 // see if the memory is allocated & readable.
-                if (process->GetLoadAddressPermissions(cfa_regval, permissions)
+                if (process->GetLoadAddressPermissions(m_cfa, permissions)
                     && (permissions & ePermissionsReadable) == 0)
                 {
                     m_frame_type = eNotAValidFrame;
                     UnwindLogMsg ("the CFA points to a region of memory that is not readable");
                     return;
                 }
             }
             else
RegisterContextLLDB::InitializeNonZerothFrame()
 
     if (!active_row.get())
     {
         m_frame_type = eNotAValidFrame;
         UnwindLogMsg ("could not find unwind row for this pc");
         return;
     }
 
-    addr_t cfa_regval = LLDB_INVALID_ADDRESS;
-    if (!ReadGPRValue (row_register_kind, active_row->GetCFARegister(), cfa_regval))
+    if (!ReadCFAValueForRow (row_register_kind, active_row, m_cfa))
     {
         UnwindLogMsg ("failed to get cfa reg %d/%d", row_register_kind, active_row->GetCFARegister());
         m_frame_type = eNotAValidFrame;
         return;
     }
 
     cfa_offset = active_row->GetCFAOffset ();
-    m_cfa = cfa_regval + cfa_offset;
 
-    UnwindLogMsg ("cfa_regval = 0x%16.16" PRIx64 " (cfa_regval = 0x%16.16" PRIx64 ", cfa_offset = %i)", m_cfa, cfa_regval, cfa_offset);
+    UnwindLogMsg ("m_cfa = 0x%16.16" PRIx64 " (cfa_offset = %i)", m_cfa, cfa_offset);
 
     // A couple of sanity checks..
-    if (cfa_regval == LLDB_INVALID_ADDRESS || cfa_regval == 0 || cfa_regval == 1)
+    if (m_cfa == LLDB_INVALID_ADDRESS || m_cfa == (addr_t)cfa_offset || m_cfa == (addr_t)cfa_offset + 1)
     {
         UnwindLogMsg ("could not find a valid cfa address");
         m_frame_type = eNotAValidFrame;
         return;
     }
 
     if (CheckIfLoopingStack ())
     {
RegisterContextLLDB::TryFallbackUnwindPlan ()
 
     UnwindPlanSP original_full_unwind_plan_sp = m_full_unwind_plan_sp;
     UnwindPlan::RowSP active_row = m_fallback_unwind_plan_sp->GetRowForFunctionOffset (m_current_offset);
     
     if (active_row && active_row->GetCFARegister() != LLDB_INVALID_REGNUM)
     {
         m_registers.clear();
         m_full_unwind_plan_sp = m_fallback_unwind_plan_sp;
-        addr_t cfa_regval = LLDB_INVALID_ADDRESS;
-        if (ReadGPRValue (m_fallback_unwind_plan_sp->GetRegisterKind(), active_row->GetCFARegister(), cfa_regval))
-        {
-            m_cfa = cfa_regval + active_row->GetCFAOffset ();
-        }
+        m_cfa = LLDB_INVALID_ADDRESS;
+        ReadCFAValueForRow(m_fallback_unwind_plan_sp->GetRegisterKind(), active_row, m_cfa);
 
         UnwindLogMsg ("trying to unwind from this function with the UnwindPlan '%s' because UnwindPlan '%s' failed.", 
                       m_fallback_unwind_plan_sp->GetSourceName().GetCString(),
                       original_full_unwind_plan_sp->GetSourceName().GetCString());
         m_fallback_unwind_plan_sp.reset();
     }
 
     return true;
 }
 
+bool
+RegisterContextLLDB::ReadCFAValueForRow (lldb::RegisterKind row_register_kind,
+                                         const UnwindPlan::RowSP &row,
+                                         addr_t &value)
+{
+    uint32_t cfa_regnum = row->GetCFARegister();
+    RegisterValue reg_value;
+    addr_t tmp;
+
+    value = LLDB_INVALID_ADDRESS;
+
+    if (ReadGPRValue (row_register_kind, cfa_regnum, value))
+    {
+        if (row->GetCFAType() == UnwindPlan::Row::CFAIsRegisterDereferenced)
+        {
+            const RegisterInfo *reg_info = GetRegisterInfoAtIndex(cfa_regnum);
+            RegisterValue reg_value;
+            tmp = value;
+            Error error = ReadRegisterValueFromMemory(reg_info,
+                                                      value,
+                                                      reg_info->byte_size,
+                                                      reg_value);
+            value = reg_value.GetAsUInt64();
+            UnwindLogMsg("dereferenced address: 0x%16.16" PRIx64 " yields: %lx", tmp, value);
+            return error.Success();
+        }
+        value = value + row->GetCFAOffset ();
+        return true;
+    }
+    return false;
+}
+
 // Retrieve a general purpose register value for THIS frame, as saved by the NEXT frame, i.e. the frame that
 // this frame called.  e.g.
 //
 //  foo () { }
 //  bar () { foo (); }
 //  main () { bar (); }
 //
 //  stopped in foo() so

lldb/trunk/source/Symbol/UnwindPlan.cpp

switch (m_type)
             break;
         
     }
 }
 
 void
 UnwindPlan::Row::Clear ()
 {
+    m_cfa_type = CFAIsRegisterPlusOffset;
     m_offset = 0;
     m_cfa_reg_num = LLDB_INVALID_REGNUM;
     m_cfa_offset = 0;
     m_register_locations.clear();
 }
 
 void
 UnwindPlan::Row::Dump (Stream& s, const UnwindPlan* unwind_plan, Thread* thread, addr_t base_addr) const
for (collection::const_iterator idx = m_register_locations.begin (); idx != m_register_locations.end (); ++idx)
         const bool verbose = false;
         idx->second.Dump(s, unwind_plan, this, thread, verbose);
         s.PutChar (' ');
     }
     s.EOL();
 }
 
 UnwindPlan::Row::Row() :
-    m_offset(0),
-    m_cfa_reg_num(LLDB_INVALID_REGNUM),
-    m_cfa_offset(0),
-    m_register_locations()
+    m_offset (0),
+    m_cfa_type (CFAIsRegisterPlusOffset),
+    m_cfa_reg_num (LLDB_INVALID_REGNUM),
+    m_cfa_offset (0),
+    m_register_locations ()
 {
 }
 
 bool
 UnwindPlan::Row::GetRegisterInfo (uint32_t reg_num, UnwindPlan::Row::RegisterLocation& register_location) const
 {
     collection::const_iterator pos = m_register_locations.find(reg_num);
     if (pos != m_register_locations.end())
UnwindPlan::Row::SetCFARegister (uint32_t reg_num)
     m_cfa_reg_num = reg_num;
 }
 
 bool
 UnwindPlan::Row::operator == (const UnwindPlan::Row& rhs) const
 {
     if (m_offset != rhs.m_offset || m_cfa_reg_num != rhs.m_cfa_reg_num || m_cfa_offset != rhs.m_cfa_offset)
         return false;
+
+    if (m_cfa_type != rhs.m_cfa_type)
+        return false;
+
+    if (m_cfa_type == CFAIsRegisterPlusOffset)
+    {
+        if (m_cfa_reg_num != rhs.m_cfa_reg_num)
+            return false;
+        if (m_cfa_offset != rhs.m_cfa_offset)
+            return false;
+    }
+    if (m_cfa_type == CFAIsRegisterDereferenced)
+    {
+        if (m_cfa_reg_num != rhs.m_cfa_reg_num)
+            return false;
+    }
+
     return m_register_locations == rhs.m_register_locations;
 }
 
 void
 UnwindPlan::AppendRow (const UnwindPlan::RowSP &row_sp)
 {
     if (m_row_list.empty() || m_row_list.back()->GetOffset() != row_sp->GetOffset())
         m_row_list.push_back(row_sp);