[lldb] Parse qSupported MultiMemRead tag in GDB Remote Client

[felipepiovezan]

This is in preparation for the new MultiMemRead packet discussed in the RFC 1.

An alternative to using qSupported would be having clients send an empty MultiMemRead packet. However, this is problematic because the already-existing packet M is a prefix of MultiMemRead; an empty reply would be ambiguous in this case. It is also risky that the stub might interpret the MultiMemRead as a valid M packet.

Another advantage of qSupported is that this packet is already exchanged, so parsing a new field is simpler than having to exchange one extra packet.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

This is in preparation for the new MultiMemRead packet discussed in the RFC 1.

---

Full diff: https://github.com/llvm/llvm-project/pull/163249.diff

3 Files Affected:

• (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+10)
• (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (+3)
• (modified) lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp (+11)

diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 7d2bd452acca9..11f164c2426ce 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -211,6 +211,12 @@ bool GDBRemoteCommunicationClient::GetReverseStepSupported() {
   return m_supports_reverse_step == eLazyBoolYes;
 }
 
+bool GDBRemoteCommunicationClient::GetMultiMemReadSupported() {
+  if (m_supports_multi_mem_read == eLazyBoolCalculate)
+    GetRemoteQSupported();
+  return m_supports_multi_mem_read == eLazyBoolYes;
+}
+
 bool GDBRemoteCommunicationClient::QueryNoAckModeSupported() {
   if (m_supports_not_sending_acks == eLazyBoolCalculate) {
     m_send_acks = true;
@@ -339,6 +345,7 @@ void GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
     m_supported_async_json_packets_is_valid = false;
     m_supported_async_json_packets_sp.reset();
     m_supports_jModulesInfo = true;
+    m_supports_multi_mem_read = eLazyBoolCalculate;
   }
 
   // These flags should be reset when we first connect to a GDB server and when
@@ -365,6 +372,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_x_packet_state.reset();
   m_supports_reverse_continue = eLazyBoolNo;
   m_supports_reverse_step = eLazyBoolNo;
+  m_supports_multi_mem_read = eLazyBoolNo;
 
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
                                   // not, we assume no limit
@@ -424,6 +432,8 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
         m_supports_reverse_continue = eLazyBoolYes;
       else if (x == "ReverseStep+")
         m_supports_reverse_step = eLazyBoolYes;
+      else if (x == "MultiMemRead+")
+        m_supports_multi_mem_read = eLazyBoolYes;
       // Look for a list of compressions in the features list e.g.
       // qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib-
       // deflate,lzma
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index a765e95bf9814..ad590a25d0f16 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -342,6 +342,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
 
   bool GetReverseStepSupported();
 
+  bool GetMultiMemReadSupported();
+
   LazyBool SupportsAllocDeallocMemory() // const
   {
     // Uncomment this to have lldb pretend the debug server doesn't respond to
@@ -574,6 +576,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
   std::optional<xPacketState> m_x_packet_state;
   LazyBool m_supports_reverse_continue = eLazyBoolCalculate;
   LazyBool m_supports_reverse_step = eLazyBoolCalculate;
+  LazyBool m_supports_multi_mem_read = eLazyBoolCalculate;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,
diff --git a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
index f940229985887..ce042e5ddf422 100644
--- a/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ b/lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -639,3 +639,14 @@ TEST_F(GDBRemoteCommunicationClientTest, CalculateMD5) {
   EXPECT_EQ(expected_high, result->high());
 }
 #endif
+
+TEST_F(GDBRemoteCommunicationClientTest, MultiMemReadSupported) {
+  std::future<bool> async_result = std::async(std::launch::async, [&] {
+    StringExtractorGDBRemote qSupported_packet_request;
+    server.GetPacket(qSupported_packet_request);
+    server.SendPacket("MultiMemRead+;");
+    return true;
+  });
+  ASSERT_TRUE(client.GetMultiMemReadSupported());
+  async_result.wait();
+}

[DavidSpickett]

Code is fine.

Would like to see the answer to https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/88441/33?u=davidspickett in the PR description as well, so we have that documented.

[DavidSpickett]

Test that the default is not supported?

[felipepiovezan]

Updated commit message (and PR description) with a justification for qSupported, and added a negative test

[jasonmolenda]

Looks good to me.