llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) <details> <summary>Changes</summary> .. which prepares us for handling of discontinuous functions. The main change there is that we can have multiple FDEs contributing towards an unwind plan of a single function. This patch separates the logic for parsing of a single FDE from the construction of an UnwindPlan (so that another patch can change the latter to combine several unwind plans). Note to reviewers: This patch consists of three commits. The first two correspond to #<!-- -->134630 and #<!-- -->134662. For reviewing, I recommend looking at the last commit in isolation. --- Patch is 22.70 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/134806.diff 9 Files Affected: - (modified) lldb/include/lldb/Symbol/DWARFCallFrameInfo.h (+8-2) - (modified) lldb/include/lldb/Symbol/FuncUnwinders.h (-13) - (modified) lldb/include/lldb/Symbol/UnwindPlan.h (+6-27) - (modified) lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp (+3-2) - (modified) lldb/source/Symbol/CompactUnwindInfo.cpp (-12) - (modified) lldb/source/Symbol/DWARFCallFrameInfo.cpp (+49-76) - (modified) lldb/source/Symbol/FuncUnwinders.cpp (-36) - (modified) lldb/source/Symbol/UnwindPlan.cpp (+3-16) - (modified) lldb/unittests/Symbol/UnwindPlanTest.cpp (+5) ``````````diff diff --git a/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h b/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h index 6cc24a02de257..526f322a770e4 100644 --- a/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h +++ b/lldb/include/lldb/Symbol/DWARFCallFrameInfo.h @@ -128,8 +128,14 @@ class DWARFCallFrameInfo { void GetFDEIndex(); - bool FDEToUnwindPlan(dw_offset_t offset, Address startaddr, - UnwindPlan &unwind_plan); + // Parsed representation of a Frame Descriptor Entry. + struct FDE { + AddressRange range; + bool for_signal_trap = false; + uint32_t return_addr_reg_num = LLDB_INVALID_REGNUM; + std::vector<UnwindPlan::Row> rows; + }; + std::optional<FDE> ParseFDE(dw_offset_t offset, const Address &startaddr); const CIE *GetCIE(dw_offset_t cie_offset); diff --git a/lldb/include/lldb/Symbol/FuncUnwinders.h b/lldb/include/lldb/Symbol/FuncUnwinders.h index 479ccf87b6e2c..c21a1af5c56a2 100644 --- a/lldb/include/lldb/Symbol/FuncUnwinders.h +++ b/lldb/include/lldb/Symbol/FuncUnwinders.h @@ -61,19 +61,6 @@ class FuncUnwinders { }); } - // A function may have a Language Specific Data Area specified -- a block of - // data in - // the object file which is used in the processing of an exception throw / - // catch. If any of the UnwindPlans have the address of the LSDA region for - // this function, this will return it. - Address GetLSDAAddress(Target &target); - - // A function may have a Personality Routine associated with it -- used in the - // processing of throwing an exception. If any of the UnwindPlans have the - // address of the personality routine, this will return it. Read the target- - // pointer at this address to get the personality function address. - Address GetPersonalityRoutinePtrAddress(Target &target); - // The following methods to retrieve specific unwind plans should rarely be // used. Instead, clients should ask for the *behavior* they are looking for, // using one of the above UnwindPlan retrieval methods. diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h b/lldb/include/lldb/Symbol/UnwindPlan.h index 6640a23a3e868..cd5451fed6c0c 100644 --- a/lldb/include/lldb/Symbol/UnwindPlan.h +++ b/lldb/include/lldb/Symbol/UnwindPlan.h @@ -356,11 +356,11 @@ class UnwindPlan { void RemoveRegisterInfo(uint32_t reg_num); - lldb::addr_t GetOffset() const { return m_offset; } + int64_t GetOffset() const { return m_offset; } - void SetOffset(lldb::addr_t offset) { m_offset = offset; } + void SetOffset(int64_t offset) { m_offset = offset; } - void SlideOffset(lldb::addr_t offset) { m_offset += offset; } + void SlideOffset(int64_t offset) { m_offset += offset; } const FAValue &GetCFAValue() const { return m_cfa_value; } FAValue &GetCFAValue() { return m_cfa_value; } @@ -420,7 +420,7 @@ class UnwindPlan { protected: typedef std::map<uint32_t, AbstractRegisterLocation> collection; - lldb::addr_t m_offset = 0; // Offset into the function for this row + int64_t m_offset = 0; // Offset into the function for this row FAValue m_cfa_value; FAValue m_afa_value; @@ -445,9 +445,7 @@ class UnwindPlan { m_plan_is_sourced_from_compiler(rhs.m_plan_is_sourced_from_compiler), m_plan_is_valid_at_all_instruction_locations( rhs.m_plan_is_valid_at_all_instruction_locations), - m_plan_is_for_signal_trap(rhs.m_plan_is_for_signal_trap), - m_lsda_address(rhs.m_lsda_address), - m_personality_func_addr(rhs.m_personality_func_addr) { + m_plan_is_for_signal_trap(rhs.m_plan_is_for_signal_trap) { m_row_list.reserve(rhs.m_row_list.size()); for (const RowSP &row_sp : rhs.m_row_list) m_row_list.emplace_back(new Row(*row_sp)); @@ -472,7 +470,7 @@ class UnwindPlan { // practice, the UnwindPlan for a function with no known start address will be // the architectural default UnwindPlan which will only have one row. const UnwindPlan::Row * - GetRowForFunctionOffset(std::optional<int> offset) const; + GetRowForFunctionOffset(std::optional<int64_t> offset) const; lldb::RegisterKind GetRegisterKind() const { return m_register_kind; } @@ -553,22 +551,10 @@ class UnwindPlan { m_plan_is_sourced_from_compiler = eLazyBoolCalculate; m_plan_is_valid_at_all_instruction_locations = eLazyBoolCalculate; m_plan_is_for_signal_trap = eLazyBoolCalculate; - m_lsda_address.Clear(); - m_personality_func_addr.Clear(); } const RegisterInfo *GetRegisterInfo(Thread *thread, uint32_t reg_num) const; - Address GetLSDAAddress() const { return m_lsda_address; } - - void SetLSDAAddress(Address lsda_addr) { m_lsda_address = lsda_addr; } - - Address GetPersonalityFunctionPtr() const { return m_personality_func_addr; } - - void SetPersonalityFunctionPtr(Address presonality_func_ptr) { - m_personality_func_addr = presonality_func_ptr; - } - private: std::vector<RowSP> m_row_list; std::vector<AddressRange> m_plan_valid_ranges; @@ -583,13 +569,6 @@ class UnwindPlan { lldb_private::LazyBool m_plan_is_sourced_from_compiler; lldb_private::LazyBool m_plan_is_valid_at_all_instruction_locations; lldb_private::LazyBool m_plan_is_for_signal_trap; - - Address m_lsda_address; // Where the language specific data area exists in the - // module - used - // in exception handling. - Address m_personality_func_addr; // The address of a pointer to the - // personality function - used in - // exception handling. }; // class UnwindPlan } // namespace lldb_private diff --git a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp index 3eaa2f33fce3e..aaff278ca31e2 100644 --- a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp +++ b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp @@ -1390,11 +1390,12 @@ bool x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite( // If we already have one row for this instruction, we can continue. while (row_id < unwind_plan.GetRowCount() && - unwind_plan.GetRowAtIndex(row_id)->GetOffset() <= offset) { + unwind_plan.GetRowAtIndex(row_id)->GetOffset() <= + static_cast<int64_t>(offset)) { row_id++; } const UnwindPlan::Row *original_row = unwind_plan.GetRowAtIndex(row_id - 1); - if (original_row->GetOffset() == offset) { + if (original_row->GetOffset() == static_cast<int64_t>(offset)) { row = *original_row; continue; } diff --git a/lldb/source/Symbol/CompactUnwindInfo.cpp b/lldb/source/Symbol/CompactUnwindInfo.cpp index 3c97d2ca11fbc..cdbbeb554c688 100644 --- a/lldb/source/Symbol/CompactUnwindInfo.cpp +++ b/lldb/source/Symbol/CompactUnwindInfo.cpp @@ -741,9 +741,6 @@ bool CompactUnwindInfo::CreateUnwindPlan_x86_64(Target &target, unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo); unwind_plan.SetRegisterKind(eRegisterKindEHFrame); - unwind_plan.SetLSDAAddress(function_info.lsda_address); - unwind_plan.SetPersonalityFunctionPtr(function_info.personality_ptr_address); - UnwindPlan::Row row; const int wordsize = 8; @@ -1011,9 +1008,6 @@ bool CompactUnwindInfo::CreateUnwindPlan_i386(Target &target, unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo); unwind_plan.SetRegisterKind(eRegisterKindEHFrame); - unwind_plan.SetLSDAAddress(function_info.lsda_address); - unwind_plan.SetPersonalityFunctionPtr(function_info.personality_ptr_address); - UnwindPlan::Row row; const int wordsize = 4; @@ -1306,9 +1300,6 @@ bool CompactUnwindInfo::CreateUnwindPlan_arm64(Target &target, unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo); unwind_plan.SetRegisterKind(eRegisterKindEHFrame); - unwind_plan.SetLSDAAddress(function_info.lsda_address); - unwind_plan.SetPersonalityFunctionPtr(function_info.personality_ptr_address); - UnwindPlan::Row row; const int wordsize = 8; @@ -1437,9 +1428,6 @@ bool CompactUnwindInfo::CreateUnwindPlan_armv7(Target &target, unwind_plan.SetUnwindPlanForSignalTrap(eLazyBoolNo); unwind_plan.SetRegisterKind(eRegisterKindEHFrame); - unwind_plan.SetLSDAAddress(function_info.lsda_address); - unwind_plan.SetPersonalityFunctionPtr(function_info.personality_ptr_address); - UnwindPlan::Row row; const int wordsize = 4; diff --git a/lldb/source/Symbol/DWARFCallFrameInfo.cpp b/lldb/source/Symbol/DWARFCallFrameInfo.cpp index 957818e8d077f..a508adc82e167 100644 --- a/lldb/source/Symbol/DWARFCallFrameInfo.cpp +++ b/lldb/source/Symbol/DWARFCallFrameInfo.cpp @@ -168,9 +168,32 @@ bool DWARFCallFrameInfo::GetUnwindPlan(const AddressRange &range, module_sp->GetObjectFile() != &m_objfile) return false; - if (std::optional<FDEEntryMap::Entry> entry = GetFirstFDEEntryInRange(range)) - return FDEToUnwindPlan(entry->data, addr, unwind_plan); - return false; + std::optional<FDEEntryMap::Entry> entry = GetFirstFDEEntryInRange(range); + if (!entry) + return false; + + std::optional<FDE> fde = ParseFDE(entry->data, addr); + if (!fde) + return false; + + unwind_plan.SetSourceName(m_type == EH ? "eh_frame CFI" : "DWARF CFI"); + // In theory the debug_frame info should be valid at all call sites + // ("asynchronous unwind info" as it is sometimes called) but in practice + // gcc et al all emit call frame info for the prologue and call sites, but + // not for the epilogue or all the other locations during the function + // reliably. + unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo); + unwind_plan.SetSourcedFromCompiler(eLazyBoolYes); + unwind_plan.SetRegisterKind(GetRegisterKind()); + + unwind_plan.SetPlanValidAddressRanges({fde->range}); + unwind_plan.SetUnwindPlanForSignalTrap( + fde->for_signal_trap ? eLazyBoolYes : eLazyBoolNo); + unwind_plan.SetReturnAddressRegister(fde->return_addr_reg_num); + for (UnwindPlan::Row &row : fde->rows) + unwind_plan.AppendRow(std::move(row)); + + return true; } bool DWARFCallFrameInfo::GetAddressRange(Address addr, AddressRange &range) { @@ -522,15 +545,15 @@ void DWARFCallFrameInfo::GetFDEIndex() { m_fde_index_initialized = true; } -bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, - Address startaddr, - UnwindPlan &unwind_plan) { +std::optional<DWARFCallFrameInfo::FDE> +DWARFCallFrameInfo::ParseFDE(dw_offset_t dwarf_offset, + const Address &startaddr) { Log *log = GetLog(LLDBLog::Unwind); lldb::offset_t offset = dwarf_offset; lldb::offset_t current_entry = offset; - if (m_section_sp.get() == nullptr || m_section_sp->IsEncrypted()) - return false; + if (!m_section_sp || m_section_sp->IsEncrypted()) + return std::nullopt; if (!m_cfi_data_initialized) GetCFIData(); @@ -550,20 +573,8 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // Translate the CIE_id from the eh_frame format, which is relative to the // FDE offset, into a __eh_frame section offset - if (m_type == EH) { - unwind_plan.SetSourceName("eh_frame CFI"); + if (m_type == EH) cie_offset = current_entry + (is_64bit ? 12 : 4) - cie_offset; - unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo); - } else { - unwind_plan.SetSourceName("DWARF CFI"); - // In theory the debug_frame info should be valid at all call sites - // ("asynchronous unwind info" as it is sometimes called) but in practice - // gcc et al all emit call frame info for the prologue and call sites, but - // not for the epilogue or all the other locations during the function - // reliably. - unwind_plan.SetUnwindPlanValidAtAllInstructions(eLazyBoolNo); - } - unwind_plan.SetSourcedFromCompiler(eLazyBoolYes); const CIE *cie = GetCIE(cie_offset); assert(cie != nullptr); @@ -583,52 +594,19 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, m_objfile.GetSectionList()); range.SetByteSize(range_len); - addr_t lsda_data_file_address = LLDB_INVALID_ADDRESS; - - if (cie->augmentation[0] == 'z') { - uint32_t aug_data_len = (uint32_t)m_cfi_data.GetULEB128(&offset); - if (aug_data_len != 0 && cie->lsda_addr_encoding != DW_EH_PE_omit) { - lldb::offset_t saved_offset = offset; - lsda_data_file_address = - GetGNUEHPointer(m_cfi_data, &offset, cie->lsda_addr_encoding, - pc_rel_addr, text_addr, data_addr); - if (offset - saved_offset != aug_data_len) { - // There is more in the augmentation region than we know how to process; - // don't read anything. - lsda_data_file_address = LLDB_INVALID_ADDRESS; - } - offset = saved_offset; - } - offset += aug_data_len; - } - unwind_plan.SetUnwindPlanForSignalTrap( - strchr(cie->augmentation, 'S') ? eLazyBoolYes : eLazyBoolNo); - - Address lsda_data; - Address personality_function_ptr; + // Skip the LSDA, if present. + if (cie->augmentation[0] == 'z') + offset += (uint32_t)m_cfi_data.GetULEB128(&offset); - if (lsda_data_file_address != LLDB_INVALID_ADDRESS && - cie->personality_loc != LLDB_INVALID_ADDRESS) { - m_objfile.GetModule()->ResolveFileAddress(lsda_data_file_address, - lsda_data); - m_objfile.GetModule()->ResolveFileAddress(cie->personality_loc, - personality_function_ptr); - } - - if (lsda_data.IsValid() && personality_function_ptr.IsValid()) { - unwind_plan.SetLSDAAddress(lsda_data); - unwind_plan.SetPersonalityFunctionPtr(personality_function_ptr); - } + FDE fde; + fde.for_signal_trap = strchr(cie->augmentation, 'S') != nullptr; + fde.range = range; + fde.return_addr_reg_num = cie->return_addr_reg_num; uint32_t code_align = cie->code_align; int32_t data_align = cie->data_align; - unwind_plan.SetPlanValidAddressRanges({range}); UnwindPlan::Row row = cie->initial_row; - - unwind_plan.SetRegisterKind(GetRegisterKind()); - unwind_plan.SetReturnAddressRegister(cie->return_addr_reg_num); - std::vector<UnwindPlan::Row> stack; UnwindPlan::Row::AbstractRegisterLocation reg_location; @@ -648,7 +626,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // that is computed by taking the current entry's location value and // adding (delta * code_align). All other values in the new row are // initially identical to the current row. - unwind_plan.AppendRow(row); + fde.rows.push_back(row); row.SlideOffset(extended_opcode * code_align); break; } @@ -664,9 +642,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // state, so we need to convert our eh_frame register number from the // EH frame info, to a register index - if (unwind_plan.IsValidRowIndex(0) && - unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num, - reg_location)) + if (fde.rows[0].GetRegisterInfo(reg_num, reg_location)) row.SetRegisterInfo(reg_num, reg_location); else { // If the register was not set in the first row, remove the @@ -685,7 +661,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // specified address as the location. All other values in the new row // are initially identical to the current row. The new location value // should always be greater than the current one. - unwind_plan.AppendRow(row); + fde.rows.push_back(row); row.SetOffset(m_cfi_data.GetAddress(&offset) - startaddr.GetFileAddress()); break; @@ -696,7 +672,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // takes a single uword argument that represents a constant delta. // This instruction is identical to DW_CFA_advance_loc except for the // encoding and size of the delta argument. - unwind_plan.AppendRow(row); + fde.rows.push_back(row); row.SlideOffset(m_cfi_data.GetU8(&offset) * code_align); break; } @@ -706,7 +682,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // takes a single uword argument that represents a constant delta. // This instruction is identical to DW_CFA_advance_loc except for the // encoding and size of the delta argument. - unwind_plan.AppendRow(row); + fde.rows.push_back(row); row.SlideOffset(m_cfi_data.GetU16(&offset) * code_align); break; } @@ -716,7 +692,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // takes a single uword argument that represents a constant delta. // This instruction is identical to DW_CFA_advance_loc except for the // encoding and size of the delta argument. - unwind_plan.AppendRow(row); + fde.rows.push_back(row); row.SlideOffset(m_cfi_data.GetU32(&offset) * code_align); break; } @@ -727,9 +703,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, // number. This instruction is identical to DW_CFA_restore except for // the encoding and size of the register argument. uint32_t reg_num = (uint32_t)m_cfi_data.GetULEB128(&offset); - if (unwind_plan.IsValidRowIndex(0) && - unwind_plan.GetRowAtIndex(0)->GetRegisterInfo(reg_num, - reg_location)) + if (fde.rows[0].GetRegisterInfo(reg_num, reg_location)) row.SetRegisterInfo(reg_num, reg_location); break; } @@ -765,7 +739,7 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, __FUNCTION__, dwarf_offset, startaddr.GetFileAddress()); break; } - lldb::addr_t offset = row.GetOffset(); + int64_t offset = row.GetOffset(); row = std::move(stack.back()); stack.pop_back(); row.SetOffset(offset); @@ -792,9 +766,8 @@ bool DWARFCallFrameInfo::FDEToUnwindPlan(dw_offset_t dwarf_offset, } } } - unwind_plan.AppendRow(row); - - return true; + fde.rows.push_back(row); + return fde; } bool DWARFCallFrameInfo::HandleCommonDwarfOpcode(uint8_t primary_opcode, diff --git a/lldb/source/Symbol/FuncUnwinders.cpp b/lldb/source/Symbol/FuncUnwinders.cpp index a74029d8343c7..11600825e8e38 100644 --- a/lldb/source/Symbol/FuncUnwinders.cpp +++ b/lldb/source/Symbol/FuncUnwinders.cpp @@ -537,39 +537,3 @@ FuncUnwinders::GetUnwindAssemblyProfiler(Target &target) { } return assembly_profiler_sp; } - -Address FuncUnwinders::GetLSDAAddress(Target &target) { - Address lsda_addr; - - std::shared_ptr<const UnwindPlan> unwind_plan_sp = - GetEHFrameUnwindPlan(target); - if (unwind_plan_sp.get() == nullptr) { - unwind_plan_sp = GetCompactUnwindUnwindPlan(target); - } - if (unwind_plan_sp.get() == nullptr) { - unwind_plan_sp = GetObjectFileUnwindPlan(target); - } - if (unwind_plan_sp.get() && unwind_plan_sp->GetLSDAAddress().IsValid()) { - lsda_addr = unwind_plan_sp->GetLSDAAddress(); - } - return lsda_addr; -} - -Address FuncUnwinders::GetPersonalityRoutinePtrAddress(Target &target) { - Address personality_addr; - - std::shared_ptr<const UnwindPlan> unwind_plan_sp = - GetEHFrameUnwindPlan(target); - if (unwind_plan_sp.get() == nullptr) { - unwind_plan_sp = GetCompactUnwindUnwindPlan(target); - } - if (unwind_plan_sp.get() == nullptr) { - unwind_plan_sp = GetObjectFileUnwindPlan(target); - } - if (unwind_plan_sp.get() && - unwind_plan_sp->GetPersonalityFunctionPtr().IsValid()) { - personality_addr = unwind_plan_sp->GetPersonalityFunctionPtr(); - } - - return personality_addr; -} diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp index cfa8eefaa55bb..137466025c982 100644 --- a/lldb/source/Symbol/UnwindPlan.cpp +++ b/lldb/source... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/134806 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
