The Function calling thread plan was replacing the stored stop info too soon, causing recursive entry into the

breakpoint StopInfo's PerformAction, which is bad.  Reworked this so that it is now correct.

<rdar://problem/12501259>

llvm-svn: 168634

Author: jimingham

lldb/include/lldb/Target/Thread.h

@@ -731,6 +731,9 @@ friend class ThreadEventData;
     virtual bool
     CheckpointThreadState (ThreadStateCheckpoint &saved_state);
 
+    virtual bool
+    RestoreRegisterStateFromCheckpoint (ThreadStateCheckpoint &saved_state);
+    
     virtual bool
     RestoreThreadStateFromCheckpoint (ThreadStateCheckpoint &saved_state);
 

lldb/include/lldb/Target/ThreadPlan.h

@@ -494,6 +494,17 @@ class ThreadPlan :
         return lldb::ValueObjectSP();
     }
 
+    // If a thread plan stores the state before it was run, then you might
+    // want to restore the state when it is done.  This will do that job.
+    // This is mostly useful for artificial plans like CallFunction plans.
+    
+    virtual bool
+    RestoreThreadState()
+    {
+        // Nothing to do in general.
+        return true;
+    }
+    
 protected:
     //------------------------------------------------------------------
     // Classes that inherit from ThreadPlan can see and modify these

lldb/include/lldb/Target/ThreadPlanCallFunction.h

@@ -127,6 +127,9 @@ class ThreadPlanCallFunction : public ThreadPlan
         return m_stop_address;
     }
 
+    virtual bool
+    RestoreThreadState();
+
 protected:
     void ReportRegisterState (const char *message);
 private:
@@ -154,8 +157,6 @@ class ThreadPlanCallFunction : public ThreadPlan
     Address                                         m_function_addr;
     Address                                         m_start_addr;
     lldb::addr_t                                    m_function_sp;
-//    Process                                        &m_process;
-//    Thread                                         &m_thread;
     Thread::RegisterCheckpoint                      m_register_backup;
     lldb::ThreadPlanSP                              m_subplan_sp;
     LanguageRuntime                                *m_cxx_language_runtime;

lldb/source/Target/Process.cpp

@@ -4377,6 +4377,9 @@ Process::RunThreadPlan (ExecutionContext &exe_ctx,
         const uint64_t default_one_thread_timeout_usec = 250000;
         uint64_t computed_timeout = 0;
 
+        // This while loop must exit out the bottom, there's cleanup that we need to do when we are done.
+        // So don't call return anywhere within it.
+        
         while (1)
         {
             // We usually want to resume the process if we get to the top of the loop.
@@ -4784,6 +4787,12 @@ Process::RunThreadPlan (ExecutionContext &exe_ctx,
 
         }
 
+        // Restore the thread state if we are going to discard the plan execution.
+        
+        if (return_value == eExecutionCompleted || discard_on_error)
+        {
+            thread_plan_sp->RestoreThreadState();
+        }
 
         // Now do some processing on the results of the run:
         if (return_value == eExecutionInterrupted)

lldb/source/Target/Thread.cpp

@@ -396,9 +396,15 @@ Thread::CheckpointThreadState (ThreadStateCheckpoint &saved_state)
 }
 
 bool
-Thread::RestoreThreadStateFromCheckpoint (ThreadStateCheckpoint &saved_state)
+Thread::RestoreRegisterStateFromCheckpoint (ThreadStateCheckpoint &saved_state)
 {
     RestoreSaveFrameZero(saved_state.register_backup);
+    return true;
+}
+
+bool
+Thread::RestoreThreadStateFromCheckpoint (ThreadStateCheckpoint &saved_state)
+{
     if (saved_state.stop_info_sp)
         saved_state.stop_info_sp->MakeStopInfoValid();
     SetStopInfo(saved_state.stop_info_sp);

lldb/source/Target/ThreadPlanCallFunction.cpp

@@ -112,9 +112,6 @@ ThreadPlanCallFunction::ConstructorSetup (Thread &thread,
             log->Printf ("ThreadPlanCallFunction(%p): Setting up ThreadPlanCallFunction, failed to checkpoint thread state.", this);
         return false;
     }
-    // Now set the thread state to "no reason" so we don't run with whatever signal was outstanding...
-    thread.SetStopInfoToNothing();
-    
     function_load_addr = m_function_addr.GetLoadAddress (target_sp.get());
 
     return true;
@@ -196,10 +193,11 @@ ThreadPlanCallFunction::ThreadPlanCallFunction (Thread &thread,
     m_valid (false),
     m_stop_other_threads (stop_other_threads),
     m_function_addr (function),
-    m_function_sp(0),
+    m_function_sp (0),
     m_return_type (return_type),
     m_takedown_done (false),
-    m_stop_address (LLDB_INVALID_ADDRESS)
+    m_stop_address (LLDB_INVALID_ADDRESS),
+    m_discard_on_error (discard_on_error)
 {
     lldb::addr_t start_load_addr;
     ABI *abi;
@@ -289,7 +287,7 @@ ThreadPlanCallFunction::DoTakedown (bool success)
         m_takedown_done = true;
         m_stop_address = m_thread.GetStackFrameAtIndex(0)->GetRegisterContext()->GetPC();
         m_real_stop_info_sp = GetPrivateStopReason();
-        m_thread.RestoreThreadStateFromCheckpoint(m_stored_thread_state);
+        m_thread.RestoreRegisterStateFromCheckpoint(m_stored_thread_state);
         SetPlanComplete(success);
         ClearBreakpoints();
         if (log && log->GetVerbose())
@@ -460,6 +458,11 @@ ThreadPlanCallFunction::DidPush ()
 {
 //#define SINGLE_STEP_EXPRESSIONS
 
+    // Now set the thread state to "no reason" so we don't run with whatever signal was outstanding...
+    // Wait till the plan is pushed so we aren't changing the stop info till we're about to run.
+    
+    GetThread().SetStopInfoToNothing();
+    
 #ifndef SINGLE_STEP_EXPRESSIONS
     m_subplan_sp.reset(new ThreadPlanRunToAddress(m_thread, m_start_addr, m_stop_other_threads));
 
@@ -533,3 +536,10 @@ ThreadPlanCallFunction::BreakpointsExplainStop()
 
     return false;
 }
+
+bool
+ThreadPlanCallFunction::RestoreThreadState()
+{
+    return GetThread().RestoreThreadStateFromCheckpoint(m_stored_thread_state);
+}
+