Fix TestThreadSpecificBreakpoint with LLGS

[tberghammer]

Summary

Fix TestThreadSpecificBreakpoint with LLGS

• Set the state of the process into running/stepping on continue/step operations
• Add mutex to use transactions in Thread State Coordinator
  • It is required because the events from two Signal Handler or form a Signal handler and a Resume request shouldn't overlap
• Send Stop Replay Packet only when the state of the process changed

[vharron]

LGTM but ovyalov must review.

[ovyalov]

Please make sure that there are no test regressions with LLGS_LOCAL on.

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2612 ↗

(On Diff #19231)

Should we update process state only if the following Resume call succeeds?

2627 ↗

(On Diff #19231)

Should we update process state only if the following SingleStep call succeeds?

2727 ↗

(On Diff #19231)

minor optimization - could you move locker down to "for (auto thread_sp : m_threads)"?

[tberghammer]

Address issues requested in review

[tberghammer]

No regression with LLGS LOCAL (I think it uses different code path)

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2612 ↗

(On Diff #19231)

Fixed

2627 ↗

(On Diff #19231)

Fixed (if step is unsuccessful then the assert will fire)

2727 ↗

(On Diff #19231)

Moved it just before the for loop. It can't be moved inside as the main purpose of this mutex is to make events inside the loop unbreakable by other thread state changing events

lldb/trunk/source/Host/common/NativeProcessProtocol.cpp

NativeProcessProtocol::GetState () const
     Mutex::Locker locker (m_state_mutex);
     return m_state;
 }
 
 void
 NativeProcessProtocol::SetState (lldb::StateType state, bool notify_delegates)
 {
     Mutex::Locker locker (m_state_mutex);
+
+    if (state == m_state)
+        return;
+
     m_state = state;
 
     if (StateIsStoppedState (state, false))
     {
         ++m_stop_id;
 
         // Give process a chance to do any stop id bump processing, such as
         // clearing cached data that is invalidated each time the process runs.

lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp

 NativeProcessLinux::Launch(LaunchArgs *args)
 {
     assert (args && "null args");
     if (!args)
         return false;
 
     NativeProcessLinux *monitor = args->m_monitor;
     assert (monitor && "monitor is NULL");
-    if (!monitor)
-        return false;
 
     const char **argv = args->m_argv;
     const char **envp = args->m_envp;
     const char *working_dir = args->m_working_dir;
 
     lldb_utility::PseudoTerminal terminal;
     const size_t err_len = 1024;
     char err_str[err_len];

 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
     const bool is_main_thread = (pid == GetID ());
 
     assert(info && info->si_signo == SIGTRAP && "Unexpected child signal!");
     if (!info)
         return;
 
+    Mutex::Locker locker (m_threads_mutex);
+
     // See if we can find a thread for this signal.
     NativeThreadProtocolSP thread_sp = GetThreadByID (pid);
     if (!thread_sp)
     {
         if (log)
             log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 " no thread found for tid %" PRIu64, __FUNCTION__, GetID (), pid);
     }
 
case (SIGTRAP | (PTRACE_EVENT_EXEC << 8)):
         NativeThreadProtocolSP main_thread_sp;
         if (log)
             log->Printf ("NativeProcessLinux::%s() received exec event, code = %d", __FUNCTION__, info->si_code ^ SIGTRAP);
 
         // The thread state coordinator needs to reset due to the exec.
         m_coordinator_up->ResetForExec ();
 
         // Remove all but the main thread here.  Linux fork creates a new process which only copies the main thread.  Mutexes are in undefined state.
-        {
-            Mutex::Locker locker (m_threads_mutex);
-
-            if (log)
-                log->Printf ("NativeProcessLinux::%s exec received, stop tracking all but main thread", __FUNCTION__);
 
-            for (auto thread_sp : m_threads)
-            {
-                const bool is_main_thread = thread_sp && thread_sp->GetID () == GetID ();
-                if (is_main_thread)
-                {
-                    main_thread_sp = thread_sp;
-                    if (log)
-                        log->Printf ("NativeProcessLinux::%s found main thread with tid %" PRIu64 ", keeping", __FUNCTION__, main_thread_sp->GetID ());
-                }
-                else
-                {
-                    // Tell thread coordinator this thread is dead.
-                    if (log)
-                        log->Printf ("NativeProcessLinux::%s discarding non-main-thread tid %" PRIu64 " due to exec", __FUNCTION__, thread_sp->GetID ());
-                }
-            }
 
-            m_threads.clear ();
 
-            if (main_thread_sp)
-            {
-                m_threads.push_back (main_thread_sp);
-                SetCurrentThreadID (main_thread_sp->GetID ());
-                reinterpret_cast<NativeThreadLinux*>(main_thread_sp.get())->SetStoppedByExec ();
-            }
-            else
-            {
-                SetCurrentThreadID (LLDB_INVALID_THREAD_ID);
-                if (log)
-                    log->Printf ("NativeProcessLinux::%s pid %" PRIu64 "no main thread found, discarded all threads, we're in a no-thread state!", __FUNCTION__, GetID ());
-            }
-        }
 
         // Tell coordinator about about the "new" (since exec) stopped main thread.
         const lldb::tid_t main_thread_tid = GetID ();
         NotifyThreadCreateStopped (main_thread_tid);
 
         // NOTE: ideally these next statements would execute at the same time as the coordinator thread create was executed.
         // Consider a handler that can execute when that happens.
         // Let our delegate know we have just exec'd.
         NotifyDidExec ();
 
         // If we have a main thread, indicate we are stopped.
         assert (main_thread_sp && "exec called during ptraced process but no main thread metadata tracked");
 
         // Let the process know we're stopped.
-        SetState (StateType::eStateStopped);
+        CallAfterRunningThreadsStop (pid,
+                                     [=] (lldb::tid_t signaling_tid)
+                                     {
+                                         SetState (StateType::eStateStopped, true);
+                                     });
 
         break;
     }
 
     case (SIGTRAP | (PTRACE_EVENT_EXIT << 8)):
     {
         // The inferior process or one of its threads is about to exit.
 
case TRAP_TRACE:
         NotifyThreadStop (pid);
 
         // Here we don't have to request the rest of the threads to stop or request a deferred stop.
         // This would have already happened at the time the Resume() with step operation was signaled.
         // At this point, we just need to say we stopped, and the deferred notifcation will fire off
         // once all running threads have checked in as stopped.
         SetCurrentThreadID (pid);
         // Tell the process we have a stop (from software breakpoint).
+        CallAfterRunningThreadsStop (pid,
+                                     [=] (lldb::tid_t signaling_tid)
+                                     {
                                          SetState (StateType::eStateStopped, true);
+                                     });
         break;
 
     case SI_KERNEL:
     case TRAP_BRKPT:
         if (log)
             log->Printf ("NativeProcessLinux::%s() received breakpoint event, pid = %" PRIu64, __FUNCTION__, pid);
 
         // This thread is currently stopped.
NativeProcessLinux::MonitorSignal(const siginfo_t *info, lldb::pid_t pid, bool exited)
     // SIGILL, SIGSEGV, or SIGBUS *unless* that signal was generated by a
     // kill(2) or raise(3).  Similarly for tgkill(2) on Linux.
     //
     // IOW, user generated signals never generate what we consider to be a
     // "crash".
     //
     // Similarly, ACK signals generated by this monitor.
 
+    Mutex::Locker locker (m_threads_mutex);
+
     // See if we can find a thread for this signal.
     NativeThreadProtocolSP thread_sp = GetThreadByID (pid);
     if (!thread_sp)
     {
         if (log)
             log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 " no thread found for tid %" PRIu64, __FUNCTION__, GetID (), pid);
     }
 
CallAfterRunningThreadsStop (pid,
                                      SetCurrentThreadID (signaling_tid);
                                      SetState (StateType::eStateStopped, true);
                                  });
 }
 
 Error
 NativeProcessLinux::Resume (const ResumeActionList &resume_actions)
 {
-    Error error;
-
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
     if (log)
         log->Printf ("NativeProcessLinux::%s called: pid %" PRIu64, __FUNCTION__, GetID ());
 
     lldb::tid_t deferred_signal_tid = LLDB_INVALID_THREAD_ID;
     lldb::tid_t deferred_signal_skip_tid = LLDB_INVALID_THREAD_ID;
     int deferred_signo = 0;
     NativeThreadProtocolSP deferred_signal_thread_sp;
-    int resume_count = 0;
     bool stepping = false;
 
-
-    // std::vector<NativeThreadProtocolSP> new_stop_threads;
-
-    // Scope for threads mutex.
-    {
-        Mutex::Locker locker (m_threads_mutex);
-        for (auto thread_sp : m_threads)
-        {
-            assert (thread_sp && "thread list should not contain NULL threads");
+
 
-            const ResumeAction *const action = resume_actions.GetActionForThread (thread_sp->GetID (), true);
 
-            if (action == nullptr)
-            {
-                if (log)
-                    log->Printf ("NativeProcessLinux::%s no action specified for pid %" PRIu64 " tid %" PRIu64,
-                        __FUNCTION__, GetID (), thread_sp->GetID ());
-                continue;
-            }
 
-            if (log)
-            {
-                log->Printf ("NativeProcessLinux::%s processing resume action state %s for pid %" PRIu64 " tid %" PRIu64, 
-                        __FUNCTION__, StateAsCString (action->state), GetID (), thread_sp->GetID ());
-            }
 
-            switch (action->state)
-            {
-            case eStateRunning:
-            {
-                // Run the thread, possibly feeding it the signal.
-                const int signo = action->signal;
-                m_coordinator_up->RequestThreadResumeAsNeeded (thread_sp->GetID (),
-                                                               [=](lldb::tid_t tid_to_resume, bool supress_signal)
-                                                               {
-                                                                   reinterpret_cast<NativeThreadLinux*> (thread_sp.get ())->SetRunning ();
-                                                                   // Pass this signal number on to the inferior to handle.
-                                                                   return Resume (tid_to_resume, (signo > 0 && !supress_signal) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
-                                                               },
-                                                               CoordinatorErrorHandler);
-                ++resume_count;
-                break;
-            }
+                                                               const auto resume_result = Resume (tid_to_resume, (signo > 0 && !supress_signal) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
+                                                               if (resume_result.Success())
+                                                                   SetState(eStateRunning, true);
+                                                               return resume_result;
 
-            case eStateStepping:
-            {
-                // Request the step.
-                const int signo = action->signal;
-                m_coordinator_up->RequestThreadResume (thread_sp->GetID (),
-                                                       [=](lldb::tid_t tid_to_step, bool supress_signal)
-                                                       {
-                                                           reinterpret_cast<NativeThreadLinux*> (thread_sp.get ())->SetStepping ();
-                                                           const auto step_result = SingleStep (tid_to_step,(signo > 0 && !supress_signal) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
-                                                           assert (step_result.Success() && "SingleStep() failed");
-                                                           return step_result;
-                                                       },
-                                                       CoordinatorErrorHandler);
-                stepping = true;
-                break;
-            }
+                                                       if (step_result.Success())
+                                                           SetState(eStateStepping, true);
 
-            case eStateSuspended:
-            case eStateStopped:
-                // if we haven't chosen a deferred signal tid yet, use this one.
-                if (deferred_signal_tid == LLDB_INVALID_THREAD_ID)
-                {
-                    deferred_signal_tid = thread_sp->GetID ();
-                    deferred_signal_thread_sp = thread_sp;
-                    deferred_signo = SIGSTOP;
-                }
-                break;
 
-            default:
-                return Error ("NativeProcessLinux::%s (): unexpected state %s specified for pid %" PRIu64 ", tid %" PRIu64,
-                        __FUNCTION__, StateAsCString (action->state), GetID (), thread_sp->GetID ());
-            }
-        }
-    }
 
     // If we had any thread stopping, then do a deferred notification of the chosen stop thread id and signal
     // after all other running threads have stopped.
     // If there is a stepping thread involved we'll be eventually stopped by SIGTRAP trace signal.
     if (deferred_signal_tid != LLDB_INVALID_THREAD_ID && !stepping)
     {
         CallAfterRunningThreadsStopWithSkipTID (deferred_signal_tid,
                                                 deferred_signal_skip_tid,
                                      [=](lldb::tid_t deferred_notification_tid)
                                      {
                                          // Set the signal thread to the current thread.
                                          SetCurrentThreadID (deferred_notification_tid);
 
                                          // Set the thread state as stopped by the deferred signo.
                                          reinterpret_cast<NativeThreadLinux*> (deferred_signal_thread_sp.get ())->SetStoppedBySignal (deferred_signo);
 
                                          // Tell the process delegate that the process is in a stopped state.
                                          SetState (StateType::eStateStopped, true);
                                      });
     }
 
-    return error;
+    return Error();
 }
 
 Error
 NativeProcessLinux::Halt ()
 {
     Error error;
 
     if (kill (GetID (), SIGSTOP) != 0)
NativeProcessLinux::Signal (int signo)
     return error;
 }
 
 Error
 NativeProcessLinux::Interrupt ()
 {
     // Pick a running thread (or if none, a not-dead stopped thread) as
     // the chosen thread that will be the stop-reason thread.
-    Error error;
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
 
     NativeThreadProtocolSP running_thread_sp;
     NativeThreadProtocolSP stopped_thread_sp;
-    {
-        Mutex::Locker locker (m_threads_mutex);
         
-        if (log)
-            log->Printf ("NativeProcessLinux::%s selecting running thread for interrupt target", __FUNCTION__);
 
-        for (auto thread_sp : m_threads)
-        {
-            // The thread shouldn't be null but lets just cover that here.
-            if (!thread_sp)
-                continue;
+    Mutex::Locker locker (m_threads_mutex);
+
 
-            // If we have a running or stepping thread, we'll call that the
-            // target of the interrupt.
-            const auto thread_state = thread_sp->GetState ();
-            if (thread_state == eStateRunning ||
-                thread_state == eStateStepping)
-            {
-                running_thread_sp = thread_sp;
-                break;
-            }
-            else if (!stopped_thread_sp && StateIsStoppedState (thread_state, true))
-            {
-                // Remember the first non-dead stopped thread.  We'll use that as a backup if there are no running threads.
-                stopped_thread_sp = thread_sp;
-            }
-        }
-    }
 
     if (!running_thread_sp && !stopped_thread_sp)
     {
-        error.SetErrorString ("found no running/stepping or live stopped threads as target for interrupt");
+        Error error("found no running/stepping or live stopped threads as target for interrupt");
         if (log)
-        {
             log->Printf ("NativeProcessLinux::%s skipping due to error: %s", __FUNCTION__, error.AsCString ());
-        }
+
         return error;
     }
 
     NativeThreadProtocolSP deferred_signal_thread_sp = running_thread_sp ? running_thread_sp : stopped_thread_sp;
 
     if (log)
         log->Printf ("NativeProcessLinux::%s pid %" PRIu64 " %s tid %" PRIu64 " chosen for interrupt target",
                      __FUNCTION__,
                      GetID (),
                      running_thread_sp ? "running" : "stopped",
                      deferred_signal_thread_sp->GetID ());
 
     CallAfterRunningThreadsStop (deferred_signal_thread_sp->GetID (),
                                  [=](lldb::tid_t deferred_notification_tid)
                                  {
                                      // Set the signal thread to the current thread.
                                      SetCurrentThreadID (deferred_notification_tid);
 
                                      // Set the thread state as stopped by the deferred signo.
                                      reinterpret_cast<NativeThreadLinux*> (deferred_signal_thread_sp.get ())->SetStoppedBySignal (SIGSTOP);
 
-                                                // Tell the process delegate that the process is in a stopped state.
-                                                SetState (StateType::eStateStopped, true);
-                                            });
-    return error;
+    return Error();
 }
 
 Error
 NativeProcessLinux::Kill ()
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
     if (log)
         log->Printf ("NativeProcessLinux::%s called for PID %" PRIu64, __FUNCTION__, GetID ());