Skip to content

Commit 461706e

Browse files
committed
Reapply "[lldb] Remove UnwindPlan::Row shared_ptrs (llvm#132370)"
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.
1 parent 1de7588 commit 461706e

File tree

2 files changed

+21
-38
lines changed

2 files changed

+21
-38
lines changed

Diff for: lldb/include/lldb/Symbol/UnwindPlan.h

+5-22
Original file line numberDiff line numberDiff line change
@@ -428,34 +428,17 @@ class UnwindPlan {
428428
bool m_unspecified_registers_are_undefined = false;
429429
}; // class Row
430430

431-
typedef std::shared_ptr<Row> RowSP;
432-
433431
UnwindPlan(lldb::RegisterKind reg_kind)
434432
: m_register_kind(reg_kind), m_return_addr_register(LLDB_INVALID_REGNUM),
435433
m_plan_is_sourced_from_compiler(eLazyBoolCalculate),
436434
m_plan_is_valid_at_all_instruction_locations(eLazyBoolCalculate),
437435
m_plan_is_for_signal_trap(eLazyBoolCalculate) {}
438436

439437
// Performs a deep copy of the plan, including all the rows (expensive).
440-
UnwindPlan(const UnwindPlan &rhs)
441-
: m_plan_valid_ranges(rhs.m_plan_valid_ranges),
442-
m_register_kind(rhs.m_register_kind),
443-
m_return_addr_register(rhs.m_return_addr_register),
444-
m_source_name(rhs.m_source_name),
445-
m_plan_is_sourced_from_compiler(rhs.m_plan_is_sourced_from_compiler),
446-
m_plan_is_valid_at_all_instruction_locations(
447-
rhs.m_plan_is_valid_at_all_instruction_locations),
448-
m_plan_is_for_signal_trap(rhs.m_plan_is_for_signal_trap),
449-
m_lsda_address(rhs.m_lsda_address),
450-
m_personality_func_addr(rhs.m_personality_func_addr) {
451-
m_row_list.reserve(rhs.m_row_list.size());
452-
for (const RowSP &row_sp : rhs.m_row_list)
453-
m_row_list.emplace_back(new Row(*row_sp));
454-
}
438+
UnwindPlan(const UnwindPlan &rhs) = default;
439+
UnwindPlan &operator=(const UnwindPlan &rhs) = default;
440+
455441
UnwindPlan(UnwindPlan &&rhs) = default;
456-
UnwindPlan &operator=(const UnwindPlan &rhs) {
457-
return *this = UnwindPlan(rhs); // NB: moving from a temporary (deep) copy
458-
}
459442
UnwindPlan &operator=(UnwindPlan &&) = default;
460443

