[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex

2019-09-30 Thread Jim Ingham via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL373280: Allow the internal-state-thread free access to the 
TargetAPI mutex. (authored by jingham, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D68174?vs=74=222527#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68174/new/

https://reviews.llvm.org/D68174

Files:
  lldb/trunk/include/lldb/Target/Process.h
  lldb/trunk/include/lldb/Target/Target.h
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
  
lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
  lldb/trunk/source/Target/Process.cpp
  lldb/trunk/source/Target/Target.cpp

Index: lldb/trunk/source/Target/Target.cpp
===
--- lldb/trunk/source/Target/Target.cpp
+++ lldb/trunk/source/Target/Target.cpp
@@ -4129,3 +4129,10 @@
 module_list = event_data->m_module_list;
   return module_list;
 }
+
+std::recursive_mutex ::GetAPIMutex() { 
+  if (GetProcessSP() && GetProcessSP()->CurrentThreadIsPrivateStateThread())
+return m_private_mutex;
+  else
+return m_mutex;
+}
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -5538,6 +5538,12 @@
 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();
Index: lldb/trunk/include/lldb/Target/Target.h
===
--- lldb/trunk/include/lldb/Target/Target.h
+++ lldb/trunk/include/lldb/Target/Target.h
@@ -535,7 +535,7 @@
 
   static const lldb::TargetPropertiesSP ();
 
-  std::recursive_mutex () { return m_mutex; }
+  std::recursive_mutex ();
 
   void DeleteCurrentProcess();
 
@@ -1288,6 +1288,12 @@
   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).
Index: lldb/trunk/include/lldb/Target/Process.h
===
--- lldb/trunk/include/lldb/Target/Process.h
+++ lldb/trunk/include/lldb/Target/Process.h
@@ -2272,6 +2272,8 @@
   void ClearPreResumeAction(PreResumeActionCallback callback, void *baton);
 
   ProcessRunLock ();
+  
+  bool CurrentThreadIsPrivateStateThread();
 
   virtual Status SendEventData(const char *data) {
 Status return_error("Sending an event is not supported for this process.");
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
@@ -1,7 +1,10 @@
 #include 
 
 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() {
Index: lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
@@ -35,3 +35,38 @@
 
 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")
+

[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex

2019-09-27 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

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.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68174/new/

https://reviews.llvm.org/D68174



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D68174: Allow private-state-thread to call SB API's that take the TargetAPI mutex

2019-09-27 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision.
jingham added a reviewer: clayborg.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

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).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68174

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
  
lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -4129,3 +4129,10 @@
 module_list = event_data->m_module_list;
   return module_list;
 }
+
+std::recursive_mutex ::GetAPIMutex() { 
+  if (GetProcessSP() && GetProcessSP()->CurrentThreadIsPrivateStateThread())
+return m_private_mutex;
+  else
+return m_mutex;
+}
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -5538,6 +5538,12 @@
 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();
Index: lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
===
--- lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
+++ lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/main.c
@@ -1,7 +1,10 @@
 #include 
 
 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() {
Index: lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
===
--- lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
@@ -12,23 +12,23 @@
 
 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",
@@ -43,6 +43,7 @@
 frame = thread.GetFrameAtIndex(0)