[Lldb-commits] [lldb] r237448 - [NativeProcessLinux] Fix removal of temporary breakpoints

[Pavel Labath]

Author: labath
Date: Fri May 15 08:49:01 2015
New Revision: 237448

URL: http://llvm.org/viewvc/llvm-project?rev=237448&view=rev
Log:
[NativeProcessLinux] Fix removal of temporary breakpoints

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.

Test Plan: TestExitDuringStep passes, other tests remain unchanged.

Reviewers: tberghammer

Subscribers: tberghammer, aemerson, lldb-commits

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

Modified:
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.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=237448&r1=237447&r2=237448&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Fri May 15 08:49:01 2015
@@ -2601,18 +2601,8 @@ NativeProcessLinux::MonitorBreakpoint(ll
                 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)
@@ -3023,7 +3013,6 @@ NativeProcessLinux::Resume (const Resume
     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);
@@ -3109,7 +3098,6 @@ NativeProcessLinux::Resume (const Resume
                         return step_result;
                     },
                     false);
-            stepping = true;
             break;
         }
 
@@ -4081,7 +4069,7 @@ NativeProcessLinux::StopTrackingThread (
     if (m_pending_notification_up)
     {
         m_pending_notification_up->wait_for_stop_tids.erase(thread_id);
-        SignalIfRequirementsSatisfied();
+        SignalIfAllThreadsStopped();
     }
 
     return found;
@@ -4331,10 +4319,24 @@ NativeProcessLinux::StopRunningThreads(c
 }
 
 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();
@@ -4386,7 +4388,7 @@ NativeProcessLinux::ThreadDidStop (lldb:
     if (m_pending_notification_up)
     {
         m_pending_notification_up->wait_for_stop_tids.erase(tid);
-        SignalIfRequirementsSatisfied();
+        SignalIfAllThreadsStopped();
     }
 
     Error error;
@@ -4423,7 +4425,7 @@ NativeProcessLinux::DoStopThreads(Pendin
 
     RequestStopOnAllRunningThreads();
 
-    SignalIfRequirementsSatisfied();
+    SignalIfAllThreadsStopped();
 }
 
 void

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=237448&r1=237447&r2=237448&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h Fri May 15 08:49:01 2015
@@ -385,8 +385,8 @@ namespace process_linux {
         };
         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();