[Lldb-commits] [lldb] r221238 - Back out r221229 -- instead of trying to identify the end of the unwind,
Jason Molenda
jmolenda at apple.com
Mon Nov 3 21:28:40 PST 2014
Author: jmolenda
Date: Mon Nov 3 23:28:40 2014
New Revision: 221238
URL: http://llvm.org/viewvc/llvm-project?rev=221238&view=rev
Log:
Back out r221229 -- instead of trying to identify the end of the unwind,
let's let lldb try the arch default unwind every time but not destructively --
it doesn't permanently replace the main unwind method for that function from
now on.
This fix is for <rdar://problem/18683658>.
I tested it against Ryan Brown's go program test case and also a
collection of core files of tricky unwind scenarios
<rdar://problem/15664282> <rdar://problem/15835846>
<rdar://problem/15982682> <rdar://problem/16099440>
<rdar://problem/17364005> <rdar://problem/18556719>
that I've fixed over the last 6-9 months.
Modified:
lldb/trunk/include/lldb/Symbol/FuncUnwinders.h
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.h
lldb/trunk/source/Plugins/Process/Utility/UnwindLLDB.cpp
lldb/trunk/source/Symbol/FuncUnwinders.cpp
Modified: lldb/trunk/include/lldb/Symbol/FuncUnwinders.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/FuncUnwinders.h?rev=221238&r1=221237&r2=221238&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/FuncUnwinders.h (original)
+++ lldb/trunk/include/lldb/Symbol/FuncUnwinders.h Mon Nov 3 23:28:40 2014
@@ -16,7 +16,8 @@ public:
// FuncUnwinders objects are used to track UnwindPlans for a function
// (named or not - really just an address range)
- // We'll record three different UnwindPlans for each address range:
+ // We'll record four different UnwindPlans for each address range:
+ //
// 1. Unwinding from a call site (a valid exception throw location)
// This is often sourced from the eh_frame exception handling info
// 2. Unwinding from a non-call site (any location in the function)
@@ -67,14 +68,6 @@ public:
return m_range.ContainsFileAddress (addr);
}
- // When we're doing an unwind using the UnwindPlanAtNonCallSite and we find an
- // impossible unwind condition, we know that the UnwindPlan is invalid. Calling
- // this method on the FuncUnwinder will tell it to replace that UnwindPlan with
- // the architectural default UnwindPlan so hopefully our stack walk will get past
- // this frame.
- void
- InvalidateNonCallSiteUnwindPlan (lldb_private::Thread& Thread);
-
private:
lldb::UnwindAssemblySP
@@ -90,6 +83,8 @@ private:
lldb::UnwindPlanSP m_unwind_plan_arch_default_sp;
lldb::UnwindPlanSP m_unwind_plan_arch_default_at_func_entry_sp;
+ // Fetching the UnwindPlans can be expensive - if we've already attempted
+ // to get one & failed, don't try again.
bool m_tried_unwind_at_call_site:1,
m_tried_unwind_at_non_call_site:1,
m_tried_unwind_fast:1,
Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp?rev=221238&r1=221237&r2=221238&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.cpp Mon Nov 3 23:28:40 2014
@@ -62,8 +62,7 @@ RegisterContextLLDB::RegisterContextLLDB
m_sym_ctx_valid (false),
m_frame_number (frame_number),
m_registers(),
- m_parent_unwind (unwind_lldb),
- m_completed_stack_walk (false)
+ m_parent_unwind (unwind_lldb)
{
m_sym_ctx.Clear(false);
m_sym_ctx_valid = false;
@@ -307,7 +306,6 @@ RegisterContextLLDB::InitializeNonZeroth
if (pc == 0)
{
m_frame_type = eNotAValidFrame;
- m_completed_stack_walk = true;
UnwindLogMsg ("this frame has a pc of 0x0");
return;
}
@@ -388,10 +386,6 @@ RegisterContextLLDB::InitializeNonZeroth
{
UnwindLogMsg ("could not find a valid cfa address");
m_frame_type = eNotAValidFrame;
- if (cfa_regval == 0 || cfa_regval == 1)
- {
- m_completed_stack_walk = true;
- }
return;
}
@@ -588,10 +582,6 @@ RegisterContextLLDB::InitializeNonZeroth
{
UnwindLogMsg ("could not find a valid cfa address");
m_frame_type = eNotAValidFrame;
- if (cfa_regval == 0 || cfa_regval == 1)
- {
- m_completed_stack_walk = true;
- }
return;
}
@@ -1040,11 +1030,10 @@ RegisterContextLLDB::IsValid () const
return m_frame_type != eNotAValidFrame;
}
-bool
-RegisterContextLLDB::IsCompletedStackWalk () const
-{
- return m_completed_stack_walk;
-}
+// After the final stack frame in a stack walk we'll get one invalid (eNotAValidFrame) stack frame --
+// one past the end of the stack walk. But higher-level code will need to tell the differnece between
+// "the unwind plan below this frame failed" versus "we successfully completed the stack walk" so
+// this method helps to disambiguate that.
bool
RegisterContextLLDB::IsTrapHandlerFrame () const
@@ -1420,15 +1409,6 @@ RegisterContextLLDB::TryFallbackUnwindPl
if (active_row && active_row->GetCFARegister() != LLDB_INVALID_REGNUM)
{
- FuncUnwindersSP func_unwinders_sp;
- if (m_sym_ctx_valid && m_current_pc.IsValid() && m_current_pc.GetModule())
- {
- func_unwinders_sp = m_current_pc.GetModule()->GetObjectFile()->GetUnwindTable().GetFuncUnwindersContainingAddress (m_current_pc, m_sym_ctx);
- if (func_unwinders_sp)
- {
- func_unwinders_sp->InvalidateNonCallSiteUnwindPlan (m_thread);
- }
- }
m_registers.clear();
m_full_unwind_plan_sp = m_fallback_unwind_plan_sp;
addr_t cfa_regval = LLDB_INVALID_ADDRESS;
@@ -1437,8 +1417,9 @@ RegisterContextLLDB::TryFallbackUnwindPl
m_cfa = cfa_regval + active_row->GetCFAOffset ();
}
- UnwindLogMsg ("full unwind plan '%s' has been replaced by architecture default unwind plan '%s' for this function from now on.",
- original_full_unwind_plan_sp->GetSourceName().GetCString(), m_fallback_unwind_plan_sp->GetSourceName().GetCString());
+ UnwindLogMsg ("trying to unwind from this function with the UnwindPlan '%s' because UnwindPlan '%s' failed.",
+ m_fallback_unwind_plan_sp->GetSourceName().GetCString(),
+ original_full_unwind_plan_sp->GetSourceName().GetCString());
m_fallback_unwind_plan_sp.reset();
}
Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.h?rev=221238&r1=221237&r2=221238&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.h (original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextLLDB.h Mon Nov 3 23:28:40 2014
@@ -73,9 +73,6 @@ public:
IsValid () const;
bool
- IsCompletedStackWalk () const;
-
- bool
IsTrapHandlerFrame () const;
bool
@@ -243,10 +240,6 @@ private:
lldb_private::UnwindLLDB& m_parent_unwind; // The UnwindLLDB that is creating this RegisterContextLLDB
- bool m_completed_stack_walk; // indicates that we completed a full stack walk
- // (this frame is likely eNotAValidFrame aka !IsValid())
- // and we should not continue trying to unwind
-
//------------------------------------------------------------------
// For RegisterContextLLDB only
//------------------------------------------------------------------
Modified: lldb/trunk/source/Plugins/Process/Utility/UnwindLLDB.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/UnwindLLDB.cpp?rev=221238&r1=221237&r2=221238&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Utility/UnwindLLDB.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Utility/UnwindLLDB.cpp Mon Nov 3 23:28:40 2014
@@ -174,8 +174,7 @@ UnwindLLDB::AddOneMoreFrame (ABI *abi)
{
// If the RegisterContextLLDB has a fallback UnwindPlan, it will switch to that and return
// true. Subsequent calls to TryFallbackUnwindPlan() will return false.
- if (reg_ctx_sp->IsCompletedStackWalk() == false
- && m_frames[cur_idx - 1]->reg_ctx_lldb_sp->TryFallbackUnwindPlan())
+ if (m_frames[cur_idx - 1]->reg_ctx_lldb_sp->TryFallbackUnwindPlan())
{
return AddOneMoreFrame (abi);
}
Modified: lldb/trunk/source/Symbol/FuncUnwinders.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/FuncUnwinders.cpp?rev=221238&r1=221237&r2=221238&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/FuncUnwinders.cpp (original)
+++ lldb/trunk/source/Symbol/FuncUnwinders.cpp Mon Nov 3 23:28:40 2014
@@ -25,13 +25,9 @@ using namespace lldb;
using namespace lldb_private;
-FuncUnwinders::FuncUnwinders
-(
- UnwindTable& unwind_table,
- AddressRange range
-) :
- m_unwind_table(unwind_table),
- m_range(range),
+FuncUnwinders::FuncUnwinders (UnwindTable& unwind_table, AddressRange range) :
+ m_unwind_table (unwind_table),
+ m_range (range),
m_mutex (Mutex::eMutexTypeRecursive),
m_unwind_plan_call_site_sp (),
m_unwind_plan_non_call_site_sp (),
@@ -42,29 +38,17 @@ FuncUnwinders::FuncUnwinders
m_tried_unwind_fast (false),
m_tried_unwind_arch_default (false),
m_tried_unwind_arch_default_at_func_entry (false),
- m_first_non_prologue_insn()
+ m_first_non_prologue_insn ()
{
}
-FuncUnwinders::~FuncUnwinders ()
+FuncUnwinders::~FuncUnwinders ()
{
}
UnwindPlanSP
FuncUnwinders::GetUnwindPlanAtCallSite (int current_offset)
{
- // Lock the mutex to ensure we can always give out the most appropriate
- // information. We want to make sure if someone requests a call site unwind
- // plan, that they get one and don't run into a race condition where one
- // thread has started to create the unwind plan and has put it into
- // m_unwind_plan_call_site_sp, and have another thread enter this function
- // and return the partially filled in m_unwind_plan_call_site_sp pointer.
- // We also want to make sure that we lock out other unwind plans from
- // being accessed until this one is done creating itself in case someone
- // had some code like:
- // UnwindPlan *best_unwind_plan = ...GetUnwindPlanAtCallSite (...)
- // if (best_unwind_plan == NULL)
- // best_unwind_plan = GetUnwindPlanAtNonCallSite (...)
Mutex::Locker locker (m_mutex);
if (m_tried_unwind_at_call_site == false && m_unwind_plan_call_site_sp.get() == nullptr)
{
@@ -96,18 +80,6 @@ FuncUnwinders::GetUnwindPlanAtCallSite (
UnwindPlanSP
FuncUnwinders::GetUnwindPlanAtNonCallSite (Target& target, Thread& thread, int current_offset)
{
- // Lock the mutex to ensure we can always give out the most appropriate
- // information. We want to make sure if someone requests an unwind
- // plan, that they get one and don't run into a race condition where one
- // thread has started to create the unwind plan and has put it into
- // the unique pointer member variable, and have another thread enter this function
- // and return the partially filled pointer contained in the unique pointer.
- // We also want to make sure that we lock out other unwind plans from
- // being accessed until this one is done creating itself in case someone
- // had some code like:
- // UnwindPlan *best_unwind_plan = ...GetUnwindPlanAtCallSite (...)
- // if (best_unwind_plan == NULL)
- // best_unwind_plan = GetUnwindPlanAtNonCallSite (...)
Mutex::Locker locker (m_mutex);
if (m_tried_unwind_at_non_call_site == false && m_unwind_plan_non_call_site_sp.get() == nullptr)
{
@@ -121,9 +93,11 @@ FuncUnwinders::GetUnwindPlanAtNonCallSit
// For 0th frame on i386 & x86_64, we fetch eh_frame and try using assembly profiler
// to augment it into asynchronous unwind table.
GetUnwindPlanAtCallSite(current_offset);
- if (m_unwind_plan_call_site_sp) {
+ if (m_unwind_plan_call_site_sp)
+ {
UnwindPlan* plan = new UnwindPlan (*m_unwind_plan_call_site_sp);
- if (assembly_profiler_sp->AugmentUnwindPlanFromCallSite (m_range, thread, *plan)) {
+ if (assembly_profiler_sp->AugmentUnwindPlanFromCallSite (m_range, thread, *plan))
+ {
m_unwind_plan_non_call_site_sp.reset (plan);
return m_unwind_plan_non_call_site_sp;
}
@@ -141,18 +115,6 @@ FuncUnwinders::GetUnwindPlanAtNonCallSit
UnwindPlanSP
FuncUnwinders::GetUnwindPlanFastUnwind (Thread& thread)
{
- // Lock the mutex to ensure we can always give out the most appropriate
- // information. We want to make sure if someone requests an unwind
- // plan, that they get one and don't run into a race condition where one
- // thread has started to create the unwind plan and has put it into
- // the unique pointer member variable, and have another thread enter this function
- // and return the partially filled pointer contained in the unique pointer.
- // We also want to make sure that we lock out other unwind plans from
- // being accessed until this one is done creating itself in case someone
- // had some code like:
- // UnwindPlan *best_unwind_plan = ...GetUnwindPlanAtCallSite (...)
- // if (best_unwind_plan == NULL)
- // best_unwind_plan = GetUnwindPlanAtNonCallSite (...)
Mutex::Locker locker (m_mutex);
if (m_tried_unwind_fast == false && m_unwind_plan_fast_sp.get() == nullptr)
{
@@ -171,18 +133,6 @@ FuncUnwinders::GetUnwindPlanFastUnwind (
UnwindPlanSP
FuncUnwinders::GetUnwindPlanArchitectureDefault (Thread& thread)
{
- // Lock the mutex to ensure we can always give out the most appropriate
- // information. We want to make sure if someone requests an unwind
- // plan, that they get one and don't run into a race condition where one
- // thread has started to create the unwind plan and has put it into
- // the unique pointer member variable, and have another thread enter this function
- // and return the partially filled pointer contained in the unique pointer.
- // We also want to make sure that we lock out other unwind plans from
- // being accessed until this one is done creating itself in case someone
- // had some code like:
- // UnwindPlan *best_unwind_plan = ...GetUnwindPlanAtCallSite (...)
- // if (best_unwind_plan == NULL)
- // best_unwind_plan = GetUnwindPlanAtNonCallSite (...)
Mutex::Locker locker (m_mutex);
if (m_tried_unwind_arch_default == false && m_unwind_plan_arch_default_sp.get() == nullptr)
{
@@ -207,20 +157,9 @@ FuncUnwinders::GetUnwindPlanArchitecture
UnwindPlanSP
FuncUnwinders::GetUnwindPlanArchitectureDefaultAtFunctionEntry (Thread& thread)
{
- // Lock the mutex to ensure we can always give out the most appropriate
- // information. We want to make sure if someone requests an unwind
- // plan, that they get one and don't run into a race condition where one
- // thread has started to create the unwind plan and has put it into
- // the unique pointer member variable, and have another thread enter this function
- // and return the partially filled pointer contained in the unique pointer.
- // We also want to make sure that we lock out other unwind plans from
- // being accessed until this one is done creating itself in case someone
- // had some code like:
- // UnwindPlan *best_unwind_plan = ...GetUnwindPlanAtCallSite (...)
- // if (best_unwind_plan == NULL)
- // best_unwind_plan = GetUnwindPlanAtNonCallSite (...)
Mutex::Locker locker (m_mutex);
- if (m_tried_unwind_arch_default_at_func_entry == false && m_unwind_plan_arch_default_at_func_entry_sp.get() == nullptr)
+ if (m_tried_unwind_arch_default_at_func_entry == false
+ && m_unwind_plan_arch_default_at_func_entry_sp.get() == nullptr)
{
m_tried_unwind_arch_default_at_func_entry = true;
Address current_pc;
@@ -249,7 +188,6 @@ FuncUnwinders::GetFirstNonPrologueInsn (
ExecutionContext exe_ctx (target.shared_from_this(), false);
UnwindAssemblySP assembly_profiler_sp (GetUnwindAssemblyProfiler());
if (assembly_profiler_sp)
- if (assembly_profiler_sp)
assembly_profiler_sp->FirstNonPrologueInsn (m_range, exe_ctx, m_first_non_prologue_insn);
return m_first_non_prologue_insn;
}
@@ -260,16 +198,6 @@ FuncUnwinders::GetFunctionStartAddress (
return m_range.GetBaseAddress();
}
-void
-FuncUnwinders::InvalidateNonCallSiteUnwindPlan (lldb_private::Thread& thread)
-{
- UnwindPlanSP arch_default = GetUnwindPlanArchitectureDefault (thread);
- if (arch_default && m_tried_unwind_at_call_site)
- {
- m_unwind_plan_call_site_sp = arch_default;
- }
-}
-
lldb::UnwindAssemblySP
FuncUnwinders::GetUnwindAssemblyProfiler ()
{
More information about the lldb-commits
mailing list