[nfc][lldb] Move FastSearch from CommandObjectMemoryFind to Process

[mbucko]

Summary:
Moving CommandObjectMemoryFind::FastSearch() to Process::FindInMemory(). Plan to expose FindInMemory as public API in SBProcess.

Test Plan:
ninja check-lldb

Reviewers: clayborg

Subscribers:

Tasks:
lldb

Tags:

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Miro Bucko (mbucko)

Changes

Summary:
Moving CommandObjectMemoryFind::FastSearch() to Process::FindInMemory(). Plan to expose FindInMemory as public API in SBProcess.

Test Plan:
ninja check-lldb

Reviewers: clayborg

Subscribers:

Tasks:
lldb

Tags:

---

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

3 Files Affected:

• (modified) lldb/include/lldb/Target/Process.h (+22)
• (modified) lldb/source/Commands/CommandObjectMemory.cpp (+2-59)
• (modified) lldb/source/Target/Process.cpp (+55)

diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 637d34c29715c1..f293d1e78494f7 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2663,6 +2663,28 @@ void PruneThreadPlans();
     return m_source_file_cache;
   }
 
+  /// Find a pattern within a memory region.
+  ///
+  /// This function searches for a pattern represented by the provided buffer
+  /// within the memory range specified by the low and high addresses. It uses
+  /// a bad character heuristic to optimize the search process.
+  ///
+  /// \param[in] low The starting address of the memory region to be searched.
+  /// (inclusive)
+  ///
+  /// \param[in] high The ending address of the memory region to be searched.
+  /// (exclusive)
+  ///
+  /// \param[in] buf A pointer to the buffer containing the pattern to be
+  /// searched.
+  ///
+  /// \param[in] buffer_size The size of the buffer in bytes.
+  ///
+  /// \return The address where the pattern was found or LLDB_INVALID_ADDRESS if
+  /// not found.
+  lldb::addr_t FindInMemory(lldb::addr_t low, lldb::addr_t high, 
+                            const uint8_t *buf, size_t size);
+
 protected:
   friend class Trace;
 
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index b78a0492cca558..1c13484dede648 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -977,35 +977,6 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
   Options *GetOptions() override { return &m_option_group; }
 
 protected:
-  class ProcessMemoryIterator {
-  public:
-    ProcessMemoryIterator(ProcessSP process_sp, lldb::addr_t base)
-        : m_process_sp(process_sp), m_base_addr(base) {
-      lldbassert(process_sp.get() != nullptr);
-    }
-
-    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_sp->ReadMemory(m_base_addr + offset, &retval, 1, error)) {
-        m_is_valid = false;
-        return 0;
-      }
-
-      return retval;
-    }
-
-  private:
-    ProcessSP m_process_sp;
-    lldb::addr_t m_base_addr;
-    bool m_is_valid = true;
-  };
   void DoExecute(Args &command, CommandReturnObject &result) override {
     // No need to check "process" for validity as eCommandRequiresProcess
     // ensures it is valid
@@ -1106,8 +1077,8 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
     found_location = low_addr;
     bool ever_found = false;
     while (count) {
-      found_location = FastSearch(found_location, high_addr, buffer.GetBytes(),
-                                  buffer.GetByteSize());
+      found_location = process->FindInMemory(
+          found_location, high_addr, buffer.GetBytes(), buffer.GetByteSize());
       if (found_location == LLDB_INVALID_ADDRESS) {
         if (!ever_found) {
           result.AppendMessage("data not found within the range.\n");
@@ -1144,34 +1115,6 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
     result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
   }
 
-  lldb::addr_t FastSearch(lldb::addr_t low, lldb::addr_t high, uint8_t *buffer,
-                          size_t buffer_size) {
-    const size_t region_size = high - low;
-
-    if (region_size < buffer_size)
-      return LLDB_INVALID_ADDRESS;
-
-    std::vector<size_t> bad_char_heuristic(256, buffer_size);
-    ProcessSP process_sp = m_exe_ctx.GetProcessSP();
-    ProcessMemoryIterator iterator(process_sp, low);
-
-    for (size_t idx = 0; idx < buffer_size - 1; idx++) {
-      decltype(bad_char_heuristic)::size_type bcu_idx = buffer[idx];
-      bad_char_heuristic[bcu_idx] = buffer_size - idx - 1;
-    }
-    for (size_t s = 0; s <= (region_size - buffer_size);) {
-      int64_t j = buffer_size - 1;
-      while (j >= 0 && buffer[j] == iterator[s + j])
-        j--;
-      if (j < 0)
-        return low + s;
-      else
-        s += bad_char_heuristic[iterator[s + buffer_size - 1]];
-    }
-
-    return LLDB_INVALID_ADDRESS;
-  }
-
   OptionGroupOptions m_option_group;
   OptionGroupFindMemory m_memory_options;
   OptionGroupMemoryTag m_memory_tag_options;
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 216d2f21abfef0..2e0a96b07b3d55 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -112,6 +112,34 @@ 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,
@@ -3191,6 +3219,33 @@ Status Process::Halt(bool clear_thread_plans, bool use_run_lock) {
   return Status();
 }
 
+lldb::addr_t Process::FindInMemory(lldb::addr_t low, lldb::addr_t high,
+                                   const uint8_t *buf, size_t size) {
+  const size_t region_size = high - low;
+
+  if (region_size < size)
+    return LLDB_INVALID_ADDRESS;
+
+  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);) {
+    int64_t j = size - 1;
+    while (j >= 0 && buf[j] == iterator[s + j])
+      j--;
+    if (j < 0)
+      return low + s;
+    else
+      s += bad_char_heuristic[iterator[s + size - 1]];
+  }
+
+  return LLDB_INVALID_ADDRESS;
+}
+
 Status Process::StopForDestroyOrDetach(lldb::EventSP &exit_event_sp) {
   Status error;
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[jimingham]

It's not a good idea in general to have useful algorithms in the CommandObjects, that leads to code duplication - people can't be expected to scour the CommandObjects for stealable code.... So this sort of move is a good tidying up even if you didn't plan to use it somewhere else.