[Lldb-commits] [lldb] r227912 - llgs: more work on thread stepping.
Chaoren Lin
chaorenl at google.com
Mon Feb 2 17:50:49 PST 2015
Author: chaoren
Date: Mon Feb 2 19:50:49 2015
New Revision: 227912
URL: http://llvm.org/viewvc/llvm-project?rev=227912&view=rev
Log:
llgs: more work on thread stepping.
See https://github.com/tfiala/lldb/issues/75. Not fixed yet but
continuing to push this further.
Fixes:
* Resume() now skips doing deferred notifications if we're doing a
vCont;{c,C}. In this case, we're trying to start something up,
not defer a stop notification. The default thread action stop
mode pickup was triggering a stop because it had at least one
stop, which was wrong in the case of a continue. (Bug introduced
by previous change.)
* Added a variant to ThreadStateCoordinator to specify a set of
thread ids to be skipped when triggering stop notifications to
non-stopped threads on a deferred signal call. For the case of
a stepping thread, it is actually told to step (and is running)
for a brief moment, but the thread state coordinator would think
it needed to send the stepping thread a stop, which id doesn't
need to do. This facility allows me to get around that cleanly.
With this change, behavior is now reduced to something I think is
essentially a different bug:
* Doing a step into libc code from my code crashes llgs.
* Doing a next out of a function in my own code crashes llgs.
Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp
lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.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=227912&r1=227911&r2=227912&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Mon Feb 2 19:50:49 2015
@@ -2532,8 +2532,10 @@ NativeProcessLinux::MonitorSignal(const
GetID (),
pid);
- // An inferior thread just stopped. Mark it as such.
- reinterpret_cast<NativeThreadLinux*> (thread_sp.get ())->SetStoppedBySignal (signo);
+ // An inferior thread just stopped, but was not the primary cause of the process stop.
+ // Instead, something else (like a breakpoint or step) caused the stop. Mark the
+ // stop signal as 0 to let lldb know this isn't the important stop.
+ reinterpret_cast<NativeThreadLinux*> (thread_sp.get ())->SetStoppedBySignal (0);
SetCurrentThreadID (thread_sp->GetID ());
// Tell the thread state coordinator about the stop.
@@ -2656,11 +2658,14 @@ NativeProcessLinux::Resume (const Resume
if (log)
log->Printf ("NativeProcessLinux::%s called: pid %" PRIu64, __FUNCTION__, GetID ());
- int deferred_signal_tid = LLDB_INVALID_THREAD_ID;
+ lldb::tid_t deferred_signal_tid = LLDB_INVALID_THREAD_ID;
+ lldb::tid_t deferred_signal_skip_tid = LLDB_INVALID_THREAD_ID;
int deferred_signo = 0;
NativeThreadProtocolSP deferred_signal_thread_sp;
+ int resume_count = 0;
- std::vector<NativeThreadProtocolSP> new_stop_threads;
+
+ // std::vector<NativeThreadProtocolSP> new_stop_threads;
// Scope for threads mutex.
{
@@ -2692,6 +2697,7 @@ NativeProcessLinux::Resume (const Resume
Resume (tid_to_resume, (signo > 0) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
},
CoordinatorErrorHandler);
+ ++resume_count;
break;
}
@@ -2713,6 +2719,16 @@ NativeProcessLinux::Resume (const Resume
deferred_signal_tid = thread_sp->GetID ();
deferred_signal_thread_sp = thread_sp;
+ // Don't send a stop request to this thread. The thread resume request
+ // above will actually run the step thread, and it will finish the step
+ // by sending a SIGTRAP with the appropriate bits set. So, the deferred
+ // signal call that happens at the end of the loop below needs to let
+ // the pending signal handling to *not* send a stop for this thread here
+ // since the start/stop step functionality will end up with a stop state.
+ // Otherwise, this stepping thread will get sent an erroneous tgkill for
+ // with a SIGSTOP signal.
+ deferred_signal_skip_tid = thread_sp->GetID ();
+
// And the stop signal we should apply for it is a SIGTRAP.
deferred_signo = SIGTRAP;
break;
@@ -2736,13 +2752,17 @@ NativeProcessLinux::Resume (const Resume
}
}
- // We don't need to tell the delegate when we're running, so don't do that here.
+ // If we resumed anything, this command was about starting a stopped thread,
+ // not about stopping something that we should trigger later.
+ if (resume_count > 0)
+ return error;
// If we had any thread stopping, then do a deferred notification of the chosen stop thread id and signal
// after all other running threads have stopped.
if (deferred_signal_tid != LLDB_INVALID_THREAD_ID)
{
- CallAfterRunningThreadsStop (deferred_signal_tid,
+ CallAfterRunningThreadsStopWithSkipTID (deferred_signal_tid,
+ deferred_signal_skip_tid,
[=](lldb::tid_t deferred_notification_tid)
{
// Set the signal thread to the current thread.
@@ -3904,5 +3924,20 @@ NativeProcessLinux::CallAfterRunningThre
},
call_after_function,
CoordinatorErrorHandler);
+}
+void
+NativeProcessLinux::CallAfterRunningThreadsStopWithSkipTID (lldb::tid_t deferred_signal_tid,
+ lldb::tid_t skip_stop_request_tid,
+ const std::function<void (lldb::tid_t tid)> &call_after_function)
+{
+ const lldb::pid_t pid = GetID ();
+ m_coordinator_up->CallAfterRunningThreadsStopWithSkipTIDs (deferred_signal_tid,
+ skip_stop_request_tid != LLDB_INVALID_THREAD_ID ? ThreadStateCoordinator::ThreadIDSet {skip_stop_request_tid} : ThreadStateCoordinator::ThreadIDSet (),
+ [=](lldb::tid_t request_stop_tid)
+ {
+ tgkill (pid, request_stop_tid, SIGSTOP);
+ },
+ call_after_function,
+ CoordinatorErrorHandler);
}
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=227912&r1=227911&r2=227912&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h Mon Feb 2 19:50:49 2015
@@ -398,6 +398,11 @@ namespace lldb_private
CallAfterRunningThreadsStop (lldb::tid_t tid,
const std::function<void (lldb::tid_t tid)> &call_after_function);
+ void
+ CallAfterRunningThreadsStopWithSkipTID (lldb::tid_t deferred_signal_tid,
+ lldb::tid_t skip_stop_request_tid,
+ const std::function<void (lldb::tid_t tid)> &call_after_function);
+
lldb_private::Error
Detach(lldb::tid_t tid);
};
Modified: lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp?rev=227912&r1=227911&r2=227912&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp Mon Feb 2 19:50:49 2015
@@ -83,7 +83,8 @@ public:
m_request_thread_stop_function (request_thread_stop_function),
m_call_after_function (call_after_function),
m_error_function (error_function),
- m_request_stop_on_all_unstopped_threads (false)
+ m_request_stop_on_all_unstopped_threads (false),
+ m_skip_stop_request_tids ()
{
}
@@ -98,7 +99,25 @@ public:
m_request_thread_stop_function (request_thread_stop_function),
m_call_after_function (call_after_function),
m_error_function (error_function),
- m_request_stop_on_all_unstopped_threads (true)
+ m_request_stop_on_all_unstopped_threads (true),
+ m_skip_stop_request_tids ()
+ {
+ }
+
+ EventCallAfterThreadsStop (lldb::tid_t triggering_tid,
+ const ThreadIDFunction &request_thread_stop_function,
+ const ThreadIDFunction &call_after_function,
+ const ThreadIDSet &skip_stop_request_tids,
+ const ErrorFunction &error_function) :
+ EventBase (),
+ m_triggering_tid (triggering_tid),
+ m_wait_for_stop_tids (),
+ m_original_wait_for_stop_tids (),
+ m_request_thread_stop_function (request_thread_stop_function),
+ m_call_after_function (call_after_function),
+ m_error_function (error_function),
+ m_request_stop_on_all_unstopped_threads (true),
+ m_skip_stop_request_tids (skip_stop_request_tids)
{
}
@@ -203,7 +222,7 @@ public:
GetDescription () override
{
std::ostringstream description;
- description << "EventCallAfterThreadsStop (triggering_tid=" << m_triggering_tid << ", request_stop_on_all_unstopped_threads=" << m_request_stop_on_all_unstopped_threads << ")";
+ description << "EventCallAfterThreadsStop (triggering_tid=" << m_triggering_tid << ", request_stop_on_all_unstopped_threads=" << m_request_stop_on_all_unstopped_threads << ", skip_stop_request_tids.size()=" << m_skip_stop_request_tids.size () << ")";
return description.str ();
}
@@ -268,6 +287,10 @@ private:
ThreadIDSet sent_tids;
for (auto it = coordinator.m_tid_stop_map.begin(); it != coordinator.m_tid_stop_map.end(); ++it)
{
+ // Ignore threads we explicitly listed as skipped for stop requests.
+ if (m_skip_stop_request_tids.count (it->first) > 0)
+ continue;
+
// We only care about threads not stopped.
const bool running = !it->second;
if (running)
@@ -290,6 +313,7 @@ private:
ThreadIDFunction m_call_after_function;
ErrorFunction m_error_function;
const bool m_request_stop_on_all_unstopped_threads;
+ ThreadIDSet m_skip_stop_request_tids;
};
//===----------------------------------------------------------------------===//
@@ -601,6 +625,21 @@ ThreadStateCoordinator::CallAfterRunning
}
void
+ThreadStateCoordinator::CallAfterRunningThreadsStopWithSkipTIDs (lldb::tid_t triggering_tid,
+ const ThreadIDSet &skip_stop_request_tids,
+ const ThreadIDFunction &request_thread_stop_function,
+ const ThreadIDFunction &call_after_function,
+ const ErrorFunction &error_function)
+{
+ EnqueueEvent (EventBaseSP (new EventCallAfterThreadsStop (triggering_tid,
+ request_thread_stop_function,
+ call_after_function,
+ skip_stop_request_tids,
+ error_function)));
+}
+
+
+void
ThreadStateCoordinator::ThreadDidStop (lldb::tid_t tid, ErrorFunction &error_function)
{
// Ensure we know about the thread.
Modified: lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h?rev=227912&r1=227911&r2=227912&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h (original)
+++ lldb/trunk/source/Plugins/Process/Linux/ThreadStateCoordinator.h Mon Feb 2 19:50:49 2015
@@ -81,6 +81,19 @@ namespace lldb_private
const ThreadIDFunction &call_after_function,
const ErrorFunction &error_function);
+ // This method is the main purpose of the class: triggering a deferred
+ // action after all non-stopped threads stop. The triggering_tid is the
+ // thread id passed to the call_after_function. The error_function will
+ // be fired if the triggering tid is unknown at the time of execution.
+ // This variant will send stop requests to all non-stopped threads except
+ // for any contained in skip_stop_request_tids.
+ void
+ CallAfterRunningThreadsStopWithSkipTIDs (lldb::tid_t triggering_tid,
+ const ThreadIDSet &skip_stop_request_tids,
+ const ThreadIDFunction &request_thread_stop_function,
+ const ThreadIDFunction &call_after_function,
+ const ErrorFunction &error_function);
+
// Notify the thread stopped. Will trigger error at time of execution if we
// already think it is stopped.
void
More information about the lldb-commits
mailing list