Do a better job of detecting when a breakpoint command has set the target running again (except you have to ignore

cases where the breakpoint runs expressions, those don't count as really "running again").

llvm-svn: 144064

lldb/include/lldb/Target/Process.h

@@ -890,8 +890,11 @@ class ProcessModID
 friend bool operator== (const ProcessModID &lhs, const ProcessModID &rhs);   
 public:
     ProcessModID () : 
-        m_stop_id (0), 
+        m_stop_id (0),
-        m_memory_id (0) 
+        m_resume_id (0), 
+        m_memory_id (0),
+        m_last_user_expression_resume (0),
+        m_running_user_expression (false)
     {}
     
     ProcessModID (const ProcessModID &rhs) :
@@ -911,11 +914,22 @@ public:
     
     ~ProcessModID () {}
     
-    void BumpStopID () { m_stop_id++; }
+    void BumpStopID () { 
+        m_stop_id++; 
+    }
+    
     void BumpMemoryID () { m_memory_id++; }
     
+    void BumpResumeID () {
+        m_resume_id++;
+        if (m_running_user_expression > 0)
+            m_last_user_expression_resume = m_resume_id;
+    }
+    
     uint32_t GetStopID() const { return m_stop_id; }
     uint32_t GetMemoryID () const { return m_memory_id; }
+    uint32_t GetResumeID () const { return m_resume_id; }
+    uint32_t GetLastUserExpressionResumeID () const { return m_last_user_expression_resume; }
     
     bool MemoryIDEqual (const ProcessModID &compare) const
     {
@@ -936,9 +950,23 @@ public:
     {
         return m_stop_id != UINT32_MAX;
     }
+    
+    void
+    SetRunningUserExpression (bool on)
+    {
+        // REMOVEME printf ("Setting running user expression %s at resume id %d - value: %d.\n", on ? "on" : "off", m_resume_id, m_running_user_expression);
+        if (on)
+            m_running_user_expression++;
+        else
+            m_running_user_expression--;
+    }
+    
 private:
     uint32_t m_stop_id;
+    uint32_t m_resume_id;
     uint32_t m_memory_id;
+    uint32_t m_last_user_expression_resume;
+    uint32_t m_running_user_expression;
 };
 inline bool operator== (const ProcessModID &lhs, const ProcessModID &rhs)
 {
@@ -1957,12 +1985,30 @@ public:
         return m_mod_id;
     }
     
+    const ProcessModID &
+    GetModIDRef () const
+    {
+        return m_mod_id;
+    }
+    
     uint32_t
     GetStopID () const
     {
         return m_mod_id.GetStopID();
     }
     
+    uint32_t
+    GetResumeID () const
+    {
+        return m_mod_id.GetResumeID();
+    }
+    
+    uint32_t
+    GetLastUserExpressionResumeID () const
+    {
+        return m_mod_id.GetLastUserExpressionResumeID();
+    }
+    
     //------------------------------------------------------------------
     /// Set accessor for the process exit status (return code).
     ///
@@ -2608,6 +2654,9 @@ public:
         return true;
     }
     
+    void
+    SetRunningUserExpression (bool on);
+    
     //------------------------------------------------------------------
     // lldb::ExecutionContextScope pure virtual functions
     //------------------------------------------------------------------

lldb/include/lldb/Target/StopInfo.h

