Simplify FuncUnwinders::GetEHFrameAugmentedUnwindPlan

[labath]

Summary

GetEHFrameAugmentedUnwindPlan duplicated the work of GetEHFrameUnwindPlan in getting the original
plan from DWARF CFI. This changes the function to call GetEHFrameUnwindPlan instead of doing all
the work itself.

[clayborg]

I am ok as long as Jason is ok with this. Deferring to Jason.

[jasonmolenda]

My only concern about the patch is the UnwindPlan copy constructor which is the default cc that the compiler generates. I think that's why I duplicated the code originally -- I wasn't sure if it was safe.

The UnwindPlan has a vector<> of Row shared pointers (std::Vector<RowSP>) - won't both copies of the UnwindPlan be pointing to the same vector of rows, and then when we add the epilogue Rows to the m_unwind_plan_eh_frame_augmented_sp unwind plan, m_unwind_plan_eh_frame_sp will have its rows updated as well?

The right answer here is probably to make the copy constructor do a deep copy of the object. Or am I misremembering how this works?

[labath]

You are correct. I have seen copies being made this way, but I guess that was of individual rows and not whole plans... I'll fix this and get back.

[labath]

Add deep copy UnwindPlan constructor.

So, the case you mentioned (adding Rows to the copied UnwindPlan) would actually work fine, as
the copies shared the individual rows, not their vector container. The problematic case would be
if someone went in and changed one of the rows. Then this change would unexpectedly propagate to
the other plan. However, I have checked the all the users of unwind plans and none of them does
this (and I don't see a reason why they should). So, another way of avoiding the deep copy would
be to make the UnwindPlan contain a std::vector<std::shared_ptr<const Row>>. Then the Rows could
no longer be modified after creation and you could share them among unwind plans freely.

What do you think?

[jasonmolenda]

This looks good to me, thanks for adding the deep copy constructor, this will make the class easier for people to use in the future without introducing bugs by accident.

lldb/trunk/include/lldb/Symbol/UnwindPlan.h

UnwindPlan (lldb::RegisterKind reg_kind) :
         m_source_name (),
         m_plan_is_sourced_from_compiler (eLazyBoolCalculate),
         m_plan_is_valid_at_all_instruction_locations (eLazyBoolCalculate),
         m_lsda_address (),
         m_personality_func_addr ()
     {
     }
 
+    // Performs a deep copy of the plan, including all the rows (expensive).
+    UnwindPlan (const UnwindPlan &rhs) :
+        m_plan_valid_address_range (rhs.m_plan_valid_address_range),
+        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_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 ()
   {
   }
 
     void 
     Dump (Stream& s, Thread* thread, lldb::addr_t base_addr) const;
 
     void 

lldb/trunk/source/Symbol/FuncUnwinders.cpp

FuncUnwinders::GetEHFrameAugmentedUnwindPlan (Target &target, Thread &thread, int current_offset)
     {
             m_tried_unwind_plan_eh_frame_augmented = true;
             return m_unwind_plan_eh_frame_augmented_sp;
     }
 
     Mutex::Locker lock (m_mutex);
     m_tried_unwind_plan_eh_frame_augmented = true;
 
-    if (m_range.GetBaseAddress().IsValid())
-    {
-        Address current_pc (m_range.GetBaseAddress ());
-        if (current_offset != -1)
-            current_pc.SetOffset (current_pc.GetOffset() + current_offset);
-        DWARFCallFrameInfo *eh_frame = m_unwind_table.GetEHFrameInfo();
-        if (eh_frame)
-        {
-            m_unwind_plan_eh_frame_augmented_sp.reset (new UnwindPlan (lldb::eRegisterKindGeneric));
-            if (!eh_frame->GetUnwindPlan (current_pc, *m_unwind_plan_eh_frame_augmented_sp))
-            {
-                m_unwind_plan_eh_frame_augmented_sp.reset();
-            }
-            else
-            {
-                // Augment the eh_frame instructions with epilogue descriptions if necessary so the
-                // UnwindPlan can be used at any instruction in the function.
+    UnwindPlanSP eh_frame_plan = GetEHFrameUnwindPlan (target, current_offset);
+    if (!eh_frame_plan)
+        return m_unwind_plan_eh_frame_augmented_sp;
+
+    m_unwind_plan_eh_frame_augmented_sp.reset(new UnwindPlan(*eh_frame_plan));
+
 
-                UnwindAssemblySP assembly_profiler_sp (GetUnwindAssemblyProfiler());
-                if (assembly_profiler_sp)
-                {
-                    if (!assembly_profiler_sp->AugmentUnwindPlanFromCallSite (m_range, thread, *m_unwind_plan_eh_frame_augmented_sp))
-                    {
-                        m_unwind_plan_eh_frame_augmented_sp.reset();
-                    }
-                }
-                else
-                {
-                    m_unwind_plan_eh_frame_augmented_sp.reset();
-                }
-            }
-        }
-    }
     return m_unwind_plan_eh_frame_augmented_sp;
 }
 
 
 UnwindPlanSP
 FuncUnwinders::GetAssemblyUnwindPlan (Target &target, Thread &thread, int current_offset)
 {
     if (m_unwind_plan_assembly_sp.get() || m_tried_unwind_plan_assembly)