Skip to content

Commit b8af31d

Browse files
chaorenchaoren
committedFeb 3, 2015
Fix some bugs in llgs thread state handling.
* When the thread state coordinator is told to skip sending a stop request for a running thread that is ignored (e.g. the thread that steps in a step operation is technically running and should not have a stop sent to it, since it will stop of its own accord per the kernel step operation), ensure the deferred signal notification logic still waits for the skipped thread. (i.e. we want to defer the notification until the stepping thread is indeed stopped, we just don't want to send it a tgkill). * Add ThreadStateCoordinator::RequestResumeAsNeeded(). This variant of the RequestResume() method does not call the error function when the thread is already running. Instead, it just logs that the thread is already running and skips the resume operation. This is useful for the case of vCont;c handling, where we tell all threads that they should be running. At the place we're calling, all we know is "we want this thread running if it isn't already," and that's exactly what this command does. * Formatting change (minor) in NativeThreadLinux logging. llvm-svn: 227913
1 parent 03f12d6 commit b8af31d

File tree

4 files changed

+64
-24
lines changed

4 files changed

+64
-24
lines changed
 

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

+9-9
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ namespace
166166
{
167167
Log *const log = GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD);
168168
if (log)
169-
log->Printf ("NativePlatformLinux::%s ThreadStateCoordinator error received: %s", __FUNCTION__, error_message.c_str ());
169+
log->Printf ("NativePlatformLinux::%s %s", __FUNCTION__, error_message.c_str ());
170170
assert (false && "ThreadStateCoordinator error reported");
171171
}
172172

@@ -2689,14 +2689,14 @@ NativeProcessLinux::Resume (const ResumeActionList &resume_actions)
26892689
{
26902690
// Run the thread, possibly feeding it the signal.
26912691
const int signo = action->signal;
2692-
m_coordinator_up->RequestThreadResume (thread_sp->GetID (),
2693-
[=](lldb::tid_t tid_to_resume)
2694-
{
2695-
reinterpret_cast<NativeThreadLinux*> (thread_sp.get ())->SetRunning ();
2696-
// Pass this signal number on to the inferior to handle.
2697-
Resume (tid_to_resume, (signo > 0) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
2698-
},
2699-
CoordinatorErrorHandler);
2692+
m_coordinator_up->RequestThreadResumeAsNeeded (thread_sp->GetID (),
2693+
[=](lldb::tid_t tid_to_resume)
2694+
{
2695+
reinterpret_cast<NativeThreadLinux*> (thread_sp.get ())->SetRunning ();
2696+
// Pass this signal number on to the inferior to handle.
2697+
Resume (tid_to_resume, (signo > 0) ? signo : LLDB_INVALID_SIGNAL_NUMBER);
2698+
},
2699+
CoordinatorErrorHandler);
27002700
++resume_count;
27012701
break;
27022702
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ NativeThreadLinux::SetStoppedBySignal (uint32_t signo)
250250
{
251251
Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD));
252252
if (log)
253-
log->Printf ("NativeThreadLinux::%s called with signal 0x%" PRIx32, __FUNCTION__, signo);
253+
log->Printf ("NativeThreadLinux::%s called with signal 0x%02" PRIx32, __FUNCTION__, signo);
254254

255255
const StateType new_state = StateType::eStateStopped;
256256
MaybeLogStateChange (new_state);

‎lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.cpp

+43-13
Original file line numberDiff line numberDiff line change
@@ -287,17 +287,19 @@ class ThreadStateCoordinator::EventCallAfterThreadsStop : public ThreadStateCoor
287287
ThreadIDSet sent_tids;
288288
for (auto it = coordinator.m_tid_stop_map.begin(); it != coordinator.m_tid_stop_map.end(); ++it)
289289
{
290-
// Ignore threads we explicitly listed as skipped for stop requests.
291-
if (m_skip_stop_request_tids.count (it->first) > 0)
292-
continue;
293-
294290
// We only care about threads not stopped.
295291
const bool running = !it->second;
296292
if (running)
297293
{
298-
// Request this thread stop.
299294
const lldb::tid_t tid = it->first;
300-
m_request_thread_stop_function (tid);
295+
296+
// Request this thread stop if the tid stop request is not explicitly ignored.
297+
const bool skip_stop_request = m_skip_stop_request_tids.count (tid) > 0;
298+
if (!skip_stop_request)
299+
m_request_thread_stop_function (tid);
300+
301+
// Even if we skipped sending the stop request for other reasons (like stepping),
302+
// we still need to wait for that stepping thread to notify completion/stop.
301303
sent_tids.insert (tid);
302304
}
303305
}
@@ -452,11 +454,13 @@ class ThreadStateCoordinator::EventRequestResume : public ThreadStateCoordinator
452454
public:
453455
EventRequestResume (lldb::tid_t tid,
454456
const ThreadIDFunction &request_thread_resume_function,
455-
const ErrorFunction &error_function):
457+
const ErrorFunction &error_function,
458+
bool error_when_already_running):
456459
EventBase (),
457460
m_tid (tid),
458461
m_request_thread_resume_function (request_thread_resume_function),
459-
m_error_function (error_function)
462+
m_error_function (error_function),
463+
m_error_when_already_running (error_when_already_running)
460464
{
461465
}
462466

@@ -478,10 +482,25 @@ class ThreadStateCoordinator::EventRequestResume : public ThreadStateCoordinator
478482
const bool is_stopped = find_it->second;
479483
if (!is_stopped)
480484
{
481-
// Skip the resume call - we have tracked it to be running.
482-
std::ostringstream error_message;
483-
error_message << "error: tid " << m_tid << " asked to resume but we think it is already running";
484-
m_error_function (error_message.str ());
485+
// It's not an error, just a log, if the m_already_running_no_error flag is set.
486+
// This covers cases where, for instance, we're just trying to resume all threads
487+
// from the user side.
488+
if (!m_error_when_already_running)
489+
{
490+
coordinator.Log ("EventRequestResume::%s tid %" PRIu64 " optional resume skipped since it is already running",
491+
__FUNCTION__,
492+
m_tid);
493+
}
494+
else
495+
{
496+
// Skip the resume call - we have tracked it to be running. And we unconditionally
497+
// expected to resume this thread. Flag this as an error.
498+
std::ostringstream error_message;
499+
error_message << "error: tid " << m_tid << " asked to resume but we think it is already running";
500+
m_error_function (error_message.str ());
501+
}
502+
503+
// Error or not, we're done.
485504
return eventLoopResultContinue;
486505
}
487506

@@ -534,6 +553,7 @@ class ThreadStateCoordinator::EventRequestResume : public ThreadStateCoordinator
534553
const lldb::tid_t m_tid;
535554
ThreadIDFunction m_request_thread_resume_function;
536555
ErrorFunction m_error_function;
556+
const bool m_error_when_already_running;
537557
};
538558

539559
//===----------------------------------------------------------------------===//
@@ -763,7 +783,17 @@ ThreadStateCoordinator::RequestThreadResume (lldb::tid_t tid,
763783
const ThreadIDFunction &request_thread_resume_function,
764784
const ErrorFunction &error_function)
765785
{
766-
EnqueueEvent (EventBaseSP (new EventRequestResume (tid, request_thread_resume_function, error_function)));
786+
const bool error_when_already_running = true;
787+
EnqueueEvent (EventBaseSP (new EventRequestResume (tid, request_thread_resume_function, error_function, error_when_already_running)));
788+
}
789+
790+
void
791+
ThreadStateCoordinator::RequestThreadResumeAsNeeded (lldb::tid_t tid,
792+
const ThreadIDFunction &request_thread_resume_function,
793+
const ErrorFunction &error_function)
794+
{
795+
const bool error_when_already_running = false;
796+
EnqueueEvent (EventBaseSP (new EventRequestResume (tid, request_thread_resume_function, error_function, error_when_already_running)));
767797
}
768798

769799
void

‎lldb/source/Plugins/Process/Linux/ThreadStateCoordinator.h

+11-1
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,22 @@ namespace lldb_private
102102

103103
// Request that the given thread id should have the request_thread_resume_function
104104
// called. Will trigger the error_function if the thread is thought to be running
105-
// already at that point.
105+
// already at that point. This call signals an error if the thread resume is for
106+
// a thread that is already in a running state.
106107
void
107108
RequestThreadResume (lldb::tid_t tid,
108109
const ThreadIDFunction &request_thread_resume_function,
109110
const ErrorFunction &error_function);
110111

112+
// Request that the given thread id should have the request_thread_resume_function
113+
// called. Will trigger the error_function if the thread is thought to be running
114+
// already at that point. This call ignores threads that are already running and
115+
// does not trigger an error in that case.
116+
void
117+
RequestThreadResumeAsNeeded (lldb::tid_t tid,
118+
const ThreadIDFunction &request_thread_resume_function,
119+
const ErrorFunction &error_function);
120+
111121
// Indicate the calling process did an exec and that the thread state
112122
// should be 100% cleared.
113123
//

0 commit comments

Comments
 (0)
Please sign in to comment.