Fix some bugs in llgs thread state handling.

* When the thread state coordinator is told to skip sending a stop request
  for a running thread that is ignored (e.g. the thread that steps in a
  step operation is technically running and should not have a stop sent
  to it, since it will stop of its own accord per the kernel step operation),
  ensure the deferred signal notification logic still waits for the
  skipped thread.  (i.e. we want to defer the notification until the
  stepping thread is indeed stopped, we just don't want to send it a tgkill).

* Add ThreadStateCoordinator::RequestResumeAsNeeded().  This variant of the
  RequestResume() method does not call the error function when the thread
  is already running.  Instead, it just logs that the thread is already
  running and skips the resume operation.  This is useful for the case of
  vCont;c handling, where we tell all threads that they should be running.
  At the place we're calling, all we know is "we want this thread running if
  it isn't already," and that's exactly what this command does.

* Formatting change (minor) in NativeThreadLinux logging.

llvm-svn: 227913

diff --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 5c29758..da71e15 100644 (file)
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -166,7 +166,7 @@ namespace
     {
         Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
         if (log)
-            log->Printf ("NativePlatformLinux::%s ThreadStateCoordinator error received: %s", __FUNCTION__, error_message.c_str ());
+            log->Printf ("NativePlatformLinux::%s %s", __FUNCTION__, error_message.c_str ());
         assert (false && "ThreadStateCoordinator error reported");
     }
 
@@ -2689,14 +2689,14 @@ NativeProcessLinux::Resume (const ResumeActionList &resume_actions)
             {
                 // Run the thread, possibly feeding it the signal.
                 const int signo = action->signal;
-                m_coordinator_up->RequestThreadResume (thread_sp->GetID (),
-                                                       [=](lldb::tid_t tid_to_resume)
-                                                       {
-                                                           reinterpret_cast<NativeThreadLinux*> (thread_sp.get ())->SetRunning ();
-                                                           // Pass this signal number on to the inferior to handle.
-                                                           Resume (tid_to_resume, (signo > 0) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
-                                                       },
-                                                       CoordinatorErrorHandler);
+                m_coordinator_up->RequestThreadResumeAsNeeded (thread_sp->GetID (),
+                                                               [=](lldb::tid_t tid_to_resume)
+                                                               {
+                                                                   reinterpret_cast<NativeThreadLinux*> (thread_sp.get ())->SetRunning ();
+                                                                   // Pass this signal number on to the inferior to handle.
+                                                                   Resume (tid_to_resume, (signo > 0) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
+                                                               },
+                                                               CoordinatorErrorHandler);
                 ++resume_count;
                 break;
             }

diff --git a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
index f5cbe23..c24e3fd 100644 (file)
--- a/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp
@@ -250,7 +250,7 @@ NativeThreadLinux::SetStoppedBySignal (uint32_t signo)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD));
     if (log)
-        log->Printf ("NativeThreadLinux::%s called with signal 0x%" PRIx32, __FUNCTION__, signo);
+        log->Printf ("NativeThreadLinux::%s called with signal 0x%02" PRIx32, __FUNCTION__, signo);
 
     const StateType new_state = StateType::eStateStopped;
     MaybeLogStateChange (new_state);

diff --git a/lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp b/lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp
index 09a5598..cff0b96 100644 (file)
--- a/lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp
+++ b/lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp
@@ -287,17 +287,19 @@ private:
         ThreadIDSet sent_tids;
         for (auto it = coordinator.m_tid_stop_map.begin(); it != coordinator.m_tid_stop_map.end(); ++it)
         {
-            // Ignore threads we explicitly listed as skipped for stop requests.
-            if (m_skip_stop_request_tids.count (it->first) > 0)
-                continue;
-
             // We only care about threads not stopped.
             const bool running = !it->second;
             if (running)
             {
-                // Request this thread stop.
                 const lldb::tid_t tid = it->first;
-                m_request_thread_stop_function (tid);
+
+                // Request this thread stop if the tid stop request is not explicitly ignored.
+                const bool skip_stop_request = m_skip_stop_request_tids.count (tid) > 0;
+                if (!skip_stop_request)
+                    m_request_thread_stop_function (tid);
+
+                // Even if we skipped sending the stop request for other reasons (like stepping),
+                // we still need to wait for that stepping thread to notify completion/stop.
                 sent_tids.insert (tid);
             }
         }
@@ -452,11 +454,13 @@ class ThreadStateCoordinator::EventRequestResume : public ThreadStateCoordinator
 public:
     EventRequestResume (lldb::tid_t tid,
                         const ThreadIDFunction &request_thread_resume_function,
-                        const ErrorFunction &error_function):
+                        const ErrorFunction &error_function,
+                        bool error_when_already_running):
     EventBase (),
     m_tid (tid),
     m_request_thread_resume_function (request_thread_resume_function),
