[NativeProcessLinux] Fix a bug in instruction-stepping over thread creation

[labath]

Summary

There was a bug in NativeProcessLinux, where doing an instruction-level single-step over the
thread-creation syscall resulted in loss of control over the inferior. This happened because
after the inferior entered the thread-creation maintenance stop, we unconditionally performed a
PTRACE_CONT, even though the original intention was to do a PTRACE_SINGLESTEP. This is fixed by
storing the original state of the thread before the stop (stepping or running) and then
performing the appropriate action when resuming.

I also get rid of the callback in the ThreadContext structure, which stored the lambda used to
resume the thread, but which was not used consistently.

A test verifying the correctness of the new behavior is included.

[ovyalov]

Please see my comments.

source/Plugins/Process/Linux/NativeProcessLinux.cpp

1457 ↗

(On Diff #32415)

Since AddThread is private method of NPL we can make it to return NativeThreadLinuxSP.

2822 ↗

(On Diff #32415)

It seems the comment is no longer actual.

3056 ↗

(On Diff #32415)

Should it be eStateStepping?

3060 ↗

(On Diff #32415)

Could you print state value here?

source/Plugins/Process/Linux/NativeProcessLinux.h

314 ↗

(On Diff #32415)

Please update the comment.

source/Plugins/Process/Linux/NativeThreadLinux.h

115 ↗

(On Diff #32415)

Nit - include <memory> for shared_ptr?

[labath]

• Address review comments

source/Plugins/Process/Linux/NativeProcessLinux.cpp

1457 ↗

(On Diff #32415)

I've been wanting to do this as well, but I'd prefer to do this as a separate commit, if possible.

3056 ↗

(On Diff #32415)

That's a good question. Previously, the state of the whole process was indeed set to eStateStepping, and I have (accidentally) failed to preserve that while refactoring.

However, now that I think about it, this had the problem that the state of the process was nondeterministic and depended on the order in which you resume threads (If you are continuing one thread and stepping another, the process state would depend on which thread you resumed last). The test suite passes, so i think we can assume no one is depending on this stepping state now, but to avoid introducing nondeterministic bugs in the future, I think we should just abolish the stepping state and say that the process is running if any of its threads are stepping or running.

What do you think?

[labath]

Fix test on android arm - skip single-stepping over some functions

[tberghammer]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

1049–1051 ↗

(On Diff #32545)

(nit): You don't need this cast (the value is never used).

1148 ↗

(On Diff #32545)

You call SetStoppedWithNoReason before almost all ResumeThread but as far as I see it isn't used in ResumeThread (except checked in an assert) and ResumeThread overwrites it. I would prefer to remove these as for me they complicate the code without any reason, but I might miss their purpose.

test/functionalities/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py

60 ↗

(On Diff #32545)

Please make this list architecture specific or add a comment for each function with the architecture where it imposes an issue

[labath]

Address review comments

[ovyalov]

LGTM

source/Plugins/Process/Linux/NativeProcessLinux.cpp

3051 ↗

(On Diff #32571)

Let's go ahead with eStateRunning instead of eStateStepping - I believe it sounds reasonable.

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

private:
 
         MainLoop::SignalHandleUP m_sigchld_handle;
         ArchSpec m_arch;
 
         LazyBool m_supports_mem_region;
         std::vector<MemoryRegionInfo> m_mem_region_cache;
         Mutex m_mem_region_cache_mutex;
 
+        lldb::tid_t m_pending_notification_tid;
+
         // List of thread ids stepping with a breakpoint with the address of
         // the relevan breakpoint
         std::map<lldb::tid_t, lldb::addr_t> m_threads_stepping_with_breakpoint;
 
         /// @class LauchArgs
         ///
         /// @brief Simple structure to pass data to the thread responsible for
         /// launching a child process.
#endif
 
         void
         NotifyThreadDeath (lldb::tid_t tid);
 
         Error
         Detach(lldb::tid_t tid);
 
 
-        // Typedefs.
-        typedef std::unordered_set<lldb::tid_t> ThreadIDSet;
-
         // This method is requests a stop on all threads which are still running. It sets up a
         // deferred delegate notification, which will fire once threads report as stopped. The
         // triggerring_tid will be set as the current thread (main stop reason).
         void
         StopRunningThreads(lldb::tid_t triggering_tid);
 
-        struct PendingNotification
-        {
-            PendingNotification (lldb::tid_t triggering_tid):
-                triggering_tid (triggering_tid),
-                wait_for_stop_tids ()
-            {
-            }
-
-            const lldb::tid_t  triggering_tid;
-            ThreadIDSet        wait_for_stop_tids;
-        };
-        typedef std::unique_ptr<PendingNotification> PendingNotificationUP;
-
         // Notify the delegate if all threads have stopped.
         void SignalIfAllThreadsStopped();
 
-        void
-        RequestStopOnAllRunningThreads();
-
-        Error
-        ThreadDidStop(lldb::tid_t tid, bool initiated_by_llgs);
-
-        // Resume the thread with the given thread id using the request_thread_resume_function
-        // called. If error_when_already_running is then then an error is raised if we think this
-        // thread is already running.
+        // Resume the given thread, optionally passing it the given signal. The type of resume
+        // operation (continue, single-step) depends on the state parameter.
         Error
-        ResumeThread(lldb::tid_t tid, NativeThreadLinux::ResumeThreadFunction request_thread_resume_function,
-                bool error_when_already_running);
-
-        void
-        DoStopThreads(PendingNotificationUP &&notification_up);
+        ResumeThread(const NativeThreadLinuxSP &thread_sp, lldb::StateType state, int signo);
 
         void
         ThreadWasCreated (lldb::tid_t tid);
 
         void
         SigchldHandler();
-
-        // Member variables.
-        PendingNotificationUP m_pending_notification_up;
     };
 
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef liblldb_NativeProcessLinux_H_

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

 // Public Instance Methods
 // -----------------------------------------------------------------------------
 
 NativeProcessLinux::NativeProcessLinux () :
     NativeProcessProtocol (LLDB_INVALID_PROCESS_ID),
     m_arch (),
     m_supports_mem_region (eLazyBoolCalculate),
     m_mem_region_cache (),
-    m_mem_region_cache_mutex ()
+    m_mem_region_cache_mutex(),
+    m_pending_notification_tid(LLDB_INVALID_THREAD_ID)
 {
 }
 
 void
 NativeProcessLinux::LaunchInferior (
     MainLoop &mainloop,
     Module *module,
     const char *argv[],
NativeProcessLinux::MonitorCallback(lldb::pid_t pid,
     }
 }
 
 void
 NativeProcessLinux::WaitForNewThread(::pid_t tid)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS));
 
-    NativeThreadProtocolSP new_thread_sp = GetThreadByID(tid);
+    NativeThreadLinuxSP new_thread_sp = std::static_pointer_cast<NativeThreadLinux>(GetThreadByID(tid));
 
     if (new_thread_sp)
     {
         // We are already tracking the thread - we got the event on the new thread (see
         // MonitorSignal) before this one. We are done.
         return;
     }
 
if (((info.si_pid != 0) || (info.si_code != SI_USER)) && log)
         // created properly.
         log->Printf ("NativeProcessLinux::%s() GetSignalInfo for tid %" PRIu32 " received unexpected signal with code %d from pid %d.", __FUNCTION__, tid, info.si_code, info.si_pid);
     }
 
     if (log)
         log->Printf ("NativeProcessLinux::%s() pid = %" PRIu64 ": tracking new thread tid %" PRIu32,
                  __FUNCTION__, GetID (), tid);
 
-    new_thread_sp = AddThread(tid);
-    std::static_pointer_cast<NativeThreadLinux> (new_thread_sp)->SetRunning ();
-    Resume (tid, LLDB_INVALID_SIGNAL_NUMBER);
+    new_thread_sp = std::static_pointer_cast<NativeThreadLinux>(AddThread(tid));
+    ResumeThread(new_thread_sp, eStateRunning, LLDB_INVALID_SIGNAL_NUMBER);
     ThreadWasCreated(tid);
 }
 
 void
 NativeProcessLinux::MonitorSIGTRAP(const siginfo_t *info, lldb::pid_t pid)
 {
     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);
+    NativeThreadLinuxSP thread_sp = std::static_pointer_cast<NativeThreadLinux>(GetThreadByID(pid));
     if (!thread_sp)
     {
         if (log)
             log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 " no thread found for tid %" PRIu64, __FUNCTION__, GetID (), pid);
     }
 
     switch (info->si_code)
     {
case (SIGTRAP | (PTRACE_EVENT_CLONE << 8)):
         unsigned long event_message = 0;
         if (GetEventMessage (pid, &event_message).Fail())
         {
             if (log)
                 log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 " received thread creation event but GetEventMessage failed so we don't know the new tid", __FUNCTION__, pid);
         } else 
             WaitForNewThread(event_message);
 
-        Resume (pid, LLDB_INVALID_SIGNAL_NUMBER);
+        ResumeThread(thread_sp, thread_sp->GetState(), LLDB_INVALID_SIGNAL_NUMBER);
         break;
     }
 
     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);
 
         // Exec clears any pending notifications.
-        m_pending_notification_up.reset ();
+        m_pending_notification_tid = LLDB_INVALID_THREAD_ID;
 
         // Remove all but the main thread here.  Linux fork creates a new process which only copies the main thread.  Mutexes are in undefined state.
         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 ();
case (SIGTRAP | (PTRACE_EVENT_EXIT << 8)):
                     is_main_thread ? "is main thread" : "not main thread");
         }
 
         if (is_main_thread)
         {
             SetExitStatus (convert_pid_status_to_exit_type (data), convert_pid_status_to_return_code (data), nullptr, true);
         }
 
-        Resume(pid, LLDB_INVALID_SIGNAL_NUMBER);
+        ResumeThread(thread_sp, thread_sp->GetState(), LLDB_INVALID_SIGNAL_NUMBER);
 
         break;
     }
 
     case 0:
     case TRAP_TRACE:  // We receive this on single stepping.
     case TRAP_HWBKPT: // We receive this on watchpoint hit
         if (thread_sp)
case TRAP_BRKPT:
         break;
 
     case SIGTRAP:
     case (SIGTRAP | 0x80):
         if (log)
             log->Printf ("NativeProcessLinux::%s() received unknown SIGTRAP system call stop event, pid %" PRIu64 "tid %" PRIu64 ", resuming", __FUNCTION__, GetID (), pid);
 
         // Ignore these signals until we know more about them.
-        Resume(pid, LLDB_INVALID_SIGNAL_NUMBER);
+        ResumeThread(thread_sp, thread_sp->GetState(), LLDB_INVALID_SIGNAL_NUMBER);
         break;
 
     default:
         assert(false && "Unexpected SIGTRAP code!");
         if (log)
             log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 "tid %" PRIu64 " received unhandled SIGTRAP code: 0x%d",
                     __FUNCTION__, GetID (), pid, info->si_code);
         break;
         
     }
 }
 
 void
 NativeProcessLinux::MonitorTrace(lldb::pid_t pid, NativeThreadProtocolSP thread_sp)
 {
     Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
     if (log)
         log->Printf("NativeProcessLinux::%s() received trace event, pid = %" PRIu64 " (single stepping)",
                 __FUNCTION__, pid);
 
+    // This thread is currently stopped.
     if (thread_sp)
         std::static_pointer_cast<NativeThreadLinux>(thread_sp)->SetStoppedByTrace();
 
-    // This thread is currently stopped.
-    ThreadDidStop(pid, false);
-
     // 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).
     StopRunningThreads(pid);
 }
 
 void
 NativeProcessLinux::MonitorBreakpoint(lldb::pid_t pid, NativeThreadProtocolSP thread_sp)
 {
     Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_BREAKPOINTS));
     if (log)
         log->Printf("NativeProcessLinux::%s() received breakpoint event, pid = %" PRIu64,
                 __FUNCTION__, pid);
 
