> On Oct 8, 2020, at 10:37 PM, Greg Clayton wrote:
>
>
>
>> On Oct 8, 2020, at 10:29 PM, Jason Molenda wrote:
>>
>>
>>
>>> On Oct 8, 2020, at 10:06 PM, Jason Molenda wrote:
>>>
>>>
>>>
On Oct 8, 2020, at 9:17 PM, Greg Clayton wrote:
> On Oct 8, 2020, at 8:55 PM, Jason Molenda wrote:
>
> Good bug find!
>
> It seems to me that DWARFCallFrameInfo should push the initial CIE
> register setup instructions as being the state at offset 0 in the
> function (in fact I'd say it's a bug if it's not). If that were done,
> then getting RowAtIndex(0) should be synonymous with
> get-the-CIE-register-unwind-rules, and this code would be correct.
>
> Looking at DWARFCallFrameInfo::FDEToUnwindPlan, we do
>
> unwind_plan.SetPlanValidAddressRange(range);
> UnwindPlan::Row *cie_initial_row = new UnwindPlan::Row;
> *cie_initial_row = cie->initial_row;
> UnwindPlan::RowSP row(cie_initial_row);
>
> unwind_plan.SetRegisterKind(GetRegisterKind());
> unwind_plan.SetReturnAddressRegister(cie->return_addr_reg_num);
>
> cie->initial_row is set by DWARFCallFrameInfo::HandleCommonDwarfOpcode.
>
> I think the bug here is DWARFCallFrameInfo::FDEToUnwindPlan not pushing
> that initial row at offset 0, isn't it? We don't really use DWARF CFI on
> darwin any more so I don't have a lot of real world experience here.
The only opcodes that push a row are DW_CFA_advance_locXXX and
DW_CFA_set_loc, so I don't think that is the right fix. I think we need to
pass a copy of just the registers from the "cie->initial_row" object
around to the opcode parsing code for restoration purposes.
>>>
>>>
>>> I think everything I'm saying here is besides the point, though. Unless we
>>> ALWAYS push the initial unwind state (from the CIE) to an UnwindPlan, the
>>> DW_CFA_restore is not going to work. If an unwind rule for r12 says
>>> "DW_CFA_restore" and at offset 0 in the function, the unwind rule for r12
>>> was "same" (i.e. no rule), but we return the RowAtIndex(0) and the first
>>> instruction, I don't know, spills it or something, then the DW_CFA_restore
>>> would set the r12 rule to "r12 was spilled" instead of "r12 is same".
>>>
>>> So the only way DW_CFA_restore would behave correctly, with this, is if we
>>> always push a Row at offset 0 with the rules from the CIE, or with no rules
>>> at all, just the initial unwind state showing how the CFA is set and no
>>> register rules.
>>
>>
>>
>> just to be clear, though, my initial reaction to this bug is "we should
>> always push a row at offset 0." I don't want to sound dumb or anything, but
>> I don't understand how unwinding would work if we didn't have a Row at
>> offset 0. You step into the function, you're at the first instruction, you
>> want to find the caller stack frame, and without knowing the rule for
>> establishing the CFA and finding the saved pc, I don't know how you get
>> that. And the only way to get the CFA / saved pc rule is to get the Row.
>> Do we really not have a Row at offset 0 when an UnwindPlan is created from
>> CFI? I might be forgetting some trick of UnwindPlans, but I don't see how
>> it works.
>
> What you are saying makes sense, but that isn't how it is encoded. A quick
> example:
>
> 0010 CIE
> Version: 4
> Augmentation: ""
> Address size: 4
> Segment desc size: 0
> Code alignment factor: 1
> Data alignment factor: -4
> Return address column: 14
>
> DW_CFA_def_cfa: reg13 +0
> DW_CFA_nop:
> DW_CFA_nop:
>
> 0014 0024 FDE cie= pc=0001bb2c...0001bc90
> DW_CFA_advance_loc: 4
> DW_CFA_def_cfa_offset: +32
> DW_CFA_offset: reg14 -4
> DW_CFA_offset: reg10 -8
> DW_CFA_offset: reg9 -12
> DW_CFA_offset: reg8 -16
> DW_CFA_offset: reg7 -20
> DW_CFA_offset: reg6 -24
> DW_CFA_offset: reg5 -28
> DW_CFA_offset: reg4 -32
> DW_CFA_advance_loc: 2
> DW_CFA_def_cfa_offset: +112
> DW_CFA_nop:
> DW_CFA_nop:
>
>
> DW_CFA_advance_loc is what pushes a row. As you can see in the FDE, it is the
> first thing it does.
Ah, cool, thanks for the example CFI. I believe
DWARFCallFrameInfo::FDEToUnwindPlan is doing the right thing here. We start by
initializing the local variable 'row' to the CIE's initial register state:
UnwindPlan::Row *cie_initial_row = new UnwindPlan::Row;
*cie_initial_row = cie->initial_row;
UnwindPlan::RowSP row(cie_initial_row);
The first instruction we hit is the advance loc,
if (primary_opcode) {
switch (primary_opcode) {
case DW_CFA_advance_loc: // (Row Creation Instruction)
unwind_plan.AppendRow(row);
UnwindPlan::Row *newrow = new UnwindPlan::Row;
*newrow = *row.get();
row.reset(newrow);
row->SlideOffset(extended_opcode * code_align);
break;
