[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb7c140335beb: [lldb] [gdb-remote server] Support selecting
process via Hg (authored by mgorny).
Herald added a project: LLDB.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -16,7 +16,7 @@
self.reset_test_sequence()
# continue and expect fork
-fork_regex = "[$]T.*;{}:p([0-9a-f]*)[.]([0-9a-f]*).*".format(variant)
+fork_regex = "[$]T.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
self.test_sequence.add_log_lines([
"read packet: $c#00",
{"direction": "send", "regex": fork_regex,
@@ -57,3 +57,137 @@
{"direction": "send", "regex": r"[$]W00#.*"},
], True)
self.expect_gdbremote_sequence()
+
+def fork_and_follow_test(self, variant):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=[variant])
+self.add_qSupported_packets(["multiprocess+",
+ "{}-events+".format(variant)])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+procinfo_regex = "[$]pid:([0-9a-f]+);.*"
+fork_regex = "[$]T.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
+self.test_sequence.add_log_lines([
+"read packet: $qProcessInfo#00",
+{"direction": "send", "regex": procinfo_regex,
+ "capture": {1: "parent_pid"}},
+"read packet: $c#00",
+{"direction": "send", "regex": fork_regex,
+ "capture": {1: "pid", 2: "tid"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid, pid, tid = (int(ret[x], 16) for x
+in ("parent_pid", "pid", "tid"))
+self.reset_test_sequence()
+
+# switch to the forked child
+self.test_sequence.add_log_lines([
+"read packet: $Hgp{:x}.{:x}#00".format(pid, tid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+"read packet: $Hcp{:x}.{:x}#00".format(pid, tid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+
+# detach the parent
+self.test_sequence.add_log_lines([
+"read packet: $D;{:x}#00".format(parent_pid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+# resume the child
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": r"[$]W00#.*"},
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_fork_follow(self):
+self.fork_and_follow_test("fork")
+
+@add_test_categories(["fork"])
+def test_vfork_follow(self):
+self.fork_and_follow_test("vfork")
+
+@add_test_categories(["fork"])
+def test_select_wrong_pid(self):
+self.build()
+self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets(["multiprocess+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("multiprocess+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# get process pid
+procinfo_regex = "[$]pid:([0-9a-f]+);.*"
+self.test_sequence.add_log_lines([
+"read packet: $qProcessInfo#00",
+{"direction": "send", "regex": procinfo_regex,
+ "capture": {1: "pid"}},
+"read packet: $qC#00",
+{"direction": "send", "regex": "[$]QC([0-9a-f]+)#.*",
+ "capture": {1: "tid"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+pid, tid = (int(ret[x], 16) for x in ("pid", "tid"))
+self.reset_test_sequence()
+
+# try switching to correct pid
+self.test_sequence.add_log_lines([
+"read packet: $Hgp{:x}.{:x}#00".format(pid, tid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+"read packet: $Hcp{:x}.{:x}#00".format(pid, tid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+ret = self.expect_gdbremote_sequence()
+
+# try switching to invalid tid
+self.test_sequence.add_log_lines([
+"read
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
JDevlieghere added inline comments.
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2121
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "Malformed thread-id"));
+
mgorny wrote:
> JDevlieghere wrote:
> > Would it be useful to say "Malformed thread-id for process-id {}"?
> In this context, 'thread-id' is GDB protocol jargon for 'PID and/or TID'. So
> if it's malformed, then we don't really have a process ID here. I suppose I
> could try replacing 'thread-id' with something more verbose but I'm not sure
> if this is the kind of error end users will be dealing with.
Fair enough. Thanks for the explanation!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
mgorny added inline comments.
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2121
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "Malformed thread-id"));
+
JDevlieghere wrote:
> Would it be useful to say "Malformed thread-id for process-id {}"?
In this context, 'thread-id' is GDB protocol jargon for 'PID and/or TID'. So if
it's malformed, then we don't really have a process ID here. I suppose I could
try replacing 'thread-id' with something more verbose but I'm not sure if this
is the kind of error end users will be dealing with.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
I'm probably not the most qualified to sign off on this, but the review has
stalled and I don't see anything obviously wrong here so LGTM.
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2121
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "Malformed thread-id"));
+
Would it be useful to say "Malformed thread-id for process-id {}"?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
mgorny added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2130 +return SendErrorResponse(llvm::make_error( +inconvertibleErrorCode(), "No current process and no PID provided")); + rovka wrote: > Just a drive-by comment: The previous behaviour was to send 0x15 when the > current process ID was invalid. Should we do that here? I've originally did that but it was kinda hackish and @labath didn't like it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100261/new/ https://reviews.llvm.org/D100261 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
rovka added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2130 +return SendErrorResponse(llvm::make_error( +inconvertibleErrorCode(), "No current process and no PID provided")); + Just a drive-by comment: The previous behaviour was to send 0x15 when the current process ID was invalid. Should we do that here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100261/new/ https://reviews.llvm.org/D100261 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
mgorny updated this revision to Diff 340310.
mgorny added a comment.
Add tests.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -16,7 +16,7 @@
self.reset_test_sequence()
# continue and expect fork
-fork_regex = "[$]T.*;{}:p([0-9a-f]*)[.]([0-9a-f]*).*".format(variant)
+fork_regex = "[$]T.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
self.test_sequence.add_log_lines([
"read packet: $c#00",
{"direction": "send", "regex": fork_regex,
@@ -57,3 +57,137 @@
{"direction": "send", "regex": r"[$]W00#.*"},
], True)
self.expect_gdbremote_sequence()
+
+def fork_and_follow_test(self, variant):
+self.build()
+self.prep_debug_monitor_and_inferior(inferior_args=[variant])
+self.add_qSupported_packets(["multiprocess+",
+ "{}-events+".format(variant)])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("{}-events+".format(variant), ret["qSupported_response"])
+self.reset_test_sequence()
+
+# continue and expect fork
+procinfo_regex = "[$]pid:([0-9a-f]+);.*"
+fork_regex = "[$]T.*;{}:p([0-9a-f]+)[.]([0-9a-f]+).*".format(variant)
+self.test_sequence.add_log_lines([
+"read packet: $qProcessInfo#00",
+{"direction": "send", "regex": procinfo_regex,
+ "capture": {1: "parent_pid"}},
+"read packet: $c#00",
+{"direction": "send", "regex": fork_regex,
+ "capture": {1: "pid", 2: "tid"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+parent_pid, pid, tid = (int(ret[x], 16) for x
+in ("parent_pid", "pid", "tid"))
+self.reset_test_sequence()
+
+# switch to the forked child
+self.test_sequence.add_log_lines([
+"read packet: $Hgp{:x}.{:x}#00".format(pid, tid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+"read packet: $Hcp{:x}.{:x}#00".format(pid, tid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+
+# detach the parent
+self.test_sequence.add_log_lines([
+"read packet: $D;{:x}#00".format(parent_pid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+ret = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+
+# resume the child
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": r"[$]W00#.*"},
+], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["fork"])
+def test_fork_follow(self):
+self.fork_and_follow_test("fork")
+
+@add_test_categories(["fork"])
+def test_vfork_follow(self):
+self.fork_and_follow_test("vfork")
+
+@add_test_categories(["fork"])
+def test_select_wrong_pid(self):
+self.build()
+self.prep_debug_monitor_and_inferior()
+self.add_qSupported_packets(["multiprocess+"])
+ret = self.expect_gdbremote_sequence()
+self.assertIn("multiprocess+", ret["qSupported_response"])
+self.reset_test_sequence()
+
+# get process pid
+procinfo_regex = "[$]pid:([0-9a-f]+);.*"
+self.test_sequence.add_log_lines([
+"read packet: $qProcessInfo#00",
+{"direction": "send", "regex": procinfo_regex,
+ "capture": {1: "pid"}},
+"read packet: $qC#00",
+{"direction": "send", "regex": "[$]QC([0-9a-f]+)#.*",
+ "capture": {1: "tid"}},
+], True)
+ret = self.expect_gdbremote_sequence()
+pid, tid = (int(ret[x], 16) for x in ("pid", "tid"))
+self.reset_test_sequence()
+
+# try switching to correct pid
+self.test_sequence.add_log_lines([
+"read packet: $Hgp{:x}.{:x}#00".format(pid, tid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+"read packet: $Hcp{:x}.{:x}#00".format(pid, tid),
+{"direction": "send", "regex": r"[$]OK#.*"},
+], True)
+ret = self.expect_gdbremote_sequence()
+
+# try switching to invalid tid
+self.test_sequence.add_log_lines([
+"read packet: $Hgp{:x}.{:x}#00".format(pid, tid+1),
+{"direction": "send", "regex": r"[$]E15#.*"},
+"read packet: $Hcp{:x}.{:x}#00".format(pid, tid+1),
+{"direction": "send", "regex": r"[$]E15#.*"},
+
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
mgorny updated this revision to Diff 337541.
mgorny marked an inline comment as done.
mgorny added a comment.
Revert accidental whitespace change.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
Index:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2055,16 +2055,6 @@
GDBRemoteCommunicationServerLLGS::Handle_H(StringExtractorGDBRemote ) {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
- // Fail if we don't have a current process.
- if (!m_current_process ||
- (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
-LLDB_LOGF(
-log,
-"GDBRemoteCommunicationServerLLGS::%s failed, no process available",
-__FUNCTION__);
-return SendErrorResponse(0x15);
- }
-
// Parse out which variant of $H is requested.
packet.SetFilePos(strlen("H"));
if (packet.GetBytesLeft() < 1) {
@@ -2076,14 +2066,14 @@
}
const char h_variant = packet.GetChar();
- lldb::pid_t default_pid;
+ NativeProcessProtocol *default_process;
switch (h_variant) {
case 'g':
-default_pid = m_current_process->GetID();
+default_process = m_current_process;
break;
case 'c':
-default_pid = m_continue_process->GetID();
+default_process = m_continue_process;
break;
default:
@@ -2096,16 +2086,32 @@
}
// Parse out the thread number.
- llvm::Expected tid_ret =
- ReadTid(packet, /*allow_all=*/true, default_pid);
- if (!tid_ret)
-return SendErrorResponse(tid_ret.takeError());
+ auto pid_tid = packet.GetPidTid(default_process ? default_process->GetID()
+ : LLDB_INVALID_PROCESS_ID);
+ if (!pid_tid)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "Malformed thread-id"));
+
+ lldb::pid_t pid = pid_tid->first;
+ lldb::tid_t tid = pid_tid->second;
+
+ if (pid == StringExtractorGDBRemote::AllProcesses)
+return SendUnimplementedResponse("Selecting all processes not supported");
+ if (pid == LLDB_INVALID_PROCESS_ID)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "No current process and no PID provided"));
+
+ // Check the process ID and find respective process instance.
+ auto new_process_it = m_debugged_processes.find(pid);
+ if (new_process_it == m_debugged_processes.end())
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(),
+llvm::formatv("No process with PID {0} debugged", pid)));
- lldb::tid_t tid = tid_ret.get();
// Ensure we have the given thread when not specifying -1 (all threads) or 0
// (any thread).
if (tid != LLDB_INVALID_THREAD_ID && tid != 0) {
-NativeThreadProtocol *thread = m_current_process->GetThreadByID(tid);
+NativeThreadProtocol *thread = new_process_it->second->GetThreadByID(tid);
if (!thread) {
LLDB_LOGF(log,
"GDBRemoteCommunicationServerLLGS::%s failed, tid %" PRIu64
@@ -2115,13 +2121,15 @@
}
}
- // Now switch the given thread type.
+ // Now switch the given process and thread type.
switch (h_variant) {
case 'g':
+m_current_process = new_process_it->second.get();
SetCurrentThreadID(tid);
break;
case 'c':
+m_continue_process = new_process_it->second.get();
SetContinueThreadID(tid);
break;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2055,16 +2055,6 @@
GDBRemoteCommunicationServerLLGS::Handle_H(StringExtractorGDBRemote ) {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
- // Fail if we don't have a current process.
- if (!m_current_process ||
- (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
-LLDB_LOGF(
-log,
-"GDBRemoteCommunicationServerLLGS::%s failed, no process available",
-__FUNCTION__);
-return SendErrorResponse(0x15);
- }
-
// Parse out which variant of $H is requested.
packet.SetFilePos(strlen("H"));
if (packet.GetBytesLeft() < 1) {
@@ -2076,14 +2066,14 @@
}
const char h_variant = packet.GetChar();
- lldb::pid_t default_pid;
+ NativeProcessProtocol *default_process;
switch (h_variant) {
case 'g':
-default_pid = m_current_process->GetID();
+default_process = m_current_process;
break;
case 'c':
-default_pid = m_continue_process->GetID();
+default_process =
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
mgorny added inline comments.
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2102
+ if (pid == StringExtractorGDBRemote::AllProcesses)
+return SendUnimplementedResponse("Selecting all processes not supported");
+ if (pid == LLDB_INVALID_PROCESS_ID)
labath wrote:
> Does it ever make sense to have more than one (process or thread) selected?
> If not, an error response may be more appropriate..
Good question.
As for PID, I don't think that the client would do something like that within
foreseeable future.
However, sending all-TIDs are technically supported in our LLGS client code. I
don't know whether it's actually used anywhere, though.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
labath added inline comments.
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2102
+ if (pid == StringExtractorGDBRemote::AllProcesses)
+return SendUnimplementedResponse("Selecting all processes not supported");
+ if (pid == LLDB_INVALID_PROCESS_ID)
Does it ever make sense to have more than one (process or thread) selected? If
not, an error response may be more appropriate..
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:234
const llvm::ArrayRef client_features) override;
-
private:
revert
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
mgorny updated this revision to Diff 337241.
mgorny added a comment.
Rebase.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100261/new/
https://reviews.llvm.org/D100261
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -231,7 +231,6 @@
virtual std::vector HandleFeatures(
const llvm::ArrayRef client_features) override;
-
private:
llvm::Expected> BuildTargetXml();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -2058,16 +2058,6 @@
GDBRemoteCommunicationServerLLGS::Handle_H(StringExtractorGDBRemote ) {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD));
- // Fail if we don't have a current process.
- if (!m_current_process ||
- (m_current_process->GetID() == LLDB_INVALID_PROCESS_ID)) {
-LLDB_LOGF(
-log,
-"GDBRemoteCommunicationServerLLGS::%s failed, no process available",
-__FUNCTION__);
-return SendErrorResponse(0x15);
- }
-
// Parse out which variant of $H is requested.
packet.SetFilePos(strlen("H"));
if (packet.GetBytesLeft() < 1) {
@@ -2079,14 +2069,14 @@
}
const char h_variant = packet.GetChar();
- lldb::pid_t default_pid;
+ NativeProcessProtocol *default_process;
switch (h_variant) {
case 'g':
-default_pid = m_current_process->GetID();
+default_process = m_current_process;
break;
case 'c':
-default_pid = m_continue_process->GetID();
+default_process = m_continue_process;
break;
default:
@@ -2099,16 +2089,32 @@
}
// Parse out the thread number.
- llvm::Expected tid_ret =
- ReadTid(packet, /*allow_all=*/true, default_pid);
- if (!tid_ret)
-return SendErrorResponse(tid_ret.takeError());
+ auto pid_tid = packet.GetPidTid(default_process ? default_process->GetID()
+ : LLDB_INVALID_PROCESS_ID);
+ if (!pid_tid)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "Malformed thread-id"));
+
+ lldb::pid_t pid = pid_tid->first;
+ lldb::tid_t tid = pid_tid->second;
+
+ if (pid == StringExtractorGDBRemote::AllProcesses)
+return SendUnimplementedResponse("Selecting all processes not supported");
+ if (pid == LLDB_INVALID_PROCESS_ID)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "No current process and no PID provided"));
+
+ // Check the process ID and find respective process instance.
+ auto new_process_it = m_debugged_processes.find(pid);
+ if (new_process_it == m_debugged_processes.end())
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(),
+llvm::formatv("No process with PID {0} debugged", pid)));
- lldb::tid_t tid = tid_ret.get();
// Ensure we have the given thread when not specifying -1 (all threads) or 0
// (any thread).
if (tid != LLDB_INVALID_THREAD_ID && tid != 0) {
-NativeThreadProtocol *thread = m_current_process->GetThreadByID(tid);
+NativeThreadProtocol *thread = new_process_it->second->GetThreadByID(tid);
if (!thread) {
LLDB_LOGF(log,
"GDBRemoteCommunicationServerLLGS::%s failed, tid %" PRIu64
@@ -2118,13 +2124,15 @@
}
}
- // Now switch the given thread type.
+ // Now switch the given process and thread type.
switch (h_variant) {
case 'g':
+m_current_process = new_process_it->second.get();
SetCurrentThreadID(tid);
break;
case 'c':
+m_continue_process = new_process_it->second.get();
SetContinueThreadID(tid);
break;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D100261: [lldb] [gdb-remote server] Support selecting process via Hg
mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.
Support using the extended thread-id syntax with Hg packet to select
a subprocess. This makes it possible to start providing support for
running some of the debugger packets against another subprocesses.
https://reviews.llvm.org/D100261
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -90,6 +90,7 @@
const NativeProcessProtocol::Factory _process_factory;
lldb::tid_t m_current_tid = LLDB_INVALID_THREAD_ID;
lldb::tid_t m_continue_tid = LLDB_INVALID_THREAD_ID;
+ lldb::pid_t m_current_pid = LLDB_INVALID_PROCESS_ID;
std::recursive_mutex m_debugged_process_mutex;
std::unique_ptr m_debugged_process_up;
std::unordered_map>
@@ -233,9 +234,17 @@
// Verify and get the process instance selected by the client.
// If run is true, the process selected by "Hc" packet will be returned,
- // otherwise the one selected by "Hg" will be used. Returns nullptr if there
- // is no process debugged.
- NativeProcessProtocol *GetCurrentProcess(bool run) const;
+ // otherwise the one selected by "Hg" will be used.
+ //
+ // If allow_subprocess is true, a subprocess instance may be returned.
+ // Otherwise, only the main process can be returned and the function will
+ // return nullptr if a subprocess is selected.
+ //
+ // Returns nullptr if there is no process debugged.
+ NativeProcessProtocol *GetCurrentProcess(bool run,
+ bool allow_subprocess = false) const;
+
+ void SetCurrentProcessID(lldb::pid_t pid, lldb::tid_t tid);
private:
llvm::Expected> BuildTargetXml();
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1624,6 +1624,9 @@
m_current_tid = tid;
if (m_debugged_process_up)
m_debugged_process_up->SetCurrentThreadID(m_current_tid);
+
+ // Reset subprocess support
+ m_current_pid = LLDB_INVALID_PROCESS_ID;
}
void GDBRemoteCommunicationServerLLGS::SetContinueThreadID(lldb::tid_t tid) {
@@ -2081,12 +2084,32 @@
}
// Parse out the thread number.
- llvm::Expected tid_ret =
- ReadTid(packet, /*run=*/h_variant == 'c', /*allow_all=*/true);
- if (!tid_ret)
-return SendErrorResponse(tid_ret.takeError());
+ auto *process = GetCurrentProcess(h_variant == 'c', true);
+ auto pid_tid =
+ packet.GetPidTid(process ? process->GetID() : LLDB_INVALID_PROCESS_ID);
+ if (!pid_tid)
+return SendErrorResponse(llvm::make_error(
+inconvertibleErrorCode(), "Malformed thread-id"));
+
+ lldb::pid_t pid = pid_tid->first;
+ lldb::tid_t tid = pid_tid->second;
+
+ if (pid == StringExtractorGDBRemote::AllProcesses)
+return SendUnimplementedResponse("Selecting all processes not supported");
+ if (pid != m_debugged_process_up->GetID()) {
+// Is it a subprocess?
+if (m_additional_processes.find(pid) == m_additional_processes.end())
+ return SendErrorResponse(llvm::make_error(
+ inconvertibleErrorCode(),
+ llvm::formatv("PID {0} not debugged", pid)));
+if (h_variant != 'g')
+ return SendUnimplementedResponse(
+ "Hc packet for subprocess not supported");
+
+SetCurrentProcessID(pid, tid);
+return SendOKResponse();
+ }
- lldb::tid_t tid = tid_ret.get();
// Ensure we have the given thread when not specifying -1 (all threads) or 0
// (any thread).
if (tid != LLDB_INVALID_THREAD_ID && tid != 0) {
@@ -2622,7 +2645,7 @@
GDBRemoteCommunication::PacketResult
GDBRemoteCommunicationServerLLGS::Handle_z(StringExtractorGDBRemote ) {
// Ensure we have a process.
- auto *process = GetCurrentProcess(false);
+ auto *process = GetCurrentProcess(false, true);
if (!process) {
Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS));
LLDB_LOG(log, "failed, no process available");
@@ -3622,12 +3645,33 @@
"Enabling protocol extensions failed: {0}");
}
-NativeProcessProtocol *
-GDBRemoteCommunicationServerLLGS::GetCurrentProcess(bool run) const {
+NativeProcessProtocol *GDBRemoteCommunicationServerLLGS::GetCurrentProcess(
+bool run, bool allow_subprocess) const {
// Fail if we don't have a current process.
if (!m_debugged_process_up ||
(m_debugged_process_up->GetID() == LLDB_INVALID_PROCESS_ID))
return nullptr;
