augusto2112 updated this revision to Diff 329385. augusto2112 marked 7 inline comments as done and an inline comment as not done. augusto2112 added a comment.
Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96176/new/ https://reviews.llvm.org/D96176 Files: lldb/include/lldb/Target/Process.h lldb/include/lldb/Utility/StringExtractorGDBRemote.h lldb/include/lldb/lldb-enumerations.h lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Commands/Options.td lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Utility/StringExtractorGDBRemote.cpp
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp =================================================================== --- lldb/source/Utility/StringExtractorGDBRemote.cpp +++ lldb/source/Utility/StringExtractorGDBRemote.cpp @@ -200,7 +200,10 @@ if (PACKET_MATCHES("qHostInfo")) return eServerPacketType_qHostInfo; break; - + case 'J': + if (PACKET_MATCHES("qJAttachSupported")) + return eServerPacketType_qJAttachSupported; + break; case 'K': if (PACKET_STARTS_WITH("qKillSpawnedProcess")) return eServerPacketType_qKillSpawnedProcess; @@ -294,6 +297,8 @@ break; case 'j': + if (PACKET_STARTS_WITH("jAttach:")) + return eServerPacketType_jAttach; if (PACKET_STARTS_WITH("jModulesInfo:")) return eServerPacketType_jModulesInfo; if (PACKET_MATCHES("jSignalsInfo")) 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 @@ -1160,41 +1160,72 @@ return error; } +static StreamGDBRemote MakeJAttachPacket(const char *process_name, + const ProcessAttachInfo &attach_info) { + + StreamString json_string; + json_string.PutCString("jAttach:"); + + StructuredData::Dictionary json_packet; + json_packet.AddStringItem("process_name", process_name); + + if (auto interval = attach_info.GetWaitForLaunchInterval()) + json_packet.AddIntegerItem("waitfor-interval-usec", interval->count()); + + if (auto duration = attach_info.GetWaitForLaunchDuration()) + json_packet.AddIntegerItem("waitfor-duration-sec", duration->count()); + + json_packet.AddBooleanItem("include-existing", + !attach_info.GetIgnoreExisting()); + + json_packet.Dump(json_string, false); + StreamGDBRemote escaped_packet; + escaped_packet.PutEscapedBytes(json_string.GetData(), json_string.GetSize()); + return escaped_packet; +} + Status ProcessGDBRemote::DoAttachToProcessWithName( const char *process_name, const ProcessAttachInfo &attach_info) { Status error; // Clear out and clean up from any current state Clear(); - if (process_name && process_name[0]) { - error = EstablishConnectionIfNeeded(attach_info); - if (error.Success()) { - StreamString packet; + if (!process_name || !process_name[0]) + return error; - m_gdb_comm.SetDetachOnError(attach_info.GetDetachOnError()); + error = EstablishConnectionIfNeeded(attach_info); + if (!error.Success()) { + SetExitStatus(-1, error.AsCString()); + return error; + } - if (attach_info.GetWaitForLaunch()) { - if (!m_gdb_comm.GetVAttachOrWaitSupported()) { + m_gdb_comm.SetDetachOnError(attach_info.GetDetachOnError()); + if (attach_info.GetWaitForLaunch() && m_gdb_comm.GetJAttachSupported()) { + auto gdb_stream = MakeJAttachPacket(process_name, attach_info); + m_async_broadcaster.BroadcastEvent( + eBroadcastBitAsyncContinue, + new EventDataBytes(gdb_stream.GetString().data(), + gdb_stream.GetSize())); + } else { + StreamString packet; + if (attach_info.GetWaitForLaunch()) { + if (!m_gdb_comm.GetVAttachOrWaitSupported()) { + packet.PutCString("vAttachWait"); + } else { + if (attach_info.GetIgnoreExisting()) packet.PutCString("vAttachWait"); - } else { - if (attach_info.GetIgnoreExisting()) - packet.PutCString("vAttachWait"); - else - packet.PutCString("vAttachOrWait"); - } - } else - packet.PutCString("vAttachName"); - packet.PutChar(';'); - packet.PutBytesAsRawHex8(process_name, strlen(process_name), - endian::InlHostByteOrder(), - endian::InlHostByteOrder()); - - m_async_broadcaster.BroadcastEvent( - eBroadcastBitAsyncContinue, - new EventDataBytes(packet.GetString().data(), packet.GetSize())); - + else + packet.PutCString("vAttachOrWait"); + } } else - SetExitStatus(-1, error.AsCString()); + packet.PutCString("vAttachName"); + packet.PutChar(';'); + packet.PutBytesAsRawHex8(process_name, strlen(process_name), + endian::InlHostByteOrder(), + endian::InlHostByteOrder()); + m_async_broadcaster.BroadcastEvent( + eBroadcastBitAsyncContinue, + new EventDataBytes(packet.GetString().data(), packet.GetSize())); } return error; } Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h @@ -65,10 +65,26 @@ /// with a given name and attaching llgs to that via the configured /// Platform. /// + /// \param process_name + /// The process we'd like to attach to. + /// \param include_existing + /// Look for the process we'd like to attach to among the processes + /// already running. + /// \param custom_polling_interval + /// The polling interval the function should use in microseconds. Defaults + /// to 1000. + /// \param timeout + /// How long we should look for the process in seconds. Defaults to + /// infinity. + /// /// \return /// An Status object indicating the success or failure of the /// attach operation. - Status AttachWaitProcess(llvm::StringRef process_name, bool include_existing); + Status AttachWaitProcess( + llvm::StringRef process_name, bool include_existing, + llvm::Optional<std::chrono::microseconds> custom_polling_interval = + llvm::None, + llvm::Optional<std::chrono::seconds> timeout = llvm::None); // NativeProcessProtocol::NativeDelegate overrides void InitializeDelegate(NativeProcessProtocol *process) override; @@ -187,6 +203,10 @@ PacketResult Handle_vAttachOrWait(StringExtractorGDBRemote &packet); + PacketResult Handle_qJAttachSupported(StringExtractorGDBRemote &packet); + + PacketResult Handle_jAttach(StringExtractorGDBRemote &packet); + PacketResult Handle_D(StringExtractorGDBRemote &packet); PacketResult Handle_qThreadStopInfo(StringExtractorGDBRemote &packet); Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -168,6 +168,12 @@ RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_vAttachOrWait, &GDBRemoteCommunicationServerLLGS::Handle_vAttachOrWait); + RegisterMemberFunctionHandler( + StringExtractorGDBRemote::eServerPacketType_qJAttachSupported, + &GDBRemoteCommunicationServerLLGS::Handle_qJAttachSupported); + RegisterMemberFunctionHandler( + StringExtractorGDBRemote::eServerPacketType_jAttach, + &GDBRemoteCommunicationServerLLGS::Handle_jAttach); RegisterMemberFunctionHandler( StringExtractorGDBRemote::eServerPacketType_vCont, &GDBRemoteCommunicationServerLLGS::Handle_vCont); @@ -344,10 +350,13 @@ } Status GDBRemoteCommunicationServerLLGS::AttachWaitProcess( - llvm::StringRef process_name, bool include_existing) { + llvm::StringRef process_name, bool include_existing, + llvm::Optional<std::chrono::microseconds> custom_polling_interval, + llvm::Optional<std::chrono::seconds> timeout) { Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS)); - std::chrono::milliseconds polling_interval = std::chrono::milliseconds(1); + auto polling_interval = + custom_polling_interval.getValueOr(std::chrono::microseconds(1000)); // Create the matcher used to search the process list. ProcessInstanceInfoList exclusion_list; @@ -376,8 +385,13 @@ return false; }; + auto target = std::chrono::steady_clock::now(); + if (timeout.hasValue()) + target = std::chrono::steady_clock::now() + + std::chrono::seconds(timeout.getValue()); + ProcessInstanceInfoList loop_process_list; - while (true) { + do { loop_process_list.clear(); if (Host::FindProcesses(match_info, loop_process_list)) { // Remove all the elements that are in the exclusion list. @@ -407,9 +421,14 @@ } } // No matches, we have not found the process. Sleep until next poll. - LLDB_LOG(log, "sleep {0} seconds", polling_interval); + LLDB_LOG(log, "sleep for {0}", polling_interval); std::this_thread::sleep_for(polling_interval); - } + } while (!timeout.hasValue() || std::chrono::steady_clock::now() < target); + + // Timed out + Status error; + error.SetErrorString("attach timed out."); + return error; } void GDBRemoteCommunicationServerLLGS::InitializeDelegate( @@ -3332,6 +3351,55 @@ return SendStopReasonForState(m_debugged_process_up->GetState()); } +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::Handle_qJAttachSupported( + StringExtractorGDBRemote &packet) { + return SendOKResponse(); +} + +GDBRemoteCommunication::PacketResult +GDBRemoteCommunicationServerLLGS::Handle_jAttach( + StringExtractorGDBRemote &packet) { + Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS)); + + if (!packet.ConsumeFront("jAttach:")) + return SendIllFormedResponse(packet, "jAttach: Ill formed packet "); + + auto json_object = StructuredData::ParseJSON(packet.Peek()); + + if (!json_object || + json_object->GetType() != lldb::eStructuredDataTypeDictionary) + return SendIllFormedResponse(packet, "jAttach: Ill formed packet "); + + auto *json_dict = json_object->GetAsDictionary(); + + StringRef process_name; + llvm::Optional<std::chrono::microseconds> polling_interval; + llvm::Optional<std::chrono::seconds> polling_duration; + bool include_existing = false; + uint64_t temp; + + json_dict->GetValueForKeyAsString("process_name", process_name); + json_dict->GetValueForKeyAsBoolean("include-existing", include_existing); + if (json_dict->GetValueForKeyAsInteger("waitfor-interval-usec", temp)) + polling_interval = std::chrono::microseconds(temp); + if (json_dict->GetValueForKeyAsInteger("waitfor-duration-sec", temp)) + polling_duration = std::chrono::seconds(temp); + + LLDB_LOG(log, "attempting to attach to process named '{0}'", process_name); + + Status error = AttachWaitProcess(process_name, include_existing, + polling_interval, polling_duration); + if (error.Fail()) { + LLDB_LOG(log, "failed to attach to process named '{0}': {1}", process_name, + error); + return SendErrorResponse(error); + } + + // Notify we attached by sending a stop packet. + return SendStopReasonForState(m_debugged_process_up->GetState()); +} + GDBRemoteCommunication::PacketResult GDBRemoteCommunicationServerLLGS::Handle_D(StringExtractorGDBRemote &packet) { Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS)); 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 @@ -257,6 +257,8 @@ bool GetVAttachOrWaitSupported(); + bool GetJAttachSupported(); + bool GetSyncThreadStateSupported(); void ResetDiscoverableSettings(bool did_exec); @@ -542,6 +544,7 @@ LazyBool m_supports_detach_stay_stopped; LazyBool m_watchpoints_trigger_after_instruction; LazyBool m_attach_or_wait_reply; + LazyBool m_j_attach_wait_reply; LazyBool m_prepare_for_reg_writing_reply; LazyBool m_supports_p; LazyBool m_supports_x; 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 @@ -74,6 +74,7 @@ m_supports_detach_stay_stopped(eLazyBoolCalculate), m_watchpoints_trigger_after_instruction(eLazyBoolCalculate), m_attach_or_wait_reply(eLazyBoolCalculate), + m_j_attach_wait_reply(eLazyBoolCalculate), m_prepare_for_reg_writing_reply(eLazyBoolCalculate), m_supports_p(eLazyBoolCalculate), m_supports_x(eLazyBoolCalculate), m_avoid_g_packets(eLazyBoolCalculate), @@ -257,6 +258,20 @@ return m_attach_or_wait_reply == eLazyBoolYes; } +bool GDBRemoteCommunicationClient::GetJAttachSupported() { + if (m_j_attach_wait_reply == eLazyBoolCalculate) { + m_j_attach_wait_reply = eLazyBoolNo; + + StringExtractorGDBRemote response; + if (SendPacketAndWaitForResponse("qJAttachSupported", response, false) == + PacketResult::Success) { + if (response.IsOKResponse()) + m_j_attach_wait_reply = eLazyBoolYes; + } + } + return m_j_attach_wait_reply == eLazyBoolYes; +} + bool GDBRemoteCommunicationClient::GetSyncThreadStateSupported() { if (m_prepare_for_reg_writing_reply == eLazyBoolCalculate) { m_prepare_for_reg_writing_reply = eLazyBoolNo; @@ -291,6 +306,7 @@ m_supports_memory_region_info = eLazyBoolCalculate; m_prepare_for_reg_writing_reply = eLazyBoolCalculate; m_attach_or_wait_reply = eLazyBoolCalculate; + m_j_attach_wait_reply = eLazyBoolCalculate; m_avoid_g_packets = eLazyBoolCalculate; m_supports_qXfer_auxv_read = eLazyBoolCalculate; m_supports_qXfer_libraries_read = eLazyBoolCalculate; Index: lldb/source/Commands/Options.td =================================================================== --- lldb/source/Commands/Options.td +++ lldb/source/Commands/Options.td @@ -688,6 +688,12 @@ Group<2>, Desc<"Include existing processes when doing attach -w.">; def process_attach_waitfor : Option<"waitfor", "w">, Group<2>, Desc<"Wait for the process with <process-name> to launch.">; + def process_attach_waitfor_interval : Option<"waitfor-interval", "I">, + Group<2>, Arg<"WaitforInterval">, + Desc<"The interval that waitfor uses to poll the list of processes.">; + def process_attach_waitfor_duration : Option<"waitfor-duration", "d">, + Group<2>, Arg<"WaitforDuration">, + Desc<"The interval that waitfor uses to poll the list of processes.">; } let Command = "process continue" in { Index: lldb/source/Commands/CommandObjectProcess.cpp =================================================================== --- lldb/source/Commands/CommandObjectProcess.cpp +++ lldb/source/Commands/CommandObjectProcess.cpp @@ -307,6 +307,27 @@ attach_info.SetIgnoreExisting(false); break; + case 'I': { + uint64_t interval; + if (option_arg.getAsInteger(0, interval)) { + error.SetErrorStringWithFormat("invalid polling interval '%s'", + option_arg.str().c_str()); + } else { + attach_info.SetWaitForLaunchInterval( + std::chrono::microseconds(interval)); + } + break; + } + case 'd': { + uint64_t duration; + if (option_arg.getAsInteger(0, duration)) { + error.SetErrorStringWithFormat("invalid duration '%s'", + option_arg.str().c_str()); + } else { + attach_info.SetWaitForLaunchDuration(std::chrono::seconds(duration)); + } + break; + } default: llvm_unreachable("Unimplemented option"); } Index: lldb/include/lldb/lldb-enumerations.h =================================================================== --- lldb/include/lldb/lldb-enumerations.h +++ lldb/include/lldb/lldb-enumerations.h @@ -597,6 +597,8 @@ eArgTypeCommand, eArgTypeColumnNum, eArgTypeModuleUUID, + eArgTypeWaitforInterval, + eArgTypeWaitforDuration, eArgTypeLastArg // Always keep this entry as the last entry in this // enumeration!! }; Index: lldb/include/lldb/Utility/StringExtractorGDBRemote.h =================================================================== --- lldb/include/lldb/Utility/StringExtractorGDBRemote.h +++ lldb/include/lldb/Utility/StringExtractorGDBRemote.h @@ -122,6 +122,7 @@ eServerPacketType_qThreadExtraInfo, eServerPacketType_qThreadStopInfo, eServerPacketType_qVAttachOrWaitSupported, + eServerPacketType_qJAttachSupported, eServerPacketType_qWatchpointSupportInfo, eServerPacketType_qWatchpointSupportInfoSupported, eServerPacketType_qXfer, @@ -132,6 +133,7 @@ eServerPacketType_vAttach, eServerPacketType_vAttachWait, eServerPacketType_vAttachOrWait, + eServerPacketType_jAttach, eServerPacketType_vAttachName, eServerPacketType_vCont, eServerPacketType_vCont_actions, // vCont? Index: lldb/include/lldb/Target/Process.h =================================================================== --- lldb/include/lldb/Target/Process.h +++ lldb/include/lldb/Target/Process.h @@ -130,6 +130,24 @@ void SetWaitForLaunch(bool b) { m_wait_for_launch = b; } + llvm::Optional<std::chrono::microseconds> GetWaitForLaunchInterval() const { + return m_wait_for_launch_interval; + } + + void SetWaitForLaunchInterval( + llvm::Optional<std::chrono::microseconds> new_interval) { + m_wait_for_launch_interval = new_interval; + } + + llvm::Optional<std::chrono::seconds> GetWaitForLaunchDuration() const { + return m_wait_for_launch_duration; + } + + void + SetWaitForLaunchDuration(llvm::Optional<std::chrono::seconds> new_duration) { + m_wait_for_launch_duration = new_duration; + } + bool GetAsync() const { return m_async; } void SetAsync(bool b) { m_async = b; } @@ -159,6 +177,8 @@ m_plugin_name.clear(); m_resume_count = 0; m_wait_for_launch = false; + m_wait_for_launch_interval = llvm::None; + m_wait_for_launch_duration = llvm::None; m_ignore_existing = true; m_continue_once_attached = false; } @@ -198,6 +218,8 @@ std::string m_plugin_name; uint32_t m_resume_count; // How many times do we resume after launching bool m_wait_for_launch; + llvm::Optional<std::chrono::microseconds> m_wait_for_launch_interval; + llvm::Optional<std::chrono::seconds> m_wait_for_launch_duration; bool m_ignore_existing; bool m_continue_once_attached; // Supports the use-case scenario of // immediately continuing the process once
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits