jingham updated this revision to Diff 344213. jingham added a comment. When the ReadPacket call in SendContinuePacketAndWaitForResponse wakes up due to the wakeup timeout, and there's an interrupt which hasn't reached it's endpoint, shorten the wakeup interval so we don't wait longer than the interrupt endpoint.
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102085/new/ https://reviews.llvm.org/D102085 Files: lldb/include/lldb/Target/Process.h lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Target/Process.cpp lldb/source/Target/TargetProperties.td lldb/test/API/functionalities/gdb_remote_client/TestHaltFails.py lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp lldb/unittests/tools/lldb-server/tests/TestClient.cpp
Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp =================================================================== --- lldb/unittests/tools/lldb-server/tests/TestClient.cpp +++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp @@ -193,7 +193,7 @@ PacketResult expected_result) { StringExtractorGDBRemote response; GTEST_LOG_(INFO) << "Send Packet: " << message.str(); - PacketResult result = SendPacketAndWaitForResponse(message, response, false); + PacketResult result = SendPacketAndWaitForResponse(message, response); response.GetEscapedBinaryData(response_string); GTEST_LOG_(INFO) << "Read Packet: " << response_string; if (result != expected_result) Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp =================================================================== --- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp +++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp @@ -385,8 +385,9 @@ TraceSupportedResponse trace_type; std::string error_message; auto callback = [&] { + std::chrono::seconds timeout(10); if (llvm::Expected<TraceSupportedResponse> trace_type_or_err = - client.SendTraceSupported()) { + client.SendTraceSupported(timeout)) { trace_type = *trace_type_or_err; error_message = ""; return true; Index: lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp =================================================================== --- lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp +++ lldb/unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp @@ -55,6 +55,8 @@ } protected: + // We don't have a process to get the interrupt timeout from, so make one up. + static std::chrono::seconds g_timeout; TestClient client; MockServer server; MockDelegate delegate; @@ -62,7 +64,8 @@ StateType SendCPacket(StringExtractorGDBRemote &response) { return client.SendContinuePacketAndWaitForResponse(delegate, LinuxSignals(), - "c", response); + "c", g_timeout, + response); } void WaitForRunEvent() { @@ -72,6 +75,8 @@ } }; +std::chrono::seconds GDBRemoteClientBaseTest::g_timeout(10); + } // end anonymous namespace TEST_F(GDBRemoteClientBaseTest, SendContinueAndWait) { @@ -103,7 +108,7 @@ StringExtractorGDBRemote continue_response, response; // SendAsyncSignal should do nothing when we are not running. - ASSERT_FALSE(client.SendAsyncSignal(0x47)); + ASSERT_FALSE(client.SendAsyncSignal(0x47, g_timeout)); // Continue. After the run packet is sent, send an async signal. std::future<StateType> continue_state = std::async( @@ -112,8 +117,9 @@ ASSERT_EQ("c", response.GetStringRef()); WaitForRunEvent(); - std::future<bool> async_result = std::async( - std::launch::async, [&] { return client.SendAsyncSignal(0x47); }); + std::future<bool> async_result = std::async(std::launch::async, [&] { + return client.SendAsyncSignal(0x47, g_timeout); + }); // First we'll get interrupted. ASSERT_EQ(PacketResult::Success, server.GetPacket(response)); @@ -133,7 +139,6 @@ TEST_F(GDBRemoteClientBaseTest, SendContinueAndAsyncPacket) { StringExtractorGDBRemote continue_response, async_response, response; - const bool send_async = true; // Continue. After the run packet is sent, send an async packet. std::future<StateType> continue_state = std::async( @@ -143,13 +148,12 @@ WaitForRunEvent(); // Sending without async enabled should fail. - ASSERT_EQ( - PacketResult::ErrorSendFailed, - client.SendPacketAndWaitForResponse("qTest1", response, !send_async)); + ASSERT_EQ(PacketResult::ErrorSendFailed, + client.SendPacketAndWaitForResponse("qTest1", response)); std::future<PacketResult> async_result = std::async(std::launch::async, [&] { return client.SendPacketAndWaitForResponse("qTest2", async_response, - send_async); + g_timeout); }); // First we'll get interrupted. @@ -178,7 +182,7 @@ StringExtractorGDBRemote continue_response, response; // Interrupt should do nothing when we're not running. - ASSERT_FALSE(client.Interrupt()); + ASSERT_FALSE(client.Interrupt(g_timeout)); // Continue. After the run packet is sent, send an interrupt. std::future<StateType> continue_state = std::async( @@ -187,8 +191,8 @@ ASSERT_EQ("c", response.GetStringRef()); WaitForRunEvent(); - std::future<bool> async_result = - std::async(std::launch::async, [&] { return client.Interrupt(); }); + std::future<bool> async_result = std::async( + std::launch::async, [&] { return client.Interrupt(g_timeout); }); // We get interrupted. ASSERT_EQ(PacketResult::Success, server.GetPacket(response)); @@ -211,8 +215,8 @@ ASSERT_EQ("c", response.GetStringRef()); WaitForRunEvent(); - std::future<bool> async_result = - std::async(std::launch::async, [&] { return client.Interrupt(); }); + std::future<bool> async_result = std::async( + std::launch::async, [&] { return client.Interrupt(g_timeout); }); // However, the target stops due to a different reason than the original // interrupt. @@ -233,10 +237,9 @@ TEST_F(GDBRemoteClientBaseTest, SendContinueAndInterrupt2PacketBug) { StringExtractorGDBRemote continue_response, async_response, response; - const bool send_async = true; // Interrupt should do nothing when we're not running. - ASSERT_FALSE(client.Interrupt()); + ASSERT_FALSE(client.Interrupt(g_timeout)); // Continue. After the run packet is sent, send an async signal. std::future<StateType> continue_state = std::async( @@ -245,8 +248,8 @@ ASSERT_EQ("c", response.GetStringRef()); WaitForRunEvent(); - std::future<bool> interrupt_result = - std::async(std::launch::async, [&] { return client.Interrupt(); }); + std::future<bool> interrupt_result = std::async( + std::launch::async, [&] { return client.Interrupt(g_timeout); }); // We get interrupted. We'll send two packets to simulate a buggy stub. ASSERT_EQ(PacketResult::Success, server.GetPacket(response)); @@ -261,8 +264,7 @@ // Packet stream should remain synchronized. std::future<PacketResult> send_result = std::async(std::launch::async, [&] { - return client.SendPacketAndWaitForResponse("qTest", async_response, - !send_async); + return client.SendPacketAndWaitForResponse("qTest", async_response); }); ASSERT_EQ(PacketResult::Success, server.GetPacket(response)); ASSERT_EQ("qTest", response.GetStringRef()); @@ -328,8 +330,8 @@ ASSERT_EQ("c", response.GetStringRef()); WaitForRunEvent(); - std::future<bool> async_result = - std::async(std::launch::async, [&] { return client.Interrupt(); }); + std::future<bool> async_result = std::async( + std::launch::async, [&] { return client.Interrupt(g_timeout); }); // We get interrupted, but we don't send a stop packet. ASSERT_EQ(PacketResult::Success, server.GetPacket(response)); @@ -352,7 +354,7 @@ ASSERT_EQ(PacketResult::Success, server.SendPacket("OK")); PacketResult result = client.SendPacketAndReceiveResponseWithOutputSupport( - "qRcmd,test", response, true, + "qRcmd,test", response, g_timeout, [&command_output](llvm::StringRef output) { command_output << output; }); ASSERT_EQ(PacketResult::Success, result); Index: lldb/test/API/functionalities/gdb_remote_client/TestHaltFails.py =================================================================== --- lldb/test/API/functionalities/gdb_remote_client/TestHaltFails.py +++ lldb/test/API/functionalities/gdb_remote_client/TestHaltFails.py @@ -8,14 +8,17 @@ class TestHaltFails(GDBRemoteTestBase): class MyResponder(MockGDBServerResponder): - + def __init__(self, timeout): + MockGDBServerResponder.__init__(self) + self.timeout = timeout + def setBreakpoint(self, packet): return "OK" def interrupt(self): # Simulate process waiting longer than the interrupt # timeout to stop, then sending the reply. - time.sleep(14) + time.sleep(self.timeout) return "T02reason:signal" def cont(self): @@ -30,9 +33,11 @@ event_type = lldb.SBProcess.GetStateFromEvent(event) self.assertEqual(event_type, value) - def get_to_running(self): - self.server.responder = self.MyResponder() + def get_to_running(self, sleep_interval, interrupt_timeout = 5): + self.server.responder = self.MyResponder(sleep_interval) self.target = self.createTarget("a.yaml") + # Set a shorter (and known) interrupt timeout: + self.dbg.HandleCommand("settings set target.process.interrupt-timeout {0}".format(interrupt_timeout)) process = self.connect(self.target) self.dbg.SetAsync(True) @@ -46,7 +51,7 @@ @skipIfReproducer # FIXME: Unexpected packet during (passive) replay def test_destroy_while_running(self): - process = self.get_to_running() + process = self.get_to_running(10) process.Destroy() # Again pretend that after failing to be interrupted, we delivered the stop @@ -59,7 +64,7 @@ Test that explicitly calling AsyncInterrupt, which then fails, leads to an "eStateExited" state. """ - process = self.get_to_running() + process = self.get_to_running(10) # Now do the interrupt: process.SendAsyncInterrupt() @@ -67,6 +72,19 @@ # be in eStateExited: self.wait_for_and_check_event(15, lldb.eStateExited) + @skipIfReproducer # FIXME: Unexpected packet during (passive) replay + def test_interrupt_timeout(self): + """ + Test that explicitly calling AsyncInterrupt but this time with + an interrupt timeout longer than we're going to wait succeeds. + """ + process = self.get_to_running(10, 20) + # Now do the interrupt: + process.SendAsyncInterrupt() + + # That should have caused the Halt to time out and we should + # be in eStateExited: + self.wait_for_and_check_event(20, lldb.eStateStopped) Index: lldb/source/Target/TargetProperties.td =================================================================== --- lldb/source/Target/TargetProperties.td +++ lldb/source/Target/TargetProperties.td @@ -230,6 +230,9 @@ def UtilityExpressionTimeout: Property<"utility-expression-timeout", "UInt64">, DefaultUnsignedValue<15>, Desc<"The time in seconds to wait for LLDB-internal utility expressions.">; + def InterruptTimeout: Property<"interrupt-timeout", "UInt64">, + DefaultUnsignedValue<20>, + Desc<"The time in seconds to wait for an interrupt succeed in stopping the target.">; def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">, DefaultFalse, Desc<"If true, stepping operations will run all threads. This is equivalent to setting the run-mode option to 'all-threads'.">; Index: lldb/source/Target/Process.cpp =================================================================== --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -296,6 +296,13 @@ return std::chrono::seconds(value); } +std::chrono::seconds ProcessProperties::GetInterruptTimeout() const { + const uint32_t idx = ePropertyInterruptTimeout; + uint64_t value = m_collection_sp->GetPropertyAtIndexAsUInt64( + nullptr, idx, g_process_properties[idx].default_uint_value); + return std::chrono::seconds(value); +} + bool ProcessProperties::GetSteppingRunsAllThreads() const { const uint32_t idx = ePropertySteppingRunsAllThreads; return m_collection_sp->GetPropertyAtIndexAsBoolean( @@ -1333,8 +1340,8 @@ Status error = PrivateResume(); if (error.Success()) { - StateType state = - WaitForProcessToStop(llvm::None, nullptr, true, listener_sp, stream); + StateType state = WaitForProcessToStop(GetInterruptTimeout(), nullptr, true, + listener_sp, stream); const bool must_be_alive = false; // eStateExited is ok, so this must be false if (!StateIsStoppedState(state, must_be_alive)) @@ -3074,9 +3081,10 @@ return Status(); } - // Wait for 10 second for the process to stop. - StateType state = WaitForProcessToStop( - seconds(10), &event_sp, true, halt_listener_sp, nullptr, use_run_lock); + // Wait for the process halt timeout seconds for the process to stop. + StateType state = + WaitForProcessToStop(GetInterruptTimeout(), &event_sp, true, + halt_listener_sp, nullptr, use_run_lock); RestoreProcessEvents(); if (state == eStateInvalid || !event_sp) { @@ -3107,8 +3115,8 @@ SendAsyncInterrupt(); // Consume the interrupt event. - StateType state = - WaitForProcessToStop(seconds(10), &exit_event_sp, true, listener_sp); + StateType state = WaitForProcessToStop(GetInterruptTimeout(), + &exit_event_sp, true, listener_sp); RestoreProcessEvents(); Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -465,7 +465,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (m_gdb_comm.SendPacketAndWaitForResponse(packet, response, false) == + if (m_gdb_comm.SendPacketAndWaitForResponse(packet, response) == GDBRemoteCommunication::PacketResult::Success) { response_type = response.GetResponseType(); if (response_type == StringExtractorGDBRemote::eResponse) { @@ -1015,7 +1015,7 @@ for (size_t idx = 0; idx < num_cmds; idx++) { StringExtractorGDBRemote response; m_gdb_comm.SendPacketAndWaitForResponse( - GetExtraStartupCommands().GetArgumentAtIndex(idx), response, false); + GetExtraStartupCommands().GetArgumentAtIndex(idx), response); } return error; } @@ -1210,25 +1210,25 @@ } llvm::Expected<TraceSupportedResponse> ProcessGDBRemote::TraceSupported() { - return m_gdb_comm.SendTraceSupported(); + return m_gdb_comm.SendTraceSupported(GetInterruptTimeout()); } llvm::Error ProcessGDBRemote::TraceStop(const TraceStopRequest &request) { - return m_gdb_comm.SendTraceStop(request); + return m_gdb_comm.SendTraceStop(request, GetInterruptTimeout()); } llvm::Error ProcessGDBRemote::TraceStart(const llvm::json::Value &request) { - return m_gdb_comm.SendTraceStart(request); + return m_gdb_comm.SendTraceStart(request, GetInterruptTimeout()); } llvm::Expected<std::string> ProcessGDBRemote::TraceGetState(llvm::StringRef type) { - return m_gdb_comm.SendTraceGetState(type); + return m_gdb_comm.SendTraceGetState(type, GetInterruptTimeout()); } llvm::Expected<std::vector<uint8_t>> ProcessGDBRemote::TraceGetBinaryData(const TraceGetBinaryDataRequest &request) { - return m_gdb_comm.SendTraceGetBinaryData(request); + return m_gdb_comm.SendTraceGetBinaryData(request, GetInterruptTimeout()); } void ProcessGDBRemote::DidExit() { @@ -1473,7 +1473,7 @@ while (true) { // Send vStopped StringExtractorGDBRemote response; - m_gdb_comm.SendPacketAndWaitForResponse("vStopped", response, false); + m_gdb_comm.SendPacketAndWaitForResponse("vStopped", response); // OK represents end of signal list if (response.IsOKResponse()) @@ -2415,7 +2415,7 @@ // file handle and debugserver will go away, and we can be done... m_gdb_comm.Disconnect(); } else - caused_stop = m_gdb_comm.Interrupt(); + caused_stop = m_gdb_comm.Interrupt(GetInterruptTimeout()); return error; } @@ -2564,11 +2564,11 @@ if (m_gdb_comm.IsConnected()) { if (m_public_state.GetValue() != eStateAttaching) { StringExtractorGDBRemote response; - bool send_async = true; GDBRemoteCommunication::ScopedTimeout(m_gdb_comm, std::chrono::seconds(3)); - if (m_gdb_comm.SendPacketAndWaitForResponse("k", response, send_async) == + if (m_gdb_comm.SendPacketAndWaitForResponse("k", response, + GetInterruptTimeout()) == GDBRemoteCommunication::PacketResult::Success) { char packet_cmd = response.GetChar(0); @@ -2732,7 +2732,8 @@ assert(packet_len + 1 < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (m_gdb_comm.SendPacketAndWaitForResponse(packet, response, true) == + if (m_gdb_comm.SendPacketAndWaitForResponse(packet, response, + GetInterruptTimeout()) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsNormalResponse()) { error.Clear(); @@ -2858,7 +2859,7 @@ StringExtractorGDBRemote response; if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response, - true) == + GetInterruptTimeout()) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsOKResponse()) { m_erased_flash_ranges.Insert(range, true); @@ -2887,7 +2888,8 @@ if (m_erased_flash_ranges.IsEmpty()) return status; StringExtractorGDBRemote response; - if (m_gdb_comm.SendPacketAndWaitForResponse("vFlashDone", response, true) == + if (m_gdb_comm.SendPacketAndWaitForResponse("vFlashDone", response, + GetInterruptTimeout()) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsOKResponse()) { m_erased_flash_ranges.Clear(); @@ -2948,7 +2950,7 @@ } StringExtractorGDBRemote response; if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response, - true) == + GetInterruptTimeout()) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsOKResponse()) { error.Clear(); @@ -3124,7 +3126,7 @@ (!bp_site->HardwareRequired())) { // Try to send off a software breakpoint packet ($Z0) uint8_t error_no = m_gdb_comm.SendGDBStoppointTypePacket( - eBreakpointSoftware, true, addr, bp_op_size); + eBreakpointSoftware, true, addr, bp_op_size, GetInterruptTimeout()); if (error_no == 0) { // The breakpoint was placed successfully bp_site->SetEnabled(true); @@ -3164,7 +3166,7 @@ if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) { // Try to send off a hardware breakpoint packet ($Z1) uint8_t error_no = m_gdb_comm.SendGDBStoppointTypePacket( - eBreakpointHardware, true, addr, bp_op_size); + eBreakpointHardware, true, addr, bp_op_size, GetInterruptTimeout()); if (error_no == 0) { // The breakpoint was placed successfully bp_site->SetEnabled(true); @@ -3228,13 +3230,15 @@ case BreakpointSite::eHardware: if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, false, - addr, bp_op_size)) + addr, bp_op_size, + GetInterruptTimeout())) error.SetErrorToGenericError(); break; case BreakpointSite::eExternal: { if (m_gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false, - addr, bp_op_size)) + addr, bp_op_size, + GetInterruptTimeout())) error.SetErrorToGenericError(); } break; } @@ -3290,7 +3294,8 @@ // Pass down an appropriate z/Z packet... if (m_gdb_comm.SupportsGDBStoppointPacket(type)) { if (m_gdb_comm.SendGDBStoppointTypePacket(type, true, addr, - wp->GetByteSize()) == 0) { + wp->GetByteSize(), + GetInterruptTimeout()) == 0) { wp->SetEnabled(true, notify); return error; } else @@ -3336,7 +3341,8 @@ GDBStoppointType type = GetGDBStoppointType(wp); // Pass down an appropriate z/Z packet... if (m_gdb_comm.SendGDBStoppointTypePacket(type, false, addr, - wp->GetByteSize()) == 0) { + wp->GetByteSize(), + GetInterruptTimeout()) == 0) { wp->SetEnabled(false, notify); return error; } else @@ -3361,7 +3367,7 @@ Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); LLDB_LOGF(log, "ProcessGDBRemote::DoSignal (signal = %d)", signo); - if (!m_gdb_comm.SendAsyncSignal(signo)) + if (!m_gdb_comm.SendAsyncSignal(signo, GetInterruptTimeout())) error.SetErrorStringWithFormat("failed to send signal %i", signo); return error; } @@ -3739,7 +3745,7 @@ // send the vCont packet if (!process->GetGDBRemote().SendvContPacket( llvm::StringRef(continue_cstr, continue_cstr_len), - response)) { + process->GetInterruptTimeout(), response)) { // Something went wrong done = true; break; @@ -3751,6 +3757,7 @@ process->GetGDBRemote().SendContinuePacketAndWaitForResponse( *process, *process->GetUnixSignals(), llvm::StringRef(continue_cstr, continue_cstr_len), + process->GetInterruptTimeout(), response); // We need to immediately clear the thread ID list so we are sure @@ -4045,8 +4052,7 @@ StringExtractorGDBRemote response; response.SetResponseValidatorToJSON(); - if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response, - false) == + if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response) == GDBRemoteCommunication::PacketResult::Success) { StringExtractorGDBRemote::ResponseType response_type = response.GetResponseType(); @@ -4118,8 +4124,7 @@ StringExtractorGDBRemote response; response.SetResponseValidatorToJSON(); - if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response, - false) == + if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response) == GDBRemoteCommunication::PacketResult::Success) { StringExtractorGDBRemote::ResponseType response_type = response.GetResponseType(); @@ -4152,8 +4157,7 @@ StringExtractorGDBRemote response; response.SetResponseValidatorToJSON(); - if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response, - false) == + if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response) == GDBRemoteCommunication::PacketResult::Success) { StringExtractorGDBRemote::ResponseType response_type = response.GetResponseType(); @@ -4919,8 +4923,7 @@ packet.PutStringAsRawHex8(file_path); StringExtractorGDBRemote response; - if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response, - false) != + if (m_gdb_comm.SendPacketAndWaitForResponse(packet.GetString(), response) != GDBRemoteCommunication::PacketResult::Success) return Status("Sending qFileLoadAddress packet failed"); @@ -5297,10 +5300,9 @@ if (process) { for (size_t i = 0; i < argc; ++i) { const char *packet_cstr = command.GetArgumentAtIndex(0); - bool send_async = true; StringExtractorGDBRemote response; process->GetGDBRemote().SendPacketAndWaitForResponse( - packet_cstr, response, send_async); + packet_cstr, response, process->GetInterruptTimeout()); result.SetStatus(eReturnStatusSuccessFinishResult); Stream &output_strm = result.GetOutputStream(); output_strm.Printf(" packet: %s\n", packet_cstr); @@ -5349,11 +5351,10 @@ packet.PutCString("qRcmd,"); packet.PutBytesAsRawHex8(command.data(), command.size()); - bool send_async = true; StringExtractorGDBRemote response; Stream &output_strm = result.GetOutputStream(); process->GetGDBRemote().SendPacketAndReceiveResponseWithOutputSupport( - packet.GetString(), response, send_async, + packet.GetString(), response, process->GetInterruptTimeout(), [&output_strm](llvm::StringRef output) { output_strm << output; }); result.SetStatus(eReturnStatusSuccessFinishResult); output_strm.Printf(" packet: %s\n", packet.GetData()); Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp @@ -364,7 +364,7 @@ reg_info->byte_size, // dst length m_reg_data.GetByteOrder())) // dst byte order { - GDBRemoteClientBase::Lock lock(gdb_comm, false); + GDBRemoteClientBase::Lock lock(gdb_comm); if (lock) { if (m_write_all_at_once) { // Invalidate all register values @@ -508,7 +508,7 @@ const bool use_g_packet = !gdb_comm.AvoidGPackets((ProcessGDBRemote *)process); - GDBRemoteClientBase::Lock lock(gdb_comm, false); + GDBRemoteClientBase::Lock lock(gdb_comm); if (lock) { if (gdb_comm.SyncThreadState(m_thread.GetProtocolID())) InvalidateAllRegisters(); @@ -574,7 +574,7 @@ const bool use_g_packet = !gdb_comm.AvoidGPackets((ProcessGDBRemote *)process); - GDBRemoteClientBase::Lock lock(gdb_comm, false); + GDBRemoteClientBase::Lock lock(gdb_comm); if (lock) { // The data_sp contains the G response packet. if (use_g_packet) { 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 @@ -321,7 +321,8 @@ GDBStoppointType type, // Type of breakpoint or watchpoint bool insert, // Insert or remove? lldb::addr_t addr, // Address of breakpoint or watchpoint - uint32_t length); // Byte Size of breakpoint or watchpoint + uint32_t length, // Byte Size of breakpoint or watchpoint + std::chrono::seconds interrupt_timeout); // Time to wait for an interrupt bool SetNonStopMode(const bool enable); @@ -510,16 +511,22 @@ ConfigureRemoteStructuredData(ConstString type_name, const StructuredData::ObjectSP &config_sp); - llvm::Expected<TraceSupportedResponse> SendTraceSupported(); + llvm::Expected<TraceSupportedResponse> + SendTraceSupported(std::chrono::seconds interrupt_timeout); - llvm::Error SendTraceStart(const llvm::json::Value &request); + llvm::Error SendTraceStart(const llvm::json::Value &request, + std::chrono::seconds interrupt_timeout); - llvm::Error SendTraceStop(const TraceStopRequest &request); + llvm::Error SendTraceStop(const TraceStopRequest &request, + std::chrono::seconds interrupt_timeout); - llvm::Expected<std::string> SendTraceGetState(llvm::StringRef type); + llvm::Expected<std::string> + SendTraceGetState(llvm::StringRef type, + std::chrono::seconds interrupt_timeout); llvm::Expected<std::vector<uint8_t>> - SendTraceGetBinaryData(const TraceGetBinaryDataRequest &request); + SendTraceGetBinaryData(const TraceGetBinaryDataRequest &request, + std::chrono::seconds interrupt_timeout); protected: LazyBool m_supports_not_sending_acks; @@ -614,7 +621,7 @@ PacketResult SendThreadSpecificPacketAndWaitForResponse( lldb::tid_t tid, StreamString &&payload, - StringExtractorGDBRemote &response, bool send_async); + StringExtractorGDBRemote &response); Status SendGetTraceDataPacket(StreamGDBRemote &packet, lldb::user_id_t uid, lldb::tid_t thread_id, 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 @@ -219,7 +219,7 @@ ScopedTimeout timeout(*this, std::max(GetPacketTimeout(), seconds(6))); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("QStartNoAckMode", response, false) == + if (SendPacketAndWaitForResponse("QStartNoAckMode", response) == PacketResult::Success) { if (response.IsOKResponse()) { m_send_acks = false; @@ -236,8 +236,8 @@ m_supports_threads_in_stop_reply = eLazyBoolNo; StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("QListThreadsInStopReply", response, - false) == PacketResult::Success) { + if (SendPacketAndWaitForResponse("QListThreadsInStopReply", response) == + PacketResult::Success) { if (response.IsOKResponse()) m_supports_threads_in_stop_reply = eLazyBoolYes; } @@ -249,8 +249,8 @@ m_attach_or_wait_reply = eLazyBoolNo; StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qVAttachOrWaitSupported", response, - false) == PacketResult::Success) { + if (SendPacketAndWaitForResponse("qVAttachOrWaitSupported", response) == + PacketResult::Success) { if (response.IsOKResponse()) m_attach_or_wait_reply = eLazyBoolYes; } @@ -263,8 +263,8 @@ m_prepare_for_reg_writing_reply = eLazyBoolNo; StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qSyncThreadStateSupported", response, - false) == PacketResult::Success) { + if (SendPacketAndWaitForResponse("qSyncThreadStateSupported", response) == + PacketResult::Success) { if (response.IsOKResponse()) m_prepare_for_reg_writing_reply = eLazyBoolYes; } @@ -362,8 +362,7 @@ } StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, - /*send_async=*/false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { // Hang on to the qSupported packet, so that platforms can do custom // configuration of the transport before attaching/launching the process. @@ -419,8 +418,8 @@ if (m_supports_thread_suffix == eLazyBoolCalculate) { StringExtractorGDBRemote response; m_supports_thread_suffix = eLazyBoolNo; - if (SendPacketAndWaitForResponse("QThreadSuffixSupported", response, - false) == PacketResult::Success) { + if (SendPacketAndWaitForResponse("QThreadSuffixSupported", response) == + PacketResult::Success) { if (response.IsOKResponse()) m_supports_thread_suffix = eLazyBoolYes; } @@ -436,7 +435,7 @@ m_supports_vCont_C = eLazyBoolNo; m_supports_vCont_s = eLazyBoolNo; m_supports_vCont_S = eLazyBoolNo; - if (SendPacketAndWaitForResponse("vCont?", response, false) == + if (SendPacketAndWaitForResponse("vCont?", response) == PacketResult::Success) { const char *response_cstr = response.GetStringRef().data(); if (::strstr(response_cstr, ";c")) @@ -488,9 +487,9 @@ GDBRemoteCommunication::PacketResult GDBRemoteCommunicationClient::SendThreadSpecificPacketAndWaitForResponse( - lldb::tid_t tid, StreamString &&payload, StringExtractorGDBRemote &response, - bool send_async) { - Lock lock(*this, send_async); + lldb::tid_t tid, StreamString &&payload, + StringExtractorGDBRemote &response) { + Lock lock(*this); if (!lock) { if (Log *log = ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet( GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)) @@ -527,7 +526,7 @@ payload.PutCString(packetStr); StringExtractorGDBRemote response; if (SendThreadSpecificPacketAndWaitForResponse( - tid, std::move(payload), response, false) == PacketResult::Success && + tid, std::move(payload), response) == PacketResult::Success && response.IsNormalResponse()) { return eLazyBoolYes; } @@ -541,7 +540,7 @@ if (m_supports_jThreadsInfo) { StringExtractorGDBRemote response; response.SetResponseValidatorToJSON(); - if (SendPacketAndWaitForResponse("jThreadsInfo", response, false) == + if (SendPacketAndWaitForResponse("jThreadsInfo", response) == PacketResult::Success) { if (response.IsUnsupportedResponse()) { m_supports_jThreadsInfo = false; @@ -558,7 +557,7 @@ if (m_supports_jThreadExtendedInfo == eLazyBoolCalculate) { StringExtractorGDBRemote response; m_supports_jThreadExtendedInfo = eLazyBoolNo; - if (SendPacketAndWaitForResponse("jThreadExtendedInfo:", response, false) == + if (SendPacketAndWaitForResponse("jThreadExtendedInfo:", response) == PacketResult::Success) { if (response.IsOKResponse()) { m_supports_jThreadExtendedInfo = eLazyBoolYes; @@ -574,7 +573,7 @@ // We try to enable error strings in remote packets but if we fail, we just // work in the older way. m_supports_error_string_reply = eLazyBoolNo; - if (SendPacketAndWaitForResponse("QEnableErrorStrings", response, false) == + if (SendPacketAndWaitForResponse("QEnableErrorStrings", response) == PacketResult::Success) { if (response.IsOKResponse()) { m_supports_error_string_reply = eLazyBoolYes; @@ -588,8 +587,7 @@ StringExtractorGDBRemote response; m_supports_jLoadedDynamicLibrariesInfos = eLazyBoolNo; if (SendPacketAndWaitForResponse("jGetLoadedDynamicLibrariesInfos:", - response, - false) == PacketResult::Success) { + response) == PacketResult::Success) { if (response.IsOKResponse()) { m_supports_jLoadedDynamicLibrariesInfos = eLazyBoolYes; } @@ -602,7 +600,7 @@ if (m_supports_jGetSharedCacheInfo == eLazyBoolCalculate) { StringExtractorGDBRemote response; m_supports_jGetSharedCacheInfo = eLazyBoolNo; - if (SendPacketAndWaitForResponse("jGetSharedCacheInfo:", response, false) == + if (SendPacketAndWaitForResponse("jGetSharedCacheInfo:", response) == PacketResult::Success) { if (response.IsOKResponse()) { m_supports_jGetSharedCacheInfo = eLazyBoolYes; @@ -618,7 +616,7 @@ m_supports_x = eLazyBoolNo; char packet[256]; snprintf(packet, sizeof(packet), "x0,0"); - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsOKResponse()) m_supports_x = eLazyBoolYes; @@ -630,7 +628,7 @@ GDBRemoteCommunicationClient::PacketResult GDBRemoteCommunicationClient::SendPacketsAndConcatenateResponses( const char *payload_prefix, std::string &response_string) { - Lock lock(*this, false); + Lock lock(*this); if (!lock) { Log *log(ProcessGDBRemoteLog::GetLogIfAnyCategoryIsSet(GDBR_LOG_PROCESS | GDBR_LOG_PACKETS)); @@ -691,8 +689,7 @@ // the thread id, which newer debugserver and lldb-gdbserver stubs return // correctly. StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qC", response, false) == - PacketResult::Success) { + if (SendPacketAndWaitForResponse("qC", response) == PacketResult::Success) { if (response.GetChar() == 'Q') { if (response.GetChar() == 'C') { m_curr_pid = response.GetHexMaxU64(false, LLDB_INVALID_PROCESS_ID); @@ -727,7 +724,7 @@ bool GDBRemoteCommunicationClient::GetLaunchSuccess(std::string &error_str) { error_str.clear(); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qLaunchSuccess", response, false) == + if (SendPacketAndWaitForResponse("qLaunchSuccess", response) == PacketResult::Success) { if (response.IsOKResponse()) return true; @@ -781,7 +778,7 @@ } StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; @@ -831,7 +828,7 @@ if (m_supports_QEnvironmentHexEncoded) { packet.PutCString("QEnvironmentHexEncoded:"); packet.PutBytesAsRawHex8(name_equal_value, strlen(name_equal_value)); - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; @@ -845,7 +842,7 @@ } else if (m_supports_QEnvironment) { packet.Printf("QEnvironment:%s", name_equal_value); - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; @@ -865,7 +862,7 @@ StreamString packet; packet.Printf("QLaunchArch:%s", arch); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; @@ -883,7 +880,7 @@ StreamString packet; packet.Printf("QSetProcessEvent:%s", data); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) { if (was_supported) @@ -968,7 +965,7 @@ m_qGDBServerVersion_is_valid = eLazyBoolNo; StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qGDBServerVersion", response, false) == + if (SendPacketAndWaitForResponse("qGDBServerVersion", response) == PacketResult::Success) { if (response.IsNormalResponse()) { llvm::StringRef name, value; @@ -1060,7 +1057,7 @@ if (avail_type != CompressionType::None) { StringExtractorGDBRemote response; llvm::Twine packet = "QEnableCompression:type:" + avail_name + ";"; - if (SendPacketAndWaitForResponse(packet.str(), response, false) != + if (SendPacketAndWaitForResponse(packet.str(), response) != PacketResult::Success) return; @@ -1086,8 +1083,7 @@ bool GDBRemoteCommunicationClient::GetDefaultThreadId(lldb::tid_t &tid) { StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qC", response, false) != - PacketResult::Success) + if (SendPacketAndWaitForResponse("qC", response) != PacketResult::Success) return false; if (!response.IsNormalResponse()) @@ -1137,7 +1133,7 @@ ScopedTimeout timeout(*this, seconds(10)); m_qHostInfo_is_valid = eLazyBoolNo; StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qHostInfo", response, false) == + if (SendPacketAndWaitForResponse("qHostInfo", response) == PacketResult::Success) { if (response.IsNormalResponse()) { llvm::StringRef name; @@ -1329,7 +1325,7 @@ ::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, pid); UNUSED_IF_ASSERT_DISABLED(packet_len); assert(packet_len < (int)sizeof(packet)); - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsErrorResponse()) return response.GetError(); @@ -1345,7 +1341,7 @@ packet.PutCString("I"); packet.PutBytesAsRawHex8(data, data_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { return 0; } @@ -1383,7 +1379,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsUnsupportedResponse()) m_supports_alloc_dealloc_memory = eLazyBoolNo; @@ -1405,7 +1401,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsUnsupportedResponse()) m_supports_alloc_dealloc_memory = eLazyBoolNo; @@ -1429,7 +1425,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success && response.IsOKResponse()) { m_supports_detach_stay_stopped = eLazyBoolYes; @@ -1443,15 +1439,13 @@ return error; } else { StringExtractorGDBRemote response; - PacketResult packet_result = - SendPacketAndWaitForResponse("D1", response, false); + PacketResult packet_result = SendPacketAndWaitForResponse("D1", response); if (packet_result != PacketResult::Success) error.SetErrorString("Sending extended disconnect packet failed."); } } else { StringExtractorGDBRemote response; - PacketResult packet_result = - SendPacketAndWaitForResponse("D", response, false); + PacketResult packet_result = SendPacketAndWaitForResponse("D", response); if (packet_result != PacketResult::Success) error.SetErrorString("Sending disconnect packet failed."); } @@ -1471,7 +1465,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success && response.GetResponseType() == StringExtractorGDBRemote::eResponse) { llvm::StringRef name; @@ -1708,8 +1702,8 @@ num = 0; if (m_supports_watchpoint_support_info != eLazyBoolNo) { StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qWatchpointSupportInfo:", response, - false) == PacketResult::Success) { + if (SendPacketAndWaitForResponse("qWatchpointSupportInfo:", response) == + PacketResult::Success) { m_supports_watchpoint_support_info = eLazyBoolYes; llvm::StringRef name; llvm::StringRef value; @@ -1777,7 +1771,7 @@ packet.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; @@ -1797,7 +1791,7 @@ packet.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; @@ -1817,7 +1811,7 @@ packet.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; @@ -1831,7 +1825,7 @@ bool GDBRemoteCommunicationClient::GetWorkingDir(FileSpec &working_dir) { StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qGetWorkingDir", response, false) == + if (SendPacketAndWaitForResponse("qGetWorkingDir", response) == PacketResult::Success) { if (response.IsUnsupportedResponse()) return false; @@ -1853,7 +1847,7 @@ packet.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; @@ -1872,8 +1866,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == - PacketResult::Success) { + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; uint8_t error = response.GetError(); @@ -1890,8 +1883,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == - PacketResult::Success) { + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsOKResponse()) return 0; uint8_t error = response.GetError(); @@ -2009,7 +2001,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { return DecodeProcessInfoResponse(response, process_info); } else { @@ -2034,7 +2026,7 @@ GetHostInfo(); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qProcessInfo", response, false) == + if (SendPacketAndWaitForResponse("qProcessInfo", response) == PacketResult::Success) { if (response.IsNormalResponse()) { llvm::StringRef name; @@ -2230,7 +2222,7 @@ // Increase timeout as the first qfProcessInfo packet takes a long time on // Android. The value of 1min was arrived at empirically. ScopedTimeout timeout(*this, minutes(1)); - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) == + if (SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success) { do { ProcessInstanceInfo process_info; @@ -2238,7 +2230,7 @@ break; process_infos.push_back(process_info); response = StringExtractorGDBRemote(); - } while (SendPacketAndWaitForResponse("qsProcessInfo", response, false) == + } while (SendPacketAndWaitForResponse("qsProcessInfo", response) == PacketResult::Success); } else { m_supports_qfProcessInfo = false; @@ -2257,7 +2249,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsNormalResponse()) { // Make sure we parsed the right number of characters. The response is @@ -2284,7 +2276,7 @@ assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsNormalResponse()) { // Make sure we parsed the right number of characters. The response is @@ -2312,8 +2304,7 @@ StringExtractorGDBRemote response; // Send to target - if (SendPacketAndWaitForResponse(packet, response, false) == - PacketResult::Success) + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) if (response.IsOKResponse()) return true; @@ -2385,7 +2376,7 @@ for (i = 0; i < num_packets; ++i) { const auto packet_start_time = steady_clock::now(); StringExtractorGDBRemote response; - SendPacketAndWaitForResponse(packet.GetString(), response, false); + SendPacketAndWaitForResponse(packet.GetString(), response); const auto packet_end_time = steady_clock::now(); packet_times.push_back(packet_end_time - packet_start_time); } @@ -2439,7 +2430,7 @@ uint32_t packet_count = 0; while (bytes_read < recv_amount) { StringExtractorGDBRemote response; - SendPacketAndWaitForResponse(packet.GetString(), response, false); + SendPacketAndWaitForResponse(packet.GetString(), response); bytes_read += recv_size; ++packet_count; } @@ -2493,7 +2484,7 @@ } StringExtractorGDBRemote response; - return SendPacketAndWaitForResponse(packet.GetString(), response, false) == + return SendPacketAndWaitForResponse(packet.GetString(), response) == PacketResult::Success; } @@ -2523,7 +2514,7 @@ // give the process a few seconds to startup ScopedTimeout timeout(*this, seconds(10)); - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { llvm::StringRef name; llvm::StringRef value; @@ -2547,7 +2538,7 @@ connection_urls.clear(); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qQueryGDBServer", response, false) != + if (SendPacketAndWaitForResponse("qQueryGDBServer", response) != PacketResult::Success) return 0; @@ -2586,7 +2577,7 @@ stream.Printf("qKillSpawnedProcess:%" PRId64, pid); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.IsOKResponse()) return true; @@ -2607,8 +2598,7 @@ assert(packet_len + 1 < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == - PacketResult::Success) { + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsOKResponse()) { m_curr_tid = tid; return true; @@ -2643,8 +2633,7 @@ assert(packet_len + 1 < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) == - PacketResult::Success) { + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsOKResponse()) { m_curr_tid_run = tid; return true; @@ -2667,8 +2656,7 @@ bool GDBRemoteCommunicationClient::GetStopReply( StringExtractorGDBRemote &response) { - if (SendPacketAndWaitForResponse("?", response, false) == - PacketResult::Success) + if (SendPacketAndWaitForResponse("?", response) == PacketResult::Success) return response.IsNormalResponse(); return false; } @@ -2681,7 +2669,7 @@ ::snprintf(packet, sizeof(packet), "qThreadStopInfo%" PRIx64, tid); assert(packet_len < (int)sizeof(packet)); UNUSED_IF_ASSERT_DISABLED(packet_len); - if (SendPacketAndWaitForResponse(packet, response, false) == + if (SendPacketAndWaitForResponse(packet, response) == PacketResult::Success) { if (response.IsUnsupportedResponse()) m_supports_qThreadStopInfo = false; @@ -2697,7 +2685,8 @@ } uint8_t GDBRemoteCommunicationClient::SendGDBStoppointTypePacket( - GDBStoppointType type, bool insert, addr_t addr, uint32_t length) { + GDBStoppointType type, bool insert, addr_t addr, uint32_t length, + std::chrono::seconds timeout) { Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_BREAKPOINTS)); LLDB_LOGF(log, "GDBRemoteCommunicationClient::%s() %s at addr = 0x%" PRIx64, __FUNCTION__, insert ? "add" : "remove", addr); @@ -2718,7 +2707,7 @@ // or "" (unsupported) response.SetResponseValidatorToOKErrorNotSupported(); // Try to send the breakpoint packet, and check that it was correctly sent - if (SendPacketAndWaitForResponse(packet, response, true) == + if (SendPacketAndWaitForResponse(packet, response, timeout) == PacketResult::Success) { // Receive and OK packet when the breakpoint successfully placed if (response.IsOKResponse()) @@ -2761,7 +2750,7 @@ bool &sequence_mutex_unavailable) { std::vector<std::pair<lldb::pid_t, lldb::tid_t>> ids; - Lock lock(*this, false); + Lock lock(*this); if (lock) { sequence_mutex_unavailable = false; StringExtractorGDBRemote response; @@ -2833,7 +2822,7 @@ lldb::addr_t GDBRemoteCommunicationClient::GetShlibInfoAddr() { StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse("qShlibInfoAddr", response, false) != + if (SendPacketAndWaitForResponse("qShlibInfoAddr", response) != PacketResult::Success || !response.IsNormalResponse()) return LLDB_INVALID_ADDRESS; @@ -2866,7 +2855,7 @@ stream.PutStringAsRawHex8(path); } StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() != 'F') return Status("malformed reply"); @@ -2904,8 +2893,7 @@ llvm::StringRef packet = stream.GetString(); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) != - PacketResult::Success) + if (SendPacketAndWaitForResponse(packet, response) != PacketResult::Success) return Status("failed to send '%s' packet", packet.str().c_str()); if (response.GetChar() != 'F') @@ -2926,8 +2914,7 @@ llvm::StringRef packet = stream.GetString(); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet, response, false) != - PacketResult::Success) + if (SendPacketAndWaitForResponse(packet, response) != PacketResult::Success) return Status("failed to send '%s' packet", stream.GetData()); if (response.GetChar() != 'F') @@ -2969,7 +2956,7 @@ stream.PutChar(','); stream.PutHex32(mode); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { return ParseHostIOPacketResponse(response, UINT64_MAX, error); } @@ -2981,7 +2968,7 @@ lldb_private::StreamString stream; stream.Printf("vFile:close:%i", (int)fd); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { return ParseHostIOPacketResponse(response, -1, error) == 0; } @@ -2996,7 +2983,7 @@ stream.PutCString("vFile:size:"); stream.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() != 'F') return UINT64_MAX; @@ -3014,7 +3001,7 @@ stream.PutChar(','); stream.PutStringAsRawHex8(request.GetCursorArgumentPrefix()); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { StreamString strm; char ch = response.GetChar(); @@ -3040,7 +3027,7 @@ stream.PutCString("vFile:mode:"); stream.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() != 'F') { error.SetErrorStringWithFormat("invalid response to '%s' packet", @@ -3075,7 +3062,7 @@ stream.Printf("vFile:pread:%i,%" PRId64 ",%" PRId64, (int)fd, dst_len, offset); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() != 'F') return 0; @@ -3109,7 +3096,7 @@ stream.Printf("vFile:pwrite:%i,%" PRId64 ",", (int)fd, offset); stream.PutEscapedBytes(src, src_len); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() != 'F') { error.SetErrorStringWithFormat("write file failed"); @@ -3144,7 +3131,7 @@ stream.PutChar(','); stream.PutStringAsRawHex8(src_path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() == 'F') { uint32_t result = response.GetU32(UINT32_MAX); @@ -3175,7 +3162,7 @@ // so we follow suit here stream.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() == 'F') { uint32_t result = response.GetU32(UINT32_MAX); @@ -3205,7 +3192,7 @@ stream.PutCString("vFile:exists:"); stream.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() != 'F') return false; @@ -3224,7 +3211,7 @@ stream.PutCString("vFile:MD5:"); stream.PutStringAsRawHex8(path); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(stream.GetString(), response, false) == + if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() != 'F') return false; @@ -3271,7 +3258,7 @@ payload.Printf("p%x", reg); StringExtractorGDBRemote response; if (SendThreadSpecificPacketAndWaitForResponse( - tid, std::move(payload), response, false) != PacketResult::Success || + tid, std::move(payload), response) != PacketResult::Success || !response.IsNormalResponse()) return nullptr; @@ -3286,7 +3273,7 @@ payload.PutChar('g'); StringExtractorGDBRemote response; if (SendThreadSpecificPacketAndWaitForResponse( - tid, std::move(payload), response, false) != PacketResult::Success || + tid, std::move(payload), response) != PacketResult::Success || !response.IsNormalResponse()) return nullptr; @@ -3305,9 +3292,8 @@ endian::InlHostByteOrder(), endian::InlHostByteOrder()); StringExtractorGDBRemote response; - return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), - response, false) == - PacketResult::Success && + return SendThreadSpecificPacketAndWaitForResponse( + tid, std::move(payload), response) == PacketResult::Success && response.IsOKResponse(); } @@ -3319,9 +3305,8 @@ endian::InlHostByteOrder(), endian::InlHostByteOrder()); StringExtractorGDBRemote response; - return SendThreadSpecificPacketAndWaitForResponse(tid, std::move(payload), - response, false) == - PacketResult::Success && + return SendThreadSpecificPacketAndWaitForResponse( + tid, std::move(payload), response) == PacketResult::Success && response.IsOKResponse(); } @@ -3336,7 +3321,7 @@ payload.PutCString("QSaveRegisterState"); StringExtractorGDBRemote response; if (SendThreadSpecificPacketAndWaitForResponse( - tid, std::move(payload), response, false) != PacketResult::Success) + tid, std::move(payload), response) != PacketResult::Success) return false; if (response.IsUnsupportedResponse()) @@ -3362,7 +3347,7 @@ payload.Printf("QRestoreRegisterState:%u", save_id); StringExtractorGDBRemote response; if (SendThreadSpecificPacketAndWaitForResponse( - tid, std::move(payload), response, false) != PacketResult::Success) + tid, std::move(payload), response) != PacketResult::Success) return false; if (response.IsOKResponse()) @@ -3380,13 +3365,13 @@ StreamString packet; StringExtractorGDBRemote response; packet.Printf("QSyncThreadState:%4.4" PRIx64 ";", tid); - return SendPacketAndWaitForResponse(packet.GetString(), response, false) == + return SendPacketAndWaitForResponse(packet.GetString(), response) == GDBRemoteCommunication::PacketResult::Success && response.IsOKResponse(); } llvm::Expected<TraceSupportedResponse> -GDBRemoteCommunicationClient::SendTraceSupported() { +GDBRemoteCommunicationClient::SendTraceSupported(std::chrono::seconds timeout) { Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); StreamGDBRemote escaped_packet; @@ -3394,7 +3379,7 @@ StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response, - true) == + timeout) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsErrorResponse()) return response.GetStatus().ToError(); @@ -3411,7 +3396,8 @@ } llvm::Error -GDBRemoteCommunicationClient::SendTraceStop(const TraceStopRequest &request) { +GDBRemoteCommunicationClient::SendTraceStop(const TraceStopRequest &request, + std::chrono::seconds timeout) { Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); StreamGDBRemote escaped_packet; @@ -3426,7 +3412,7 @@ StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response, - true) == + timeout) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsErrorResponse()) return response.GetStatus().ToError(); @@ -3445,7 +3431,8 @@ } llvm::Error -GDBRemoteCommunicationClient::SendTraceStart(const llvm::json::Value ¶ms) { +GDBRemoteCommunicationClient::SendTraceStart(const llvm::json::Value ¶ms, + std::chrono::seconds timeout) { Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); StreamGDBRemote escaped_packet; @@ -3460,7 +3447,7 @@ StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response, - true) == + timeout) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsErrorResponse()) return response.GetStatus().ToError(); @@ -3479,7 +3466,8 @@ } llvm::Expected<std::string> -GDBRemoteCommunicationClient::SendTraceGetState(llvm::StringRef type) { +GDBRemoteCommunicationClient::SendTraceGetState(llvm::StringRef type, + std::chrono::seconds timeout) { Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); StreamGDBRemote escaped_packet; @@ -3494,7 +3482,7 @@ StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response, - true) == + timeout) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsErrorResponse()) return response.GetStatus().ToError(); @@ -3513,7 +3501,7 @@ llvm::Expected<std::vector<uint8_t>> GDBRemoteCommunicationClient::SendTraceGetBinaryData( - const TraceGetBinaryDataRequest &request) { + const TraceGetBinaryDataRequest &request, std::chrono::seconds timeout) { Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); StreamGDBRemote escaped_packet; @@ -3528,7 +3516,7 @@ StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse(escaped_packet.GetString(), response, - true) == + timeout) == GDBRemoteCommunication::PacketResult::Success) { if (response.IsErrorResponse()) return response.GetStatus().ToError(); @@ -3548,8 +3536,8 @@ llvm::Optional<QOffsets> GDBRemoteCommunicationClient::GetQOffsets() { StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse( - "qOffsets", response, /*send_async=*/false) != PacketResult::Success) + if (SendPacketAndWaitForResponse("qOffsets", response) != + PacketResult::Success) return llvm::None; if (!response.IsNormalResponse()) return llvm::None; @@ -3604,7 +3592,7 @@ packet.PutStringAsRawHex8(triple); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(packet.GetString(), response, false) != + if (SendPacketAndWaitForResponse(packet.GetString(), response) != PacketResult::Success) return false; @@ -3711,7 +3699,7 @@ ScopedTimeout timeout(*this, std::chrono::seconds(10)); StringExtractorGDBRemote response; - if (SendPacketAndWaitForResponse(payload.GetString(), response, false) != + if (SendPacketAndWaitForResponse(payload.GetString(), response) != PacketResult::Success || response.IsErrorResponse()) return llvm::None; @@ -3770,7 +3758,7 @@ << "," << std::hex << size; GDBRemoteCommunication::PacketResult res = - SendPacketAndWaitForResponse(packet.str(), chunk, false); + SendPacketAndWaitForResponse(packet.str(), chunk); if (res != GDBRemoteCommunication::PacketResult::Success) { err.SetErrorString("Error sending $qXfer packet"); @@ -3859,7 +3847,7 @@ bool first_qsymbol_query = true; if (m_supports_qSymbol && !m_qSymbol_requests_done) { - Lock lock(*this, false); + Lock lock(*this); if (lock) { StreamString packet; packet.PutCString("qSymbol::"); @@ -3987,9 +3975,8 @@ // Poll it now. StringExtractorGDBRemote response; - const bool send_async = false; - if (SendPacketAndWaitForResponse("qStructuredDataPlugins", response, - send_async) == PacketResult::Success) { + if (SendPacketAndWaitForResponse("qStructuredDataPlugins", response) == + PacketResult::Success) { m_supported_async_json_packets_sp = StructuredData::ParseJSON(std::string(response.GetStringRef())); if (m_supported_async_json_packets_sp && @@ -4033,7 +4020,7 @@ std::string packet = formatv("QPassSignals:{0:$[;]@(x-2)}", range).str(); StringExtractorGDBRemote response; - auto send_status = SendPacketAndWaitForResponse(packet, response, false); + auto send_status = SendPacketAndWaitForResponse(packet, response); if (send_status != GDBRemoteCommunication::PacketResult::Success) return Status("Sending QPassSignals packet failed"); @@ -4072,10 +4059,8 @@ stream.Flush(); // Send the packet. - const bool send_async = false; StringExtractorGDBRemote response; - auto result = - SendPacketAndWaitForResponse(stream.GetString(), response, send_async); + auto result = SendPacketAndWaitForResponse(stream.GetString(), response); if (result == PacketResult::Success) { // We failed if the config result comes back other than OK. if (strcmp(response.GetStringRef().data(), "OK") == 0) { Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h @@ -33,29 +33,46 @@ GDBRemoteClientBase(const char *comm_name, const char *listener_name); - bool SendAsyncSignal(int signo); + bool SendAsyncSignal(int signo, std::chrono::seconds interrupt_timeout); - bool Interrupt(); + bool Interrupt(std::chrono::seconds interrupt_timeout); lldb::StateType SendContinuePacketAndWaitForResponse( ContinueDelegate &delegate, const UnixSignals &signals, - llvm::StringRef payload, StringExtractorGDBRemote &response); - - PacketResult SendPacketAndWaitForResponse(llvm::StringRef payload, - StringExtractorGDBRemote &response, - bool send_async); + llvm::StringRef payload, std::chrono::seconds interrupt_timeout, + StringExtractorGDBRemote &response); + + // If interrupt_timeout == 0 seconds, don't interrupt the target. + // Only send the packet if the target is stopped. + // If you want to use this mode, use the fact that the timeout is defaulted + // so it's clear from the call-site that you are using no-interrupt. + // If it is non-zero, interrupt the target if it is running, and + // send the packet. + // It the target doesn't respond within the given timeout, it returns + // ErrorReplyTimeout. + PacketResult SendPacketAndWaitForResponse( + llvm::StringRef payload, StringExtractorGDBRemote &response, + std::chrono::seconds interrupt_timeout = std::chrono::seconds(0)); PacketResult SendPacketAndReceiveResponseWithOutputSupport( llvm::StringRef payload, StringExtractorGDBRemote &response, - bool send_async, + std::chrono::seconds interrupt_timeout, llvm::function_ref<void(llvm::StringRef)> output_callback); bool SendvContPacket(llvm::StringRef payload, + std::chrono::seconds interrupt_timeout, StringExtractorGDBRemote &response); class Lock { public: - Lock(GDBRemoteClientBase &comm, bool interrupt); + // If interrupt_timeout == 0 seconds, only take the lock if the target is + // not running. If using this option, use the fact that the + // interrupt_timeout is defaulted so it will be obvious at the call site + // that you are choosing this mode. If it is non-zero, interrupt the target + // if it is running, waiting for the given timeout for the interrupt to + // succeed. + Lock(GDBRemoteClientBase &comm, + std::chrono::seconds interrupt_timeout = std::chrono::seconds(0)); ~Lock(); explicit operator bool() { return m_acquired; } @@ -67,10 +84,11 @@ private: std::unique_lock<std::recursive_mutex> m_async_lock; GDBRemoteClientBase &m_comm; + std::chrono::seconds m_interrupt_timeout; bool m_acquired; bool m_did_interrupt; - void SyncWithContinueThread(bool interrupt); + void SyncWithContinueThread(); }; protected: @@ -109,7 +127,7 @@ /// When was the interrupt packet sent. Used to make sure we time out if the /// stub does not respond to interrupt requests. - std::chrono::time_point<std::chrono::steady_clock> m_interrupt_time; + std::chrono::time_point<std::chrono::steady_clock> m_interrupt_endpoint; /// Number of threads interested in sending. uint32_t m_async_count; Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp @@ -20,7 +20,10 @@ using namespace lldb_private::process_gdb_remote; using namespace std::chrono; -static const seconds kInterruptTimeout(5); +// When we've sent a continue packet and are waiting for the target to stop, +// we wake up the wait with this interval to make sure the stub hasn't gone +// away while we were waiting. +static const seconds kWakeupInterval(5); ///////////////////////// // GDBRemoteClientBase // @@ -35,7 +38,8 @@ StateType GDBRemoteClientBase::SendContinuePacketAndWaitForResponse( ContinueDelegate &delegate, const UnixSignals &signals, - llvm::StringRef payload, StringExtractorGDBRemote &response) { + llvm::StringRef payload, std::chrono::seconds interrupt_timeout, + StringExtractorGDBRemote &response) { Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); response.Clear(); @@ -48,16 +52,34 @@ if (!cont_lock) return eStateInvalid; OnRunPacketSent(true); - + // The main ReadPacket loop wakes up at computed_timeout intervals, just to + // check that the connection hasn't dropped. When we wake up we also check + // whether there is an interrupt request that has reached its endpoint. + // If we want a shorter interrupt timeout that kWakeupInterval, we need to + // choose the shorter interval for the wake up as well. + std::chrono::seconds computed_timeout = std::min(interrupt_timeout, + kWakeupInterval); for (;;) { - PacketResult read_result = ReadPacket(response, kInterruptTimeout, false); + PacketResult read_result = ReadPacket(response, computed_timeout, false); switch (read_result) { case PacketResult::ErrorReplyTimeout: { std::lock_guard<std::mutex> lock(m_mutex); - if (m_async_count == 0) + if (m_async_count == 0) { continue; - if (steady_clock::now() >= m_interrupt_time + kInterruptTimeout) + } + auto cur_time = steady_clock::now(); + if (cur_time >= m_interrupt_endpoint) return eStateInvalid; + else { + // We woke up and found an interrupt is in flight, but we haven't + // exceeded the interrupt wait time. So reset the wait time to the + // time left till the interrupt timeout. But don't wait longer + // than our wakeup timeout. + auto new_wait = m_interrupt_endpoint - cur_time; + computed_timeout = std::min(kWakeupInterval, + std::chrono::duration_cast<std::chrono::seconds>(new_wait)); + continue; + } break; } case PacketResult::Success: @@ -133,8 +155,9 @@ } } -bool GDBRemoteClientBase::SendAsyncSignal(int signo) { - Lock lock(*this, true); +bool GDBRemoteClientBase::SendAsyncSignal( + int signo, std::chrono::seconds interrupt_timeout) { + Lock lock(*this, interrupt_timeout); if (!lock || !lock.DidInterrupt()) return false; @@ -144,25 +167,26 @@ return true; } -bool GDBRemoteClientBase::Interrupt() { - Lock lock(*this, true); +bool GDBRemoteClientBase::Interrupt(std::chrono::seconds interrupt_timeout) { + Lock lock(*this, interrupt_timeout); if (!lock.DidInterrupt()) return false; m_should_stop = true; return true; } + GDBRemoteCommunication::PacketResult GDBRemoteClientBase::SendPacketAndWaitForResponse( llvm::StringRef payload, StringExtractorGDBRemote &response, - bool send_async) { - Lock lock(*this, send_async); + std::chrono::seconds interrupt_timeout) { + Lock lock(*this, interrupt_timeout); if (!lock) { if (Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)) LLDB_LOGF(log, "GDBRemoteClientBase::%s failed to get mutex, not sending " - "packet '%.*s' (send_async=%d)", - __FUNCTION__, int(payload.size()), payload.data(), send_async); + "packet '%.*s'", + __FUNCTION__, int(payload.size()), payload.data()); return PacketResult::ErrorSendFailed; } @@ -172,16 +196,16 @@ GDBRemoteCommunication::PacketResult GDBRemoteClientBase::SendPacketAndReceiveResponseWithOutputSupport( llvm::StringRef payload, StringExtractorGDBRemote &response, - bool send_async, + std::chrono::seconds interrupt_timeout, llvm::function_ref<void(llvm::StringRef)> output_callback) { - Lock lock(*this, send_async); + Lock lock(*this, interrupt_timeout); if (!lock) { if (Log *log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)) LLDB_LOGF(log, "GDBRemoteClientBase::%s failed to get mutex, not sending " - "packet '%.*s' (send_async=%d)", - __FUNCTION__, int(payload.size()), payload.data(), send_async); + "packet '%.*s'", + __FUNCTION__, int(payload.size()), payload.data()); return PacketResult::ErrorSendFailed; } @@ -222,13 +246,14 @@ return packet_result; } -bool GDBRemoteClientBase::SendvContPacket(llvm::StringRef payload, - StringExtractorGDBRemote &response) { +bool GDBRemoteClientBase::SendvContPacket( + llvm::StringRef payload, std::chrono::seconds interrupt_timeout, + StringExtractorGDBRemote &response) { Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); LLDB_LOGF(log, "GDBRemoteCommunicationClient::%s ()", __FUNCTION__); // we want to lock down packet sending while we continue - Lock lock(*this, true); + Lock lock(*this, interrupt_timeout); LLDB_LOGF(log, "GDBRemoteCommunicationClient::%s () sending vCont packet: %.*s", @@ -336,18 +361,20 @@ // GDBRemoteClientBase::Lock // /////////////////////////////// -GDBRemoteClientBase::Lock::Lock(GDBRemoteClientBase &comm, bool interrupt) +GDBRemoteClientBase::Lock::Lock(GDBRemoteClientBase &comm, + std::chrono::seconds interrupt_timeout) : m_async_lock(comm.m_async_mutex, std::defer_lock), m_comm(comm), - m_acquired(false), m_did_interrupt(false) { - SyncWithContinueThread(interrupt); + m_interrupt_timeout(interrupt_timeout), m_acquired(false), + m_did_interrupt(false) { + SyncWithContinueThread(); if (m_acquired) m_async_lock.lock(); } -void GDBRemoteClientBase::Lock::SyncWithContinueThread(bool interrupt) { +void GDBRemoteClientBase::Lock::SyncWithContinueThread() { Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)); std::unique_lock<std::mutex> lock(m_comm.m_mutex); - if (m_comm.m_is_running && !interrupt) + if (m_comm.m_is_running && m_interrupt_timeout == std::chrono::seconds(0)) return; // We were asked to avoid interrupting the sender. Lock is not // acquired. @@ -365,9 +392,9 @@ "interrupt packet"); return; } + m_comm.m_interrupt_endpoint = steady_clock::now() + m_interrupt_timeout; if (log) log->PutCString("GDBRemoteClientBase::Lock::Lock sent packet: \\x03"); - m_comm.m_interrupt_time = steady_clock::now(); } m_comm.m_cv.wait(lock, [this] { return !m_comm.m_is_running; }); m_did_interrupt = true; Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp =================================================================== --- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -740,8 +740,8 @@ m_remote_signals_sp = UnixSignals::Create(GetRemoteSystemArchitecture()); StringExtractorGDBRemote response; - auto result = m_gdb_client.SendPacketAndWaitForResponse("jSignalsInfo", - response, false); + auto result = + m_gdb_client.SendPacketAndWaitForResponse("jSignalsInfo", response); if (result != decltype(result)::Success || response.GetResponseType() != response.eResponse) Index: lldb/include/lldb/Target/Process.h =================================================================== --- lldb/include/lldb/Target/Process.h +++ lldb/include/lldb/Target/Process.h @@ -94,6 +94,7 @@ bool GetWarningsUnsupportedLanguage() const; bool GetStopOnExec() const; std::chrono::seconds GetUtilityExpressionTimeout() const; + std::chrono::seconds GetInterruptTimeout() const; bool GetOSPluginReportsAllThreads() const; void SetOSPluginReportsAllThreads(bool does_report); bool GetSteppingRunsAllThreads() const;
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits