Allow private-state-thread to call SB API's that take the TargetAPI mutex

[jingham]

Summary

When we do synchronous process execution in lldb, the call that restarted the target will generally have acquired first the TargetAPI lock, and then the Process RunLock. If in the process of handling that continue we need to run Python code in the private-state-thread (e.g. for an OS plugin or scripted ThreadPlan) if that Python code calls another API that takes the Target API mutex, we will deadlock. However, the private state thread is always doing work on behalf of whatever thread initiated the continue. So the private state thread should always formally be allowed to pass the lock check.

We already handled this in the case of the Process RunLock by passing out separate locks depending on whether you were running code in the private state thread or anywhere else. This patch just does the same thing for the Target API mutex.

I also added a test using a scripted ThreadPlan that calls various SBFrame and SBValue API's which take the API lock in the should_stop method of the plan (which is always run on the private state thread).

[jingham]

Note, a better way to do this would be to have some kind of "API/Runlock baton" that could get handed off from the thread that initiated the continue to the private state thread, then handed back when the process stops and returns control to the SB API that initiated the run. But I don't have time to design such a thing right now, and this was blocking writing tests for some work I am doing to handle the case where OS Plugins don't report all threads at each stop. So I'd prefer this more straightforward solution for now - especially since it's just a copy of the way we handled the same problem for the runlock.

lldb/trunk/include/lldb/Target/Process.h

public:
 
   bool RunPreResumeActions();
 
   void ClearPreResumeActions();
 
   void ClearPreResumeAction(PreResumeActionCallback callback, void *baton);
 
   ProcessRunLock &GetRunLock();
   
+  bool CurrentThreadIsPrivateStateThread();
+
   virtual Status SendEventData(const char *data) {
     Status return_error("Sending an event is not supported for this process.");
     return return_error;
   }
 
   lldb::ThreadCollectionSP GetHistoryThreads(lldb::addr_t addr);
 
   lldb::InstrumentationRuntimeSP

lldb/trunk/include/lldb/Target/Target.h

public:
   ///     will likely be provided.
   lldb::ModuleSP GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
                                    Status *error_ptr = nullptr);
 
   // Settings accessors
 
   static const lldb::TargetPropertiesSP &GetGlobalProperties();
 
-  std::recursive_mutex &GetAPIMutex() { return m_mutex; }
+  std::recursive_mutex &GetAPIMutex();
 
   void DeleteCurrentProcess();
 
   void CleanupProcess();
 
   /// Dump a description of this object to a Stream.
   ///
   /// Dump a description of the contents of this object to the
private:
     ArchSpec m_spec;
     std::unique_ptr<Architecture> m_plugin_up;
   };
   // Member variables.
   Debugger &m_debugger;
   lldb::PlatformSP m_platform_sp; ///< The platform for this target.
   std::recursive_mutex m_mutex; ///< An API mutex that is used by the lldb::SB*
                                 /// classes make the SB interface thread safe
+  /// When the private state thread calls SB API's - usually because it is 
+  /// running OS plugin or Python ThreadPlan code - it should not block on the
+  /// API mutex that is held by the code that kicked off the sequence of events
+  /// that led us to run the code.  We hand out this mutex instead when we 
+  /// detect that code is running on the private state thread.
+  std::recursive_mutex m_private_mutex; 
   Arch m_arch;
   ModuleList m_images; ///< The list of images for this process (shared
                        /// libraries and anything dynamically loaded).
   SectionLoadHistory m_section_load_history;
   BreakpointList m_breakpoint_list;
   BreakpointList m_internal_breakpoint_list;
   using BreakpointNameList = std::map<ConstString, BreakpointName *>;
   BreakpointNameList m_breakpoint_names;

lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py

def queue_child_thread_plan(self):
         return self.thread_plan.QueueThreadPlanForStepOut(0)
 
 class StepScripted(StepWithChild):
     def __init__(self, thread_plan, dict):
         StepWithChild.__init__(self, thread_plan)
 
     def queue_child_thread_plan(self):
         return self.thread_plan.QueueThreadPlanForStepScripted("Steps.StepOut")
+
+# This plan does a step-over until a variable changes value.
+class StepUntil(StepWithChild):
+    def __init__(self, thread_plan, dict):
+        self.frame = thread_plan.GetThread().frames[0]
+        self.target = thread_plan.GetThread().GetProcess().GetTarget()
+        self.value = self.frame.FindVariable("foo")
+        StepWithChild.__init__(self, thread_plan)
+
+    def queue_child_thread_plan(self):
+        le = self.frame.GetLineEntry()
+        start_addr = le.GetStartAddress() 
+        start = start_addr.GetLoadAddress(self.target)
+        end = le.GetEndAddress().GetLoadAddress(self.target)
+        print("Stepping from 0x%x to 0x%x (0x%x)"%(start, end, end - start))
+        return self.thread_plan.QueueThreadPlanForStepOverRange(start_addr,
+                                                                end - start)
+
+    def should_stop(self, event):
+        if not self.child_thread_plan.IsPlanComplete():
+            return False
+
+        # If we've stepped out of this frame, stop.
+        if not self.frame.IsValid():
+            return True
+
+        if not self.value.IsValid():
+            return True
+
+        print("Got next value: %d"%(self.value.GetValueAsUnsigned()))
+        if not self.value.GetValueDidChange():
+            self.child_thread_plan = self.queue_child_thread_plan()
+            return False
+        else:
+            return True

lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py

 """
 Tests stepping with scripted thread plans.
 """
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
 from lldbsuite.test.lldbtest import *
 
 class StepScriptedTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     NO_DEBUG_INFO_TESTCASE = True
 
+    def setUp(self):
+        TestBase.setUp(self)
+        self.main_source_file = lldb.SBFileSpec("main.c")
+        self.runCmd("command script import Steps.py")
+
     def test_standard_step_out(self):
-        """Tests stepping with the scripted thread plan laying over a standard thread plan for stepping out."""
+        """Tests stepping with the scripted thread plan laying over a standard 
+        thread plan for stepping out."""
         self.build()
-        self.main_source_file = lldb.SBFileSpec("main.c")
         self.step_out_with_scripted_plan("Steps.StepOut")
 
     def test_scripted_step_out(self):
-        """Tests stepping with the scripted thread plan laying over an another scripted thread plan for stepping out."""
+        """Tests stepping with the scripted thread plan laying over an another 
+        scripted thread plan for stepping out."""
         self.build()
-        self.main_source_file = lldb.SBFileSpec("main.c")
         self.step_out_with_scripted_plan("Steps.StepScripted")
 
-    def setUp(self):
-        TestBase.setUp(self)
-        self.main_source_file = lldb.SBFileSpec("main.c")
-        self.runCmd("command script import Steps.py")
-
     def step_out_with_scripted_plan(self, name):
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
                                                                             "Set a breakpoint here",
                                                                             self.main_source_file)
 
         frame = thread.GetFrameAtIndex(0)
         self.assertEqual("foo", frame.GetFunctionName())
 
         err = thread.StepUsingScriptedThreadPlan(name)
         self.assertTrue(err.Success(), err.GetCString())
 
         frame = thread.GetFrameAtIndex(0)
         self.assertEqual("main", frame.GetFunctionName())
 
+
     def test_misspelled_plan_name(self):
         """Test that we get a useful error if we misspell the plan class name"""
         self.build()
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
                                                                             "Set a breakpoint here",
                                                                             self.main_source_file)
         stop_id = process.GetStopID()
         # Pass a non-existent class for the plan class:
         err = thread.StepUsingScriptedThreadPlan("NoSuchModule.NoSuchPlan")
         
         # Make sure we got a good error:
         self.assertTrue(err.Fail(), "We got a failure state")
         msg = err.GetCString()
         self.assertTrue("NoSuchModule.NoSuchPlan" in msg, "Mentioned missing class")
         
         # Make sure we didn't let the process run:
         self.assertEqual(stop_id, process.GetStopID(), "Process didn't run")
+        
+    def test_checking_variable(self):
+        """Test that we can call SBValue API's from a scripted thread plan"""
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                                                            "Set a breakpoint here",
+                                                                            self.main_source_file)
+
+        frame = thread.GetFrameAtIndex(0)
+        self.assertEqual("foo", frame.GetFunctionName())
+        foo_val = frame.FindVariable("foo")
+        self.assertTrue(foo_val.GetError().Success(), "Got the foo variable")
+        self.assertEqual(foo_val.GetValueAsUnsigned(), 10, "foo starts at 10")
+
+        err = thread.StepUsingScriptedThreadPlan("Steps.StepUntil")
+        self.assertTrue(err.Success(), err.GetCString())
+
+        # We should not have exited:
+        self.assertEqual(process.GetState(), lldb.eStateStopped, "We are stopped")
+        
+        # We should still be in foo:
+        self.assertEqual("foo", frame.GetFunctionName())
+
+        # And foo should have changed:
+        self.assertTrue(foo_val.GetValueDidChange(), "Foo changed")

lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c

 #include <stdio.h>
 
 void foo() {
-  printf("Set a breakpoint here.\n");
+  int foo = 10; 
+  printf("%d\n", foo); // Set a breakpoint here. 
+  foo = 20;
+  printf("%d\n", foo);
 }
 
 int main() {
   foo();
   return 0;
 }

lldb/trunk/source/Target/Process.cpp

 
 ProcessRunLock &Process::GetRunLock() {
   if (m_private_state_thread.EqualsThread(Host::GetCurrentThread()))
     return m_private_run_lock;
   else
     return m_public_run_lock;
 }
 
+bool Process::CurrentThreadIsPrivateStateThread()
+{
+  return m_private_state_thread.EqualsThread(Host::GetCurrentThread());
+}
+
+
 void Process::Flush() {
   m_thread_list.Flush();
   m_extended_thread_list.Flush();
   m_extended_thread_stop_id = 0;
   m_queue_list.Clear();
   m_queue_list_stop_id = 0;
 }
 

lldb/trunk/source/Target/Target.cpp

 ModuleList
 Target::TargetEventData::GetModuleListFromEvent(const Event *event_ptr) {
   ModuleList module_list;
   const TargetEventData *event_data = GetEventDataFromEvent(event_ptr);
   if (event_data)
     module_list = event_data->m_module_list;
   return module_list;
 }
+
+std::recursive_mutex &Target::GetAPIMutex() { 
+  if (GetProcessSP() && GetProcessSP()->CurrentThreadIsPrivateStateThread())
+    return m_private_mutex;
+  else
+    return m_mutex;
+}