-    // This thread is currently stopped.
-    ThreadDidStop(pid, false);
-
     // Mark the thread as stopped at breakpoint.
     if (thread_sp)
     {
         std::static_pointer_cast<NativeThreadLinux>(thread_sp)->SetStoppedByBreakpoint();
         Error error = FixupBreakpointPCAsNeeded(thread_sp);
         if (error.Fail())
             if (log)
                 log->Printf("NativeProcessLinux::%s() pid = %" PRIu64 " fixup: %s",

 NativeProcessLinux::MonitorWatchpoint(lldb::pid_t pid, NativeThreadProtocolSP thread_sp, uint32_t wp_index)
 {
     Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_WATCHPOINTS));
     if (log)
         log->Printf("NativeProcessLinux::%s() received watchpoint event, "
                     "pid = %" PRIu64 ", wp_index = %" PRIu32,
                     __FUNCTION__, pid, wp_index);
 
-    // This thread is currently stopped.
-    ThreadDidStop(pid, false);
-
     // Mark the thread as stopped at watchpoint.
     // The address is at (lldb::addr_t)info->si_addr if we need it.
     lldbassert(thread_sp && "thread_sp cannot be NULL");
     std::static_pointer_cast<NativeThreadLinux>(thread_sp)->SetStoppedByWatchpoint(wp_index);
 
     // We need to tell all other running threads before we notify the delegate about this stop.
     StopRunningThreads(pid);
 }
