[Lldb-commits] [PATCH] UnwindPlan::Row refactor -- add support for CFA set by a DWARF expression

[Jason Molenda]

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.


http://reviews.llvm.org/D7755

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/