[Lldb-commits] [lldb] 7a2b09b - Revert "[lldb] [llgs] Fix multi-resume bugs with nonstop mode"
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Fri Jul 15 04:45:22 PDT 2022
Author: Michał Górny
Date: 2022-07-15T13:43:34+02:00
New Revision: 7a2b09b4798dbd5ec69e934b595eab5afddf33c5
URL: https://github.com/llvm/llvm-project/commit/7a2b09b4798dbd5ec69e934b595eab5afddf33c5
DIFF: https://github.com/llvm/llvm-project/commit/7a2b09b4798dbd5ec69e934b595eab5afddf33c5.diff
LOG: Revert "[lldb] [llgs] Fix multi-resume bugs with nonstop mode"
This reverts commit f8605da8758fbae16410e4ed5493a39429fd73ec.
This is causing buildbot failures and now I see that I have not updated
the tests to use "stop" instead of "trap".
Added:
Modified:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/test/API/tools/lldb-server/TestNonStop.py
lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 8edb49f06a334..a2ea53b0e8e19 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1512,30 +1512,6 @@ 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);
@@ -1571,14 +1547,6 @@ 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;
@@ -1612,11 +1580,14 @@ GDBRemoteCommunicationServerLLGS::Handle_C(StringExtractorGDBRemote &packet) {
}
}
- // 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;
+ // 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);
+ }
// Don't send an "OK" packet, except in non-stop mode;
// otherwise, the response is the stopped/exited message.
@@ -1651,9 +1622,14 @@ GDBRemoteCommunicationServerLLGS::Handle_c(StringExtractorGDBRemote &packet) {
ResumeActionList actions(StateType::eStateRunning,
LLDB_INVALID_SIGNAL_NUMBER);
- PacketResult resume_res = ResumeProcess(*m_continue_process, actions);
- if (resume_res != PacketResult::Success)
- return resume_res;
+ 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());
return SendContinueSuccessResponse();
}
@@ -1667,18 +1643,6 @@ 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) {
@@ -1700,6 +1664,9 @@ 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;
@@ -1766,12 +1733,6 @@ 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(
@@ -1804,43 +1765,13 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
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);
-
- // 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;
+ 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);
}
+
+ LLDB_LOG(log, "continued process {0}", x.first);
}
return SendContinueSuccessResponse();
@@ -3009,10 +2940,15 @@ GDBRemoteCommunicationServerLLGS::Handle_s(StringExtractorGDBRemote &packet) {
// All other threads stop while we're single stepping a thread.
actions.SetDefaultThreadActionIfNeeded(eStateStopped, 0);
-
- PacketResult resume_res = ResumeProcess(*m_continue_process, actions);
- if (resume_res != PacketResult::Success)
- return resume_res;
+ 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);
+ }
// No response here, unless in non-stop mode.
// Otherwise, the stop or exit will come from the resulting action.
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index 47e8a783603de..ebd656687da93 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -155,9 +155,6 @@ class GDBRemoteCommunicationServerLLGS
PacketResult Handle_QListThreadsInStopReply(StringExtractorGDBRemote &packet);
- PacketResult ResumeProcess(NativeProcessProtocol &process,
- const ResumeActionList &actions);
-
PacketResult Handle_C(StringExtractorGDBRemote &packet);
PacketResult Handle_c(StringExtractorGDBRemote &packet);
diff --git a/lldb/test/API/tools/lldb-server/TestNonStop.py b/lldb/test/API/tools/lldb-server/TestNonStop.py
index d544cdd45c48e..83772ada78e30 100644
--- a/lldb/test/API/tools/lldb-server/TestNonStop.py
+++ b/lldb/test/API/tools/lldb-server/TestNonStop.py
@@ -168,137 +168,3 @@ 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)
diff --git a/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py b/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
index b98b63ea22e03..2cc60d3d0a5c0 100644
--- a/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
+++ b/lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
@@ -75,3 +75,54 @@ 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]))
More information about the lldb-commits
mailing list