NativeProcessLinux::MonitorSignal(const siginfo_t *info, lldb::pid_t pid, bool exited)
     // 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);
+    NativeThreadLinuxSP thread_sp = std::static_pointer_cast<NativeThreadLinux>(GetThreadByID(pid));
     if (!thread_sp)
     {
         if (log)
             log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 " no thread found for tid %" PRIu64, __FUNCTION__, GetID (), pid);
     }
 
     // Handle the signal.
     if (info->si_code == SI_TKILL || info->si_code == SI_USER)
if ((info->si_pid == 0) && (info->si_code == SI_USER))
         // A new thread creation is being signaled. This is one of two parts that come in
         // a non-deterministic order. This code handles the case where the new thread event comes
         // before the event on the parent thread. For the opposite case see code in
         // MonitorSIGTRAP.
         if (log)
             log->Printf ("NativeProcessLinux::%s() pid = %" PRIu64 " tid %" PRIu64 ": new thread notification",
                      __FUNCTION__, GetID (), pid);
 
-        thread_sp = AddThread(pid);
+        thread_sp = std::static_pointer_cast<NativeThreadLinux>(AddThread(pid));
         assert (thread_sp.get() && "failed to create the tracking data for newly created inferior thread");
         // We can now resume the newly created thread.
-        std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetRunning ();
-        Resume (pid, LLDB_INVALID_SIGNAL_NUMBER);
+        ResumeThread(thread_sp, eStateRunning, LLDB_INVALID_SIGNAL_NUMBER);
         ThreadWasCreated(pid);
         // Done handling.
         return;
     }
 
     // Check for thread stop notification.
     if (is_from_llgs && (info->si_code == SI_TKILL) && (signo == SIGSTOP))
     {
if (is_from_llgs && (info->si_code == SI_TKILL) && (signo == SIGSTOP))
             {
                 // An inferior thread has stopped because of a SIGSTOP we have sent it.
                 // Generally, these are not important stops and we don't want to report them as
                 // they are just used to stop other threads when one thread (the one with the
                 // *real* stop reason) hits a breakpoint (watchpoint, etc...). However, in the
                 // case of an asynchronous Interrupt(), this *is* the real stop reason, so we
                 // leave the signal intact if this is the thread that was chosen as the
                 // triggering thread.
-                if (m_pending_notification_up && m_pending_notification_up->triggering_tid == pid)
+                if (m_pending_notification_tid != LLDB_INVALID_THREAD_ID)
+                {
+                    if (m_pending_notification_tid == pid)
                         linux_thread_sp->SetStoppedBySignal(SIGSTOP, info);
                     else
                         linux_thread_sp->SetStoppedWithNoReason();
 
                     SetCurrentThreadID (thread_sp->GetID ());
-                ThreadDidStop (thread_sp->GetID (), true);
+                    SignalIfAllThreadsStopped();
+                }
+                else
+                {
+                    // We can end up here if stop was initiated by LLGS but by this time a
+                    // thread stop has occurred - maybe initiated by another event.
+                    Error error = ResumeThread(linux_thread_sp, linux_thread_sp->GetState(), 0);
+                    if (error.Fail() && log)
+                    {
+                        log->Printf("NativeProcessLinux::%s failed to resume thread tid  %" PRIu64 ": %s",
+                                __FUNCTION__, linux_thread_sp->GetID(), error.AsCString ());
+                    }
+                }
             }
             else
             {
                 if (log)
                 {
                     // Retrieve the signal name if the thread was stopped by a signal.
                     int stop_signo = 0;
                     const bool stopped_by_signal = linux_thread_sp->IsStopped (&stop_signo);
                     const char *signal_name = stopped_by_signal ? Host::GetSignalAsCString(stop_signo) : "<not stopped by signal>";
                     if (!signal_name)
                         signal_name = "<no-signal-name>";
 
                     log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 " tid %" PRIu64 ", thread was already marked as a stopped state (state=%s, signal=%d (%s)), leaving stop signal as is",
                                  __FUNCTION__,
                                  GetID (),
                                  linux_thread_sp->GetID (),
                                  StateAsCString (thread_state),
                                  stop_signo,
                                  signal_name);
                 }
-                ThreadDidStop (thread_sp->GetID (), false);
+                SignalIfAllThreadsStopped();
             }
         }
 
         // Done handling.
         return;
     }
 
     if (log)
         log->Printf ("NativeProcessLinux::%s() received signal %s", __FUNCTION__, Host::GetSignalAsCString(signo));
 
     // This thread is stopped.
