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. > On Oct 8, 2020, at 4:01 PM, Greg Clayton <clayb...@gmail.com> wrote: > > Hello LLDB devs, > > This is a deep dive into an issue I found in the LLDB handling of DWARF call > frame information, so no need to read further if this doesn't interest you! > > I am in the process of adding some support to LLVM for parsing the opcode > state machines for CIE and FDE objects that produces unwind rows. While > making unit tests to test DW_CFA_restore and DW_CFA_restore_extended opcodes, > I read the DWARF spec that states: > > "The DW_CFA_restore instruction takes a single operand (encoded with the > opcode) that represents a register number. The required action is to change > the rule for the indicated register to the rule assigned it by the > initial_instructions in the CIE." > > Looking at the LLDB code in DWARFCallFrameInfo.cpp I see code that is > simplified to: > > case DW_CFA_restore: > if (unwind_plan.IsValidRowIndex(0) && > unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num, reg_location)) > row->SetRegisterInfo(reg_num, reg_location); > break; > > > The issue is, the CIE contains initial instructions, but it doesn't push a > row after doing these instructions, the FDE will push a row when it emits a > DW_CFA_advance_loc, DW_CFA_advance_loc1, DW_CFA_advance_loc2, > DW_CFA_advance_loc4 or DW_CFA_set_loc opcode. So the DWARF spec says we > should restore the register rule to be what it was in the CIE's initial > instructions, but we are restoring it to the first row that was parsed. This > will mostly not get us into trouble because .debug_frame and .eh_frame > usually have a DW_CFA_advance_locXXX or DW_CFA_set_loc opcode as the first > opcode, but it isn't a requirement and a FDE could modify a register value > prior to pushing the first row at index zero. So we might be restoring the > register incorrectly in some cases according to the spec. Also, what if there > was no value specified in the CIE's initial instructions for a register? > Should we remove the register value to match the state of the CIE's initial > instructions if there is no rule for the register? We are currently leaving > this register as having the same value if there is no value for the register > in the first row. > > Let me know what you think. > > Greg Clayton > _______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
