diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -293,15 +293,6 @@ 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 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/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -1674,12 +1674,7 @@ 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 @@ 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 thread_actions; while (packet.GetBytesLeft() && *packet.Peek() == ';') { // Skip the semi-colon. @@ -1737,32 +1726,62 @@ 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 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 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 @@ return result; } -llvm::Expected 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(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( - inconvertibleErrorCode(), - llvm::formatv("PID value {0} not allowed", pid == 0 ? 0 : -1)); - - if (!allow_all && tid == StringExtractorGDBRemote::AllThreads) - return llvm::make_error( - 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( - inconvertibleErrorCode(), llvm::formatv("PID {0} not debugged", pid)); - } - - return tid; -} - std::vector GDBRemoteCommunicationServerLLGS::HandleFeatures( const llvm::ArrayRef client_features) { std::vector ret = diff --git a/lldb/source/Utility/StringExtractorGDBRemote.cpp b/lldb/source/Utility/StringExtractorGDBRemote.cpp --- a/lldb/source/Utility/StringExtractorGDBRemote.cpp +++ b/lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -639,7 +639,7 @@ 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 @@ // 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 --- 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_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 @@ 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 @@ @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()