461444
~UnwindPlan() = default;
@@ -487,7 +470,7 @@ class UnwindPlan {
487470
uint32_t GetInitialCFARegister() const {
488471
if (m_row_list.empty())
489472
return LLDB_INVALID_REGNUM;
490-
return m_row_list.front()->GetCFAValue().GetRegisterNumber();
473+
return m_row_list.front().GetCFAValue().GetRegisterNumber();
491474
}
492475

493476
// This UnwindPlan may not be valid at every address of the function span.
@@ -570,7 +553,7 @@ class UnwindPlan {
570553
}
571554

572555
private:
573-
std::vector<RowSP> m_row_list;
556+
std::vector<Row> m_row_list;
574557
std::vector<AddressRange> m_plan_valid_ranges;
575558
lldb::RegisterKind m_register_kind; // The RegisterKind these register numbers
576559
// are in terms of - will need to be

Diff for: lldb/source/Symbol/UnwindPlan.cpp

+16-16
Original file line numberDiff line numberDiff line change
@@ -391,29 +391,29 @@ bool UnwindPlan::Row::operator==(const UnwindPlan::Row &rhs) const {
391391
}
392392

393393
void UnwindPlan::AppendRow(Row row) {
394-
if (m_row_list.empty() || m_row_list.back()->GetOffset() != row.GetOffset())
395-
m_row_list.push_back(std::make_shared<Row>(std::move(row)));
394+
if (m_row_list.empty() || m_row_list.back().GetOffset() != row.GetOffset())
395+
m_row_list.push_back(std::move(row));
396396
else
397-
*m_row_list.back() = std::move(row);
397+
m_row_list.back() = std::move(row);
398398
}
399399

400400
struct RowLess {
401-
bool operator()(addr_t a, const UnwindPlan::RowSP &b) const {
402-
return a < b->GetOffset();
401+
bool operator()(addr_t a, const UnwindPlan::Row &b) const {
402+
return a < b.GetOffset();
403403
}
404-
bool operator()(const UnwindPlan::RowSP &a, addr_t b) const {
405-
return a->GetOffset() < b;
404+
bool operator()(const UnwindPlan::Row &a, addr_t b) const {
405+
return a.GetOffset() < b;
406406
}
407407
};
408408

409409
void UnwindPlan::InsertRow(Row row, bool replace_existing) {
410410
auto it = llvm::lower_bound(m_row_list, row.GetOffset(), RowLess());
411-
if (it == m_row_list.end() || it->get()->GetOffset() > row.GetOffset())
412-
m_row_list.insert(it, std::make_shared<Row>(std::move(row)));
411+
if (it == m_row_list.end() || it->GetOffset() > row.GetOffset())
412+
m_row_list.insert(it, std::move(row));
413413
else {
414-
assert(it->get()->GetOffset() == row.GetOffset());
414+
assert(it->GetOffset() == row.GetOffset());
415415
if (replace_existing)
416-
**it = std::move(row);
416+
*it = std::move(row);
417417
}
418418
}
419419

@@ -425,7 +425,7 @@ UnwindPlan::GetRowForFunctionOffset(std::optional<int> offset) const {
425425
return nullptr;
426426
// upper_bound returns the row strictly greater than our desired offset, which
427427
// means that the row before it is a match.
428-
return std::prev(it)->get();
428+
return &*std::prev(it);
429429
}
430430

431431
bool UnwindPlan::IsValidRowIndex(uint32_t idx) const {
@@ -434,7 +434,7 @@ bool UnwindPlan::IsValidRowIndex(uint32_t idx) const {
434434

435435
const UnwindPlan::Row *UnwindPlan::GetRowAtIndex(uint32_t idx) const {
436436
if (idx < m_row_list.size())
437-
return m_row_list[idx].get();
437+
return &m_row_list[idx];
438438
LLDB_LOG(GetLog(LLDBLog::Unwind),
439439
"error: UnwindPlan::GetRowAtIndex(idx = {0}) invalid index "
440440
"(number rows is {1})",
@@ -448,7 +448,7 @@ const UnwindPlan::Row *UnwindPlan::GetLastRow() const {
448448
"UnwindPlan::GetLastRow() when rows are empty");
449449
return nullptr;
450450
}
451-
return m_row_list.back().get();
451+
return &m_row_list.back();
452452
}
453453

454454
bool UnwindPlan::PlanValidAtAddress(Address addr) const {
@@ -567,9 +567,9 @@ void UnwindPlan::Dump(Stream &s, Thread *thread, lldb::addr_t base_addr) const {
567567
range.Dump(&s, target_sp.get(), Address::DumpStyleSectionNameOffset);
568568
s.EOL();
569569
}
570-
for (const auto &[index, row_sp] : llvm::enumerate(m_row_list)) {
570+
for (const auto &[index, row] : llvm::enumerate(m_row_list)) {
571571
s.Format("row[{0}]: ", index);
572-
row_sp->Dump(s, this, thread, base_addr);
572+
row.Dump(s, this, thread, base_addr);
573573
s << "\n";
574574
}
575575
}

0 commit comments

Comments
 (0)