[Lldb-commits] [PATCH] D28808: Fix a bug where lldb does not respect the packet size.

[Hafiz Abid Qadeer via Phabricator via lldb-commits]

abidh created this revision.

LLDB was using packet size advertised by the target as the max memory size to write in one go. It is wrong because packets have other overhead apart from memory payload. Also memory transferred through 'm' and 'M' packets needs 2 bytes in packet to transfer 1 of memory.


https://reviews.llvm.org/D28808

Files:
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp


Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2729,16 +2729,19 @@
 size_t ProcessGDBRemote::DoReadMemory(addr_t addr, void *buf, size_t size,
                                       Error &error) {
   GetMaxMemorySize();
-  if (size > m_max_memory_size) {
+  bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
+  // M and m packets take 2 bytes for 1 byte of memory
+  size_t max_memory_size = binary_memory_read ? m_max_memory_size :
+          m_max_memory_size / 2;
+  if (size > max_memory_size) {
     // Keep memory read sizes down to a sane limit. This function will be
     // called multiple times in order to complete the task by
     // lldb_private::Process so it is ok to do this.
-    size = m_max_memory_size;
+    size = max_memory_size;
   }
 
   char packet[64];
   int packet_len;
-  bool binary_memory_read = m_gdb_comm.GetxPacketSupported();
   packet_len = ::snprintf(packet, sizeof(packet), "%c%" PRIx64 ",%" PRIx64,
                           binary_memory_read ? 'x' : 'm', (uint64_t)addr,
                           (uint64_t)size);
@@ -2785,11 +2788,13 @@
 size_t ProcessGDBRemote::DoWriteMemory(addr_t addr, const void *buf,
                                        size_t size, Error &error) {
   GetMaxMemorySize();
-  if (size > m_max_memory_size) {
+  // M and m packets take 2 bytes for 1 byte of memory
+  size_t max_memory_size = m_max_memory_size / 2;
+  if (size > max_memory_size) {
     // Keep memory read sizes down to a sane limit. This function will be
     // called multiple times in order to complete the task by
     // lldb_private::Process so it is ok to do this.
-    size = m_max_memory_size;
+    size = max_memory_size;
   }
 
   StreamString packet;
@@ -3988,6 +3993,10 @@
         stub_max_size = reasonable_largeish_default;
       }
 
+      // Memory packets have other overheads too like Maddr,size:#NN.
+      // Instead of calculating the bytes taken by size and addr every
+      // time, we take a maximum guess here.
+      stub_max_size -= 32 + 32 + 6;
       m_max_memory_size = stub_max_size;
     } else {
       m_max_memory_size = conservative_default;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28808.84674.patch
Type: text/x-patch
Size: 2336 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170117/2a2dbfc7/attachment.bin>