Skip to content

Commit

Permalink
[lldb] [llgs] Fix multi-resume bugs with nonstop mode
Browse files Browse the repository at this point in the history 
Improve handling of multiple successive continue packets in non-stop
mode.  More specifically:

1. Explicitly send error response (instead of crashing on assertion)
   if the user attempts to resume the same process twice.  Since we
   do not support thread-level non-stop mode, one needs to always stop
   the process explicitly before resuming another thread set.

2. Actually stop the process if "vCont;t" is delivered to a running
   process.  Similarly, we only support stopping all the running threads
   simultaneously (via -1) and return an error in any other case.

With this patch, running multiple processes simultaneously is still
unsupported.  The patch also employs a hack to avoid enabling stdio
forwarding on "vCont;t" packet.  Both of these issues are addressed
by followup patches.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128710
  • Loading branch information
@mgorny
mgorny committed Jul 15, 2022
1 parent 7d297de commit f8605da
Show file tree
Hide file tree
Showing 4 changed files with 235 additions and 85 deletions.
132 changes: 98 additions & 34 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
View file
Expand Up @@ -1512,6 +1512,30 @@ GDBRemoteCommunicationServerLLGS::Handle_QListThreadsInStopReply(
return SendOKResponse();
}

GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::ResumeProcess(
NativeProcessProtocol &process, const ResumeActionList &actions) {
Log *log = GetLog(LLDBLog::Process | LLDBLog::Thread);

// In non-stop protocol mode, the process could be running already.
// We do not support resuming threads independently, so just error out.
if (!process.CanResume()) {
LLDB_LOG(log, "process {0} cannot be resumed (state={1})", process.GetID(),
process.GetState());
return SendErrorResponse(0x37);
}

Status error = process.Resume(actions);
if (error.Fail()) {
LLDB_LOG(log, "process {0} failed to resume: {1}", process.GetID(), error);
return SendErrorResponse(GDBRemoteServerError::eErrorResume);
}

LLDB_LOG(log, "process {0} resumed", process.GetID());

return PacketResult::Success;
}

GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::Handle_C(StringExtractorGDBRemote &packet) {
Log *log = GetLog(LLDBLog::Process | LLDBLog::Thread);
Expand Down Expand Up @@ -1547,6 +1571,14 @@ GDBRemoteCommunicationServerLLGS::Handle_C(StringExtractorGDBRemote &packet) {
packet, "unexpected content after $C{signal-number}");
}

// In non-stop protocol mode, the process could be running already.
// We do not support resuming threads independently, so just error out.
if (!m_continue_process->CanResume()) {
LLDB_LOG(log, "process cannot be resumed (state={0})",
m_continue_process->GetState());
return SendErrorResponse(0x37);
}

ResumeActionList resume_actions(StateType::eStateRunning,
LLDB_INVALID_SIGNAL_NUMBER);
Status error;
Expand Down Expand Up @@ -1580,14 +1612,11 @@ GDBRemoteCommunicationServerLLGS::Handle_C(StringExtractorGDBRemote &packet) {
}
}

// Resume the threads.
error = m_continue_process->Resume(resume_actions);
if (error.Fail()) {
LLDB_LOG(log, "failed to resume threads for process {0}: {1}",
m_continue_process->GetID(), error);

return SendErrorResponse(0x38);
}
// NB: this checks CanResume() twice but using a single code path for
// resuming still seems worth it.
PacketResult resume_res = ResumeProcess(*m_continue_process, resume_actions);
if (resume_res != PacketResult::Success)
return resume_res;

// Don't send an "OK" packet, except in non-stop mode;
// otherwise, the response is the stopped/exited message.
Expand Down Expand Up @@ -1622,14 +1651,9 @@ GDBRemoteCommunicationServerLLGS::Handle_c(StringExtractorGDBRemote &packet) {
ResumeActionList actions(StateType::eStateRunning,
LLDB_INVALID_SIGNAL_NUMBER);

Status error = m_continue_process->Resume(actions);
if (error.Fail()) {
LLDB_LOG(log, "c failed for process {0}: {1}", m_continue_process->GetID(),
error);
return SendErrorResponse(GDBRemoteServerError::eErrorResume);
}

LLDB_LOG(log, "continued process {0}", m_continue_process->GetID());
PacketResult resume_res = ResumeProcess(*m_continue_process, actions);
if (resume_res != PacketResult::Success)
return resume_res;

return SendContinueSuccessResponse();
}
Expand All @@ -1643,6 +1667,18 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont_actions(
return SendPacketNoLock(response.GetString());
}

static bool ResumeActionListStopsAllThreads(ResumeActionList &actions) {
// We're doing a stop-all if and only if our only action is a "t" for all
// threads.
if (const ResumeAction *default_action =
actions.GetActionForThread(LLDB_INVALID_THREAD_ID, false)) {
if (default_action->state == eStateSuspended && actions.GetSize() == 1)
return true;
}

return false;
}

GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::Handle_vCont(
StringExtractorGDBRemote &packet) {
Expand All @@ -1664,9 +1700,6 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
// Move past the ';', then do a simple 's'.
packet.SetFilePos(packet.GetFilePos() + 1);
return Handle_s(packet);
} else if (m_non_stop && ::strcmp(packet.Peek(), ";t") == 0) {
// TODO: add full support for "t" action
return SendOKResponse();
}

std::unordered_map<lldb::pid_t, ResumeActionList> thread_actions;
Expand Down Expand Up @@ -1733,6 +1766,12 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
tid = pid_tid->second;
}

if (thread_action.state == eStateSuspended &&
tid != StringExtractorGDBRemote::AllThreads) {
return SendIllFormedResponse(
packet, "'t' action not supported for individual threads");
}

if (pid == StringExtractorGDBRemote::AllProcesses) {
if (m_debugged_processes.size() > 1)
return SendIllFormedResponse(
Expand Down Expand Up @@ -1765,13 +1804,43 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
return SendErrorResponse(GDBRemoteServerError::eErrorResume);
}

Status error = process_it->second.process_up->Resume(x.second);
if (error.Fail()) {
LLDB_LOG(log, "vCont failed for process {0}: {1}", x.first, error);
return SendErrorResponse(GDBRemoteServerError::eErrorResume);
}
// There are four possible scenarios here. These are:
// 1. vCont on a stopped process that resumes at least one thread.
// In this case, we call Resume().
// 2. vCont on a stopped process that leaves all threads suspended.
// A no-op.
// 3. vCont on a running process that requests suspending all
// running threads. In this case, we call Interrupt().
// 4. vCont on a running process that requests suspending a subset
// of running threads or resuming a subset of suspended threads.
// Since we do not support full nonstop mode, this is unsupported
// and we return an error.

assert(process_it->second.process_up);
if (ResumeActionListStopsAllThreads(x.second)) {
if (process_it->second.process_up->IsRunning()) {
assert(m_non_stop);

Status error = process_it->second.process_up->Interrupt();
if (error.Fail()) {
LLDB_LOG(log, "vCont failed to halt process {0}: {1}", x.first,
error);
return SendErrorResponse(GDBRemoteServerError::eErrorResume);
}

LLDB_LOG(log, "halted process {0}", x.first);

LLDB_LOG(log, "continued process {0}", x.first);
// hack to avoid enabling stdio forwarding after stop
// TODO: remove this when we improve stdio forwarding for nonstop
assert(thread_actions.size() == 1);
return SendOKResponse();
}
} else {
PacketResult resume_res =
ResumeProcess(*process_it->second.process_up, x.second);
if (resume_res != PacketResult::Success)
return resume_res;
}
}

return SendContinueSuccessResponse();
Expand Down Expand Up @@ -2940,15 +3009,10 @@ GDBRemoteCommunicationServerLLGS::Handle_s(StringExtractorGDBRemote &packet) {

// All other threads stop while we're single stepping a thread.
actions.SetDefaultThreadActionIfNeeded(eStateStopped, 0);
Status error = m_continue_process->Resume(actions);
if (error.Fail()) {
LLDB_LOGF(log,
"GDBRemoteCommunicationServerLLGS::%s pid %" PRIu64
" tid %" PRIu64 " Resume() failed with error: %s",
__FUNCTION__, m_continue_process->GetID(), tid,
error.AsCString());
return SendErrorResponse(0x49);
}

PacketResult resume_res = ResumeProcess(*m_continue_process, actions);
if (resume_res != PacketResult::Success)
return resume_res;

// No response here, unless in non-stop mode.
// Otherwise, the stop or exit will come from the resulting action.
Expand Down
3 changes: 3 additions & 0 deletions lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
View file
Expand Up @@ -155,6 +155,9 @@ class GDBRemoteCommunicationServerLLGS

PacketResult Handle_QListThreadsInStopReply(StringExtractorGDBRemote &packet);

PacketResult ResumeProcess(NativeProcessProtocol &process,
const ResumeActionList &actions);

PacketResult Handle_C(StringExtractorGDBRemote &packet);

PacketResult Handle_c(StringExtractorGDBRemote &packet);
Expand Down
134 changes: 134 additions & 0 deletions lldb/test/API/tools/lldb-server/TestNonStop.py
View file
Expand Up @@ -168,3 +168,137 @@ def test_exit_query(self):
"send packet: $OK#00",
], True)
self.expect_gdbremote_sequence()

def multiple_resume_test(self, second_command):
self.build()
self.set_inferior_startup_launch()
procs = self.prep_debug_monitor_and_inferior(
inferior_args=["sleep:15"])
self.test_sequence.add_log_lines(
["read packet: $QNonStop:1#00",
"send packet: $OK#00",
"read packet: $c#63",
"send packet: $OK#00",
"read packet: ${}#00".format(second_command),
"send packet: $E37#00",
], True)
self.expect_gdbremote_sequence()

@add_test_categories(["llgs"])
def test_multiple_C(self):
self.multiple_resume_test("C05")

@add_test_categories(["llgs"])
def test_multiple_c(self):
self.multiple_resume_test("c")

@add_test_categories(["llgs"])
def test_multiple_s(self):
self.multiple_resume_test("s")

@expectedFailureAll(archs=["arm"]) # TODO
@expectedFailureAll(archs=["aarch64"],
bugnumber="https://github.com/llvm/llvm-project/issues/56268")
@add_test_categories(["llgs"])
def test_multiple_vCont(self):
self.build()
self.set_inferior_startup_launch()
procs = self.prep_debug_monitor_and_inferior(
inferior_args=["thread:new", "trap", "sleep:15"])
self.test_sequence.add_log_lines(
["read packet: $QNonStop:1#00",
"send packet: $OK#00",
"read packet: $c#63",
"send packet: $OK#00",
{"direction": "send",
"regex": r"^%Stop:T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);",
"capture": {1: "tid1"},
},
"read packet: $vStopped#63",
{"direction": "send",
"regex": r"^[$]T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);",
"capture": {1: "tid2"},
},
"read packet: $vStopped#63",
"send packet: $OK#00",
], True)
ret = self.expect_gdbremote_sequence()

self.reset_test_sequence()
self.test_sequence.add_log_lines(
["read packet: $vCont;c:{}#00".format(ret["tid1"]),
"send packet: $OK#00",
"read packet: $vCont;c:{}#00".format(ret["tid2"]),
"send packet: $E37#00",
], True)
self.expect_gdbremote_sequence()

@add_test_categories(["llgs"])
def test_vCont_then_stop(self):
self.build()
self.set_inferior_startup_launch()
procs = self.prep_debug_monitor_and_inferior(
inferior_args=["sleep:15"])
self.test_sequence.add_log_lines(
["read packet: $QNonStop:1#00",
"send packet: $OK#00",
"read packet: $c#63",
"send packet: $OK#00",
"read packet: $vCont;t#00",
"send packet: $OK#00",
], True)
self.expect_gdbremote_sequence()

def vCont_then_partial_stop_test(self, run_both):
self.build()
self.set_inferior_startup_launch()
procs = self.prep_debug_monitor_and_inferior(
inferior_args=["thread:new", "trap", "sleep:15"])
self.test_sequence.add_log_lines(
["read packet: $QNonStop:1#00",
"send packet: $OK#00",
"read packet: $c#63",
"send packet: $OK#00",
{"direction": "send",
"regex": r"^%Stop:T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);",
"capture": {1: "tid1"},
},
"read packet: $vStopped#63",
{"direction": "send",
"regex": r"^[$]T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);",
"capture": {1: "tid2"},
},
"read packet: $vStopped#63",
"send packet: $OK#00",
], True)
ret = self.expect_gdbremote_sequence()

self.reset_test_sequence()
if run_both:
self.test_sequence.add_log_lines(
["read packet: $vCont;c#00",
], True)
else:
self.test_sequence.add_log_lines(
["read packet: $vCont;c:{}#00".format(ret["tid1"]),
], True)
self.test_sequence.add_log_lines(
["send packet: $OK#00",
"read packet: $vCont;t:{}#00".format(ret["tid2"]),
"send packet: $E03#00",
], True)
self.expect_gdbremote_sequence()

@expectedFailureAll(archs=["arm"]) # TODO
@expectedFailureAll(archs=["aarch64"],
bugnumber="https://github.com/llvm/llvm-project/issues/56268")
@add_test_categories(["llgs"])
def test_vCont_then_partial_stop(self):
self.vCont_then_partial_stop_test(False)

@expectedFailureAll(archs=["arm"]) # TODO
@expectedFailureAll(archs=["aarch64"],
bugnumber="https://github.com/llvm/llvm-project/issues/56268")
@add_test_categories(["llgs"])
def test_vCont_then_partial_stop_run_both(self):
self.vCont_then_partial_stop_test(True)
51 changes: 0 additions & 51 deletions lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
View file
Expand Up @@ -75,54 +75,3 @@ def test_vCont_cxcxt(self):
main_thread,
"c:{:x};c:{:x};t".format(*all_subthreads_list[:2])),
set(all_subthreads_list[:2]))

@skipIfWindows
@add_test_categories(["llgs"])
def test_vCont_txc(self):
main_thread, all_subthreads_list = (
self.start_vCont_run_subset_of_threads_test())
# stop one thread explicitly, resume others
self.assertEqual(
self.continue_and_get_threads_running(
main_thread,
"t:{:x};c".format(all_subthreads_list[-1])),
set(all_subthreads_list[:2]))

@skipIfWindows
@add_test_categories(["llgs"])
def test_vCont_cxtxc(self):
main_thread, all_subthreads_list = (
self.start_vCont_run_subset_of_threads_test())
# resume one thread explicitly, stop one explicitly,
# resume others
self.assertEqual(
self.continue_and_get_threads_running(
main_thread,
"c:{:x};t:{:x};c".format(*all_subthreads_list[-2:])),
set(all_subthreads_list[:2]))

@skipIfWindows
@add_test_categories(["llgs"])
def test_vCont_txcx(self):
main_thread, all_subthreads_list = (
self.start_vCont_run_subset_of_threads_test())
# resume one thread explicitly, stop one explicitly,
# stop others implicitly
self.assertEqual(
self.continue_and_get_threads_running(
main_thread,
"t:{:x};c:{:x}".format(*all_subthreads_list[:2])),
set(all_subthreads_list[1:2]))

@skipIfWindows
@add_test_categories(["llgs"])
def test_vCont_txcxt(self):
main_thread, all_subthreads_list = (
self.start_vCont_run_subset_of_threads_test())
# resume one thread explicitly, stop one explicitly,
# stop others explicitly
self.assertEqual(
self.continue_and_get_threads_running(
main_thread,
"t:{:x};c:{:x};t".format(*all_subthreads_list[:2])),
set(all_subthreads_list[1:2]))

0 comments on commit f8605da

Please sign in to comment.