[Lldb-commits] [lldb] [debugserver] Fix mutex scope in RNBRemote::CommDataReceived (PR #131077)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 13 11:23:12 PDT 2025
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/131077
>From 4d27dc4928cfd57b2c2bc8b3059d2cb773cfc7ca Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Wed, 12 Mar 2025 22:25:09 -0700
Subject: [PATCH 1/2] [debugserver] Fix mutex scope in
RNBRemote::CommDataReceived
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.
---
lldb/tools/debugserver/source/RNBRemote.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index 8a53094429aba..dd6da3db11814 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -1054,7 +1054,7 @@ 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);
@@ -1132,7 +1132,6 @@ void RNBRemote::CommDataReceived(const std::string &new_data) {
__FUNCTION__, data[idx]);
idx = idx + 1;
}
- }
}
if (!m_rx_packets.empty()) {
>From 40fd7756b8a6b467b6a7397706c718c3ddbf12d0 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Thu, 13 Mar 2025 11:22:29 -0700
Subject: [PATCH 2/2] Fix indentation
---
lldb/tools/debugserver/source/RNBRemote.cpp | 146 ++++++++++----------
1 file changed, 73 insertions(+), 73 deletions(-)
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index dd6da3db11814..02d1544c93191 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -1055,84 +1055,84 @@ 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;
+ }
+}
if (!m_rx_packets.empty()) {
// Let the main thread know we have received a packet
More information about the lldb-commits
mailing list