[Lldb-commits] [lldb] 998511c - [debugserver] Fix mutex scope in RNBRemote::CommDataReceived (#131077)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 13 15:12:22 PDT 2025
Author: Jonas Devlieghere
Date: 2025-03-13T15:12:19-07:00
New Revision: 998511c8ef577f330c90bf32db37d5f8305c53f3
URL: https://github.com/llvm/llvm-project/commit/998511c8ef577f330c90bf32db37d5f8305c53f3
DIFF: https://github.com/llvm/llvm-project/commit/998511c8ef577f330c90bf32db37d5f8305c53f3.diff
LOG: [debugserver] Fix mutex scope in RNBRemote::CommDataReceived (#131077)
The mutex in RNBRemote::CommDataReceived protects m_rx_packets and
should extend to the end of the function to cover the read where we
check if the list is empty.
Added:
Modified:
lldb/tools/debugserver/source/RNBRemote.cpp
Removed:
################################################################################
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index 8a53094429aba..c225ac1667b54 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -1054,84 +1054,82 @@ rnb_err_t RNBRemote::HandleReceivedPacket(PacketEnum *type) {
void RNBRemote::CommDataReceived(const std::string &new_data) {
// DNBLogThreadedIf (LOG_RNB_REMOTE, "%8d RNBRemote::%s called",
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__);
- {
- // Put the packet data into the buffer in a thread safe fashion
- PThreadMutex::Locker locker(m_mutex);
-
- std::string data;
- // See if we have any left over data from a previous call to this
- // function?
- if (!m_rx_partial_data.empty()) {
- // We do, so lets start with that data
- data.swap(m_rx_partial_data);
- }
- // Append the new incoming data
- data += new_data;
-
- // Parse up the packets into gdb remote packets
- size_t idx = 0;
- const size_t data_size = data.size();
-
- while (idx < data_size) {
- // end_idx must be one past the last valid packet byte. Start
- // it off with an invalid value that is the same as the current
- // index.
- size_t end_idx = idx;
-
- switch (data[idx]) {
- case '+': // Look for ack
- case '-': // Look for cancel
- case '\x03': // ^C to halt target
- end_idx = idx + 1; // The command is one byte long...
- break;
- case '$':
- // Look for a standard gdb packet?
- end_idx = data.find('#', idx + 1);
- if (end_idx == std::string::npos || end_idx + 3 > data_size) {
- end_idx = std::string::npos;
- } else {
- // Add two for the checksum bytes and 1 to point to the
- // byte just past the end of this packet
- end_idx += 3;
- }
- break;
+ // Put the packet data into the buffer in a thread safe fashion
+ PThreadMutex::Locker locker(m_mutex);
+
+ std::string data;
+ // See if we have any left over data from a previous call to this
+ // function?
+ if (!m_rx_partial_data.empty()) {
+ // We do, so lets start with that data
+ data.swap(m_rx_partial_data);
+ }
+ // Append the new incoming data
+ data += new_data;
+
+ // Parse up the packets into gdb remote packets
+ size_t idx = 0;
+ const size_t data_size = data.size();
+
+ while (idx < data_size) {
+ // end_idx must be one past the last valid packet byte. Start
+ // it off with an invalid value that is the same as the current
+ // index.
+ size_t end_idx = idx;
+
+ switch (data[idx]) {
+ case '+': // Look for ack
+ case '-': // Look for cancel
+ case '\x03': // ^C to halt target
+ end_idx = idx + 1; // The command is one byte long...
+ break;
- default:
- break;
+ case '$':
+ // Look for a standard gdb packet?
+ end_idx = data.find('#', idx + 1);
+ if (end_idx == std::string::npos || end_idx + 3 > data_size) {
+ end_idx = std::string::npos;
+ } else {
+ // Add two for the checksum bytes and 1 to point to the
+ // byte just past the end of this packet
+ end_idx += 3;
}
+ break;
- if (end_idx == std::string::npos) {
- // Not all data may be here for the packet yet, save it for
- // next time through this function.
- m_rx_partial_data += data.substr(idx);
- // DNBLogThreadedIf (LOG_RNB_MAX, "%8d RNBRemote::%s saving data for
- // later[%u, npos):
- // '%s'",(uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
- // __FUNCTION__, idx, m_rx_partial_data.c_str());
- idx = end_idx;
- } else if (idx < end_idx) {
- m_packets_recvd++;
- // Hack to get rid of initial '+' ACK???
- if (m_packets_recvd == 1 && (end_idx == idx + 1) && data[idx] == '+') {
- // DNBLogThreadedIf (LOG_RNB_REMOTE, "%8d RNBRemote::%s throwing first
- // ACK away....[%u, npos):
- // '+'",(uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
- // __FUNCTION__, idx);
- } else {
- // We have a valid packet...
- m_rx_packets.push_back(data.substr(idx, end_idx - idx));
- DNBLogThreadedIf(LOG_RNB_PACKETS, "getpkt: %s",
- m_rx_packets.back().c_str());
- }
- idx = end_idx;
+ default:
+ break;
+ }
+
+ if (end_idx == std::string::npos) {
+ // Not all data may be here for the packet yet, save it for
+ // next time through this function.
+ m_rx_partial_data += data.substr(idx);
+ // DNBLogThreadedIf (LOG_RNB_MAX, "%8d RNBRemote::%s saving data for
+ // later[%u, npos):
+ // '%s'",(uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
+ // __FUNCTION__, idx, m_rx_partial_data.c_str());
+ idx = end_idx;
+ } else if (idx < end_idx) {
+ m_packets_recvd++;
+ // Hack to get rid of initial '+' ACK???
+ if (m_packets_recvd == 1 && (end_idx == idx + 1) && data[idx] == '+') {
+ // DNBLogThreadedIf (LOG_RNB_REMOTE, "%8d RNBRemote::%s throwing first
+ // ACK away....[%u, npos):
+ // '+'",(uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
+ // __FUNCTION__, idx);
} else {
- DNBLogThreadedIf(LOG_RNB_MAX,
- "%8d RNBRemote::%s tossing junk byte at %c",
- (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
- __FUNCTION__, data[idx]);
- idx = idx + 1;
+ // We have a valid packet...
+ m_rx_packets.push_back(data.substr(idx, end_idx - idx));
+ DNBLogThreadedIf(LOG_RNB_PACKETS, "getpkt: %s",
+ m_rx_packets.back().c_str());
}
+ idx = end_idx;
+ } else {
+ DNBLogThreadedIf(LOG_RNB_MAX, "%8d RNBRemote::%s tossing junk byte at %c",
+ (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true),
+ __FUNCTION__, data[idx]);
+ idx = idx + 1;
}
}
More information about the lldb-commits
mailing list