[lldb] Fix and speedup the memory find command

This patch fixes an issue where the `memory find` command would
effectively stop searching after encountering a memory read error (which
could happen due to unreadable memory), without giving any indication
that it has done so (it would just print it could not find the pattern).

To make matters worse, it would not terminate after encountering this
error, but rather proceed to slowly increment the address pointer, which
meant that searching a large region could take a very long time (and
give the appearance that lldb is actually searching for the thing).

The patch fixes this first problem (*) by detecting read errors and
skipping over (using GetMemoryRegionInfo) the unreadable parts of memory
and resuming the search after them. It also reads the memory in bulk (up
to 1MB), which speeds up the search significantly (up to 6x for live
processes, 18x for core files).

(*) The fix does not work on windows yet, because the ReadMemory API
does not return partial results (like it does for other systems). I'm
preparing a separate patch to deal with that.

Author: labath

lldb/source/Target/Process.cpp

@@ -114,33 +114,6 @@ class ProcessOptionValueProperties
   }
 };
 
-class ProcessMemoryIterator {
-public:
-  ProcessMemoryIterator(Process &process, lldb::addr_t base)
-      : m_process(process), m_base_addr(base) {}
-
-  bool IsValid() { return m_is_valid; }
-
-  uint8_t operator[](lldb::addr_t offset) {
-    if (!IsValid())
-      return 0;
-
-    uint8_t retval = 0;
-    Status error;
-    if (0 == m_process.ReadMemory(m_base_addr + offset, &retval, 1, error)) {
-      m_is_valid = false;
-      return 0;
-    }
-
-    return retval;
-  }
-
-private:
-  Process &m_process;
-  const lldb::addr_t m_base_addr;
-  bool m_is_valid = true;
-};
-
 static constexpr OptionEnumValueElement g_follow_fork_mode_values[] = {
     {
         eFollowParent,
@@ -3367,21 +3340,48 @@ lldb::addr_t Process::FindInMemory(lldb::addr_t low, lldb::addr_t high,
   if (region_size < size)
     return LLDB_INVALID_ADDRESS;
 
+  // See "Boyer-Moore string search algorithm".
   std::vector<size_t> bad_char_heuristic(256, size);
-  ProcessMemoryIterator iterator(*this, low);
-
   for (size_t idx = 0; idx < size - 1; idx++) {
     decltype(bad_char_heuristic)::size_type bcu_idx = buf[idx];
     bad_char_heuristic[bcu_idx] = size - idx - 1;
   }
-  for (size_t s = 0; s <= (region_size - size);) {
+
+  // Memory we're currently searching through.
+  llvm::SmallVector<uint8_t, 0> mem;
+  // Position of the memory buffer.
+  addr_t mem_pos = low;
+  // Maximum number of bytes read (and buffered). We need to read at least
+  // `size` bytes for a successful match.
+  const size_t max_read_size = std::max<size_t>(size, 0x10000);
+
+  for (addr_t s = low; s <= (high - size);) {
+    if (s + size > mem.size() + mem_pos) {
+      // We need to read more data. We don't attempt to reuse the data we've
+      // already read (up to `size-1` bytes from `s` to `mem_pos+mem.size()`).
+      // This is fine for patterns much smaller than max_read_size. For very
+      // long patterns we may need to do something more elaborate.
+      mem.resize_for_overwrite(max_read_size);
+      Status error;
+      mem.resize(
+          ReadMemory(s, mem.data(), std::min(mem.size(), high - s), error));
+      mem_pos = s;
+      if (error.Fail() || size > mem.size()) {
+        // We didn't read enough data. Skip to the next memory region.
+        MemoryRegionInfo info;
+        error = GetMemoryRegionInfo(mem_pos + mem.size(), info);
+        if (error.Fail())
+          break;
+        s = info.GetRange().GetRangeEnd();
+        continue;
+      }
+    }
     int64_t j = size - 1;
-    while (j >= 0 && buf[j] == iterator[s + j])
+    while (j >= 0 && buf[j] == mem[s + j - mem_pos])
       j--;
     if (j < 0)
-      return low + s;
-    else
-      s += bad_char_heuristic[iterator[s + size - 1]];
+      return s; // We have a match.
+    s += bad_char_heuristic[mem[s + size - 1 - mem_pos]];
   }
 
   return LLDB_INVALID_ADDRESS;

lldb/test/API/functionalities/memory/find/TestMemoryFind.py

@@ -10,14 +10,16 @@
 
 
 class MemoryFindTestCase(TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.line = line_number("main.cpp", "// break here")
 
-    def test_memory_find(self):
-        """Test the 'memory find' command."""
+    def _prepare_inferior(self):
         self.build()
         exe = self.getBuildArtifact("a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
@@ -39,7 +41,10 @@ def test_memory_find(self):
         # The breakpoint should have a hit count of 1.
         lldbutil.check_breakpoint(self, bpno=1, expected_hit_count=1)
 
-        # Test the memory find commands.
+    def test_memory_find(self):
+        """Test the 'memory find' command."""
+
+        self._prepare_inferior()
 
         # Empty search string should be handled.
         self.expect(
@@ -79,3 +84,22 @@ def test_memory_find(self):
             'memory find -s "nothere" `stringdata` `stringdata+10`',
             substrs=["data not found within the range."],
         )
+
+    @expectedFailureWindows
+    def test_memory_find_with_holes(self):
+        self._prepare_inferior()
+
+        pagesize = self.frame().FindVariable("pagesize").GetValueAsUnsigned()
+        mem_with_holes = (
+            self.frame().FindVariable("mem_with_holes").GetValueAsUnsigned()
+        )
+        matches_var = self.frame().FindVariable("matches")
+        self.assertEqual(matches_var.GetNumChildren(), 4)
+        matches = [
+            f"data found at location: {matches_var.GetChildAtIndex(i).GetValueAsUnsigned():#x}"
+            for i in range(4)
+        ]
+        self.expect(
+            'memory find -c 5 -s "needle" `mem_with_holes` `mem_with_holes+5*pagesize`',
+            substrs=matches + ["no more matches within the range"],
+        )

lldb/test/API/functionalities/memory/find/main.cpp

@@ -1,9 +1,76 @@
-#include <stdio.h>
-#include <stdint.h>
-
-int main (int argc, char const *argv[])
-{
-    const char* stringdata = "hello world; I like to write text in const char pointers";
-    uint8_t bytedata[] = {0xAA,0xBB,0xCC,0xDD,0xEE,0xFF,0x00,0x11,0x22,0x33,0x44,0x55,0x66,0x77,0x88,0x99};
-    return 0; // break here
+#include <cstdint>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <initializer_list>
+#include <iostream>
+
+#ifdef _WIN32
+#include "Windows.h"
+
+int getpagesize() {
+  SYSTEM_INFO system_info;
+  GetSystemInfo(&system_info);
+  return system_info.dwPageSize;
+}
+
+char *allocate_memory_with_holes() {
+  int pagesize = getpagesize();
+  void *mem = VirtualAlloc(nullptr, 5 * pagesize, MEM_RESERVE, PAGE_NOACCESS);
+  if (!mem) {
+    std::cerr << std::system_category().message(GetLastError()) << std::endl;
+    exit(1);
+  }
+  char *bytes = static_cast<char *>(mem);
+  for (int page : {0, 2, 4}) {
+    if (!VirtualAlloc(bytes + page * pagesize, pagesize, MEM_COMMIT,
+                      PAGE_READWRITE)) {
+      std::cerr << std::system_category().message(GetLastError()) << std::endl;
+      exit(1);
+    }
+  }
+  return bytes;
+}
+#else
+#include "sys/mman.h"
+#include "unistd.h"
+
+char *allocate_memory_with_holes() {
+  int pagesize = getpagesize();
+  void *mem = mmap(nullptr, 5 * pagesize, PROT_READ | PROT_WRITE,
+                   MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (mem == MAP_FAILED) {
+    perror("mmap");
+    exit(1);
+  }
+  char *bytes = static_cast<char *>(mem);
+  for (int page : {1, 3}) {
+    if (munmap(bytes + page * pagesize, pagesize) != 0) {
+      perror("munmap");
+      exit(1);
+    }
+  }
+  return bytes;
+}
+#endif
+
+int main(int argc, char const *argv[]) {
+  const char *stringdata =
+      "hello world; I like to write text in const char pointers";
+  uint8_t bytedata[] = {0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF, 0x00, 0x11,
+                        0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99};
+
+  char *mem_with_holes = allocate_memory_with_holes();
+  int pagesize = getpagesize();
+  char *matches[] = {
+      mem_with_holes,                // Beginning of memory
+      mem_with_holes + 2 * pagesize, // After a hole
+      mem_with_holes + 2 * pagesize +
+          pagesize / 2, // Middle of a block, after an existing match.
+      mem_with_holes + 5 * pagesize - 7, // End of memory
+  };
+  for (char *m : matches)
+    strcpy(m, "needle");
+
+  return 0; // break here
 }