[Lldb-commits] [lldb] r250084 - Fix a misunderstanding of the ThreadPlan::OkayToDiscard flag in InferiorCallPOSIX. It was

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Mon Oct 12 12:11:03 PDT 2015


Author: jingham
Date: Mon Oct 12 14:11:03 2015
New Revision: 250084

URL: http://llvm.org/viewvc/llvm-project?rev=250084&view=rev
Log:
Fix a misunderstanding of the ThreadPlan::OkayToDiscard flag in InferiorCallPOSIX.  It was 
set to true, but all plans run by RunThreadPlan need to have this set to false so they will
return control to RunThreadPlan without consulting plans higher on the stack.

Since this seems like a common error, I also modified RunThreadPlan to enforce this behavior.

<rdar://problem/22543166>

Modified:
    lldb/trunk/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
    lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp?rev=250084&r1=250083&r2=250084&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Utility/InferiorCallPOSIX.cpp Mon Oct 12 14:11:03 2015
@@ -103,9 +103,6 @@ lldb_private::InferiorCallMmap (Process
                 if (call_plan_sp)
                 {
                     StreamFile error_strm;
-                    // This plan is a utility plan, so set it to discard itself when done.
-                    call_plan_sp->SetIsMasterPlan (true);
-                    call_plan_sp->SetOkayToDiscard(true);
                     
                     StackFrame *frame = thread->GetStackFrameAtIndex (0).get();
                     if (frame)
@@ -188,9 +185,6 @@ lldb_private::InferiorCallMunmap (Proces
                 if (call_plan_sp)
                 {
                     StreamFile error_strm;
-                    // This plan is a utility plan, so set it to discard itself when done.
-                    call_plan_sp->SetIsMasterPlan (true);
-                    call_plan_sp->SetOkayToDiscard(true);
                    
                     StackFrame *frame = thread->GetStackFrameAtIndex (0).get();
                     if (frame)
@@ -244,9 +238,6 @@ lldb_private::InferiorCall (Process *pro
     if (call_plan_sp)
     {
         StreamString error_strm;
-        // This plan is a utility plan, so set it to discard itself when done.
-        call_plan_sp->SetIsMasterPlan (true);
-        call_plan_sp->SetOkayToDiscard(true);
 
         StackFrame *frame = thread->GetStackFrameAtIndex (0).get();
         if (frame)

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=250084&r1=250083&r2=250084&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Mon Oct 12 14:11:03 2015
@@ -5367,6 +5367,53 @@ Process::SettingsTerminate ()
     Thread::SettingsTerminate ();
 }
 
+namespace
+{
+    // RestorePlanState is used to record the "is private", "is master" and "okay to discard" fields of
+    // the plan we are running, and reset it on Clean or on destruction.
+    // It will only reset the state once, so you can call Clean and then monkey with the state and it
+    // won't get reset on you again.
+    
+    class RestorePlanState
+    {
+    public:
+        RestorePlanState (lldb::ThreadPlanSP thread_plan_sp) :
+            m_thread_plan_sp(thread_plan_sp),
+            m_already_reset(false)
+        {
+            if (m_thread_plan_sp)
+            {
+                m_private = m_thread_plan_sp->GetPrivate();
+                m_is_master = m_thread_plan_sp->IsMasterPlan();
+                m_okay_to_discard = m_thread_plan_sp->OkayToDiscard();
+            }
+        }
+        
+        ~RestorePlanState()
+        {
+            Clean();
+        }
+        
+        void
+        Clean ()
+        {
+            if (!m_already_reset && m_thread_plan_sp)
+            {
+                m_already_reset = true;
+                m_thread_plan_sp->SetPrivate(m_private);
+                m_thread_plan_sp->SetIsMasterPlan (m_is_master);
+                m_thread_plan_sp->SetOkayToDiscard(m_okay_to_discard);
+            }
+        }
+    private:
+        lldb::ThreadPlanSP m_thread_plan_sp;
+        bool m_already_reset;
+        bool m_private;
+        bool m_is_master;
+        bool m_okay_to_discard;
+    };
+}
+
 ExpressionResults
 Process::RunThreadPlan (ExecutionContext &exe_ctx,
                         lldb::ThreadPlanSP &thread_plan_sp,
@@ -5400,12 +5447,22 @@ Process::RunThreadPlan (ExecutionContext
         return eExpressionSetupError;
     }
 
+    // We need to change some of the thread plan attributes for the thread plan runner.  This will restore them
+    // when we are done:
+    
+    RestorePlanState thread_plan_restorer(thread_plan_sp);
+    
     // We rely on the thread plan we are running returning "PlanCompleted" if when it successfully completes.
     // For that to be true the plan can't be private - since private plans suppress themselves in the
     // GetCompletedPlan call. 
 
-    bool orig_plan_private = thread_plan_sp->GetPrivate();
     thread_plan_sp->SetPrivate(false);
+    
+    // The plans run with RunThreadPlan also need to be terminal master plans or when they are done we will end
+    // up asking the plan above us whether we should stop, which may give the wrong answer.
+    
+    thread_plan_sp->SetIsMasterPlan (true);
+    thread_plan_sp->SetOkayToDiscard(false);
 
     if (m_private_state.GetValue() != eStateStopped)
     {
@@ -5835,10 +5892,10 @@ Process::RunThreadPlan (ExecutionContext
                                         {
                                             if (log)
                                                 log->PutCString ("Process::RunThreadPlan(): execution completed successfully.");
-                                            // Now mark this plan as private so it doesn't get reported as the stop reason
-                                            // after this point.  
-                                            if (thread_plan_sp)
-                                                thread_plan_sp->SetPrivate (orig_plan_private);
+                                            
+                                            // Restore the plan state so it will get reported as intended when we are done.
+                                            thread_plan_restorer.Clean();
+                                            
                                             return_value = eExpressionCompleted;
                                         }
                                         else
@@ -5851,6 +5908,13 @@ Process::RunThreadPlan (ExecutionContext
                                                 return_value = eExpressionHitBreakpoint;
                                                 if (!options.DoesIgnoreBreakpoints())
                                                 {
+                                                    // Restore the plan state and then force Private to false.  We are
+                                                    // going to stop because of this plan so we need it to become a public
+                                                    // plan or it won't report correctly when we continue to its termination
+                                                    // later on.
+                                                    thread_plan_restorer.Clean();
+                                                    if (thread_plan_sp)
+                                                        thread_plan_sp->SetPrivate(false);
                                                     event_to_broadcast_sp = event_sp;
                                                 }
                                             }
@@ -6069,6 +6133,19 @@ Process::RunThreadPlan (ExecutionContext
                 m_public_state.SetValueNoLock(old_state);
         }
 
+        if (return_value != eExpressionCompleted && log)
+        {
+            // Print a backtrace into the log so we can figure out where we are:
+            StreamString s;
+            s.PutCString("Thread state after unsuccessful completion: \n");
+            thread->GetStackFrameStatus (s,
+                                         0,
+                                         UINT32_MAX,
+                                         true,
+                                         UINT32_MAX);
+            log->PutCString(s.GetData());
+
+        }
         // Restore the thread state if we are going to discard the plan execution.  There are three cases where this
         // could happen:
         // 1) The execution successfully completed
@@ -6157,7 +6234,8 @@ Process::RunThreadPlan (ExecutionContext
                             else
                                 ts.Printf("[ip unknown] ");
 
-                            lldb::StopInfoSP stop_info_sp = thread->GetStopInfo();
+                            // Show the private stop info here, the public stop info will be from the last natural stop.
+                            lldb::StopInfoSP stop_info_sp = thread->GetPrivateStopInfo();
                             if (stop_info_sp)
                             {
                                 const char *stop_desc = stop_info_sp->GetDescription();
@@ -6183,7 +6261,6 @@ Process::RunThreadPlan (ExecutionContext
                     log->Printf ("Process::RunThreadPlan: ExecutionInterrupted - discarding thread plans up to %p.",
                                  static_cast<void*>(thread_plan_sp.get()));
                 thread->DiscardThreadPlansUpToPlan (thread_plan_sp);
-                thread_plan_sp->SetPrivate (orig_plan_private);
             }
             else
             {
@@ -6200,7 +6277,6 @@ Process::RunThreadPlan (ExecutionContext
             if (options.DoesUnwindOnError())
             {
                 thread->DiscardThreadPlansUpToPlan (thread_plan_sp);
-                thread_plan_sp->SetPrivate (orig_plan_private);
             }
         }
         else
@@ -6226,7 +6302,6 @@ Process::RunThreadPlan (ExecutionContext
                     if (log)
                         log->PutCString("Process::RunThreadPlan(): discarding thread plan 'cause unwind_on_error is set.");
                     thread->DiscardThreadPlansUpToPlan (thread_plan_sp);
-                    thread_plan_sp->SetPrivate (orig_plan_private);
                 }
             }
         }




More information about the lldb-commits mailing list