[Lldb-commits] [PATCH] UnwindPlan::Row refactor -- add support for CFA set by a DWARF expression
jmolenda at apple.com
Sat Feb 21 01:52:49 PST 2015
Yeah, I can see what you're talking about with CFAValue::SetOffset() assuming the type of the CFA value but for instance in UnwindAssembly-x86, when it knows that the CFA is set in terms of rsp and the stack pointer value changes, it already knows that the CFA value type is IsRegisterPlusOffset - it is safe to simply call SetOffset() here.
In the case of UnwindAssembly-x86, it needs to track the stack pointer as it is modified throughout the lifetime of the function. When a register is saved on the stack, it records where that register was saved. It would be possible for it to track all of this in the CFAValue class - calling GetOffset() instead of using a local, calling IncrementOffset() whenever the stack value is moved down. There'd need to be a matching DecrementOffset() to match, of course. But I think it's more natural (to me, anyway) to have a local var tracking these changes and calling SetOffset() whenever needed.
I don't think the CFAValue class needs to dictate how the users behave - I'm fine with having methods to adjust the value - but I'd rather classes like UnwindAssembly-x86 continue to call SetOffset(). We could make it return a bool if the operation was invalid, but IncrementOffset() would have the same issues for that matter. I don't think it's worth worrying about.
Thanks for making the adjustment to the patch.
More information about the lldb-commits