@@ -22,6 +22,7 @@ namespace lldb_private {
 
 class StopInfo
 {
+    friend class Process;
 public:
     //------------------------------------------------------------------
     // Constructors and Destructors
@@ -136,13 +137,13 @@ protected:
     //------------------------------------------------------------------
     Thread &        m_thread;   // The thread corresponding to the stop reason.
     uint32_t        m_stop_id;  // The process stop ID for which this stop info is valid
+    uint32_t        m_resume_id; // This is the resume ID when we made this stop ID.
     uint64_t        m_value;    // A generic value that can be used for things pertaining to this stop info
     std::string     m_description; // A textual description describing this stop.
     
-    // This provides an accessor to the PrivateEventState of the process for StopInfo's w/o having to make each
+    // This determines whether the target has run since this stop info.
-    // StopInfo subclass a friend of Process.
+    // N.B. running to evaluate a user expression does not count. 
-    lldb::StateType
+    bool HasTargetRunSinceMe ();
-    GetPrivateState ();
 
 private:
     friend class Thread;

lldb/source/Expression/ClangUserExpression.cpp

@@ -567,6 +567,9 @@ ClangUserExpression::Execute (Stream &error_stream,
         if (log)
             log->Printf("-- [ClangUserExpression::Execute] Execution of expression begins --");
         
+        if (exe_ctx.GetProcessPtr())
+            exe_ctx.GetProcessPtr()->SetRunningUserExpression(true);
+            
         ExecutionResults execution_result = exe_ctx.GetProcessRef().RunThreadPlan (exe_ctx, 
                                                                                    call_plan_sp, 
                                                                                    stop_others, 
@@ -575,6 +578,9 @@ ClangUserExpression::Execute (Stream &error_stream,
                                                                                    single_thread_timeout_usec, 
                                                                                    error_stream);
         
+        if (exe_ctx.GetProcessPtr())
+            exe_ctx.GetProcessPtr()->SetRunningUserExpression(false);
+            
         if (log)
             log->Printf("-- [ClangUserExpression::Execute] Execution of expression completed --");
 

lldb/source/Target/Process.cpp

@@ -1156,6 +1156,12 @@ Process::SetPrivateState (StateType new_state)
     }
 }
 
+void
+Process::SetRunningUserExpression (bool on)
+{
+    m_mod_id.SetRunningUserExpression (on);
+}
+
 addr_t
 Process::GetImageInfoAddress()
 {
@@ -2425,6 +2431,7 @@ Process::Resume ()
         // to see if they are suppoed to start back up with a signal.
         if (m_thread_list.WillResume())
         {
+            m_mod_id.BumpResumeID();
             error = DoResume();
             if (error.Success())
             {
@@ -2986,18 +2993,47 @@ Process::ProcessEventData::DoOnRemoval (Event *event_ptr)
         
     // If we're stopped and haven't restarted, then do the breakpoint commands here:
     if (m_state == eStateStopped && ! m_restarted)
-    {
+    {        
-        int num_threads = m_process_sp->GetThreadList().GetSize();
+        ThreadList &curr_thread_list = m_process_sp->GetThreadList();
+        int num_threads = curr_thread_list.GetSize();
         int idx;
 
         // The actions might change one of the thread's stop_info's opinions about whether we should
         // stop the process, so we need to query that as we go.
+        
+        // One other complication here, is that we try to catch any case where the target has run (except for expressions)
+        // and immediately exit, but if we get that wrong (which is possible) then the thread list might have changed, and
+        // that would cause our iteration here to crash.  We could make a copy of the thread list, but we'd really like
+        // to also know if it has changed at all, so we make up a vector of the thread ID's and check what we get back 
+        // against this list & bag out if anything differs.
+        std::vector<lldb::tid_t> thread_index_array(num_threads);
+        for (idx = 0; idx < num_threads; ++idx)
+            thread_index_array[idx] = curr_thread_list.GetThreadAtIndex(idx)->GetIndexID();
+        
         bool still_should_stop = true;
         
         for (idx = 0; idx < num_threads; ++idx)
         {
-            lldb::ThreadSP thread_sp = m_process_sp->GetThreadList().GetThreadAtIndex(idx);
+            curr_thread_list = m_process_sp->GetThreadList();
-
+            if (curr_thread_list.GetSize() != num_threads)
+            {
+                lldb::LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_STEP | LIBLLDB_LOG_PROCESS));
+                log->Printf("Number of threads changed from %d to %d while processing event.", num_threads, curr_thread_list.GetSize());
+                break;
+            }
+            
+            lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx);
+            
+            if (thread_sp->GetIndexID() != thread_index_array[idx])
+            {
+                lldb::LogSP log(lldb_private::GetLogIfAnyCategoriesSet (LIBLLDB_LOG_STEP | LIBLLDB_LOG_PROCESS));
+                log->Printf("The thread at position %d changed from %d  to %d while processing event.", 
+                            idx, 
+                            thread_index_array[idx],
+                            thread_sp->GetIndexID());
+                break;
+            }
+            
             StopInfoSP stop_info_sp = thread_sp->GetStopInfo ();
             if (stop_info_sp)
             {
@@ -3006,7 +3042,9 @@ Process::ProcessEventData::DoOnRemoval (Event *event_ptr)
                 // event so that whoever is receiving it will know to wait for the running event and reflect
                 // that state appropriately.
                 // We also need to stop processing actions, since they aren't expecting the target to be running.
-                if (m_process_sp->GetPrivateState() == eStateRunning)
+                
+                // FIXME: we might have run.
+                if (stop_info_sp->HasTargetRunSinceMe())
                 {
                     SetRestarted (true);
                     break;

lldb/source/Target/StopInfo.cpp

@@ -35,6 +35,7 @@ using namespace lldb_private;
 StopInfo::StopInfo (Thread &thread, uint64_t value) :
     m_thread (thread),
     m_stop_id (thread.GetProcess().GetStopID()),
+    m_resume_id (thread.GetProcess().GetResumeID()),
     m_value (value)
 {
 }
@@ -49,12 +50,37 @@ void
 StopInfo::MakeStopInfoValid ()
 {
     m_stop_id = m_thread.GetProcess().GetStopID();
+    m_resume_id = m_thread.GetProcess().GetResumeID();
 }
 
-lldb::StateType
+bool
-StopInfo::GetPrivateState ()
+StopInfo::HasTargetRunSinceMe ()
 {
-    return m_thread.GetProcess().GetPrivateState();
+    lldb::StateType ret_type = m_thread.GetProcess().GetPrivateState();
+    if (ret_type == eStateRunning)
+    {
+        return true;
+    }
+    else if (ret_type == eStateStopped)
+    {
+        // This is a little tricky.  We want to count "run and stopped again before you could
+        // ask this question as a "TRUE" answer to HasTargetRunSinceMe.  But we don't want to 
+        // include any running of the target done for expressions.  So we track both resumes,
+        // and resumes caused by expressions, and check if there are any resumes NOT caused
+        // by expressions.
+        
+        uint32_t curr_resume_id = m_thread.GetProcess().GetResumeID();
+        uint32_t last_user_expression_id = m_thread.GetProcess().GetLastUserExpressionResumeID ();
+        if (curr_resume_id == m_resume_id)
+        {
+            return false;
+        }
+        else if (curr_resume_id > last_user_expression_id)
+        {
+            return true;
+        }
+    }
+    return false;
 }
 
 //----------------------------------------------------------------------
@@ -161,7 +187,7 @@ public:
             // However we want to run all the callbacks, except of course if one of them actually
             // resumes the target.
             // So we use stop_requested to track what we're were asked to do.
-            bool stop_requested = true;
+            bool stop_requested = false;
             for (size_t j = 0; j < num_owners; j++)
             {
                 lldb::BreakpointLocationSP bp_loc_sp = bp_site_sp->GetOwnerAtIndex(j);
@@ -170,11 +196,30 @@ public:
                                                   &m_thread, 
                                                   m_thread.GetStackFrameAtIndex(0).get(),
                                                   false);
-                stop_requested = bp_loc_sp->InvokeCallback (&context);
+                bool callback_return;
+                
+                // FIXME: For now the callbacks have to run in async mode - the first time we restart we need
+                // to get out of there.  So set it here.
+                // When we figure out how to stack breakpoint hits then this will change.
+                
+                Debugger &debugger = m_thread.GetProcess().GetTarget().GetDebugger();
+                bool old_async = debugger.GetAsyncExecution();
+                debugger.SetAsyncExecution (true);
+                
+                callback_return = bp_loc_sp->InvokeCallback (&context);
+                
+                debugger.SetAsyncExecution (old_async);
+                
+                if (callback_return)
+                    stop_requested = true;
+                    
                 // Also make sure that the callback hasn't continued the target.  
                 // If it did, when we'll set m_should_start to false and get out of here.
-                if (GetPrivateState() == eStateRunning)
+                if (HasTargetRunSinceMe ())
+                {
                     m_should_stop = false;
+                    break;
+                }
             }
             
             if (m_should_stop && !stop_requested)
@@ -423,7 +468,7 @@ public:
             bool stop_requested = wp_sp->InvokeCallback (&context);
             // Also make sure that the callback hasn't continued the target.  
             // If it did, when we'll set m_should_start to false and get out of here.
-            if (GetPrivateState() == eStateRunning)
+            if (HasTargetRunSinceMe ())
                 m_should_stop = false;
             
             if (m_should_stop && !stop_requested)

lldb/test/functionalities/conditional_break/TestConditionalBreak.py

@@ -71,6 +71,7 @@ class ConditionalBreakTestCase(TestBase):
         # The 10 in range(10) is just an arbitrary number, which means we would
         # like to try for at most 10 times.
         for j in range(10):
+            print "j is: ", j
             thread = process.GetThreadAtIndex(0)
             
             if thread.GetNumFrames() >= 2:
@@ -102,9 +103,13 @@ class ConditionalBreakTestCase(TestBase):
         # executable, sets the breakpoint on c(), and adds the callback for the
         # breakpoint such that lldb only stops when the caller of c() is a().
         # the "my" package that defines the date() function.
+        print "About to source .lldb"
+
         self.runCmd("command source .lldb")
 
+        print "About to run."
         self.runCmd("run", RUN_SUCCEEDED)
+        print "Done running"
 
         # The stop reason of the thread should be breakpoint.
         self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,

lldb/test/functionalities/conditional_break/conditional_break.py

@@ -7,7 +7,8 @@ def stop_if_called_from_a():
     dbg = lldb.SBDebugger.FindDebuggerWithID(lldb.debugger_unique_id)
 
     # Perform synchronous interaction with the debugger.
-    dbg.SetAsync(False)
+    old_async = dbg.GetAsync()
+    dbg.SetAsync(True)
 
     # Retrieve the target, process, and the only thread.
     target = dbg.GetSelectedTarget()
@@ -18,17 +19,19 @@ def stop_if_called_from_a():
     # of the leaf function c() is a().  If it's not the right caller, we ask the
     # command interpreter to continue execution.
 
-    #print >> sys.stdout, "Checking call frames..."
+    print >> sys.stdout, "Checking call frames..."
-    #lldbutil.print_stacktrace(thread)
+    lldbutil.print_stacktrace(thread)
+    should_stop = True
     if thread.GetNumFrames() >= 2:
         funcs = lldbutil.get_function_names(thread)
-        #print >> sys.stdout, funcs[0], "called from", funcs[1]
+        print >> sys.stdout, funcs[0], "called from", funcs[1]
         if (funcs[0] == 'c' and funcs[1] == 'a'):
-            #print >> sys.stdout, "Stopped at c() with immediate caller as a()."
+            should_stop = True
-            pass
         else:
-            #print >> sys.stdout, "Continuing..."
             process.Continue()
+            should_stop = False
+
+    dbg.SetAsync(old_async)
+    return should_stop
 
-    return True