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

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

1426

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

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

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

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

1426

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

1461

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

source/Symbol/UnwindPlan.cpp

23

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).

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_cfa_deref_reg_num  (rhs.m_cfa_deref_reg_num),
             m_register_locations (rhs.m_register_locations)
         {
         }
 
         bool
         operator == (const Row &rhs) const;
 
         bool
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;
+        }
+
+        // This should be used when GetCFAType() returns CFAIsRegisterDereferenced
+        uint32_t
+        GetCFARegisterNumberToDereference () const
+        {
+            return m_cfa_deref_reg_num;
+        }
+
+        // This should be used when GetCFAType() is set to CFAIsRegisterDereferenced
+        void
+        SetCFARegisterNumberToDereference (uint32_t reg_num) 
+        {
+            m_cfa_deref_reg_num = reg_num;
+        }
+
         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
+
+        // if m_cfa_type == CFAIsRegisterDereferenced, the following is used
+        uint32_t m_cfa_deref_reg_num;
+
         collection m_register_locations;
     }; // class Row
 
 public:
 
     typedef std::shared_ptr<Row> RowSP;
 
     UnwindPlan (lldb::RegisterKind reg_kind) : 

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)));

source/Plugins/Process/Utility/RegisterContextLLDB.cpp

RegisterContextLLDB::InitializeZerothFrame()
     {
         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, cfa_regval))
     {
         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;
if (!m_current_pc.IsValid() || !pc_module_sp)
             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, cfa_regval))
                 {
                     UnwindLogMsg ("failed to get cfa value");
                     if (m_frame_type != eSkipFrame)   // don't override eSkipFrame
                     {
                         m_frame_type = eNormalFrame;
                     }
                     return;
                 }
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, cfa_regval))
     {
         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;
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;
+        uint32_t cfa_regnum = active_row->GetCFARegister();
         addr_t cfa_regval = LLDB_INVALID_ADDRESS;
-        if (ReadGPRValue (m_fallback_unwind_plan_sp->GetRegisterKind(), active_row->GetCFARegister(), cfa_regval))
+        if (ReadCFAValueForRow(m_fallback_unwind_plan_sp->GetRegisterKind(), active_row, cfa_regval))
         {
             m_cfa = cfa_regval + active_row->GetCFAOffset ();
         }
 
         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)
+        {
+            tmp = value;
+            const RegisterInfo *reg_info = GetRegisterInfoAtIndex(cfa_regnum);
+            RegisterValue reg_value;
+            Error error = ReadRegisterValueFromMemory(reg_info,
+                                                      value,
+                                                      reg_info->byte_size,
+                                                      reg_value);
+            value = reg_value.GetAsUInt64();
+            UnwindLogMsg("dereferenced address: %p yields: %lx\n", tmp, value);
+            return error.Success();
+        }
+        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

source/Symbol/UnwindPlan.cpp

 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Thread.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 bool
 UnwindPlan::Row::RegisterLocation::operator == (const UnwindPlan::Row::RegisterLocation& rhs) const
 {
+    bool is_match = true;
+
+    if (m_type != rhs.m_type)
+        is_match = false;
+
     if (m_type == rhs.m_type)
     {
         switch (m_type)
         {
             case unspecified:
             case undefined:
             case same:
-                return true;
+                break;
                 
             case atCFAPlusOffset:
             case isCFAPlusOffset:
-                return m_location.offset == rhs.m_location.offset;
+                if (m_location.offset != rhs.m_location.offset)
+                    is_match = false;
+                break;
 
             case inOtherRegister:
-                return m_location.reg_num == rhs.m_location.reg_num;
+                if (m_location.reg_num != rhs.m_location.reg_num)
+                    is_match = false;
+                break;
             
             case atDWARFExpression:
             case isDWARFExpression:
                 if (m_location.expr.length == rhs.m_location.expr.length)
-                    return !memcmp (m_location.expr.opcodes, rhs.m_location.expr.opcodes, m_location.expr.length);
+                    if (memcmp (m_location.expr.opcodes, rhs.m_location.expr.opcodes, m_location.expr.length) != 0)
+                        is_match = false;
                 break;
         }
     }
-    return false;
+    return is_match;
 }
 
 // This function doesn't copy the dwarf expression bytes; they must remain in allocated
 // memory for the lifespan of this UnwindPlan object.
 void
 UnwindPlan::Row::RegisterLocation::SetAtDWARFExpression (const uint8_t *opcodes, uint32_t len)
 {
     m_type = atDWARFExpression;
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_cfa_deref_reg_num = LLDB_INVALID_REGNUM;
     m_register_locations.clear();
 }
 
 void
 UnwindPlan::Row::Dump (Stream& s, const UnwindPlan* unwind_plan, Thread* thread, addr_t base_addr) const
 {
     const RegisterInfo *reg_info = unwind_plan->GetRegisterInfo (thread, GetCFARegister());
 
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_cfa_deref_reg_num (LLDB_INVALID_REGNUM),
+    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_deref_reg_num != rhs.m_cfa_deref_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);