-    ThreadDidStop (pid, false);
-
     if (thread_sp)
         std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetStoppedBySignal(signo, info);
 
     // Send a stop to the debugger after we get all other threads to stop.
     StopRunningThreads (pid);
 }
 
 namespace {
for (auto thread_sp : m_threads)
         {
             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;
-            ResumeThread(thread_sp->GetID (),
-                    [=](lldb::tid_t tid_to_resume, bool supress_signal)
-                    {
-                        std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetRunning ();
-                        // Pass this signal number on to the inferior to handle.
-                        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;
-                    },
-                    false);
-            break;
-        }
-
         case eStateStepping:
         {
-            // Request the step.
+            // Run the thread, possibly feeding it the signal.
             const int signo = action->signal;
-            ResumeThread(thread_sp->GetID (),
-                    [=](lldb::tid_t tid_to_step, bool supress_signal)
-                    {
-                        std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetStepping ();
-
-                        Error step_result;
-                        if (software_single_step)
-                            step_result = Resume (tid_to_step, (signo > 0 && !supress_signal) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
-                        else
-                            step_result = SingleStep (tid_to_step,(signo > 0 && !supress_signal) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
-
-                        assert (step_result.Success() && "SingleStep() failed");
-                        if (step_result.Success())
-                            SetState(eStateStepping, true);
-                        return step_result;
-                    },
-                    false);
+            ResumeThread(std::static_pointer_cast<NativeThreadLinux>(thread_sp), action->state, signo);
             break;
         }
 
         case eStateSuspended:
         case eStateStopped:
             lldbassert(0 && "Unexpected state");
 
         default:

 Error
 NativeProcessLinux::SingleStep(lldb::tid_t tid, uint32_t signo)
 {
     intptr_t data = 0;
 
     if (signo != LLDB_INVALID_SIGNAL_NUMBER)
         data = signo;
 
-    return PtraceWrapper(PTRACE_SINGLESTEP, tid, nullptr, (void*)data);
+    // If hardware single-stepping is not supported, we just do a continue. The breakpoint on the
+    // next instruction has been setup in NativeProcessLinux::Resume.
+    return PtraceWrapper(SupportHardwareSingleStepping() ? PTRACE_SINGLESTEP : PTRACE_CONT,
+            tid, nullptr, (void*)data);
 }
 
 Error
 NativeProcessLinux::GetSignalInfo(lldb::tid_t tid, void *siginfo)
 {
     return PtraceWrapper(PTRACE_GETSIGINFO, tid, nullptr, siginfo);
 }
 
for (auto it = m_threads.begin (); it != m_threads.end (); ++it)
         if (*it && ((*it)->GetID () == thread_id))
         {
             m_threads.erase (it);
             found = true;
             break;
         }
     }
 
-    // If we have a pending notification, remove this from the set.
-    if (m_pending_notification_up)
-    {
-        m_pending_notification_up->wait_for_stop_tids.erase(thread_id);
-        SignalIfAllThreadsStopped();
-    }
 
     return found;
 }
 
 NativeThreadProtocolSP
 NativeProcessLinux::AddThread (lldb::tid_t thread_id)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD));
Error error = ProcFileReader::ProcessLineByLine (GetID (), "maps",
  
             // Return true to continue reading the proc file
             return true;
         });
     return error; 
 }
 
 Error
-NativeProcessLinux::ResumeThread(
-        lldb::tid_t tid,
-        NativeThreadLinux::ResumeThreadFunction request_thread_resume_function,
-        bool error_when_already_running)
+NativeProcessLinux::ResumeThread(const NativeThreadLinuxSP &thread_sp, lldb::StateType state, int signo)
 {
     Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
 
     if (log)
-        log->Printf("NativeProcessLinux::%s (tid: %" PRIu64 ", error_when_already_running: %s)",
-                __FUNCTION__, tid, error_when_already_running?"true":"false");
-    
-    auto thread_sp = std::static_pointer_cast<NativeThreadLinux>(GetThreadByID(tid));
-    lldbassert(thread_sp != nullptr);
-
-    auto& context = thread_sp->GetThreadContext();
-    // Tell the thread to resume if we don't already think it is running.
-    const bool is_stopped = StateIsStoppedState(thread_sp->GetState(), true);
-
-    lldbassert(!(error_when_already_running && !is_stopped));
-
-    if (!is_stopped)
-    {
-        // It's not an error, just a log, if the error_when_already_running flag is not set.
-        // This covers cases where, for instance, we're just trying to resume all threads
-        // from the user side.
-        if (log)
-            log->Printf("NativeProcessLinux::%s tid %" PRIu64 " optional resume skipped since it is already running",
-                    __FUNCTION__,
-                    tid);
-        return Error();
-    }
+        log->Printf("NativeProcessLinux::%s (tid: %" PRIu64 ")",
+                __FUNCTION__, thread_sp->GetID());
     
     // Before we do the resume below, first check if we have a pending
     // stop notification that is currently waiting for
-    // this thread to stop.  This is potentially a buggy situation since
+    // all threads to stop.  This is potentially a buggy situation since
     // we're ostensibly waiting for threads to stop before we send out the
     // pending notification, and here we are resuming one before we send
     // out the pending stop notification.
-    if (m_pending_notification_up && log && m_pending_notification_up->wait_for_stop_tids.count (tid) > 0)
+    if (m_pending_notification_tid != LLDB_INVALID_THREAD_ID && log)
     {
-        log->Printf("NativeProcessLinux::%s about to resume tid %" PRIu64 " per explicit request but we have a pending stop notification (tid %" PRIu64 ") that is actively waiting for this thread to stop. Valid sequence of events?", __FUNCTION__, tid, m_pending_notification_up->triggering_tid);
+        log->Printf("NativeProcessLinux::%s about to resume tid %" PRIu64 " per explicit request but we have a pending stop notification (tid %" PRIu64 ") that is actively waiting for this thread to stop. Valid sequence of events?", __FUNCTION__, thread_sp->GetID(), m_pending_notification_tid);
     }
 
     // Request a resume.  We expect this to be synchronous and the system
     // to reflect it is running after this completes.
-    const auto error = request_thread_resume_function (tid, false);
-    if (error.Success())
-        context.request_resume_function = request_thread_resume_function;
-    else if (log)
+    switch (state)
     {
-        log->Printf("NativeProcessLinux::%s failed to resume thread tid  %" PRIu64 ": %s",
-                         __FUNCTION__, tid, error.AsCString ());
+    case eStateRunning:
+    {
+        thread_sp->SetRunning();
+        const auto resume_result = Resume(thread_sp->GetID(), signo);
+        if (resume_result.Success())
+            SetState(eStateRunning, true);
+        return resume_result;
+    }
+    case eStateStepping:
+    {
+        thread_sp->SetStepping();
+        const auto step_result = SingleStep(thread_sp->GetID(), signo);
+        if (step_result.Success())
+            SetState(eStateRunning, true);
+        return step_result;
+    }
+    default:
+        if (log)
+            log->Printf("NativeProcessLinux::%s Unhandled state %s.",
+                    __FUNCTION__, StateAsCString(state));
+        llvm_unreachable("Unhandled state for resume");
     }
-
-    return error;
 }
 
 //===----------------------------------------------------------------------===//
 
 void
 NativeProcessLinux::StopRunningThreads(const lldb::tid_t triggering_tid)
 {
     Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
 
     if (log)
     {
         log->Printf("NativeProcessLinux::%s about to process event: (triggering_tid: %" PRIu64 ")",
                 __FUNCTION__, triggering_tid);
     }
 
-    DoStopThreads(PendingNotificationUP(new PendingNotification(triggering_tid)));
+    m_pending_notification_tid = triggering_tid;
+
+    // Request a stop for all the thread stops that need to be stopped
+    // and are not already known to be stopped.
+    for (const auto &thread_sp: m_threads)
+    {
+        if (StateIsRunningState(thread_sp->GetState()))
+            static_pointer_cast<NativeThreadLinux>(thread_sp)->RequestStop();
+    }
+
+    SignalIfAllThreadsStopped();
 
     if (log)
     {
         log->Printf("NativeProcessLinux::%s event processing done", __FUNCTION__);
     }
 }
 
 void
 NativeProcessLinux::SignalIfAllThreadsStopped()
 {
-    if (m_pending_notification_up && m_pending_notification_up->wait_for_stop_tids.empty ())
+    if (m_pending_notification_tid == LLDB_INVALID_THREAD_ID)
+        return; // No pending notification. Nothing to do.
+
+    for (const auto &thread_sp: m_threads)
     {
-        Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_BREAKPOINTS));
+        if (StateIsRunningState(thread_sp->GetState()))
+            return; // Some threads are still running. Don't signal yet.
+    }
+
+    // We have a pending notification and all threads have stopped.
 
-        // Clear any temporary breakpoints we used to implement software single stepping.
-        for (const auto &thread_info: m_threads_stepping_with_breakpoint)
-        {
-            Error error = RemoveBreakpoint (thread_info.second);
-            if (error.Fail())
-                if (log)
-                    log->Printf("NativeProcessLinux::%s() pid = %" PRIu64 " remove stepping breakpoint: %s",
-                            __FUNCTION__, thread_info.first, error.AsCString());
-        }
-        m_threads_stepping_with_breakpoint.clear();
 
-        // Notify the delegate about the stop
-        SetCurrentThreadID(m_pending_notification_up->triggering_tid);
-        SetState(StateType::eStateStopped, true);
-        m_pending_notification_up.reset();
-    }
-}
-
-void
-NativeProcessLinux::RequestStopOnAllRunningThreads()
-{
-    // Request a stop for all the thread stops that need to be stopped
-    // and are not already known to be stopped.  Keep a list of all the
-    // threads from which we still need to hear a stop reply.
-
-    ThreadIDSet sent_tids;
-    for (const auto &thread_sp: m_threads)
-    {
-        // We only care about running threads
-        if (StateIsStoppedState(thread_sp->GetState(), true))
-            continue;
-
-        static_pointer_cast<NativeThreadLinux>(thread_sp)->RequestStop();
-        sent_tids.insert (thread_sp->GetID());
-    }
-
-    // Set the wait list to the set of tids for which we requested stops.
-    m_pending_notification_up->wait_for_stop_tids.swap (sent_tids);
-}
-
-
-Error
-NativeProcessLinux::ThreadDidStop (lldb::tid_t tid, bool initiated_by_llgs)
-{
-    Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
-
-    if (log)
-        log->Printf("NativeProcessLinux::%s (tid: %" PRIu64 ", %sinitiated by llgs)",
-                __FUNCTION__, tid, initiated_by_llgs?"":"not ");
-
-    // Ensure we know about the thread.
-    auto thread_sp = std::static_pointer_cast<NativeThreadLinux>(GetThreadByID(tid));
-    lldbassert(thread_sp != nullptr);
-
-    // Update the global list of known thread states.  This one is definitely stopped.
-    auto& context = thread_sp->GetThreadContext();
-    const auto stop_was_requested = context.stop_requested;
-    context.stop_requested = false;
-
-    // If we have a pending notification, remove this from the set.
-    if (m_pending_notification_up)
-    {
-        m_pending_notification_up->wait_for_stop_tids.erase(tid);
-        SignalIfAllThreadsStopped();
-    }
-
-    Error error;
-    if (initiated_by_llgs && context.request_resume_function && !stop_was_requested)
-    {
-        // We can end up here if stop was initiated by LLGS but by this time a
-        // thread stop has occurred - maybe initiated by another event.
-        if (log)
-            log->Printf("Resuming thread %"  PRIu64 " since stop wasn't requested", tid);
-        error = context.request_resume_function (tid, true);
-        if (error.Fail() && log)
-        {
-                log->Printf("NativeProcessLinux::%s failed to resume thread tid  %" PRIu64 ": %s",
-                        __FUNCTION__, tid, error.AsCString ());
-        }
-    }
-    return error;
-}
-
-void
-NativeProcessLinux::DoStopThreads(PendingNotificationUP &&notification_up)
-{
-    Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
-    if (m_pending_notification_up && log)
-    {
-        // Yikes - we've already got a pending signal notification in progress.
-        // Log this info.  We lose the pending notification here.
-        log->Printf("NativeProcessLinux::%s dropping existing pending signal notification for tid %" PRIu64 ", to be replaced with signal for tid %" PRIu64,
-                   __FUNCTION__,
-                   m_pending_notification_up->triggering_tid,
-                   notification_up->triggering_tid);
-    }
-    m_pending_notification_up = std::move(notification_up);
-
-    RequestStopOnAllRunningThreads();
-
-    SignalIfAllThreadsStopped();
+    SetCurrentThreadID(m_pending_notification_tid);
+    m_pending_notification_tid = LLDB_INVALID_THREAD_ID;
 }
 
 void
 NativeProcessLinux::ThreadWasCreated (lldb::tid_t tid)
 {
     Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
 
     if (log)
         log->Printf("NativeProcessLinux::%s (tid: %" PRIu64 ")", __FUNCTION__, tid);
 
     auto thread_sp = std::static_pointer_cast<NativeThreadLinux>(GetThreadByID(tid));
     lldbassert(thread_sp != nullptr);
 
-    if (m_pending_notification_up && StateIsRunningState(thread_sp->GetState()))
+    if (m_pending_notification_tid != LLDB_INVALID_THREAD_ID && StateIsRunningState(thread_sp->GetState()))
     {
         // We will need to wait for this new thread to stop as well before firing the
         // notification.
-        m_pending_notification_up->wait_for_stop_tids.insert(tid);
         thread_sp->RequestStop();
     }
 }
 
 void
 NativeProcessLinux::SigchldHandler()
 {
     Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));

lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h

 //===-- NativeThreadLinux.h ----------------------------------- -*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_NativeThreadLinux_H_
 #define liblldb_NativeThreadLinux_H_
 
 #include "lldb/lldb-private-forward.h"
 #include "lldb/Host/common/NativeThreadProtocol.h"
 
 #include <map>
+#include <memory>
 #include <string>
 
 namespace lldb_private {
 namespace process_linux {
 
     class NativeProcessLinux;
 
     class NativeThreadLinux : public NativeThreadProtocol
private:
         SetStoppedWithNoReason ();
 
         void
         SetExited ();
 
         Error
         RequestStop ();
 
-        typedef std::function<Error (lldb::tid_t tid, bool supress_signal)> ResumeThreadFunction;
-        struct ThreadContext
-        {
-            bool stop_requested = false;
-            ResumeThreadFunction request_resume_function;
-        };
-
-        ThreadContext &
-        GetThreadContext() { return m_thread_context; }
-
         // ---------------------------------------------------------------------
         // Private interface
         // ---------------------------------------------------------------------
         void
         MaybeLogStateChange (lldb::StateType new_state);
 
         // ---------------------------------------------------------------------
         // Member Variables
         // ---------------------------------------------------------------------
         lldb::StateType m_state;
         ThreadStopInfo m_stop_info;
         NativeRegisterContextSP m_reg_context_sp;
         std::string m_stop_description;
         using WatchpointIndexMap = std::map<lldb::addr_t, uint32_t>;
         WatchpointIndexMap m_watchpoint_index_map;
-        ThreadContext m_thread_context;
     };
 
+    typedef std::shared_ptr<NativeThreadLinux> NativeThreadLinuxSP;
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef liblldb_NativeThreadLinux_H_

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

NativeThreadLinux::RequestStop ()
     Error err;
     errno = 0;
     if (::tgkill (pid, tid, SIGSTOP) != 0)
     {
         err.SetErrorToErrno ();
         if (log)
             log->Printf ("NativeThreadLinux::%s tgkill(%" PRIu64 ", %" PRIu64 ", SIGSTOP) failed: %s", __FUNCTION__, pid, tid, err.AsCString ());
     }
-    else
-        m_thread_context.stop_requested = true;
 
     return err;
 }
 
 void
 NativeThreadLinux::MaybeLogStateChange (lldb::StateType new_state)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD));

