[Lldb-commits] [PATCH] Improve PowerPC unwind support

[Justin Hibbits]

On Nov 9, 2014 1:25 PM, "Jason Molenda" <jmolenda at apple.com> wrote:
>
> ================
> Comment at: source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp:1015
> @@ -1014,4 +1014,3 @@
>
> -    const int32_t ptr_size = 8;
> -    row->SetCFARegister (LLDB_REGNUM_GENERIC_SP);
> -    row->SetCFAOffset (8);
> +    const int32_t ptr_size = 4;
> +    row->SetCFARegister (sp_reg_num);
> ----------------
> emaste wrote:
> > I'd specifically call out this fix in the commit message
> 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.

This was a plain and simple bug caused by copy and paste. Since this coffee
really didn't work from the beginning I didn't notice until I got the new
unwind code in.

>
> ================
> Comment at: source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp:1016
> @@ +1015,3 @@
> +    const int32_t ptr_size = 4;
> +    row->SetCFARegister (sp_reg_num);
> +    row->SetCFAOffset (0);
> ----------------
> You should be calling SetCFARegisterNumberToDereference() here, shouldn't
you?

I completely missed that API when making the last patch and instead used
the CFARegister and the type to determine how to get the CFA. One of these
has to go, which do you think is the better API? I'll just remove the
unnecessary one.

>
> ================
> Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:1016
> @@ -1015,4 +1015,3 @@
>      const int32_t ptr_size = 8;
> -    row->SetCFARegister (LLDB_REGNUM_GENERIC_SP);
> -    row->SetCFAOffset (48);
> +    row->SetCFARegister (sp_reg_num);
>      row->SetOffset (0);
> ----------------
> Same thihng, this should be SetCFARegisterNumberToDereference().
>
> http://reviews.llvm.org/D6183
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141109/ba6438cc/attachment.html>