Reapply "[lldb] Remove UnwindPlan::Row shared_ptrs (https://github.com/llvm/llvm-project/pull/132370)" #134821
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesThis reverts commit 48864a5, reapplying Full diff: https://github.com/llvm/llvm-project/pull/134821.diff 3 Files Affected:
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";
}
}
|
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
This reverts commit llvm@48864a5, reapplying llvm@d7cea2b. It also fixes the dangling pointers caused by the previous version by creating copies of the Rows in x86AssemblyInspectionEngine.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 48864a5, reapplying
d7cea2b. It also fixes the dangling
pointers caused by the previous version by creating copies of the Rows
in x86AssemblyInspectionEngine.