Reapply "[lldb] Remove UnwindPlan::Row shared_ptrs" (#134821)

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.

Author: labath

lldb/include/lldb/Symbol/UnwindPlan.h

@@ -428,34 +428,17 @@ 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),
         m_plan_is_valid_at_all_instruction_locations(eLazyBoolCalculate),
         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

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;

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";
   }
 }