jingham created this revision. jingham added reviewers: clayborg, labath, JDevlieghere. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits.
This patch started as an attempt to rationalize the handling of the timeout for interrupting a running process. Process::Halt sends a 10 second timeout to WaitForProcessToStop, but that timeout didn't have any effect on how the interrupt was actually handled in ProcessGDBRemote & its GDBRemoteClient client, and there was no way to plumb the desired timeout to that layer. This patch adds an interrupt-timeout setting to process and plumbs that through. That then allowed me to write some more tests on the interrupt handling, in the course of which I found a bug in the handling of the interrupt timeout. The lowest level waiting on the remote stub doesn't block when the process is running. Instead, it passes in a 5 second wakeup timeout when it calls ReadPacket. That's there so we can detect if the connection has dropped, but in the current code it is also the de facto interrupt timeout as well. When ProcessGDBRemote wants to interrupt a running process, it sends a break to debugserver, and sets some ivars that say "interrupt is in flight" and to provide the "interrupt timepoint" to the client. When the GDBRemoteClientBase object wakes up from ReadPacket because of the timeout, it checks whether there was an interrupt in flight, and if so, next checks whether 5 seconds have elapsed since the interrupt request. If that latter test is NOT true, it is supposed to go back into ReadPacket to wait out the remaining time (*). But it flubs this part. Instead of calling "continue" to go back to top of the ReadPacket loop, it just breaks from the "result checking" switch. The next thing after the switch is a check to see if the response packet is empty, and if it is empty, we exit from the ReadPacket loop, returning eStateInvalid - the same state as if the interrupt had failed. But if we woke up before the interrupt had a chance to take effect, there won't be any response yet, and so we error out. That looks counter to the intention of the code that was checking the timeouts. It's bad because it means that if you are unlucky and happened to request the interrupt just before the ReadPacket call exceeds the timeout, you could in fact end up waiting only a very short time for the interrupt. I have lots of reports of this happening, but never reproducibly. The reports always come in contexts where lots of processes were being started and stopped quickly. For instance, Xcode runs all its test plans in the debugger when not running in a test harness, and runs the tests in parallel. So it was starting & stopping a lot of processes in parallel, and that this should occasionally cause the system to stall didn't seem altogether unreasonable. That was the motivation for adding the interrupt timeout in the first place. That still may be true, but it looks like this bug was making the chance of exceeding the timeout more likely. I still think having the timeout is valuable, however, if for no other reason than that was how I was able to write a test for the behavior that triggered this bug... (*) BTW, this is not an exact timer. We don't have a way to stop ReadPacket & readjust the wakeup time if the interrupt time happens to be shorter. And when ReadPacket wakes up and finds we have an interrupt pending, but haven't waited long enough, formally we should change our timeout to be the amount left to wait for the interrupt. I didn't change this behavior as I don't think we need to guarantee that kind of exactness. As long as we wait at least as long as the timeout requested, I think we're doing what folks want, and I don't think being more precise is worth the added complexity. Repository: rG LLVM Github Monorepo 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; @@ -72,6 +74,8 @@ } }; +std::chrono::seconds GDBRemoteClientBaseTest::g_timeout(10); + } // end anonymous namespace TEST_F(GDBRemoteClientBaseTest, SendContinueAndWait) { @@ -103,7 +107,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 +116,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 +138,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 +147,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 +181,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 +190,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 +214,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 +236,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 +247,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 +263,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 +329,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 +353,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; @@ -4045,8 +4051,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 +4123,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 +4156,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 +4922,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 +5299,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 +5350,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,44 @@ 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); + // This version sends the packet only if it doesn't have to interrupt the + // running process. PacketResult SendPacketAndWaitForResponse(llvm::StringRef payload, - StringExtractorGDBRemote &response, - bool send_async); + StringExtractorGDBRemote &response); + + // This version interrupts the target if it is running, and sends 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); 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); + // This is the constructor for use when you aren't interrupting. + Lock(GDBRemoteClientBase &comm); + // This is the one for when you are interrupting. interrupt_timeout + // is the length of time you are willing to wait for the interrupt to + // succeed. + Lock(GDBRemoteClientBase &comm, std::chrono::seconds interrupt_timeout); ~Lock(); explicit operator bool() { return m_acquired; } @@ -67,10 +82,12 @@ private: std::unique_lock<std::recursive_mutex> m_async_lock; GDBRemoteClientBase &m_comm; + std::chrono::seconds m_interrupt_timeout; bool m_acquired; + bool m_should_interrupt; bool m_did_interrupt; - void SyncWithContinueThread(bool interrupt); + void SyncWithContinueThread(); }; protected: @@ -109,7 +126,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 // @@ -50,14 +53,17 @@ OnRunPacketSent(true); for (;;) { - PacketResult read_result = ReadPacket(response, kInterruptTimeout, false); + PacketResult read_result = ReadPacket(response, kWakeupInterval, false); switch (read_result) { case PacketResult::ErrorReplyTimeout: { std::lock_guard<std::mutex> lock(m_mutex); 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 + continue; break; } case PacketResult::Success: @@ -133,8 +139,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 +151,43 @@ 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; + } + + return SendPacketAndWaitForResponseNoLock(payload, response); +} + +GDBRemoteCommunication::PacketResult +GDBRemoteClientBase::SendPacketAndWaitForResponse( + llvm::StringRef payload, StringExtractorGDBRemote &response) { + Lock lock(*this); + if (!lock) { + if (Log *log = + ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS)) + LLDB_LOGF(log, + "GDBRemoteClientBase::%s failed to get mutex, not sending " + "packet '%.*s'", + __FUNCTION__, int(payload.size()), payload.data()); return PacketResult::ErrorSendFailed; } @@ -172,16 +197,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 +247,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 +362,29 @@ // 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_interrupt_timeout(interrupt_timeout), m_acquired(false), + m_should_interrupt(true), m_did_interrupt(false) { + SyncWithContinueThread(); + if (m_acquired) + m_async_lock.lock(); +} + +GDBRemoteClientBase::Lock::Lock(GDBRemoteClientBase &comm) : 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(0), m_acquired(false), m_should_interrupt(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_should_interrupt) return; // We were asked to avoid interrupting the sender. Lock is not // acquired. @@ -365,9 +402,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