Fix stepping a virtual thread when the python operating system was enabled.

The OperatingSystem plug-ins allow code to detect threads in memory and then say "memory thread 0x11111" is backed by the actual thread 1. 

You can then single step these virtual threads. A problem arose when thread specific breakpoints were used during thread plans where we would say "set a breakpoint on thread 0x11111" and we would hit the breakpoint on the real thread 1 and the thread IDs wouldn't match and we would get rid of the "stopped at breakpoint" stop info due to this mismatch. Code was added to ensure these events get forwarded and thus allow single stepping a memory thread to work correctly.

Added a test case for this as well.

<rdar://problem/19211770>

llvm-svn: 234364

Author: Greg Clayton

lldb/include/lldb/Target/StopInfo.h

@@ -116,6 +116,12 @@ class StopInfo
         else
             m_description.clear();
     }
+
+    virtual bool
+    IsValidForOperatingSystemThread (Thread &thread)
+    {
+        return true;
+    }
 
     // Sometimes the thread plan logic will know that it wants a given stop to stop or not,
     // regardless of what the ordinary logic for that StopInfo would dictate.  The main example

lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp

@@ -178,11 +178,18 @@ OperatingSystemPython::UpdateThreadList (ThreadList &old_thread_list,
 
     Log *log(lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_OS));
 
-    // First thing we have to do is get the API lock, and the run lock.  We're going to change the thread
-    // content of the process, and we're going to use python, which requires the API lock to do it.
-    // So get & hold that.  This is a recursive lock so we can grant it to any Python code called on the stack below us.
+    // First thing we have to do is to try to get the API lock, and the run lock.
+    // We're going to change the thread content of the process, and we're going
+    // to use python, which requires the API lock to do it.
+    //
+    // If someone already has the API lock, that is ok, we just want to avoid
+    // external code from making new API calls while this call is happening.
+    //
+    // This is a recursive lock so we can grant it to any Python code called on
+    // the stack below us.
     Target &target = m_process->GetTarget();
-    Mutex::Locker api_locker (target.GetAPIMutex());
+    Mutex::Locker api_locker;
+    api_locker.TryLock(target.GetAPIMutex());
 
     if (log)
         log->Printf ("OperatingSystemPython::UpdateThreadList() fetching thread data from python for pid %" PRIu64, m_process->GetID());

lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp

@@ -498,12 +498,15 @@ StopInfoMachException::CreateStopReasonWithMachException
                         // If the breakpoint is for this thread, then we'll report the hit, but if it is for another thread,
                         // we can just report no reason.  We don't need to worry about stepping over the breakpoint here, that
                         // will be taken care of when the thread resumes and notices that there's a breakpoint under the pc.
-                        if (bp_site_sp->ValidForThisThread (&thread))
+                        // If we have an operating system plug-in, we might have set a thread specific breakpoint using the
+                        // operating system thread ID, so we can't make any assumptions about the thread ID so we must always
+                        // report the breakpoint regardless of the thread.
+                        if (bp_site_sp->ValidForThisThread (&thread) || thread.GetProcess()->GetOperatingSystem () != NULL)
                             return StopInfo::CreateStopReasonWithBreakpointSiteID (thread, bp_site_sp->GetID());
                         else
                             return StopInfoSP();
                     }
-                    
+
                     // Don't call this a trace if we weren't single stepping this thread.
                     if (is_trace_if_actual_breakpoint_missing && thread.GetTemporaryResumeState() == eStateStepping)
                     {

lldb/source/Plugins/Process/Utility/ThreadMemory.cpp

@@ -105,7 +105,7 @@ ThreadMemory::CalculateStopInfo ()
     if (m_backing_thread_sp)
     {
         lldb::StopInfoSP backing_stop_info_sp (m_backing_thread_sp->GetPrivateStopInfo());
-        if (backing_stop_info_sp)
+        if (backing_stop_info_sp && backing_stop_info_sp->IsValidForOperatingSystemThread(*this))
         {
             backing_stop_info_sp->SetThread (shared_from_this());
             SetStopInfo (backing_stop_info_sp);

lldb/source/Target/StopInfo.cpp

@@ -163,6 +163,19 @@ class StopInfoBreakpoint : public StopInfo
     {
     }
 
+    virtual bool
+    IsValidForOperatingSystemThread (Thread &thread)
+    {
+        ProcessSP process_sp (thread.GetProcess());
+        if (process_sp)
+        {
+            BreakpointSiteSP bp_site_sp (process_sp->GetBreakpointSiteList().FindByID (m_value));
+            if (bp_site_sp)
+                return bp_site_sp->ValidForThisThread (&thread);
+        }
+        return false;
+    }
+
     virtual StopReason
     GetStopReason () const
     {

lldb/test/functionalities/plugins/python_os_plugin/TestPythonOSPlugin.py

@@ -26,6 +26,19 @@ def test_python_os_plugin_dwarf(self):
         self.buildDwarf()
         self.run_python_os_funcionality()
 
+    @skipUnlessDarwin
+    @dsym_test
+    def test_python_os_step_dsym(self):
+        """Test that the Python operating system plugin works correctly when single stepping a virtual thread"""
+        self.buildDsym()
+        self.run_python_os_step()
+
+    @dwarf_test
+    def run_python_os_step_dwarf(self):
+        """Test that the Python operating system plugin works correctly when single stepping a virtual thread"""
+        self.buildDwarf()
+        self.run_python_os_step()
+
     def verify_os_thread_registers(self, thread):
         frame = thread.GetFrameAtIndex(0)
         registers = frame.GetRegisters().GetValueAtIndex(0)
@@ -94,6 +107,62 @@ def run_python_os_funcionality(self):
         thread = process.GetThreadByID(0x333333333);
         self.assertFalse (thread.IsValid(), "Make sure there is no thread 0x333333333 after we unload the python OS plug-in");
 
+    def run_python_os_step(self):
+        """Test that the Python operating system plugin works correctly and allows single stepping of a virtual thread that is backed by a real thread"""
+
+        # Set debugger into synchronous mode
+        self.dbg.SetAsync(False)
+
+        # Create a target by the debugger.
+        cwd = os.getcwd()
+        exe = os.path.join(cwd, "a.out")
+        python_os_plugin_path = os.path.join(cwd, "operating_system2.py")
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        # Set breakpoints inside and outside methods that take pointers to the containing struct.
+        lldbutil.run_break_set_by_source_regexp (self, "// Set breakpoint here")
+
+        # Register our shared libraries for remote targets so they get automatically uploaded
+        arguments = None
+        environment = None 
+
+        # Now launch the process, and do not stop at entry point.
+        process = target.LaunchSimple (arguments, environment, self.get_process_working_directory())
+        self.assertTrue(process, PROCESS_IS_VALID)
+
+        # Make sure there are no OS plug-in created thread when we first stop at our breakpoint in main
+        thread = process.GetThreadByID(0x111111111);
+        self.assertFalse (thread.IsValid(), "Make sure there is no thread 0x111111111 before we load the python OS plug-in");
+
+
+        # Now load the python OS plug-in which should update the thread list and we should have
+        # OS plug-in created threads with the IDs: 0x111111111, 0x222222222, 0x333333333
+        command = "settings set target.process.python-os-plugin-path '%s'" % python_os_plugin_path
+        self.dbg.HandleCommand(command)
+
+        # Verify our OS plug-in threads showed up
+        thread = process.GetThreadByID(0x111111111);
+        self.assertTrue (thread.IsValid(), "Make sure there is a thread 0x111111111 after we load the python OS plug-in");
+
+        frame = thread.GetFrameAtIndex(0)
+        self.assertTrue(frame.IsValid(), "Make sure we get a frame from thread 0x111111111")
+        line_entry = frame.GetLineEntry()
+
+        self.assertTrue(line_entry.GetFileSpec().GetFilename() == 'main.c', "Make sure we stopped on line 5 in main.c")
+        self.assertTrue(line_entry.GetLine() == 5, "Make sure we stopped on line 5 in main.c")
+
+        # Now single step thread 0x111111111 and make sure it does what we need it to
+        thread.StepOver()
+        
+        frame = thread.GetFrameAtIndex(0)
+        self.assertTrue(frame.IsValid(), "Make sure we get a frame from thread 0x111111111")
+        line_entry = frame.GetLineEntry()
+        
+        self.assertTrue(line_entry.GetFileSpec().GetFilename() == 'main.c', "Make sure we stepped from line 5 to line 6 in main.c")
+        self.assertTrue(line_entry.GetLine() == 6, "Make sure we stepped from line 5 to line 6 in main.c")
+        
+
 if __name__ == '__main__':
     import atexit
     lldb.SBDebugger.Initialize()