-    m_error_function (error_function)
+    m_error_function (error_function),
+    m_error_when_already_running (error_when_already_running)
     {
     }
 
@@ -478,10 +482,25 @@ public:
         const bool is_stopped = find_it->second;
         if (!is_stopped)
         {
-            // Skip the resume call - we have tracked it to be running.
-            std::ostringstream error_message;
-            error_message << "error: tid " << m_tid << " asked to resume but we think it is already running";
-            m_error_function (error_message.str ());
+            // It's not an error, just a log, if the m_already_running_no_error flag is set.
+            // This covers cases where, for instance, we're just trying to resume all threads
+            // from the user side.
+            if (!m_error_when_already_running)
+            {
+                coordinator.Log ("EventRequestResume::%s tid %" PRIu64 " optional resume skipped since it is already running",
+                                 __FUNCTION__,
+                                 m_tid);
+            }
+            else
+            {
+                // Skip the resume call - we have tracked it to be running.  And we unconditionally
+                // expected to resume this thread.  Flag this as an error.
+                std::ostringstream error_message;
+                error_message << "error: tid " << m_tid << " asked to resume but we think it is already running";
+                m_error_function (error_message.str ());
+            }
+
+            // Error or not, we're done.
             return eventLoopResultContinue;
         }
 
@@ -534,6 +553,7 @@ private:
     const lldb::tid_t m_tid;
     ThreadIDFunction m_request_thread_resume_function;
     ErrorFunction m_error_function;
+    const bool m_error_when_already_running;
 };
 
 //===----------------------------------------------------------------------===//
@@ -763,7 +783,17 @@ ThreadStateCoordinator::RequestThreadResume (lldb::tid_t tid,
                                              const ThreadIDFunction &request_thread_resume_function,
                                              const ErrorFunction &error_function)
 {
-    EnqueueEvent (EventBaseSP (new EventRequestResume (tid, request_thread_resume_function, error_function)));
+    const bool error_when_already_running = true;
+    EnqueueEvent (EventBaseSP (new EventRequestResume (tid, request_thread_resume_function, error_function, error_when_already_running)));
+}
+
+void
+ThreadStateCoordinator::RequestThreadResumeAsNeeded (lldb::tid_t tid,
+                                                     const ThreadIDFunction &request_thread_resume_function,
+                                                     const ErrorFunction &error_function)
+{
+    const bool error_when_already_running = false;
+    EnqueueEvent (EventBaseSP (new EventRequestResume (tid, request_thread_resume_function, error_function, error_when_already_running)));
 }
 
 void

diff --git a/lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.h b/lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.h
index 267cfc3..66c9452 100644 (file)
--- a/lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.h
+++ b/lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.h
@@ -102,12 +102,22 @@ namespace lldb_private
 
         // Request that the given thread id should have the request_thread_resume_function
         // called.  Will trigger the error_function if the thread is thought to be running
-        // already at that point.
+        // already at that point.  This call signals an error if the thread resume is for
+        // a thread that is already in a running state.
         void
         RequestThreadResume (lldb::tid_t tid,
                              const ThreadIDFunction &request_thread_resume_function,
                              const ErrorFunction &error_function);
 
+        // Request that the given thread id should have the request_thread_resume_function
+        // called.  Will trigger the error_function if the thread is thought to be running
+        // already at that point.  This call ignores threads that are already running and
+        // does not trigger an error in that case.
+        void
+        RequestThreadResumeAsNeeded (lldb::tid_t tid,
+                                     const ThreadIDFunction &request_thread_resume_function,
+                                     const ErrorFunction &error_function);
+
         // Indicate the calling process did an exec and that the thread state
         // should be 100% cleared.
         //