[NativeProcessLinux] Fix removal of temporary breakpoints

[labath]

Summary

There was an issue in NPL, where we attempted removal of temporary breakpoints (used to implement
software single stepping), while some threads of the process were running. This is a problem
since we currently always use the main thread's ID in the removal ptrace call. Therefore, if the
main thread was still running, the ptrace call would fail, and the software breakpoint would
remain, causing all kinds of problems. This change removes the breakpoints after all threads have
stopped. This fixes TestExitDuringStep on Android arm and can also potentially help in other
situations, as previously the breakpoint would not get removed if the thread stopped for another
reason.

[tberghammer]

LGTM

source/Plugins/Process/Linux/NativeProcessLinux.h

#endif
             {
             }
 
             const lldb::tid_t  triggering_tid;
             ThreadIDSet        wait_for_stop_tids;
         };
         typedef std::unique_ptr<PendingNotification> PendingNotificationUP;
 
-        // Fire pending notification if no pending thread stops remain.
-        void SignalIfRequirementsSatisfied();
+        // 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

source/Plugins/Process/Linux/NativeProcessLinux.cpp

NativeProcessLinux::MonitorBreakpoint(lldb::pid_t pid, NativeThreadProtocolSP 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",
                         __FUNCTION__, pid, error.AsCString());
 
-        auto it = m_threads_stepping_with_breakpoint.find(pid);
-        if (it != m_threads_stepping_with_breakpoint.end())
-        {
-            Error error = RemoveBreakpoint (it->second);
-            if (error.Fail())
-                if (log)
-                    log->Printf("NativeProcessLinux::%s() pid = %" PRIu64 " remove stepping breakpoint: %s",
-                            __FUNCTION__, pid, error.AsCString());
-
-            m_threads_stepping_with_breakpoint.erase(it);
+        if (m_threads_stepping_with_breakpoint.find(pid) != m_threads_stepping_with_breakpoint.end())
             std::static_pointer_cast<NativeThreadLinux>(thread_sp)->SetStoppedByTrace();
-        }
-    }
     else
         if (log)
             log->Printf("NativeProcessLinux::%s()  pid = %" PRIu64 ": "
                     "warning, cannot process software breakpoint since no thread metadata",
                     __FUNCTION__, pid);
 
 
     // We need to tell all other running threads before we notify the delegate about this stop.

 
 Error
 NativeProcessLinux::Resume (const ResumeActionList &resume_actions)
 {
     Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_THREAD));
     if (log)
         log->Printf ("NativeProcessLinux::%s called: pid %" PRIu64, __FUNCTION__, GetID ());
 
-    bool stepping = false;
     bool software_single_step = !SupportHardwareSingleStepping();
 
     Monitor::ScopedOperationLock monitor_lock(*m_monitor_up);
     Mutex::Locker locker (m_threads_mutex);
 
     if (software_single_step)
     {
         for (auto thread_sp : m_threads)
for (auto thread_sp : m_threads)
                             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);
-            stepping = true;
             break;
         }
 
         case eStateSuspended:
         case eStateStopped:
             lldbassert(0 && "Unexpected state");
 
         default:
for (auto it = m_threads.begin (); it != m_threads.end (); ++it)
             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);
-        SignalIfRequirementsSatisfied();
+        SignalIfAllThreadsStopped();
     }
 
     return found;
 }
 
 NativeThreadProtocolSP
 NativeProcessLinux::AddThread (lldb::tid_t thread_id)
 {
NativeProcessLinux::StopRunningThreads(const lldb::tid_t triggering_tid)
 
     if (log)
     {
         log->Printf("NativeProcessLinux::%s event processing done", __FUNCTION__);
     }
 }
 
 void
-NativeProcessLinux::SignalIfRequirementsSatisfied()
+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();
+
+        // 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()
NativeProcessLinux::ThreadDidStop (lldb::tid_t tid, bool initiated_by_llgs)
     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);
-        SignalIfRequirementsSatisfied();
+        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)
if (m_pending_notification_up && log)
                    __FUNCTION__,
                    m_pending_notification_up->triggering_tid,
                    notification_up->triggering_tid);
     }
     m_pending_notification_up = std::move(notification_up);
 
     RequestStopOnAllRunningThreads();
 
-    SignalIfRequirementsSatisfied();
+    SignalIfAllThreadsStopped();
 }
 
 void
 NativeProcessLinux::ThreadWasCreated (lldb::tid_t tid)
 {
     Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
 
     if (log)