<p dir="ltr"><br>
On Nov 9, 2014 1:25 PM, "Jason Molenda" <<a href="mailto:jmolenda@apple.com">jmolenda@apple.com</a>> wrote:<br>
><br>
> ================<br>
> Comment at: source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp:1015<br>
> @@ -1014,4 +1014,3 @@<br>
><br>
> -    const int32_t ptr_size = 8;<br>
> -    row->SetCFARegister (LLDB_REGNUM_GENERIC_SP);<br>
> -    row->SetCFAOffset (8);<br>
> +    const int32_t ptr_size = 4;<br>
> +    row->SetCFARegister (sp_reg_num);<br>
> ----------------<br>
> emaste wrote:<br>
> > I'd specifically call out this fix in the commit message<br>
> 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.</p>
<p dir="ltr">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.</p>
<p dir="ltr">><br>
> ================<br>
> Comment at: source/Plugins/ABI/SysV-ppc/ABISysV_ppc.cpp:1016<br>
> @@ +1015,3 @@<br>
> +    const int32_t ptr_size = 4;<br>
> +    row->SetCFARegister (sp_reg_num);<br>
> +    row->SetCFAOffset (0);<br>
> ----------------<br>
> You should be calling SetCFARegisterNumberToDereference() here, shouldn't you?</p>
<p dir="ltr">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.</p>
<p dir="ltr">><br>
> ================<br>
> Comment at: source/Plugins/ABI/SysV-ppc64/ABISysV_ppc64.cpp:1016<br>
> @@ -1015,4 +1015,3 @@<br>
>      const int32_t ptr_size = 8;<br>
> -    row->SetCFARegister (LLDB_REGNUM_GENERIC_SP);<br>
> -    row->SetCFAOffset (48);<br>
> +    row->SetCFARegister (sp_reg_num);<br>
>      row->SetOffset (0);<br>
> ----------------<br>
> Same thihng, this should be SetCFARegisterNumberToDereference().<br>
><br>
> <a href="http://reviews.llvm.org/D6183">http://reviews.llvm.org/D6183</a><br>
><br>
><br>
</p>