Author: ewancrawford Date: Wed May 27 09:12:34 2015 New Revision: 238323 URL: http://llvm.org/viewvc/llvm-project?rev=238323&view=rev Log: Change ProcessGDBRemote last stop packet to a container.
In ProcessGDBRemote we currently have a single packet, m_last_stop_packet, used to set the thread stop info. However in non-stop mode we can receive several stop reply packets in a sequence for different threads. As a result we need to use a container to hold them before they are processed. This patch also changes the return type of CheckPacket() so we can detect async notification packets. Reviewers: clayborg Subscribers: labath, ted, deepak2427, lldb-commits Differential Revision: http://reviews.llvm.org/D9853 Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp?rev=238323&r1=238322&r2=238323&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp Wed May 27 09:12:34 2015 @@ -329,7 +329,7 @@ GDBRemoteCommunication::WaitForPacketWit Log *log (ProcessGDBRemoteLog::GetLogIfAllCategoriesSet (GDBR_LOG_PACKETS | GDBR_LOG_VERBOSE)); // Check for a packet from our cache first without trying any reading... - if (CheckForPacket (NULL, 0, packet)) + if (CheckForPacket(NULL, 0, packet) != PacketType::Invalid) return PacketResult::Success; bool timed_out = false; @@ -349,7 +349,7 @@ GDBRemoteCommunication::WaitForPacketWit if (bytes_read > 0) { - if (CheckForPacket (buffer, bytes_read, packet)) + if (CheckForPacket(buffer, bytes_read, packet) != PacketType::Invalid) return PacketResult::Success; } else @@ -383,7 +383,7 @@ GDBRemoteCommunication::WaitForPacketWit return PacketResult::ErrorReplyFailed; } -bool +GDBRemoteCommunication::PacketType GDBRemoteCommunication::CheckForPacket (const uint8_t *src, size_t src_len, StringExtractorGDBRemote &packet) { // Put the packet data into the buffer in a thread safe fashion @@ -405,6 +405,8 @@ GDBRemoteCommunication::CheckForPacket ( m_bytes.append ((const char *)src, src_len); } + bool isNotifyPacket = false; + // Parse up the packets into gdb remote packets if (!m_bytes.empty()) { @@ -425,6 +427,9 @@ GDBRemoteCommunication::CheckForPacket ( break; case '%': // Async notify packet + isNotifyPacket = true; + // Intentional fall through + case '$': // Look for a standard gdb packet? { @@ -487,7 +492,7 @@ GDBRemoteCommunication::CheckForPacket ( if (content_length == std::string::npos) { packet.Clear(); - return false; + return GDBRemoteCommunication::PacketType::Invalid; } else if (total_length > 0) { @@ -626,11 +631,15 @@ GDBRemoteCommunication::CheckForPacket ( m_bytes.erase(0, total_length); packet.SetFilePos(0); - return success; + + if (isNotifyPacket) + return GDBRemoteCommunication::PacketType::Notify; + else + return GDBRemoteCommunication::PacketType::Standard; } } packet.Clear(); - return false; + return GDBRemoteCommunication::PacketType::Invalid; } Error Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h?rev=238323&r1=238322&r2=238323&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h Wed May 27 09:12:34 2015 @@ -49,7 +49,14 @@ public: { eBroadcastBitRunPacketSent = kLoUserBroadcastBit }; - + + enum class PacketType + { + Invalid = 0, + Standard, + Notify + }; + enum class PacketResult { Success = 0, // Success @@ -101,7 +108,7 @@ public: bool GetSequenceMutex (Mutex::Locker& locker, const char *failure_message = NULL); - bool + PacketType CheckForPacket (const uint8_t *src, size_t src_len, StringExtractorGDBRemote &packet); Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp?rev=238323&r1=238322&r2=238323&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Wed May 27 09:12:34 2015 @@ -374,7 +374,6 @@ ProcessGDBRemote::ProcessGDBRemote(Targe m_flags (0), m_gdb_comm (), m_debugserver_pid (LLDB_INVALID_PROCESS_ID), - m_last_stop_packet (), m_last_stop_packet_mutex (Mutex::eMutexTypeNormal), m_register_info (), m_async_broadcaster (NULL, "lldb.process.gdb-remote.async-broadcaster"), @@ -769,8 +768,10 @@ ProcessGDBRemote::DoConnectRemote (Strea // We have a valid process SetID (pid); GetThreadList(); - if (m_gdb_comm.GetStopReply(m_last_stop_packet)) + StringExtractorGDBRemote response; + if (m_gdb_comm.GetStopReply(response)) { + SetLastStopPacket(response); // '?' Packets must be handled differently in non-stop mode if (GetTarget().GetNonStopModeEnabled()) @@ -788,7 +789,7 @@ ProcessGDBRemote::DoConnectRemote (Strea } } - const StateType state = SetThreadStopInfo (m_last_stop_packet); + const StateType state = SetThreadStopInfo (response); if (state == eStateStopped) { SetPrivateState (state); @@ -1057,9 +1058,10 @@ ProcessGDBRemote::DoLaunch (Module *exe_ return error; } - if (m_gdb_comm.GetStopReply(m_last_stop_packet)) + StringExtractorGDBRemote response; + if (m_gdb_comm.GetStopReply(response)) { - + SetLastStopPacket(response); // '?' Packets must be handled differently in non-stop mode if (GetTarget().GetNonStopModeEnabled()) HandleStopReplySequence(); @@ -1077,7 +1079,7 @@ ProcessGDBRemote::DoLaunch (Module *exe_ m_target.MergeArchitecture(host_arch); } - SetPrivateState (SetThreadStopInfo (m_last_stop_packet)); + SetPrivateState (SetThreadStopInfo (response)); if (!disable_stdio) { @@ -2123,7 +2125,25 @@ ProcessGDBRemote::RefreshStateAfterStop // Set the thread stop info. It might have a "threads" key whose value is // a list of all thread IDs in the current process, so m_thread_ids might // get set. - SetThreadStopInfo (m_last_stop_packet); + + // Scope for the lock + { + // Lock the thread stack while we access it + Mutex::Locker stop_stack_lock(m_last_stop_packet_mutex); + // Get the number of stop packets on the stack + int nItems = m_stop_packet_stack.size(); + // Iterate over them + for (int i = 0; i < nItems; i++) + { + // Get the thread stop info + StringExtractorGDBRemote stop_info = m_stop_packet_stack[i]; + // Process thread stop info + SetThreadStopInfo(stop_info); + } + // Clear the thread stop stack + m_stop_packet_stack.clear(); + } + // Check to see if SetThreadStopInfo() filled in m_thread_ids? if (m_thread_ids.empty()) { @@ -2395,7 +2415,6 @@ ProcessGDBRemote::DoDestroy () void ProcessGDBRemote::SetLastStopPacket (const StringExtractorGDBRemote &response) { - Mutex::Locker locker (m_last_stop_packet_mutex); const bool did_exec = response.GetStringRef().find(";reason:exec;") != std::string::npos; if (did_exec) { @@ -2408,7 +2427,16 @@ ProcessGDBRemote::SetLastStopPacket (con BuildDynamicRegisterInfo (true); m_gdb_comm.ResetDiscoverableSettings(); } - m_last_stop_packet = response; + + // Scope the lock + { + // Lock the thread stack while we access it + Mutex::Locker stop_stack_lock(m_last_stop_packet_mutex); + // Add this stop packet to the stop packet stack + // This stack will get popped and examined when we switch to the + // Stopped state + m_stop_packet_stack.push_back(response); + } } Modified: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h?rev=238323&r1=238322&r2=238323&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (original) +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h Wed May 27 09:12:34 2015 @@ -333,7 +333,7 @@ protected: Flags m_flags; // Process specific flags (see eFlags enums) GDBRemoteCommunicationClient m_gdb_comm; std::atomic<lldb::pid_t> m_debugserver_pid; - StringExtractorGDBRemote m_last_stop_packet; + std::vector<StringExtractorGDBRemote> m_stop_packet_stack; // The stop packet stack replaces the last stop packet variable Mutex m_last_stop_packet_mutex; GDBRemoteDynamicRegisterInfo m_register_info; Broadcaster m_async_broadcaster; _______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
