jingham created this revision. jingham added a reviewer: friss. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. jingham requested review of this revision. Herald added a subscriber: JDevlieghere.
There are some platforms (the xnu kernel being one) where trying to run just one thread can cause problems, and you need to always run all threads whenever you do more than a single step-i. This patch adds a target.process.run-all-threads setting which overrides the default "run-mode" value for all the stepping commands. I also needed to come up with a way to test this. The easiest way to do that was to use a scripted thread plan so that I could check the incoming value. But to do that I needed to plumb the stop others through the scripted-step & the ThreadPlanPython, which this patch also does. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D85265 Files: lldb/bindings/interface/SBThreadPlan.i lldb/include/lldb/API/SBThreadPlan.h lldb/include/lldb/Target/Process.h lldb/include/lldb/Target/ThreadPlanPython.h lldb/source/API/SBThreadPlan.cpp lldb/source/Commands/CommandObjectThread.cpp lldb/source/Target/Process.cpp lldb/source/Target/TargetProperties.td lldb/source/Target/Thread.cpp lldb/source/Target/ThreadPlanPython.cpp lldb/test/API/functionalities/step_scripted/Steps.py lldb/test/API/functionalities/step_scripted/TestStepScripted.py
Index: lldb/test/API/functionalities/step_scripted/TestStepScripted.py =================================================================== --- lldb/test/API/functionalities/step_scripted/TestStepScripted.py +++ lldb/test/API/functionalities/step_scripted/TestStepScripted.py @@ -1,7 +1,7 @@ """ Tests stepping with scripted thread plans. """ - +import threading import lldb import lldbsuite.test.lldbutil as lldbutil from lldbsuite.test.decorators import * @@ -111,3 +111,58 @@ # And foo should have changed: self.assertTrue(foo_val.GetValueDidChange(), "Foo changed") + + def test_stop_others_from_command(self): + """Test that the stop-others flag is set correctly by the command line. + Also test that the run-all-threads property overrides this.""" + self.do_test_stop_others() + + def run_step(self, stop_others_value, run_mode, token): + import Steps + interp = self.dbg.GetCommandInterpreter() + result = lldb.SBCommandReturnObject() + + cmd = "thread step-scripted -C Steps.StepReportsStopOthers -k token -v %s"%(token) + if run_mode != None: + cmd = cmd + " --run-mode %s"%(run_mode) + print(cmd) + interp.HandleCommand(cmd, result) + self.assertTrue(result.Succeeded(), "Step scripted failed: %s."%(result.GetError())) + print(Steps.StepReportsStopOthers.stop_mode_dict) + value = Steps.StepReportsStopOthers.stop_mode_dict[token] + self.assertEqual(value, stop_others_value, "Stop others has the correct value.") + + def do_test_stop_others(self): + self.build() + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, + "Set a breakpoint here", + self.main_source_file) + # First run with stop others false and see that we got that. + thread_id = "" + if sys.version_info.major == 2: + thread_id = str(threading._get_ident()) + else: + thread_id = str(threading.get_ident()) + + # all-threads should set stop others to False. + self.run_step(False, "all-threads", thread_id) + + # this-thread should set stop others to True + self.run_step(True, "this-thread", thread_id) + + # The default value should be stop others: + self.run_step(True, None, thread_id) + + # The target.process.run-all-threads should override this: + interp = self.dbg.GetCommandInterpreter() + result = lldb.SBCommandReturnObject() + + interp.HandleCommand("settings set target.process.run-all-threads true", result) + self.assertTrue(result.Succeeded, "setting run-all-threads works.") + + self.run_step(False, None, thread_id) + + + + + Index: lldb/test/API/functionalities/step_scripted/Steps.py =================================================================== --- lldb/test/API/functionalities/step_scripted/Steps.py +++ lldb/test/API/functionalities/step_scripted/Steps.py @@ -75,9 +75,29 @@ 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 + +# This plan does nothing, but sets stop_mode to the +# value of GetStopOthers for this plan. +class StepReportsStopOthers(): + stop_mode_dict = {} + + def __init__(self, thread_plan, args_data, dict): + self.thread_plan = thread_plan + self.key = args_data.GetValueForKey("token").GetStringValue(1000) + + def should_stop(self, event): + self.thread_plan.SetPlanComplete(True) + print("Called in should_stop") + StepReportsStopOthers.stop_mode_dict[self.key] = self.thread_plan.GetStopOthers() + return True + + def should_step(self): + return True + + def explains_stop(self, event): + return True Index: lldb/source/Target/ThreadPlanPython.cpp =================================================================== --- lldb/source/Target/ThreadPlanPython.cpp +++ lldb/source/Target/ThreadPlanPython.cpp @@ -29,7 +29,8 @@ StructuredDataImpl *args_data) : ThreadPlan(ThreadPlan::eKindPython, "Python based Thread Plan", thread, eVoteNoOpinion, eVoteNoOpinion), - m_class_name(class_name), m_args_data(args_data), m_did_push(false) { + m_class_name(class_name), m_args_data(args_data), m_did_push(false), + m_stop_others(false) { SetIsMasterPlan(true); SetOkayToDiscard(true); SetPrivate(false); @@ -162,13 +163,6 @@ } // The ones below are not currently exported to Python. - -bool ThreadPlanPython::StopOthers() { - // For now Python plans run all threads, but we should add some controls for - // this. - return false; -} - void ThreadPlanPython::GetDescription(Stream *s, lldb::DescriptionLevel level) { s->Printf("Python thread plan implemented by class %s.", m_class_name.c_str()); Index: lldb/source/Target/Thread.cpp =================================================================== --- lldb/source/Target/Thread.cpp +++ lldb/source/Target/Thread.cpp @@ -1380,7 +1380,7 @@ ThreadPlanSP thread_plan_sp(new ThreadPlanPython(*this, class_name, extra_args_impl)); - + thread_plan_sp->SetStopOthers(stop_other_threads); status = QueueThreadPlan(thread_plan_sp, abort_other_plans); return thread_plan_sp; } Index: lldb/source/Target/TargetProperties.td =================================================================== --- lldb/source/Target/TargetProperties.td +++ lldb/source/Target/TargetProperties.td @@ -214,6 +214,9 @@ def UtilityExpressionTimeout: Property<"utility-expression-timeout", "UInt64">, DefaultUnsignedValue<15>, Desc<"The time in seconds to wait for LLDB-internal utility expressions.">; + def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">, + DefaultFalse, + Desc<"If true, stepping operations will run all threads. This is equivalent to setting the run-mode option to 'all-threads'.">; } let Definition = "platform" in { Index: lldb/source/Target/Process.cpp =================================================================== --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -278,6 +278,12 @@ return std::chrono::seconds(value); } +bool ProcessProperties::GetSteppingRunsAllThreads() const { + const uint32_t idx = ePropertySteppingRunsAllThreads; + return m_collection_sp->GetPropertyAtIndexAsBoolean( + nullptr, idx, g_process_properties[idx].default_uint_value != 0); +} + bool ProcessProperties::GetOSPluginReportsAllThreads() const { const bool fail_value = true; const Property *exp_property = Index: lldb/source/Commands/CommandObjectThread.cpp =================================================================== --- lldb/source/Commands/CommandObjectThread.cpp +++ lldb/source/Commands/CommandObjectThread.cpp @@ -482,9 +482,20 @@ // Check if we are in Non-Stop mode TargetSP target_sp = execution_context ? execution_context->GetTargetSP() : TargetSP(); - if (target_sp && target_sp->GetNonStopModeEnabled()) + if (target_sp && target_sp->GetNonStopModeEnabled()) { + // NonStopMode runs all threads down in the ProcessPlugin layer, but + // at this level we need to pretend we are actually only running this + // thread. So functionally it does the same thing as + // GetSteppingRunsAllThreads. So it overrides the runs all threads + // setting. m_run_mode = eOnlyThisThread; - + } else { + ProcessSP process_sp = + execution_context ? execution_context->GetProcessSP() : ProcessSP(); + if (process_sp && process_sp->GetSteppingRunsAllThreads()) + m_run_mode = eAllThreads; + } + m_avoid_regexp.clear(); m_step_in_target.clear(); m_step_count = 1; @@ -612,8 +623,7 @@ if (m_options.m_run_mode == eAllThreads) bool_stop_other_threads = false; else if (m_options.m_run_mode == eOnlyDuringStepping) - bool_stop_other_threads = - (m_step_type != eStepTypeOut && m_step_type != eStepTypeScripted); + bool_stop_other_threads = (m_step_type != eStepTypeOut); else bool_stop_other_threads = true; Index: lldb/source/API/SBThreadPlan.cpp =================================================================== --- lldb/source/API/SBThreadPlan.cpp +++ lldb/source/API/SBThreadPlan.cpp @@ -196,6 +196,23 @@ return false; } +bool SBThreadPlan::GetStopOthers() { + LLDB_RECORD_METHOD_NO_ARGS(bool, SBThreadPlan, GetStopOthers); + + ThreadPlanSP thread_plan_sp(GetSP()); + if (thread_plan_sp) + return thread_plan_sp->StopOthers(); + return false; +} + +void SBThreadPlan::SetStopOthers(bool stop_others) { + LLDB_RECORD_METHOD(void, SBThreadPlan, SetStopOthers, (bool), stop_others); + + ThreadPlanSP thread_plan_sp(GetSP()); + if (thread_plan_sp) + thread_plan_sp->SetStopOthers(stop_others); +} + // This section allows an SBThreadPlan to push another of the common types of // plans... // @@ -463,6 +480,8 @@ LLDB_REGISTER_METHOD(bool, SBThreadPlan, IsPlanComplete, ()); LLDB_REGISTER_METHOD(bool, SBThreadPlan, IsPlanStale, ()); LLDB_REGISTER_METHOD(bool, SBThreadPlan, IsValid, ()); + LLDB_REGISTER_METHOD(void, SBThreadPlan, SetStopOthers, (bool)); + LLDB_REGISTER_METHOD(bool, SBThreadPlan, GetStopOthers, ()); LLDB_REGISTER_METHOD(lldb::SBThreadPlan, SBThreadPlan, QueueThreadPlanForStepOverRange, (lldb::SBAddress &, lldb::addr_t)); Index: lldb/include/lldb/Target/ThreadPlanPython.h =================================================================== --- lldb/include/lldb/Target/ThreadPlanPython.h +++ lldb/include/lldb/Target/ThreadPlanPython.h @@ -45,7 +45,13 @@ bool WillStop() override; - bool StopOthers() override; + bool StopOthers() override { + return m_stop_others; + } + + void SetStopOthers(bool new_value) { + m_stop_others = new_value; + } void DidPush() override; @@ -67,6 +73,7 @@ std::string m_error_str; StructuredData::ObjectSP m_implementation_sp; bool m_did_push; + bool m_stop_others; ThreadPlanPython(const ThreadPlanPython &) = delete; const ThreadPlanPython &operator=(const ThreadPlanPython &) = delete; Index: lldb/include/lldb/Target/Process.h =================================================================== --- lldb/include/lldb/Target/Process.h +++ lldb/include/lldb/Target/Process.h @@ -91,6 +91,7 @@ std::chrono::seconds GetUtilityExpressionTimeout() const; bool GetOSPluginReportsAllThreads() const; void SetOSPluginReportsAllThreads(bool does_report); + bool GetSteppingRunsAllThreads() const; protected: Process *m_process; // Can be nullptr for global ProcessProperties Index: lldb/include/lldb/API/SBThreadPlan.h =================================================================== --- lldb/include/lldb/API/SBThreadPlan.h +++ lldb/include/lldb/API/SBThreadPlan.h @@ -76,6 +76,10 @@ bool IsPlanStale(); bool IsValid(); + + bool GetStopOthers(); + + void SetStopOthers(bool stop_others); // This section allows an SBThreadPlan to push another of the common types of // plans... Index: lldb/bindings/interface/SBThreadPlan.i =================================================================== --- lldb/bindings/interface/SBThreadPlan.i +++ lldb/bindings/interface/SBThreadPlan.i @@ -92,6 +92,14 @@ bool IsPlanStale(); + %feature("docstring", "Return whether this plan will ask to stop other threads when it runs.") GetStopOthers; + bool + GetStopOthers(); + + %feature("docstring", "Set whether this plan will ask to stop other threads when it runs.") GetStopOthers; + void + SetStopOthers(bool stop_others); + SBThreadPlan QueueThreadPlanForStepOverRange (SBAddress &start_address, lldb::addr_t range_size);
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits