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 <jo...@devlieghere.com> Date: Wed, 12 Mar 2025 22:25:09 -0700 Subject: [PATCH 1/3] [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 <jo...@devlieghere.com> Date: Thu, 13 Mar 2025 11:22:29 -0700 Subject: [PATCH 2/3] 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 >From 05c5ba32af4737cdb2c3f78b9721b3641a0fbe85 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere <jo...@devlieghere.com> Date: Thu, 13 Mar 2025 13:58:07 -0700 Subject: [PATCH 3/3] Fix formatting --- lldb/tools/debugserver/source/RNBRemote.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index 02d1544c93191..c225ac1667b54 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -1122,17 +1122,16 @@ void RNBRemote::CommDataReceived(const std::string &new_data) { // 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()); + 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]); + 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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits