[Lldb-commits] [lldb] r245545 - [NativeProcessLinux] Fix a bug in instruction-stepping over thread creation

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Aug 20 02:06:13 PDT 2015


Author: labath
Date: Thu Aug 20 04:06:12 2015
New Revision: 245545

URL: http://llvm.org/viewvc/llvm-project?rev=245545&view=rev
Log:
[NativeProcessLinux] Fix a bug in instruction-stepping over thread creation

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.

Reviewers: ovyalov, tberghammer

Subscribers: lldb-commits

Differential Revision: http://reviews.llvm.org/D12104

Added:
    lldb/trunk/test/functionalities/thread/create_during_instruction_step/
    lldb/trunk/test/functionalities/thread/create_during_instruction_step/Makefile
    lldb/trunk/test/functionalities/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
    lldb/trunk/test/functionalities/thread/create_during_instruction_step/main.cpp
Modified:
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
    lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=245545&r1=245544&r2=245545&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Thu Aug 20 04:06:12 2015
@@ -428,7 +428,8 @@ NativeProcessLinux::NativeProcessLinux (
     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)
 {
 }
 
@@ -1055,7 +1056,7 @@ NativeProcessLinux::WaitForNewThread(::p
 {
     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)
     {
@@ -1113,9 +1114,8 @@ NativeProcessLinux::WaitForNewThread(::p
         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);
 }
 
@@ -1132,7 +1132,7 @@ NativeProcessLinux::MonitorSIGTRAP(const
     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)
@@ -1161,7 +1161,7 @@ NativeProcessLinux::MonitorSIGTRAP(const
         } else 
             WaitForNewThread(event_message);
 
-        Resume (pid, LLDB_INVALID_SIGNAL_NUMBER);
+        ResumeThread(thread_sp, thread_sp->GetState(), LLDB_INVALID_SIGNAL_NUMBER);
         break;
     }
 
@@ -1172,7 +1172,7 @@ NativeProcessLinux::MonitorSIGTRAP(const
             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)
@@ -1253,7 +1253,7 @@ NativeProcessLinux::MonitorSIGTRAP(const
             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;
     }
@@ -1313,7 +1313,7 @@ NativeProcessLinux::MonitorSIGTRAP(const
             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:
@@ -1334,12 +1334,10 @@ NativeProcessLinux::MonitorTrace(lldb::p
         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
@@ -1357,9 +1355,6 @@ NativeProcessLinux::MonitorBreakpoint(ll
         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)
     {
@@ -1393,9 +1388,6 @@ NativeProcessLinux::MonitorWatchpoint(ll
                     "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");
@@ -1429,7 +1421,7 @@ NativeProcessLinux::MonitorSignal(const
     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)
@@ -1461,11 +1453,10 @@ NativeProcessLinux::MonitorSignal(const
             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;
@@ -1502,13 +1493,27 @@ NativeProcessLinux::MonitorSignal(const
                 // 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)
-                    linux_thread_sp->SetStoppedBySignal(SIGSTOP, info);
-                else
-                    linux_thread_sp->SetStoppedWithNoReason();
+                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);
+                    SetCurrentThreadID (thread_sp->GetID ());
+                    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
             {
@@ -1529,7 +1534,7 @@ NativeProcessLinux::MonitorSignal(const
                                  stop_signo,
                                  signal_name);
                 }
-                ThreadDidStop (thread_sp->GetID (), false);
+                SignalIfAllThreadsStopped();
             }
         }
 
@@ -1541,8 +1546,6 @@ NativeProcessLinux::MonitorSignal(const
         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);
 
@@ -1794,44 +1797,11 @@ NativeProcessLinux::Resume (const Resume
         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;
         }
 
@@ -2752,7 +2722,10 @@ NativeProcessLinux::SingleStep(lldb::tid
     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
@@ -2842,12 +2815,7 @@ NativeProcessLinux::StopTrackingThread (
         }
     }
 
-    // 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();
-    }
+    SignalIfAllThreadsStopped();
 
     return found;
 }
@@ -3042,61 +3010,51 @@ NativeProcessLinux::GetFileLoadAddress(c
 }
 
 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");
+        log->Printf("NativeProcessLinux::%s (tid: %" PRIu64 ")",
+                __FUNCTION__, thread_sp->GetID());
     
-    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();
-    }
-
     // 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;
 }
 
 //===----------------------------------------------------------------------===//
@@ -3112,122 +3070,54 @@ NativeProcessLinux::StopRunningThreads(c
                 __FUNCTION__, triggering_tid);
     }
 
-    DoStopThreads(PendingNotificationUP(new PendingNotification(triggering_tid)));
+    m_pending_notification_tid = triggering_tid;
 
-    if (log)
+    // 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)
     {
-        log->Printf("NativeProcessLinux::%s event processing done", __FUNCTION__);
+        if (StateIsRunningState(thread_sp->GetState()))
+            static_pointer_cast<NativeThreadLinux>(thread_sp)->RequestStop();
     }
-}
-
-void
-NativeProcessLinux::SignalIfAllThreadsStopped()
-{
-    if (m_pending_notification_up && m_pending_notification_up->wait_for_stop_tids.empty ())
-    {
-        Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_BREAKPOINTS));
 
-        // 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();
+    SignalIfAllThreadsStopped();
 
-        // Notify the delegate about the stop
-        SetCurrentThreadID(m_pending_notification_up->triggering_tid);
-        SetState(StateType::eStateStopped, true);
-        m_pending_notification_up.reset();
+    if (log)
+    {
+        log->Printf("NativeProcessLinux::%s event processing done", __FUNCTION__);
     }
 }
 
 void
-NativeProcessLinux::RequestStopOnAllRunningThreads()
+NativeProcessLinux::SignalIfAllThreadsStopped()
 {
-    // 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.
+    if (m_pending_notification_tid == LLDB_INVALID_THREAD_ID)
+        return; // No pending notification. Nothing to do.
 
-    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());
+        if (StateIsRunningState(thread_sp->GetState()))
+            return; // Some threads are still running. Don't signal yet.
     }
 
-    // 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;
-}
+    // We have a pending notification and all threads have stopped.
+    Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_BREAKPOINTS));
 
-void
-NativeProcessLinux::DoStopThreads(PendingNotificationUP &&notification_up)
-{
-    Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
-    if (m_pending_notification_up && log)
+    // Clear any temporary breakpoints we used to implement software single stepping.
+    for (const auto &thread_info: m_threads_stepping_with_breakpoint)
     {
-        // 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);
+        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_pending_notification_up = std::move(notification_up);
-
-    RequestStopOnAllRunningThreads();
+    m_threads_stepping_with_breakpoint.clear();
 
-    SignalIfAllThreadsStopped();
+    // Notify the delegate about the stop
+    SetCurrentThreadID(m_pending_notification_tid);
+    SetState(StateType::eStateStopped, true);
+    m_pending_notification_tid = LLDB_INVALID_THREAD_ID;
 }
 
 void
@@ -3241,11 +3131,10 @@ NativeProcessLinux::ThreadWasCreated (ll
     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();
     }
 }

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h?rev=245545&r1=245544&r2=245545&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h Thu Aug 20 04:06:12 2015
@@ -140,6 +140,8 @@ namespace process_linux {
         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;
@@ -300,55 +302,25 @@ namespace process_linux {
         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

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp?rev=245545&r1=245544&r2=245545&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.cpp Thu Aug 20 04:06:12 2015
@@ -418,8 +418,6 @@ NativeThreadLinux::RequestStop ()
         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;
 }

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h?rev=245545&r1=245544&r2=245545&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h Thu Aug 20 04:06:12 2015
@@ -14,6 +14,7 @@
 #include "lldb/Host/common/NativeThreadProtocol.h"
 
 #include <map>
+#include <memory>
 #include <string>
 
 namespace lldb_private {
@@ -95,16 +96,6 @@ namespace process_linux {
         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
         // ---------------------------------------------------------------------
@@ -120,9 +111,9 @@ namespace process_linux {
         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
 

Added: lldb/trunk/test/functionalities/thread/create_during_instruction_step/Makefile
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/thread/create_during_instruction_step/Makefile?rev=245545&view=auto
==============================================================================
--- lldb/trunk/test/functionalities/thread/create_during_instruction_step/Makefile (added)
+++ lldb/trunk/test/functionalities/thread/create_during_instruction_step/Makefile Thu Aug 20 04:06:12 2015
@@ -0,0 +1,5 @@
+LEVEL = ../../../make
+
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+include $(LEVEL)/Makefile.rules

Added: lldb/trunk/test/functionalities/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py?rev=245545&view=auto
==============================================================================
--- lldb/trunk/test/functionalities/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py (added)
+++ lldb/trunk/test/functionalities/thread/create_during_instruction_step/TestCreateDuringInstructionStep.py Thu Aug 20 04:06:12 2015
@@ -0,0 +1,88 @@
+"""
+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()

Added: lldb/trunk/test/functionalities/thread/create_during_instruction_step/main.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/thread/create_during_instruction_step/main.cpp?rev=245545&view=auto
==============================================================================
--- lldb/trunk/test/functionalities/thread/create_during_instruction_step/main.cpp (added)
+++ lldb/trunk/test/functionalities/thread/create_during_instruction_step/main.cpp Thu Aug 20 04:06:12 2015
@@ -0,0 +1,36 @@
+//===-- 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;
+}




More information about the lldb-commits mailing list