[Lldb-commits] [lldb] a342279 - [lldb] [llgs] Support resuming one process with PID!=current via vCont
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Fri Jun 24 08:20:38 PDT 2022
Author: Michał Górny
Date: 2022-06-24T17:20:23+02:00
New Revision: a3422793e0643fa849ff178d87fc706c81b734b7
URL: https://github.com/llvm/llvm-project/commit/a3422793e0643fa849ff178d87fc706c81b734b7
DIFF: https://github.com/llvm/llvm-project/commit/a3422793e0643fa849ff178d87fc706c81b734b7.diff
LOG: [lldb] [llgs] Support resuming one process with PID!=current via vCont
Extend vCont function to support resuming a process with an arbitrary
PID, that could be different than the one selected via Hc (or no process
at all may be selected). Resuming more than one process simultaneously
is not supported yet.
Remove the ReadTid() method that was only used by Handle_vCont(),
and furthermore it was wrongly using m_current_process rather than
m_continue_process.
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D127862
Added:
Modified:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 1ff0b7fe4ee5d..f6b61f521206b 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1674,12 +1674,7 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
return SendIllFormedResponse(packet, "Missing action from vCont package");
}
- // Check if this is all continue (no options or ";c").
- if (::strcmp(packet.Peek(), ";c") == 0) {
- // Move past the ';', then do a simple 'c'.
- packet.SetFilePos(packet.GetFilePos() + 1);
- return Handle_c(packet);
- } else if (::strcmp(packet.Peek(), ";s") == 0) {
+ if (::strcmp(packet.Peek(), ";s") == 0) {
// Move past the ';', then do a simple 's'.
packet.SetFilePos(packet.GetFilePos() + 1);
return Handle_s(packet);
@@ -1688,13 +1683,7 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
return SendOKResponse();
}
- // Ensure we have a native process.
- if (!m_continue_process) {
- LLDB_LOG(log, "no debugged process");
- return SendErrorResponse(0x36);
- }
-
- ResumeActionList thread_actions;
+ std::unordered_map<lldb::pid_t, ResumeActionList> thread_actions;
while (packet.GetBytesLeft() && *packet.Peek() == ';') {
// Skip the semi-colon.
@@ -1737,32 +1726,62 @@ GDBRemoteCommunicationServerLLGS::Handle_vCont(
break;
}
+ lldb::pid_t pid = StringExtractorGDBRemote::AllProcesses;
+ lldb::tid_t tid = StringExtractorGDBRemote::AllThreads;
+
// Parse out optional :{thread-id} value.
if (packet.GetBytesLeft() && (*packet.Peek() == ':')) {
// Consume the separator.
packet.GetChar();
- llvm::Expected<lldb::tid_t> tid_ret =
- ReadTid(packet, /*allow_all=*/true, m_continue_process->GetID());
- if (!tid_ret)
- return SendErrorResponse(tid_ret.takeError());
+ auto pid_tid = packet.GetPidTid(StringExtractorGDBRemote::AllProcesses);
+ if (!pid_tid)
+ return SendIllFormedResponse(packet, "Malformed thread-id");
- thread_action.tid = tid_ret.get();
- if (thread_action.tid == StringExtractorGDBRemote::AllThreads)
- thread_action.tid = LLDB_INVALID_THREAD_ID;
+ pid = pid_tid->first;
+ tid = pid_tid->second;
}
- thread_actions.Append(thread_action);
- }
+ if (pid == StringExtractorGDBRemote::AllProcesses) {
+ if (m_debugged_processes.size() > 1)
+ return SendIllFormedResponse(
+ packet, "Resuming multiple processes not supported yet");
+ if (!m_continue_process) {
+ LLDB_LOG(log, "no debugged process");
+ return SendErrorResponse(0x36);
+ }
+ pid = m_continue_process->GetID();
+ }
- Status error = m_continue_process->Resume(thread_actions);
- if (error.Fail()) {
- LLDB_LOG(log, "vCont failed for process {0}: {1}",
- m_continue_process->GetID(), error);
- return SendErrorResponse(GDBRemoteServerError::eErrorResume);
+ if (tid == StringExtractorGDBRemote::AllThreads)
+ tid = LLDB_INVALID_THREAD_ID;
+
+ thread_action.tid = tid;
+
+ thread_actions[pid].Append(thread_action);
}
- LLDB_LOG(log, "continued process {0}", m_continue_process->GetID());
+ assert(thread_actions.size() >= 1);
+ if (thread_actions.size() > 1)
+ return SendIllFormedResponse(
+ packet, "Resuming multiple processes not supported yet");
+
+ for (std::pair<lldb::pid_t, ResumeActionList> x : thread_actions) {
+ auto process_it = m_debugged_processes.find(x.first);
+ if (process_it == m_debugged_processes.end()) {
+ LLDB_LOG(log, "vCont failed for process {0}: process not debugged",
+ x.first);
+ return SendErrorResponse(GDBRemoteServerError::eErrorResume);
+ }
+
+ Status error = process_it->second->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();
}
@@ -4011,38 +4030,6 @@ std::string GDBRemoteCommunicationServerLLGS::XMLEncodeAttributeValue(
return result;
}
-llvm::Expected<lldb::tid_t> GDBRemoteCommunicationServerLLGS::ReadTid(
- StringExtractorGDBRemote &packet, bool allow_all, lldb::pid_t default_pid) {
- assert(m_current_process);
- assert(m_current_process->GetID() != LLDB_INVALID_PROCESS_ID);
-
- auto pid_tid = packet.GetPidTid(default_pid);
- if (!pid_tid)
- return llvm::make_error<StringError>(inconvertibleErrorCode(),
- "Malformed thread-id");
-
- lldb::pid_t pid = pid_tid->first;
- lldb::tid_t tid = pid_tid->second;
-
- if (!allow_all && pid == StringExtractorGDBRemote::AllProcesses)
- return llvm::make_error<StringError>(
- inconvertibleErrorCode(),
- llvm::formatv("PID value {0} not allowed", pid == 0 ? 0 : -1));
-
- if (!allow_all && tid == StringExtractorGDBRemote::AllThreads)
- return llvm::make_error<StringError>(
- inconvertibleErrorCode(),
- llvm::formatv("TID value {0} not allowed", tid == 0 ? 0 : -1));
-
- if (pid != StringExtractorGDBRemote::AllProcesses) {
- if (pid != m_current_process->GetID())
- return llvm::make_error<StringError>(
- inconvertibleErrorCode(), llvm::formatv("PID {0} not debugged", pid));
- }
-
- return tid;
-}
-
std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
const llvm::ArrayRef<llvm::StringRef> client_features) {
std::vector<std::string> ret =
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index 728334ac81698..983f969fd6d0e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -293,15 +293,6 @@ class GDBRemoteCommunicationServerLLGS
void StopSTDIOForwarding();
- // Read thread-id from packet. If the thread-id is correct, returns it.
- // Otherwise, returns the error.
- //
- // If allow_all is true, then the pid/tid value of -1 ('all') will be allowed.
- // In any case, the function assumes that exactly one inferior is being
- // debugged and rejects pid values that do no match that inferior.
- llvm::Expected<lldb::tid_t> ReadTid(StringExtractorGDBRemote &packet,
- bool allow_all, lldb::pid_t default_pid);
-
// Call SetEnabledExtensions() with appropriate flags on the process.
void SetEnabledExtensions(NativeProcessProtocol &process);
diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp
index 75fc74de4e215..07954408f6d02 100644
--- a/lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -639,7 +639,7 @@ llvm::Optional<std::pair<lldb::pid_t, lldb::tid_t>>
StringExtractorGDBRemote::GetPidTid(lldb::pid_t default_pid) {
llvm::StringRef view = llvm::StringRef(m_packet).substr(m_index);
size_t initial_length = view.size();
- lldb::pid_t pid = default_pid;
+ lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
lldb::tid_t tid;
if (view.consume_front("p")) {
@@ -675,5 +675,5 @@ StringExtractorGDBRemote::GetPidTid(lldb::pid_t default_pid) {
// update m_index
m_index += initial_length - view.size();
- return {{pid, tid}};
+ return {{pid != LLDB_INVALID_PROCESS_ID ? pid : default_pid, tid}};
}
diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index f5f088f506848..ec6ae2575f4e3 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -354,7 +354,7 @@ def test_vkill_parent(self):
def test_vkill_both(self):
self.vkill_test(kill_parent=True, kill_child=True)
- def resume_one_test(self, run_order):
+ def resume_one_test(self, run_order, use_vCont=False):
self.build()
self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
self.add_qSupported_packets(["multiprocess+",
@@ -395,11 +395,19 @@ def resume_one_test(self, run_order):
else:
assert False, "unexpected x={}".format(x)
+ if use_vCont:
+ self.test_sequence.add_log_lines([
+ # continue the selected process
+ "read packet: $vCont;c:p{}.{}#00".format(*pidtid),
+ ], True)
+ else:
+ self.test_sequence.add_log_lines([
+ # continue the selected process
+ "read packet: $Hcp{}.{}#00".format(*pidtid),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ ], True)
self.test_sequence.add_log_lines([
- # continue the selected process
- "read packet: $Hcp{}.{}#00".format(*pidtid),
- "send packet: $OK#00",
- "read packet: $c#00",
{"direction": "send", "regex": expect},
], True)
# if at least one process remained, check both PIDs
@@ -431,3 +439,117 @@ def test_c_child_then_parent(self):
@add_test_categories(["fork"])
def test_c_interspersed(self):
self.resume_one_test(run_order=["parent", "child", "parent", "child"])
+
+ @add_test_categories(["fork"])
+ def test_vCont_parent(self):
+ self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
+
+ @add_test_categories(["fork"])
+ def test_vCont_child(self):
+ self.resume_one_test(run_order=["child", "child"], use_vCont=True)
+
+ @add_test_categories(["fork"])
+ def test_vCont_parent_then_child(self):
+ self.resume_one_test(run_order=["parent", "parent", "child", "child"],
+ use_vCont=True)
+
+ @add_test_categories(["fork"])
+ def test_vCont_child_then_parent(self):
+ self.resume_one_test(run_order=["child", "child", "parent", "parent"],
+ use_vCont=True)
+
+ @add_test_categories(["fork"])
+ def test_vCont_interspersed(self):
+ self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ use_vCont=True)
+
+ @add_test_categories(["fork"])
+ def test_vCont_two_processes(self):
+ self.build()
+ self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+ self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ ret = self.expect_gdbremote_sequence()
+ self.assertIn("fork-events+", ret["qSupported_response"])
+ self.reset_test_sequence()
+
+ # continue and expect fork
+ self.test_sequence.add_log_lines([
+ "read packet: $c#00",
+ {"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+ ], True)
+ ret = self.expect_gdbremote_sequence()
+ parent_pid = ret["parent_pid"]
+ parent_tid = ret["parent_tid"]
+ child_pid = ret["child_pid"]
+ child_tid = ret["child_tid"]
+ self.reset_test_sequence()
+
+ self.test_sequence.add_log_lines([
+ # try to resume both processes
+ "read packet: $vCont;c:p{}.{};c:p{}.{}#00".format(
+ parent_pid, parent_tid, child_pid, child_tid),
+ "send packet: $E03#00",
+ ], True)
+ self.expect_gdbremote_sequence()
+
+ @add_test_categories(["fork"])
+ def test_vCont_all_processes_explicit(self):
+ self.build()
+ self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+ self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ ret = self.expect_gdbremote_sequence()
+ self.assertIn("fork-events+", ret["qSupported_response"])
+ self.reset_test_sequence()
+
+ # continue and expect fork
+ self.test_sequence.add_log_lines([
+ "read packet: $c#00",
+ {"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+ ], True)
+ ret = self.expect_gdbremote_sequence()
+ parent_pid = ret["parent_pid"]
+ parent_tid = ret["parent_tid"]
+ child_pid = ret["child_pid"]
+ child_tid = ret["child_tid"]
+ self.reset_test_sequence()
+
+ self.test_sequence.add_log_lines([
+ # try to resume all processes implicitly
+ "read packet: $vCont;c:p-1.-1#00",
+ "send packet: $E03#00",
+ ], True)
+ self.expect_gdbremote_sequence()
+
+ @add_test_categories(["fork"])
+ def test_vCont_all_processes_implicit(self):
+ self.build()
+ self.prep_debug_monitor_and_inferior(inferior_args=["fork", "trap"])
+ self.add_qSupported_packets(["multiprocess+",
+ "fork-events+"])
+ ret = self.expect_gdbremote_sequence()
+ self.assertIn("fork-events+", ret["qSupported_response"])
+ self.reset_test_sequence()
+
+ # continue and expect fork
+ self.test_sequence.add_log_lines([
+ "read packet: $c#00",
+ {"direction": "send", "regex": self.fork_regex.format("fork"),
+ "capture": self.fork_capture},
+ ], True)
+ ret = self.expect_gdbremote_sequence()
+ parent_pid = ret["parent_pid"]
+ parent_tid = ret["parent_tid"]
+ child_pid = ret["child_pid"]
+ child_tid = ret["child_tid"]
+ self.reset_test_sequence()
+
+ self.test_sequence.add_log_lines([
+ # try to resume all processes implicitly
+ "read packet: $vCont;c#00",
+ "send packet: $E03#00",
+ ], True)
+ self.expect_gdbremote_sequence()
More information about the lldb-commits
mailing list