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

llvm-svn: 238323
This commit is contained in:
Ewan Crawford 2015-05-27 14:12:34 +00:00
parent 86c7b46680
commit 9aa2da0025
4 changed files with 62 additions and 18 deletions

View File

@ -329,7 +329,7 @@ GDBRemoteCommunication::WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtrac
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::WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtrac
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::WaitForPacketWithTimeoutMicroSecondsNoLock (StringExtrac
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 (const uint8_t *src, size_t src_len, Stri
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 (const uint8_t *src, size_t src_len, Stri
break;
case '%': // Async notify packet
isNotifyPacket = true;
// Intentional fall through
case '$':
// Look for a standard gdb packet?
{
@ -487,7 +492,7 @@ GDBRemoteCommunication::CheckForPacket (const uint8_t *src, size_t src_len, Stri
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 (const uint8_t *src, size_t src_len, Stri
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

View File

@ -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);

View File

@ -374,7 +374,6 @@ ProcessGDBRemote::ProcessGDBRemote(Target& target, Listener &listener) :
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 (Stream *strm, const char *remote_url)
// 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 (Stream *strm, const char *remote_url)
}
}
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_module, ProcessLaunchInfo &launch_info)
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_module, ProcessLaunchInfo &launch_info)
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 (const StringExtractorGDBRemote &response)
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);
}
}

View File

@ -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;