lldb/trunk/test/functionalities/thread/create_during_instruction_step/Makefile

+LEVEL = ../../../make
+
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+include $(LEVEL)/Makefile.rules

lldb/trunk/test/functionalities/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py

+"""
+This tests that we do not lose control of the inferior, while doing an instruction-level step
+over a thread creation instruction.
+"""
+
+import os
+import unittest2
+import lldb
+from lldbtest import *
+import lldbutil
+
+class CreateDuringInstructionStepTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+        # Find the line numbers to break and continue.
+        self.breakpoint = line_number('main.cpp', '// Set breakpoint here')
+
+    @dsym_test
+    def test_step_inst_with_dsym(self):
+        self.buildDsym(dictionary=self.getBuildFlags())
+        self.create_during_step_inst_test()
+
+    @dwarf_test
+    def test_step_inst_with_dwarf(self):
+        self.buildDwarf(dictionary=self.getBuildFlags())
+        self.create_during_step_inst_test()
+
+    def create_during_step_inst_test(self):
+        exe = os.path.join(os.getcwd(), "a.out")
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target and target.IsValid(), "Target is valid")
+
+        # This should create a breakpoint in the stepping thread.
+        self.bp_num = lldbutil.run_break_set_by_file_and_line (self, "main.cpp", self.breakpoint, num_expected_locations=-1)
+
+        # Run the program.
+        process = target.LaunchSimple(None, None, self.get_process_working_directory())
+        self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
+
+        # The stop reason of the thread should be breakpoint.
+        self.assertEqual(process.GetState(), lldb.eStateStopped, PROCESS_STOPPED)
+        self.assertEqual(lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint).IsValid(), 1,
+                STOPPED_DUE_TO_BREAKPOINT)
+
+        # Get the number of threads
+        num_threads = process.GetNumThreads()
+
+        # Make sure we see only one threads
+        self.assertEqual(num_threads, 1, 'Number of expected threads and actual threads do not match.')
+
+        thread = process.GetThreadAtIndex(0)
+        self.assertTrue(thread and thread.IsValid(), "Thread is valid")
+
+        # Keep stepping until we see the thread creation
+        while process.GetNumThreads() < 2:
+            # This skips some functions we have trouble stepping into. Testing stepping
+            # through these is not the purpose of this test. We just want to find the
+            # instruction, which creates the thread.
+            if thread.GetFrameAtIndex(0).GetFunctionName() in [
+                    '__sync_fetch_and_add_4', # Android arm: unable to set a breakpoint for software single-step
+                    'pthread_mutex_lock'      # Android arm: function contains atomic instruction sequences
+                    ]:
+                thread.StepOut()
+            else:
+                thread.StepInstruction(False)
+            self.assertEqual(process.GetState(), lldb.eStateStopped, PROCESS_STOPPED)
+            self.assertEqual(thread.GetStopReason(), lldb.eStopReasonPlanComplete, "Step operation succeeded")
+            if self.TraceOn():
+                self.runCmd("disassemble --pc")
+
+        if self.TraceOn():
+            self.runCmd("thread list")
+
+        # We have successfully caught thread creation. Now just run to completion
+        process.Continue()
+
+        # At this point, the inferior process should have exited.
+        self.assertEqual(process.GetState(), lldb.eStateExited, PROCESS_EXITED)
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

lldb/trunk/test/functionalities/thread/create_during_instruction_step/main.cpp

+//===-- main.cpp ------------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <atomic>
+#include <thread>
+
+std::atomic<bool> flag(false);
+
+void do_nothing()
+{
+    while (flag)
+        ;
+}
+
+int main ()
+{
+    // Instruction-level stepping over a creation of the first thread takes a very long time, so
+    // we give the threading machinery a chance to initialize all its data structures.
+    // This way, stepping over the second thread will be much faster.
+    std::thread dummy(do_nothing);
+    dummy.join();
+
+    // Make sure the new thread does not exit before we get a chance to notice the main thread stopped
+    flag = true;
+
+    std::thread thread(do_nothing); // Set breakpoint here
+    flag = false; // Release the new thread.
+    thread.join();
+    return 0;
+}