Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h =================================================================== --- lldb/include/lldb/Utility/StringExtractorGDBRemote.h +++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h @@ -193,6 +193,13 @@ size_t GetEscapedBinaryData(std::string &str); + // Read thread-id in the | "p" ["." ] format, where + // and can be either "-1", "0" or a hex-encoded number. Note that + // the function does not check for incorrect values such as -1.. + // A pair of (llvm::None, llvm::None) is returned for malformed input. + std::pair, llvm::Optional> + GetPidTid(); + protected: ResponseValidatorCallback m_validator; void *m_validator_baton; 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/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 features = {"xmlRegisters=i386,arm,mips,arc"}; + std::vector 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/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/GDBRemoteCommunicationServerLLGS.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -244,6 +244,12 @@ void StopSTDIOForwarding(); + // Read thread-id from packet. If the thread-id is correct (and PID matches + // the current program), sets tid to it and returns llvm::None. Otherwise, + // returns the error response. + llvm::Optional + ReadTid(StringExtractorGDBRemote &packet, lldb::tid_t &tid); + // For GDBRemoteCommunicationServerLLGS only GDBRemoteCommunicationServerLLGS(const GDBRemoteCommunicationServerLLGS &) = delete; 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,10 @@ } // 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::max()); + lldb::tid_t tid; + auto tid_ret = ReadTid(packet, tid); + if (tid_ret) + return tid_ret.getValue(); // Ensure we have the given thread when not specifying -1 (all threads) or 0 // (any thread). @@ -3653,3 +3654,31 @@ } return result; } + +llvm::Optional +GDBRemoteCommunicationServerLLGS::ReadTid(StringExtractorGDBRemote &packet, + lldb::tid_t &tid) { + Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_THREAD)); + + 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"); + } + + tid = pid_tid.second.getValue(); + return llvm::None; +} Index: lldb/source/Utility/StringExtractorGDBRemote.cpp =================================================================== --- lldb/source/Utility/StringExtractorGDBRemote.cpp +++ lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -606,3 +606,52 @@ else return true; // No validator, so response is valid } + +std::pair, llvm::Optional> +StringExtractorGDBRemote::GetPidTid() { + llvm::Optional pid = llvm::None; + llvm::Optional tid = llvm::None; + llvm::StringRef view = llvm::StringRef(m_packet).substr(m_index); + + if (view.consume_front("p")) { + // process identifier + ++m_index; + if (view.consume_front("-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 + m_index = UINT64_MAX; + return {llvm::None, llvm::None}; + } + view = view.substr(m_index - prev_index); + } + + // "." must follow if we expect TID too + if (!view.consume_front(".")) + return {pid, tid}; + ++m_index; + } + + // thread identifier + if (view.consume_front("-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) + m_index = UINT64_MAX; + return {llvm::None, llvm::None}; + } + } + + return {pid, tid}; +} 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/unittests/Utility/CMakeLists.txt =================================================================== --- lldb/unittests/Utility/CMakeLists.txt +++ lldb/unittests/Utility/CMakeLists.txt @@ -29,6 +29,7 @@ StatusTest.cpp StreamTeeTest.cpp StreamTest.cpp + StringExtractorGDBRemoteTest.cpp StringExtractorTest.cpp StringLexerTest.cpp StringListTest.cpp Index: lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Utility/StringExtractorGDBRemoteTest.cpp @@ -0,0 +1,169 @@ +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include + +#include "lldb/Utility/StringExtractorGDBRemote.h" +#include "lldb/lldb-defines.h" + +namespace { +class StringExtractorGDBRemoteTest : public ::testing::Test {}; +} // namespace + +TEST_F(StringExtractorGDBRemoteTest, GetPidTid) { + StringExtractorGDBRemote 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)); +}