[Lldb-commits] [lldb] e348173 - Reapply "[lldb] Remove UnwindPlan::Row shared_ptrs" (#134821)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Apr 8 23:22:19 PDT 2025
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";
}
}
More information about the lldb-commits
mailing list