fallkrum updated this revision to Diff 270176. fallkrum added a comment. Removed _ptr from found_valid_stopinfo_ptr.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80112/new/ https://reviews.llvm.org/D80112 Files: lldb/include/lldb/Target/Process.h lldb/source/Target/Process.cpp lldb/test/API/functionalities/thread/ignore_suspended/Makefile lldb/test/API/functionalities/thread/ignore_suspended/TestIgnoreSuspendedThread.py lldb/test/API/functionalities/thread/ignore_suspended/main.cpp lldb/unittests/CMakeLists.txt lldb/unittests/Process/CMakeLists.txt lldb/unittests/Process/ProcessEventDataTest.cpp lldb/unittests/Thread/CMakeLists.txt lldb/unittests/Thread/ThreadTest.cpp
Index: lldb/unittests/Thread/ThreadTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Thread/ThreadTest.cpp @@ -0,0 +1,168 @@ +//===-- ThreadTest.cpp ------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Target/Thread.h" +#include "Plugins/Platform/Linux/PlatformLinux.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Target/Process.h" +#include "lldb/Target/StopInfo.h" +#include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/Reproducer.h" +#include "gtest/gtest.h" + +using namespace lldb_private; +using namespace lldb_private::repro; +using namespace lldb; + +namespace { +class ThreadTest : public ::testing::Test { +public: + void SetUp() override { + llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None)); + FileSystem::Initialize(); + HostInfo::Initialize(); + platform_linux::PlatformLinux::Initialize(); + } + void TearDown() override { + platform_linux::PlatformLinux::Terminate(); + HostInfo::Terminate(); + FileSystem::Terminate(); + Reproducer::Terminate(); + } +}; + +class DummyProcess : public Process { +public: + using Process::Process; + + virtual bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) { + return true; + } + virtual Status DoDestroy() { return {}; } + virtual void RefreshStateAfterStop() {} + virtual size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, + Status &error) { + return 0; + } + virtual bool UpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { + return false; + } + virtual ConstString GetPluginName() { return ConstString("Dummy"); } + virtual uint32_t GetPluginVersion() { return 0; } + + ProcessModID &GetModIDNonConstRef() { return m_mod_id; } +}; + +class DummyThread : public Thread { +public: + using Thread::Thread; + + ~DummyThread() override { DestroyThread(); } + + void RefreshStateAfterStop() override {} + + lldb::RegisterContextSP GetRegisterContext() override { return nullptr; } + + lldb::RegisterContextSP + CreateRegisterContextForFrame(StackFrame *frame) override { + return nullptr; + } + + bool CalculateStopInfo() override { return false; } + + bool IsStillAtLastBreakpointHit() override { return true; } +}; +} // namespace + +TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) { + Status error; + PlatformSP platform_sp; + TargetSP target_sp; + error = debugger_sp->GetTargetList().CreateTarget( + *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp); + + if (target_sp) { + debugger_sp->GetTargetList().SetSelectedTarget(target_sp.get()); + } + + return target_sp; +} + +TEST_F(ThreadTest, SetStopInfo) { + ArchSpec arch("powerpc64-pc-linux"); + + Platform::SetHostPlatform( + platform_linux::PlatformLinux::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared<DummyProcess>(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + DummyProcess *process = static_cast<DummyProcess *>(process_sp.get()); + + ThreadSP thread_sp = std::make_shared<DummyThread>(*process_sp.get(), 0); + ASSERT_TRUE(thread_sp); + + StopInfoSP stopinfo_sp = + StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp.get(), 0); + ASSERT_TRUE(stopinfo_sp->IsValid() == true); + + /* + Should make stopinfo valid. + */ + process->GetModIDNonConstRef().BumpStopID(); + ASSERT_TRUE(stopinfo_sp->IsValid() == false); + + thread_sp->SetStopInfo(stopinfo_sp); + ASSERT_TRUE(stopinfo_sp->IsValid() == true); +} + +TEST_F(ThreadTest, GetPrivateStopInfo) { + ArchSpec arch("powerpc64-pc-linux"); + + Platform::SetHostPlatform( + platform_linux::PlatformLinux::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared<DummyProcess>(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + DummyProcess *process = static_cast<DummyProcess *>(process_sp.get()); + + ThreadSP thread_sp = std::make_shared<DummyThread>(*process_sp.get(), 0); + ASSERT_TRUE(thread_sp); + + StopInfoSP stopinfo_sp = + StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp.get(), 0); + ASSERT_TRUE(stopinfo_sp); + + thread_sp->SetStopInfo(stopinfo_sp); + + /* + Should make stopinfo valid if thread is at last breakpoint hit. + */ + process->GetModIDNonConstRef().BumpStopID(); + ASSERT_TRUE(stopinfo_sp->IsValid() == false); + StopInfoSP new_stopinfo_sp = thread_sp->GetPrivateStopInfo(); + ASSERT_TRUE(new_stopinfo_sp && stopinfo_sp->IsValid() == true); +} Index: lldb/unittests/Thread/CMakeLists.txt =================================================================== --- /dev/null +++ lldb/unittests/Thread/CMakeLists.txt @@ -0,0 +1,15 @@ +add_lldb_unittest(ThreadTest + ThreadTest.cpp + + LINK_LIBS + lldbCore + lldbHost + lldbTarget + lldbSymbol + lldbUtility + lldbUtilityHelpers + lldbInterpreter + lldbBreakpoint + lldbPluginPlatformLinux + ) + Index: lldb/unittests/Process/ProcessEventDataTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Process/ProcessEventDataTest.cpp @@ -0,0 +1,258 @@ +//===-- ProcessEventDataTest.cpp ------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Plugins/Platform/MacOSX/PlatformMacOSX.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Target/Process.h" +#include "lldb/Target/StopInfo.h" +#include "lldb/Target/Thread.h" +#include "lldb/Utility/ArchSpec.h" +#include "lldb/Utility/Event.h" +#include "lldb/Utility/Reproducer.h" +#include "gtest/gtest.h" + +using namespace lldb_private; +using namespace lldb_private::repro; +using namespace lldb; + +namespace { +class ProcessEventDataTest : public ::testing::Test { +public: + void SetUp() override { + llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None)); + FileSystem::Initialize(); + HostInfo::Initialize(); + PlatformMacOSX::Initialize(); + } + void TearDown() override { + PlatformMacOSX::Terminate(); + HostInfo::Terminate(); + FileSystem::Terminate(); + Reproducer::Terminate(); + } +}; + +class DummyProcess : public Process { +public: + using Process::Process; + + virtual bool CanDebug(lldb::TargetSP target, bool plugin_specified_by_name) { + return true; + } + virtual Status DoDestroy() { return {}; } + virtual void RefreshStateAfterStop() {} + virtual size_t DoReadMemory(lldb::addr_t vm_addr, void *buf, size_t size, + Status &error) { + return 0; + } + virtual bool UpdateThreadList(ThreadList &old_thread_list, + ThreadList &new_thread_list) { + return false; + } + virtual ConstString GetPluginName() { return ConstString("Dummy"); } + virtual uint32_t GetPluginVersion() { return 0; } + + ProcessModID &GetModIDNonConstRef() { return m_mod_id; } +}; + +class DummyThread : public Thread { +public: + using Thread::Thread; + + ~DummyThread() override { DestroyThread(); } + + void RefreshStateAfterStop() override {} + + lldb::RegisterContextSP GetRegisterContext() override { return nullptr; } + + lldb::RegisterContextSP + CreateRegisterContextForFrame(StackFrame *frame) override { + return nullptr; + } + + bool CalculateStopInfo() override { return false; } +}; + +class DummyStopInfo : public StopInfo { +public: + DummyStopInfo(Thread &thread, uint64_t value) + : StopInfo(thread, value), m_should_stop(true), + m_stop_reason(eStopReasonBreakpoint) {} + + bool ShouldStop(Event *event_ptr) override { return m_should_stop; } + + StopReason GetStopReason() const override { return m_stop_reason; } + + bool m_should_stop; + StopReason m_stop_reason; +}; + +class DummyProcessEventData : public Process::ProcessEventData { +public: + DummyProcessEventData(ProcessSP &process_sp, StateType state) + : ProcessEventData(process_sp, state), m_should_stop_hit_count(0) {} + bool ShouldStop(Event *event_ptr, bool &found_valid_stopinfo) override { + m_should_stop_hit_count++; + return false; + } + + int m_should_stop_hit_count; +}; +} // namespace + +typedef std::shared_ptr<Process::ProcessEventData> ProcessEventDataSP; +typedef std::shared_ptr<Event> EventSP; + +TargetSP CreateTarget(DebuggerSP &debugger_sp, ArchSpec &arch) { + Status error; + PlatformSP platform_sp; + TargetSP target_sp; + error = debugger_sp->GetTargetList().CreateTarget( + *debugger_sp, "", arch, eLoadDependentsNo, platform_sp, target_sp); + + if (target_sp) { + debugger_sp->GetTargetList().SetSelectedTarget(target_sp.get()); + } + + return target_sp; +} + +ThreadSP CreateThread(ProcessSP &process_sp, bool should_stop, + bool has_valid_stopinfo) { + ThreadSP thread_sp = std::make_shared<DummyThread>(*process_sp.get(), 0); + if (thread_sp == nullptr) { + return nullptr; + } + + if (has_valid_stopinfo) { + StopInfoSP stopinfo_sp = + std::make_shared<DummyStopInfo>(*thread_sp.get(), 0); + static_cast<DummyStopInfo *>(stopinfo_sp.get())->m_should_stop = + should_stop; + if (stopinfo_sp == nullptr) { + return nullptr; + } + + thread_sp->SetStopInfo(stopinfo_sp); + } + + process_sp->GetThreadList().AddThread(thread_sp); + + return thread_sp; +} + +TEST_F(ProcessEventDataTest, DoOnRemoval) { + ArchSpec arch("x86_64-apple-macosx-"); + + Platform::SetHostPlatform(PlatformMacOSX::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared<DummyProcess>(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + /* + Should hit ShouldStop if state is eStateStopped + */ + ProcessEventDataSP event_data_sp = + std::make_shared<DummyProcessEventData>(process_sp, eStateStopped); + EventSP event_sp = std::make_shared<Event>(0, event_data_sp); + event_data_sp->SetUpdateStateOnRemoval(event_sp.get()); + event_data_sp->DoOnRemoval(event_sp.get()); + bool result = static_cast<DummyProcessEventData *>(event_data_sp.get()) + ->m_should_stop_hit_count == 1; + ASSERT_TRUE(result); + + /* + Should not hit ShouldStop if state is not eStateStopped + */ + event_data_sp = + std::make_shared<DummyProcessEventData>(process_sp, eStateStepping); + event_sp = std::make_shared<Event>(0, event_data_sp); + event_data_sp->SetUpdateStateOnRemoval(event_sp.get()); + event_data_sp->DoOnRemoval(event_sp.get()); + result = static_cast<DummyProcessEventData *>(event_data_sp.get()) + ->m_should_stop_hit_count == 0; + ASSERT_TRUE(result); +} + +TEST_F(ProcessEventDataTest, ShouldStop) { + ArchSpec arch("x86_64-apple-macosx-"); + + Platform::SetHostPlatform(PlatformMacOSX::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + + ListenerSP listener_sp(Listener::MakeListener("dummy")); + ProcessSP process_sp = std::make_shared<DummyProcess>(target_sp, listener_sp); + ASSERT_TRUE(process_sp); + + // wants to stop and has valid StopInfo + ThreadSP thread_sp = CreateThread(process_sp, true, true); + + ProcessEventDataSP event_data_sp = + std::make_shared<Process::ProcessEventData>(process_sp, eStateStopped); + EventSP event_sp = std::make_shared<Event>(0, event_data_sp); + /* + Should stop if thread has valid StopInfo and not suspended + */ + bool found_valid_stopinfo = false; + bool should_stop = + event_data_sp->ShouldStop(event_sp.get(), found_valid_stopinfo); + ASSERT_TRUE(should_stop == true && found_valid_stopinfo == true); + + /* + Should not stop if thread has valid StopInfo but was suspended + */ + found_valid_stopinfo = false; + thread_sp->SetResumeState(eStateSuspended); + should_stop = + event_data_sp->ShouldStop(event_sp.get(), found_valid_stopinfo); + ASSERT_TRUE(should_stop == false && found_valid_stopinfo == false); + + /* + Should not stop, thread-reason of stop does not want to stop in its + callback and suspended thread who wants to (from previous stop) + must be ignored. + */ + event_data_sp = + std::make_shared<Process::ProcessEventData>(process_sp, eStateStopped); + event_sp = std::make_shared<Event>(0, event_data_sp); + process_sp->GetThreadList().Clear(); + + for (int i = 0; i < 6; i++) { + if (i == 2) { + // Does not want to stop but has valid StopInfo + thread_sp = CreateThread(process_sp, false, true); + } else if (i == 5) { + // Wants to stop and has valid StopInfo + thread_sp = CreateThread(process_sp, true, true); + thread_sp->SetResumeState(eStateSuspended); + } else { + // Thread has no StopInfo i.e is not the reason of stop + thread_sp = CreateThread(process_sp, false, false); + } + } + ASSERT_TRUE(process_sp->GetThreadList().GetSize() == 6); + + found_valid_stopinfo = false; + should_stop = + event_data_sp->ShouldStop(event_sp.get(), found_valid_stopinfo); + ASSERT_TRUE(should_stop == false && found_valid_stopinfo == true); +} Index: lldb/unittests/Process/CMakeLists.txt =================================================================== --- lldb/unittests/Process/CMakeLists.txt +++ lldb/unittests/Process/CMakeLists.txt @@ -4,3 +4,17 @@ add_subdirectory(POSIX) endif() add_subdirectory(minidump) + +add_lldb_unittest(ProcessEventDataTest + ProcessEventDataTest.cpp + + LINK_LIBS + lldbCore + lldbHost + lldbTarget + lldbSymbol + lldbUtility + lldbUtilityHelpers + lldbInterpreter + lldbPluginPlatformMacOSX + ) Index: lldb/unittests/CMakeLists.txt =================================================================== --- lldb/unittests/CMakeLists.txt +++ lldb/unittests/CMakeLists.txt @@ -80,6 +80,7 @@ add_subdirectory(tools) add_subdirectory(UnwindAssembly) add_subdirectory(Utility) +add_subdirectory(Thread) if(LLDB_CAN_USE_DEBUGSERVER AND LLDB_TOOL_DEBUGSERVER_BUILD AND NOT LLDB_USE_SYSTEM_DEBUGSERVER) add_subdirectory(debugserver) Index: lldb/test/API/functionalities/thread/ignore_suspended/main.cpp =================================================================== --- /dev/null +++ lldb/test/API/functionalities/thread/ignore_suspended/main.cpp @@ -0,0 +1,35 @@ +// Test simulates situation when suspended thread could stop process +// where thread that is a real reason of stop says process +// should not stop in it's action handler. + +#include <chrono> +#include <thread> + +void thread1() { + // Will be suspended at breakpoint stop + // Set first breakpoint here +} + +void thread2() { + /* + Prevent threads from stopping simultaneously + */ + std::this_thread::sleep_for(std::chrono::seconds(1)); + // Set second breakpoint here +} + +int main() { + // Create a thread + std::thread thread_1(thread1); + + // Create another thread. + std::thread thread_2(thread2); + + // Wait for the thread that was not suspeneded + thread_2.join(); + + // Wait for thread that was suspended + thread_1.join(); // Set third breakpoint here + + return 0; +} Index: lldb/test/API/functionalities/thread/ignore_suspended/TestIgnoreSuspendedThread.py =================================================================== --- /dev/null +++ lldb/test/API/functionalities/thread/ignore_suspended/TestIgnoreSuspendedThread.py @@ -0,0 +1,94 @@ +""" +Test that suspended threads do not affect should-stop decisions. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +import lldbsuite.test.lldbutil as lldbutil + + +class IgnoreSuspendedThreadTestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + NO_DEBUG_INFO_TESTCASE = True + + def setUp(self): + #Call super's setUp(). + TestBase.setUp(self) + #Find the line numbers for our breakpoints. + self.break_1 = line_number('main.cpp', '// Set first breakpoint here') + self.break_2 = line_number('main.cpp', '// Set second breakpoint here') + self.break_3 = line_number('main.cpp', '// Set third breakpoint here') + + def printThreadsStoppedByBreakpoint(self, process): + stopped_threads = \ + lldbutil.get_stopped_threads(process, lldb.eStopReasonBreakpoint) + for thread in stopped_threads: + break_id = thread.GetStopReasonDataAtIndex(0) + print('Thread ' + str(thread.GetThreadID()) + \ + ' stopped at breakpoint ' + str(break_id)) + + def test(self): + self.build() + target = lldbutil.run_to_breakpoint_make_target(self) + + #This should create a breakpoint with 1 location. + bp1_id = \ + lldbutil.run_break_set_by_file_and_line(self, + "main.cpp", + self.break_1, + num_expected_locations=1) + + bp2_id = \ + lldbutil.run_break_set_by_file_and_line(self, + "main.cpp", + self.break_2, + num_expected_locations=1) + + bp3_id = \ + lldbutil.run_break_set_by_file_and_line(self, + "main.cpp", + self.break_3, + num_expected_locations=1) + + #Run the program. + self.runCmd("run", RUN_SUCCEEDED) + #Get the target process + process = target.GetProcess() + + if self.TraceOn(): + print('First stop:') + self.printThreadsStoppedByBreakpoint(process) + + thread_to_suspend = \ + lldbutil.get_one_thread_stopped_at_breakpoint_id(process, + bp1_id) + self.assertIsNotNone(thread_to_suspend, "Should hit breakpoint 1") + thread_to_suspend.Suspend() + + #Do not stop at bp2 and autocontinue to bp3 + target.FindBreakpointByID(bp2_id).SetAutoContinue(True) + + #Run to the third breakpoint + self.runCmd("continue") + + if self.TraceOn(): + print('Second stop:') + self.printThreadsStoppedByBreakpoint(process) + + stopped_thread = \ + lldbutil.get_one_thread_stopped_at_breakpoint_id(process, + bp3_id) + self.assertIsNotNone(stopped_thread, + "Should hit breakpoint 3") + + thread_to_suspend.Resume() + + #Run to completion + self.runCmd("continue") + + #At this point, the inferior process should have exited. + self.assertEqual(process.GetState(), + lldb.eStateExited, + PROCESS_EXITED) Index: lldb/test/API/functionalities/thread/ignore_suspended/Makefile =================================================================== --- /dev/null +++ lldb/test/API/functionalities/thread/ignore_suspended/Makefile @@ -0,0 +1,3 @@ +ENABLE_THREADS := YES +CXX_SOURCES := main.cpp +include Makefile.rules Index: lldb/source/Target/Process.cpp =================================================================== --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -3940,6 +3940,115 @@ return ProcessEventData::GetFlavorString(); } +bool Process::ProcessEventData::ShouldStop(Event *event_ptr, + bool &found_valid_stopinfo) { + found_valid_stopinfo = false; + + ProcessSP process_sp(m_process_wp.lock()); + if (!process_sp) + return false; + + + ThreadList &curr_thread_list = process_sp->GetThreadList(); + uint32_t num_threads = curr_thread_list.GetSize(); + uint32_t idx; + + // The actions might change one of the thread's stop_info's opinions about + // whether we should stop the process, so we need to query that as we go. + + // One other complication here, is that we try to catch any case where the + // target has run (except for expressions) and immediately exit, but if we + // get that wrong (which is possible) then the thread list might have + // changed, and that would cause our iteration here to crash. We could + // make a copy of the thread list, but we'd really like to also know if it + // has changed at all, so we make up a vector of the thread ID's and check + // what we get back against this list & bag out if anything differs. + ThreadList not_suspended_thread_list(process_sp.get()); + std::vector<uint32_t> thread_index_array(num_threads); + uint32_t not_suspended_idx = 0; + for (idx = 0; idx < num_threads; ++idx) { + lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx); + + /* + Filter out all suspended threads, they could not be the reason + of stop and no need to perform any actions on them. + */ + if (thread_sp->GetResumeState() != eStateSuspended) { + not_suspended_thread_list.AddThread(thread_sp); + thread_index_array[not_suspended_idx] = thread_sp->GetIndexID(); + not_suspended_idx++; + } + } + + // Use this to track whether we should continue from here. We will only + // continue the target running if no thread says we should stop. Of course + // if some thread's PerformAction actually sets the target running, then it + // doesn't matter what the other threads say... + + bool still_should_stop = false; + + // Sometimes - for instance if we have a bug in the stub we are talking to, + // we stop but no thread has a valid stop reason. In that case we should + // just stop, because we have no way of telling what the right thing to do + // is, and it's better to let the user decide than continue behind their + // backs. + + for (idx = 0; idx < not_suspended_thread_list.GetSize(); ++idx) { + curr_thread_list = process_sp->GetThreadList(); + if (curr_thread_list.GetSize() != num_threads) { + Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STEP | + LIBLLDB_LOG_PROCESS)); + LLDB_LOGF( + log, + "Number of threads changed from %u to %u while processing event.", + num_threads, curr_thread_list.GetSize()); + break; + } + + lldb::ThreadSP thread_sp = not_suspended_thread_list.GetThreadAtIndex(idx); + + if (thread_sp->GetIndexID() != thread_index_array[idx]) { + Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STEP | + LIBLLDB_LOG_PROCESS)); + LLDB_LOGF(log, + "The thread at position %u changed from %u to %u while " + "processing event.", + idx, thread_index_array[idx], thread_sp->GetIndexID()); + break; + } + + StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); + if (stop_info_sp && stop_info_sp->IsValid()) { + found_valid_stopinfo = true; + bool this_thread_wants_to_stop; + if (stop_info_sp->GetOverrideShouldStop()) { + this_thread_wants_to_stop = + stop_info_sp->GetOverriddenShouldStopValue(); + } else { + stop_info_sp->PerformAction(event_ptr); + // The stop action might restart the target. If it does, then we + // want to mark that in the event so that whoever is receiving it + // will know to wait for the running event and reflect that state + // appropriately. We also need to stop processing actions, since they + // aren't expecting the target to be running. + + // FIXME: we might have run. + if (stop_info_sp->HasTargetRunSinceMe()) { + SetRestarted(true); + break; + } + + this_thread_wants_to_stop = stop_info_sp->ShouldStop(event_ptr); + } + + if (!still_should_stop) + still_should_stop = this_thread_wants_to_stop; + } + } + + return still_should_stop; +} + void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) { ProcessSP process_sp(m_process_wp.lock()); @@ -3974,121 +4083,40 @@ if (m_interrupted) return; - // If we're stopped and haven't restarted, then do the StopInfo actions here: - if (m_state == eStateStopped && !m_restarted) { - ThreadList &curr_thread_list = process_sp->GetThreadList(); - uint32_t num_threads = curr_thread_list.GetSize(); - uint32_t idx; - - // The actions might change one of the thread's stop_info's opinions about - // whether we should stop the process, so we need to query that as we go. - - // One other complication here, is that we try to catch any case where the - // target has run (except for expressions) and immediately exit, but if we - // get that wrong (which is possible) then the thread list might have - // changed, and that would cause our iteration here to crash. We could - // make a copy of the thread list, but we'd really like to also know if it - // has changed at all, so we make up a vector of the thread ID's and check - // what we get back against this list & bag out if anything differs. - std::vector<uint32_t> thread_index_array(num_threads); - for (idx = 0; idx < num_threads; ++idx) - thread_index_array[idx] = - curr_thread_list.GetThreadAtIndex(idx)->GetIndexID(); - - // Use this to track whether we should continue from here. We will only - // continue the target running if no thread says we should stop. Of course - // if some thread's PerformAction actually sets the target running, then it - // doesn't matter what the other threads say... - - bool still_should_stop = false; - - // Sometimes - for instance if we have a bug in the stub we are talking to, - // we stop but no thread has a valid stop reason. In that case we should - // just stop, because we have no way of telling what the right thing to do - // is, and it's better to let the user decide than continue behind their - // backs. - - bool does_anybody_have_an_opinion = false; - - for (idx = 0; idx < num_threads; ++idx) { - curr_thread_list = process_sp->GetThreadList(); - if (curr_thread_list.GetSize() != num_threads) { - Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STEP | - LIBLLDB_LOG_PROCESS)); - LLDB_LOGF( - log, - "Number of threads changed from %u to %u while processing event.", - num_threads, curr_thread_list.GetSize()); - break; - } - - lldb::ThreadSP thread_sp = curr_thread_list.GetThreadAtIndex(idx); - - if (thread_sp->GetIndexID() != thread_index_array[idx]) { - Log *log(lldb_private::GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STEP | - LIBLLDB_LOG_PROCESS)); - LLDB_LOGF(log, - "The thread at position %u changed from %u to %u while " - "processing event.", - idx, thread_index_array[idx], thread_sp->GetIndexID()); - break; - } - - StopInfoSP stop_info_sp = thread_sp->GetStopInfo(); - if (stop_info_sp && stop_info_sp->IsValid()) { - does_anybody_have_an_opinion = true; - bool this_thread_wants_to_stop; - if (stop_info_sp->GetOverrideShouldStop()) { - this_thread_wants_to_stop = - stop_info_sp->GetOverriddenShouldStopValue(); - } else { - stop_info_sp->PerformAction(event_ptr); - // The stop action might restart the target. If it does, then we - // want to mark that in the event so that whoever is receiving it - // will know to wait for the running event and reflect that state - // appropriately. We also need to stop processing actions, since they - // aren't expecting the target to be running. - - // FIXME: we might have run. - if (stop_info_sp->HasTargetRunSinceMe()) { - SetRestarted(true); - break; - } + // If we're not stopped or have restarted, then skip the StopInfo actions: + if (m_state != eStateStopped || m_restarted) { + return; + } - this_thread_wants_to_stop = stop_info_sp->ShouldStop(event_ptr); - } + bool does_anybody_have_an_opinion = false; + bool still_should_stop = ShouldStop(event_ptr, does_anybody_have_an_opinion); - if (!still_should_stop) - still_should_stop = this_thread_wants_to_stop; - } - } + if (GetRestarted()) { + return; + } - if (!GetRestarted()) { - if (!still_should_stop && does_anybody_have_an_opinion) { - // We've been asked to continue, so do that here. + if (!still_should_stop && does_anybody_have_an_opinion) { + // We've been asked to continue, so do that here. + SetRestarted(true); + // Use the public resume method here, since this is just extending a + // public resume. + process_sp->PrivateResume(); + } else { + bool hijacked = process_sp->IsHijackedForEvent(eBroadcastBitStateChanged) && + !process_sp->StateChangedIsHijackedForSynchronousResume(); + + if (!hijacked) { + // If we didn't restart, run the Stop Hooks here. + // Don't do that if state changed events aren't hooked up to the + // public (or SyncResume) broadcasters. StopHooks are just for + // real public stops. They might also restart the target, + // so watch for that. + process_sp->GetTarget().RunStopHooks(); + if (process_sp->GetPrivateState() == eStateRunning) SetRestarted(true); - // Use the public resume method here, since this is just extending a - // public resume. - process_sp->PrivateResume(); - } else { - bool hijacked = - process_sp->IsHijackedForEvent(eBroadcastBitStateChanged) && - !process_sp->StateChangedIsHijackedForSynchronousResume(); - - if (!hijacked) { - // If we didn't restart, run the Stop Hooks here. - // Don't do that if state changed events aren't hooked up to the - // public (or SyncResume) broadcasters. StopHooks are just for - // real public stops. They might also restart the target, - // so watch for that. - process_sp->GetTarget().RunStopHooks(); - if (process_sp->GetPrivateState() == eStateRunning) - SetRestarted(true); - } } } } -} void Process::ProcessEventData::Dump(Stream *s) const { ProcessSP process_sp(m_process_wp.lock()); Index: lldb/include/lldb/Target/Process.h =================================================================== --- lldb/include/lldb/Target/Process.h +++ lldb/include/lldb/Target/Process.h @@ -444,6 +444,8 @@ void Dump(Stream *s) const override; + virtual bool ShouldStop(Event *event_ptr, bool &found_valid_stopinfo); + void DoOnRemoval(Event *event_ptr) override; static const Process::ProcessEventData *
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
