mgorny updated this revision to Diff 330225.
mgorny edited the summary of this revision.
mgorny added a comment.
Updated to include `multiprocess` in `qSupported`, and proposed how to handle
it server-side on the example of `H` packet. Basically, accept the correct PID
of current process, reject anything else (including `0` and `-1` that do not
make sense for `H`).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98482/new/
https://reviews.llvm.org/D98482
Files:
lldb/include/lldb/Utility/StringExtractor.h
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Utility/StringExtractor.cpp
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
lldb/unittests/Utility/StringExtractorTest.cpp
Index: lldb/unittests/Utility/StringExtractorTest.cpp
===================================================================
--- lldb/unittests/Utility/StringExtractorTest.cpp
+++ lldb/unittests/Utility/StringExtractorTest.cpp
@@ -1,7 +1,9 @@
#include "gtest/gtest.h"
+#include "gmock/gmock.h"
#include <limits.h>
#include "lldb/Utility/StringExtractor.h"
+#include "lldb/lldb-defines.h"
namespace {
class StringExtractorTest : public ::testing::Test {};
@@ -695,3 +697,193 @@
ex.Reset("123456789ABCDEF000");
EXPECT_EQ(0ull, ex.GetHexMaxU64(false, 0));
}
+
+TEST_F(StringExtractorTest, GetPidTid) {
+ StringExtractor ex("");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ // invalid/short values
+
+ ex.Reset("narf");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset(";1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset(".1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("pnarf");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p;1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p.1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p1234.");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p1234.;1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("-2");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p1234.-2");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p-2");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p-2.1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ // overflow
+
+ ex.Reset("p10000000000000000");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p10000000000000000.0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("10000000000000000");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p0.10000000000000000");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ ex.Reset("p10000000000000000.10000000000000000");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, llvm::None));
+
+ // pure thread id
+
+ ex.Reset("0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, 0));
+
+ ex.Reset("-1");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, LLDB_INVALID_THREAD_ID));
+
+ ex.Reset("1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, 0x1234ULL));
+
+ ex.Reset("123456789ABCDEF0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(llvm::None, 0x123456789ABCDEF0ULL));
+
+ // pure process id
+
+ ex.Reset("p0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0, llvm::None));
+
+ ex.Reset("p-1");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(LLDB_INVALID_PROCESS_ID, llvm::None));
+
+ ex.Reset("p1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x1234ULL, llvm::None));
+
+ ex.Reset("p123456789ABCDEF0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x123456789ABCDEF0ULL, llvm::None));
+
+ // combined thread id + process id
+
+ ex.Reset("p0.0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0, 0));
+
+ ex.Reset("p0.-1");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0, LLDB_INVALID_THREAD_ID));
+
+ // NB: technically, specific thread with unspecified process is invalid
+ // but we do not filter that in StringExtractor
+
+ ex.Reset("p0.1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0, 0x1234ULL));
+
+ ex.Reset("p0.123456789ABCDEF0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0, 0x123456789ABCDEF0ULL));
+
+ ex.Reset("p-1.0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(LLDB_INVALID_PROCESS_ID, 0));
+
+ ex.Reset("p-1.-1");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(LLDB_INVALID_PROCESS_ID, LLDB_INVALID_THREAD_ID));
+
+ ex.Reset("p-1.1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(LLDB_INVALID_PROCESS_ID, 0x1234ULL));
+
+ ex.Reset("p-1.123456789ABCDEF0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(LLDB_INVALID_PROCESS_ID, 0x123456789ABCDEF0ULL));
+
+ ex.Reset("p1234.0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x1234ULL, 0));
+
+ ex.Reset("p1234.-1");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x1234ULL, LLDB_INVALID_THREAD_ID));
+
+ ex.Reset("p1234.123456789ABCDEF0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x1234ULL, 0x123456789ABCDEF0ULL));
+
+ ex.Reset("p123456789ABCDEF0.0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x123456789ABCDEF0ULL, 0));
+
+ ex.Reset("p123456789ABCDEF0.-1");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x123456789ABCDEF0ULL, LLDB_INVALID_THREAD_ID));
+
+ ex.Reset("p123456789ABCDEF0.1234");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x123456789ABCDEF0ULL, 0x1234ULL));
+
+ ex.Reset("p123456789ABCDEF0.123456789ABCDEF0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x123456789ABCDEF0ULL, 0x123456789ABCDEF0ULL));
+
+ ex.Reset("p123456789ABCDEF0.123456789ABCDEF0");
+ EXPECT_THAT(ex.GetPidTid(),
+ ::testing::Pair(0x123456789ABCDEF0ULL, 0x123456789ABCDEF0ULL));
+}
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -349,7 +349,7 @@
# Increment loop
reg_index += 1
- def Hg_switches_to_3_threads(self):
+ def Hg_switches_to_3_threads(self, pass_pid=False):
# Startup the inferior with three threads (main + 2 new ones).
procs = self.prep_debug_monitor_and_inferior(
inferior_args=["thread:new", "thread:new"])
@@ -363,12 +363,16 @@
threads = self.wait_for_thread_count(3)
self.assertEqual(len(threads), 3)
+ pid_str = ""
+ if pass_pid:
+ pid_str = "p{0:x}.".format(procs["inferior"].pid)
+
# verify we can $H to each thead, and $qC matches the thread we set.
for thread in threads:
# Change to each thread, verify current thread id.
self.reset_test_sequence()
self.test_sequence.add_log_lines(
- ["read packet: $Hg{0:x}#00".format(thread), # Set current thread.
+ ["read packet: $Hg{0}{1:x}#00".format(pid_str, thread), # Set current thread.
"send packet: $OK#00",
"read packet: $qC#00",
{"direction": "send", "regex": r"^\$QC([0-9a-fA-F]+)#", "capture": {1: "thread_id"}}],
@@ -393,6 +397,50 @@
self.set_inferior_startup_attach()
self.Hg_switches_to_3_threads()
+ @expectedFailureAll(oslist=["windows"]) # expect 4 threads
+ def test_Hg_switches_to_3_threads_attach_pass_correct_pid(self):
+ self.build()
+ self.set_inferior_startup_attach()
+ self.Hg_switches_to_3_threads(pass_pid=True)
+
+ def Hg_fails_on_pid(self, pass_pid):
+ # Start the inferior.
+ procs = self.prep_debug_monitor_and_inferior(
+ inferior_args=["thread:new"])
+
+ self.run_process_then_stop(run_seconds=1)
+
+ threads = self.wait_for_thread_count(2)
+ self.assertEqual(len(threads), 2)
+
+ if pass_pid == -1:
+ pid_str = "p-1."
+ else:
+ pid_str = "p{0:x}.".format(pass_pid)
+ thread = threads[1]
+
+ self.test_sequence.add_log_lines(
+ ["read packet: $Hg{0}{1:x}#00".format(pid_str, thread), # Set current thread.
+ "send packet: $E15#00"],
+ True)
+
+ self.expect_gdbremote_sequence()
+
+ def test_Hg_fails_on_another_pid(self):
+ self.build()
+ self.set_inferior_startup_launch()
+ self.Hg_fails_on_pid(1)
+
+ def test_Hg_fails_on_zero_pid(self):
+ self.build()
+ self.set_inferior_startup_launch()
+ self.Hg_fails_on_pid(0)
+
+ def test_Hg_fails_on_minus_one_pid(self):
+ self.build()
+ self.set_inferior_startup_launch()
+ self.Hg_fails_on_pid(-1)
+
def Hc_then_Csignal_signals_correct_thread(self, segfault_signo):
# NOTE only run this one in inferior-launched mode: we can't grab inferior stdout when running attached,
# and the test requires getting stdout from the exe.
Index: lldb/source/Utility/StringExtractor.cpp
===================================================================
--- lldb/source/Utility/StringExtractor.cpp
+++ lldb/source/Utility/StringExtractor.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "lldb/Utility/StringExtractor.h"
+#include "lldb/lldb-defines.h"
#include "llvm/ADT/StringExtras.h"
#include <tuple>
@@ -369,3 +370,56 @@
while (m_index < n && llvm::isSpace(m_packet[m_index]))
++m_index;
}
+
+std::pair<llvm::Optional<lldb::pid_t>, llvm::Optional<lldb::tid_t>>
+StringExtractor::GetPidTid() {
+ llvm::Optional<lldb::pid_t> pid = llvm::None;
+ llvm::Optional<lldb::tid_t> tid = llvm::None;
+
+ SkipSpaces();
+ if (m_index < m_packet.size() && m_packet[m_index] == 'p') {
+ // process identifier
+ ++m_index;
+ SkipSpaces();
+ if (m_index + 1 < m_packet.size() && m_packet[m_index] == '-' &&
+ m_packet[m_index + 1] == '1') {
+ // -1 is a special case
+ m_index += 2;
+ pid = LLDB_INVALID_PROCESS_ID;
+ } else {
+ uint64_t prev_index = m_index;
+ pid = GetHexMaxU64(false, 0);
+ if (m_index == prev_index || m_index == UINT64_MAX) {
+ // if index was not incremented or we overflowed, it failed
+ pid = llvm::None;
+ return {pid, tid};
+ }
+ }
+
+ // "." must follow if we expect TID too
+ SkipSpaces();
+ if (!(m_index < m_packet.size() && m_packet[m_index] == '.'))
+ return {pid, tid};
+ ++m_index;
+ SkipSpaces();
+ }
+
+ // thread identifier
+ if (m_index + 1 < m_packet.size() && m_packet[m_index] == '-' &&
+ m_packet[m_index + 1] == '1') {
+ // -1 is a special case
+ m_index += 2;
+ tid = LLDB_INVALID_PROCESS_ID;
+ } else {
+ uint64_t prev_index = m_index;
+ tid = GetHexMaxU64(false, 0);
+ if (m_index == prev_index || m_index == UINT64_MAX) {
+ // if index was not incremented or we overflowed, it failed
+ // (also reset pid since the value is incorrect)
+ pid = llvm::None;
+ tid = llvm::None;
+ }
+ }
+
+ return {pid, tid};
+}
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
@@ -2220,9 +2220,26 @@
}
// Parse out the thread number.
- // FIXME return a parse success/fail value. All values are valid here.
- const lldb::tid_t tid =
- packet.GetHexMaxU64(false, std::numeric_limits<lldb::tid_t>::max());
+ auto pid_tid = packet.GetPidTid();
+ if (pid_tid.first) {
+ const lldb::pid_t pid = pid_tid.first.getValue();
+ // Since we require a specific thread number, the PID must not be -1/0.
+ if (pid != m_debugged_process_up->GetID()) {
+ LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerLLGS::%s failed, pid %" PRIu64
+ " not found",
+ __FUNCTION__, pid);
+ return SendErrorResponse(0x15);
+ }
+ } else if (!pid_tid.second) {
+ LLDB_LOGF(
+ log,
+ "GDBRemoteCommunicationServerLLGS::%s failed, malformed thread-id",
+ __FUNCTION__);
+ return SendIllFormedResponse(packet, "Malformed thread-id");
+ }
+
+ const lldb::tid_t tid = pid_tid.second.getValue();
// Ensure we have the given thread when not specifying -1 (all threads) or 0
// (any thread).
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -848,6 +848,7 @@
response.PutCString(";qXfer:auxv:read+");
response.PutCString(";qXfer:libraries-svr4:read+");
#endif
+ response.PutCString(";multiprocess+");
return SendPacketNoLock(response.GetString());
}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -558,6 +558,7 @@
LazyBool m_supports_jGetSharedCacheInfo;
LazyBool m_supports_QPassSignals;
LazyBool m_supports_error_string_reply;
+ LazyBool m_supports_multiprocess;
bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
m_supports_qUserName : 1, m_supports_qGroupName : 1,
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -89,6 +89,7 @@
m_supports_jGetSharedCacheInfo(eLazyBoolCalculate),
m_supports_QPassSignals(eLazyBoolCalculate),
m_supports_error_string_reply(eLazyBoolCalculate),
+ m_supports_multiprocess(eLazyBoolCalculate),
m_supports_qProcessInfoPID(true), m_supports_qfProcessInfo(true),
m_supports_qUserName(true), m_supports_qGroupName(true),
m_supports_qThreadStopInfo(true), m_supports_z0(true),
@@ -292,6 +293,7 @@
m_prepare_for_reg_writing_reply = eLazyBoolCalculate;
m_attach_or_wait_reply = eLazyBoolCalculate;
m_avoid_g_packets = eLazyBoolCalculate;
+ m_supports_multiprocess = eLazyBoolCalculate;
m_supports_qXfer_auxv_read = eLazyBoolCalculate;
m_supports_qXfer_libraries_read = eLazyBoolCalculate;
m_supports_qXfer_libraries_svr4_read = eLazyBoolCalculate;
@@ -342,11 +344,13 @@
m_supports_augmented_libraries_svr4_read = eLazyBoolNo;
m_supports_qXfer_features_read = eLazyBoolNo;
m_supports_qXfer_memory_map_read = eLazyBoolNo;
+ m_supports_multiprocess = eLazyBoolNo;
m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
// not, we assume no limit
// build the qSupported packet
- std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc"};
+ std::vector<std::string> features = {"xmlRegisters=i386,arm,mips,arc",
+ "multiprocess+"};
StreamString packet;
packet.PutCString("qSupported");
for (uint32_t i = 0; i < features.size(); ++i) {
@@ -433,6 +437,11 @@
else
m_supports_QPassSignals = eLazyBoolNo;
+ if (::strstr(response_cstr, "multiprocess+"))
+ m_supports_multiprocess = eLazyBoolYes;
+ else
+ m_supports_multiprocess = eLazyBoolNo;
+
const char *packet_size_str = ::strstr(response_cstr, "PacketSize=");
if (packet_size_str) {
StringExtractorGDBRemote packet_response(packet_size_str +
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -902,7 +902,8 @@
"qXfer:libraries-svr4:read",
"qXfer:features:read",
"qEcho",
- "QPassSignals"
+ "QPassSignals",
+ "multiprocess",
]
def parse_qSupported_response(self, context):
Index: lldb/include/lldb/Utility/StringExtractor.h
===================================================================
--- lldb/include/lldb/Utility/StringExtractor.h
+++ lldb/include/lldb/Utility/StringExtractor.h
@@ -10,8 +10,11 @@
#define LLDB_UTILITY_STRINGEXTRACTOR_H
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringRef.h"
+#include "lldb/lldb-types.h"
+
#include <stddef.h>
#include <stdint.h>
#include <string>
@@ -95,6 +98,13 @@
size_t GetHexByteStringTerminatedBy(std::string &str, char terminator);
+ // Read thread-id in the <tid> | "p" <pid> ["." <tid>] format, where <pid>
+ // and <tid> can be either "-1", "0" or a hex-encoded number. Note that
+ // the function does not check for incorrect values such as -1.<tid != -1>.
+ // A pair of (llvm::None, llvm::None) is returned for malformed input.
+ std::pair<llvm::Optional<lldb::pid_t>, llvm::Optional<lldb::tid_t>>
+ GetPidTid();
+
bool ConsumeFront(const llvm::StringRef &str);
const char *Peek() {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
