[Lldb-commits] [lldb] 3b43f00 - [lldb] Check if thread was suspended during previous stop added.

Jonas Devlieghere via lldb-commits lldb-commits at lists.llvm.org
Thu Jun 11 15:02:52 PDT 2020


Author: Ilya Bukonkin
Date: 2020-06-11T15:02:46-07:00
New Revision: 3b43f006294971b8049d4807110032169780e5b8

URL: https://github.com/llvm/llvm-project/commit/3b43f006294971b8049d4807110032169780e5b8
DIFF: https://github.com/llvm/llvm-project/commit/3b43f006294971b8049d4807110032169780e5b8.diff

LOG: [lldb] Check if thread was suspended during previous stop added.

Encountered the following situation: Let we started thread T1 and it hit
breakpoint on B1 location. We suspended T1 and continued the process.
Then we started thread T2 which hit for example the same location B1.
This time in a breakpoint callback we decided not to stop returning
false.

Expected result: process continues (as if T2 did not hit breakpoint) its
workflow with T1 still suspended. Actual result: process do stops (as if
T2 callback returned true).

Solution: We need invalidate StopInfo for threads that was previously
suspended just because something that is already inactive can not be the
reason of stop. Thread::GetPrivateStopInfo() may be appropriate place to
do it, because it gets called (through Thread::GetStopInfo()) every time
before process reports stop and user gets chance to change
m_resume_state again i.e if we see m_resume_state == eStateSuspended
it definitely means it was set during previous stop and it also means
this thread can not be stopped again (cos' it was frozen during
previous stop).

Differential revision: https://reviews.llvm.org/D80112

Added: 
    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/Process/ProcessEventDataTest.cpp
    lldb/unittests/Thread/CMakeLists.txt
    lldb/unittests/Thread/ThreadTest.cpp

Modified: 
    lldb/include/lldb/Target/Process.h
    lldb/source/Target/Process.cpp
    lldb/unittests/CMakeLists.txt
    lldb/unittests/Process/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 700f74f26958..01e2a0d77111 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -452,6 +452,8 @@ class Process : public std::enable_shared_from_this<Process>,
 
     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 *

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 25a940b66430..9cb38556a6ec 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3998,6 +3998,114 @@ ConstString Process::ProcessEventData::GetFlavor() const {
   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 
diff ers.
+  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());
 
@@ -4032,121 +4140,40 @@ void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) {
   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 
diff ers.
-    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());

diff  --git a/lldb/test/API/functionalities/thread/ignore_suspended/Makefile b/lldb/test/API/functionalities/thread/ignore_suspended/Makefile
new file mode 100644
index 000000000000..ebecfbf92416
--- /dev/null
+++ b/lldb/test/API/functionalities/thread/ignore_suspended/Makefile
@@ -0,0 +1,3 @@
+ENABLE_THREADS := YES
+CXX_SOURCES := main.cpp
+include Makefile.rules

diff  --git a/lldb/test/API/functionalities/thread/ignore_suspended/TestIgnoreSuspendedThread.py b/lldb/test/API/functionalities/thread/ignore_suspended/TestIgnoreSuspendedThread.py
new file mode 100644
index 000000000000..cf9be237efaf
--- /dev/null
+++ b/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)

diff  --git a/lldb/test/API/functionalities/thread/ignore_suspended/main.cpp b/lldb/test/API/functionalities/thread/ignore_suspended/main.cpp
new file mode 100644
index 000000000000..3bd736bbdb2a
--- /dev/null
+++ b/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;
+}

diff  --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
index 33818b5888c0..52f5c964fcee 100644
--- a/lldb/unittests/CMakeLists.txt
+++ b/lldb/unittests/CMakeLists.txt
@@ -84,6 +84,7 @@ add_subdirectory(Target)
 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)

diff  --git a/lldb/unittests/Process/CMakeLists.txt b/lldb/unittests/Process/CMakeLists.txt
index a4045692eb50..50d0e75167ea 100644
--- a/lldb/unittests/Process/CMakeLists.txt
+++ b/lldb/unittests/Process/CMakeLists.txt
@@ -4,3 +4,17 @@ if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   add_subdirectory(POSIX)
 endif()
 add_subdirectory(minidump)
+
+add_lldb_unittest(ProcessEventDataTest
+  ProcessEventDataTest.cpp
+
+  LINK_LIBS
+      lldbCore
+      lldbHost
+      lldbTarget
+      lldbSymbol
+      lldbUtility
+      lldbUtilityHelpers
+      lldbInterpreter
+      lldbPluginPlatformMacOSX
+  )

diff  --git a/lldb/unittests/Process/ProcessEventDataTest.cpp b/lldb/unittests/Process/ProcessEventDataTest.cpp
new file mode 100644
index 000000000000..0f032a786112
--- /dev/null
+++ b/lldb/unittests/Process/ProcessEventDataTest.cpp
@@ -0,0 +1,256 @@
+//===-- 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);
+}

diff  --git a/lldb/unittests/Thread/CMakeLists.txt b/lldb/unittests/Thread/CMakeLists.txt
new file mode 100644
index 000000000000..aa5b777f4b2e
--- /dev/null
+++ b/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
+  )
+

diff  --git a/lldb/unittests/Thread/ThreadTest.cpp b/lldb/unittests/Thread/ThreadTest.cpp
new file mode 100644
index 000000000000..9a79bbc12cab
--- /dev/null
+++ b/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);
+}


        


More information about the lldb-commits mailing list