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 &Target::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 <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() {
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)
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()
@@ -60,3 +61,28 @@
# 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")
Index: lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py
+++ lldb/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")
+ 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
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -535,7 +535,7 @@
static const lldb::TargetPropertiesSP &GetGlobalProperties();
- std::recursive_mutex &GetAPIMutex() { return m_mutex; }
+ std::recursive_mutex &GetAPIMutex();
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/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2272,6 +2272,8 @@
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.");
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
