Author: Pavel Labath Date: 2025-04-09T08:22:15+02:00 New Revision: e348173bef7182c7cc609d0f000452d3acf4b65c
URL: https://github.com/llvm/llvm-project/commit/e348173bef7182c7cc609d0f000452d3acf4b65c DIFF: https://github.com/llvm/llvm-project/commit/e348173bef7182c7cc609d0f000452d3acf4b65c.diff LOG: Reapply "[lldb] Remove UnwindPlan::Row shared_ptrs" (#134821) This reverts commit https://github.com/llvm/llvm-project/commit/48864a52ef547ac0477271127b510dd9e9798219, reapplying https://github.com/llvm/llvm-project/commit/d7cea2b18717f0cc31b7da4a03f772d89ee201db. It also fixes the dangling pointers caused by the previous version by creating copies of the Rows in x86AssemblyInspectionEngine. Added: Modified: lldb/include/lldb/Symbol/UnwindPlan.h lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp lldb/source/Symbol/UnwindPlan.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h b/lldb/include/lldb/Symbol/UnwindPlan.h index 6640a23a3e868..0feb20b12e184 100644 --- a/lldb/include/lldb/Symbol/UnwindPlan.h +++ b/lldb/include/lldb/Symbol/UnwindPlan.h @@ -428,8 +428,6 @@ class UnwindPlan { bool m_unspecified_registers_are_undefined = false; }; // class Row - typedef std::shared_ptr<Row> RowSP; - UnwindPlan(lldb::RegisterKind reg_kind) : m_register_kind(reg_kind), m_return_addr_register(LLDB_INVALID_REGNUM), m_plan_is_sourced_from_compiler(eLazyBoolCalculate), @@ -437,25 +435,10 @@ class UnwindPlan { m_plan_is_for_signal_trap(eLazyBoolCalculate) {} // Performs a deep copy of the plan, including all the rows (expensive). - UnwindPlan(const UnwindPlan &rhs) - : m_plan_valid_ranges(rhs.m_plan_valid_ranges), - m_register_kind(rhs.m_register_kind), - m_return_addr_register(rhs.m_return_addr_register), - m_source_name(rhs.m_source_name), - 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_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)); - } + UnwindPlan(const UnwindPlan &rhs) = default; + UnwindPlan &operator=(const UnwindPlan &rhs) = default; + UnwindPlan(UnwindPlan &&rhs) = default; - UnwindPlan &operator=(const UnwindPlan &rhs) { - return *this = UnwindPlan(rhs); // NB: moving from a temporary (deep) copy - } UnwindPlan &operator=(UnwindPlan &&) = default; ~UnwindPlan() = default; @@ -487,7 +470,7 @@ class UnwindPlan { uint32_t GetInitialCFARegister() const { if (m_row_list.empty()) return LLDB_INVALID_REGNUM; - return m_row_list.front()->GetCFAValue().GetRegisterNumber(); + return m_row_list.front().GetCFAValue().GetRegisterNumber(); } // This UnwindPlan may not be valid at every address of the function span. @@ -570,7 +553,7 @@ class UnwindPlan { } private: - std::vector<RowSP> m_row_list; + std::vector<Row> m_row_list; std::vector<AddressRange> m_plan_valid_ranges; lldb::RegisterKind m_register_kind; // The RegisterKind these register numbers // are in terms of - will need to be diff --git a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp index 3eaa2f33fce3e..823c6505d90cf 100644 --- a/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp +++ b/lldb/source/Plugins/UnwindAssembly/x86/x86AssemblyInspectionEngine.cpp @@ -1338,25 +1338,25 @@ bool x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite( if (unwind_plan.GetRowCount() < 2) return false; - const UnwindPlan::Row *first_row = unwind_plan.GetRowAtIndex(0); - if (first_row->GetOffset() != 0) + UnwindPlan::Row first_row = *unwind_plan.GetRowAtIndex(0); + if (first_row.GetOffset() != 0) return false; - uint32_t cfa_reg = first_row->GetCFAValue().GetRegisterNumber(); + uint32_t cfa_reg = first_row.GetCFAValue().GetRegisterNumber(); if (unwind_plan.GetRegisterKind() != eRegisterKindLLDB) { cfa_reg = reg_ctx->ConvertRegisterKindToRegisterNumber( unwind_plan.GetRegisterKind(), - first_row->GetCFAValue().GetRegisterNumber()); + first_row.GetCFAValue().GetRegisterNumber()); } if (cfa_reg != m_lldb_sp_regnum || - first_row->GetCFAValue().GetOffset() != m_wordsize) + first_row.GetCFAValue().GetOffset() != m_wordsize) return false; - const UnwindPlan::Row *original_last_row = unwind_plan.GetLastRow(); + UnwindPlan::Row original_last_row = *unwind_plan.GetLastRow(); size_t offset = 0; int row_id = 1; bool unwind_plan_updated = false; - UnwindPlan::Row row = *first_row; + UnwindPlan::Row row = first_row; // After a mid-function epilogue we will need to re-insert the original // unwind rules so unwinds work for the remainder of the function. These @@ -1380,7 +1380,7 @@ bool x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite( continue; if (reinstate_unwind_state) { - row = *original_last_row; + row = original_last_row; row.SetOffset(offset); unwind_plan.AppendRow(row); reinstate_unwind_state = false; @@ -1521,7 +1521,7 @@ bool x86AssemblyInspectionEngine::AugmentUnwindPlanFromCallSite( if (ret_pattern_p()) { row.SetOffset(offset); row.GetCFAValue().SetIsRegisterPlusOffset( - first_row->GetCFAValue().GetRegisterNumber(), m_wordsize); + first_row.GetCFAValue().GetRegisterNumber(), m_wordsize); unwind_plan.InsertRow(row); unwind_plan_updated = true; diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp index cfa8eefaa55bb..48999aadab1e2 100644 --- a/lldb/source/Symbol/UnwindPlan.cpp +++ b/lldb/source/Symbol/UnwindPlan.cpp @@ -391,29 +391,29 @@ bool UnwindPlan::Row::operator==(const UnwindPlan::Row &rhs) const { } void UnwindPlan::AppendRow(Row row) { - if (m_row_list.empty() || m_row_list.back()->GetOffset() != row.GetOffset()) - m_row_list.push_back(std::make_shared<Row>(std::move(row))); + if (m_row_list.empty() || m_row_list.back().GetOffset() != row.GetOffset()) + m_row_list.push_back(std::move(row)); else - *m_row_list.back() = std::move(row); + m_row_list.back() = std::move(row); } struct RowLess { - bool operator()(addr_t a, const UnwindPlan::RowSP &b) const { - return a < b->GetOffset(); + bool operator()(addr_t a, const UnwindPlan::Row &b) const { + return a < b.GetOffset(); } - bool operator()(const UnwindPlan::RowSP &a, addr_t b) const { - return a->GetOffset() < b; + bool operator()(const UnwindPlan::Row &a, addr_t b) const { + return a.GetOffset() < b; } }; void UnwindPlan::InsertRow(Row row, bool replace_existing) { auto it = llvm::lower_bound(m_row_list, row.GetOffset(), RowLess()); - if (it == m_row_list.end() || it->get()->GetOffset() > row.GetOffset()) - m_row_list.insert(it, std::make_shared<Row>(std::move(row))); + if (it == m_row_list.end() || it->GetOffset() > row.GetOffset()) + m_row_list.insert(it, std::move(row)); else { - assert(it->get()->GetOffset() == row.GetOffset()); + assert(it->GetOffset() == row.GetOffset()); if (replace_existing) - **it = std::move(row); + *it = std::move(row); } } @@ -425,7 +425,7 @@ UnwindPlan::GetRowForFunctionOffset(std::optional<int> offset) const { return nullptr; // upper_bound returns the row strictly greater than our desired offset, which // means that the row before it is a match. - return std::prev(it)->get(); + return &*std::prev(it); } bool UnwindPlan::IsValidRowIndex(uint32_t idx) const { @@ -434,7 +434,7 @@ bool UnwindPlan::IsValidRowIndex(uint32_t idx) const { const UnwindPlan::Row *UnwindPlan::GetRowAtIndex(uint32_t idx) const { if (idx < m_row_list.size()) - return m_row_list[idx].get(); + return &m_row_list[idx]; LLDB_LOG(GetLog(LLDBLog::Unwind), "error: UnwindPlan::GetRowAtIndex(idx = {0}) invalid index " "(number rows is {1})", @@ -448,7 +448,7 @@ const UnwindPlan::Row *UnwindPlan::GetLastRow() const { "UnwindPlan::GetLastRow() when rows are empty"); return nullptr; } - return m_row_list.back().get(); + return &m_row_list.back(); } bool UnwindPlan::PlanValidAtAddress(Address addr) const { @@ -567,9 +567,9 @@ void UnwindPlan::Dump(Stream &s, Thread *thread, lldb::addr_t base_addr) const { range.Dump(&s, target_sp.get(), Address::DumpStyleSectionNameOffset); s.EOL(); } - for (const auto &[index, row_sp] : llvm::enumerate(m_row_list)) { + for (const auto &[index, row] : llvm::enumerate(m_row_list)) { s.Format("row[{0}]: ", index); - row_sp->Dump(s, this, thread, base_addr); + row.Dump(s, this, thread, base_addr); s << "\n"; } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
