[Lldb-commits] [lldb] r363750 - Add ReadCStringFromMemory for faster string reads

Antonio Afonso via lldb-commits lldb-commits at lists.llvm.org
Tue Jun 18 16:27:57 PDT 2019


Author: aadsm
Date: Tue Jun 18 16:27:57 2019
New Revision: 363750

URL: http://llvm.org/viewvc/llvm-project?rev=363750&view=rev
Log:
Add ReadCStringFromMemory for faster string reads

Summary:
This is the fifth patch to improve module loading in a series that started here (where I explain the motivation and solution): D62499

Reading strings with ReadMemory is really slow when reading the path of the shared library. This is because we don't know the length of the path so use PATH_MAX (4096) and these strings are actually super close to the boundary of an unreadable page. So even though we use process_vm_readv it will usually fail because the read size spans to the unreadable page and we then default to read the string word by word with ptrace.

This new function is very similar to another ReadCStringFromMemory that already exists in lldb that makes sure it never reads cross page boundaries and checks if we already read the entire string by finding '\0'.

I was able to reduce the GetLoadedSharedLibraries call from 30ms to 4ms (or something of that order).

Reviewers: clayborg, xiaobai, labath

Reviewed By: labath

Subscribers: emaste, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D62503

Modified:
    lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
    lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
    lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
    lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp
    lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp

Modified: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h?rev=363750&r1=363749&r2=363750&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Tue Jun 18 16:27:57 2019
@@ -84,6 +84,31 @@ public:
   Status ReadMemoryWithoutTrap(lldb::addr_t addr, void *buf, size_t size,
                                size_t &bytes_read);
 
+  /// Reads a null terminated string from memory.
+  ///
+  /// Reads up to \p max_size bytes of memory until it finds a '\0'.
+  /// If a '\0' is not found then it reads max_size-1 bytes as a string and a
+  /// '\0' is added as the last character of the \p buffer.
+  ///
+  /// \param[in] addr
+  ///     The address in memory to read from.
+  ///
+  /// \param[in] buffer
+  ///     An allocated buffer with at least \p max_size size.
+  ///
+  /// \param[in] max_size
+  ///     The maximum number of bytes to read from memory until it reads the
+  ///     string.
+  ///
+  /// \param[out] total_bytes_read
+  ///     The number of bytes read from memory into \p buffer.
+  ///
+  /// \return
+  ///     Returns a StringRef backed up by the \p buffer passed in.
+  llvm::Expected<llvm::StringRef>
+  ReadCStringFromMemory(lldb::addr_t addr, char *buffer, size_t max_size,
+                        size_t &total_bytes_read);
+
   virtual Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size,
                              size_t &bytes_written) = 0;
 

Modified: lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeProcessProtocol.cpp?rev=363750&r1=363749&r2=363750&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/NativeProcessProtocol.cpp (original)
+++ lldb/trunk/source/Host/common/NativeProcessProtocol.cpp Tue Jun 18 16:27:57 2019
@@ -16,6 +16,8 @@
 #include "lldb/Utility/State.h"
 #include "lldb/lldb-enumerations.h"
 
+#include "llvm/Support/Process.h"
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -659,6 +661,58 @@ Status NativeProcessProtocol::ReadMemory
   return Status();
 }
 
+llvm::Expected<llvm::StringRef>
+NativeProcessProtocol::ReadCStringFromMemory(lldb::addr_t addr, char *buffer,
+                                             size_t max_size,
+                                             size_t &total_bytes_read) {
+  static const size_t cache_line_size =
+      llvm::sys::Process::getPageSizeEstimate();
+  size_t bytes_read = 0;
+  size_t bytes_left = max_size;
+  addr_t curr_addr = addr;
+  size_t string_size;
+  char *curr_buffer = buffer;
+  total_bytes_read = 0;
+  Status status;
+
+  while (bytes_left > 0 && status.Success()) {
+    addr_t cache_line_bytes_left =
+        cache_line_size - (curr_addr % cache_line_size);
+    addr_t bytes_to_read = std::min<addr_t>(bytes_left, cache_line_bytes_left);
+    status = ReadMemory(curr_addr, reinterpret_cast<void *>(curr_buffer),
+                        bytes_to_read, bytes_read);
+
+    if (bytes_read == 0)
+      break;
+
+    void *str_end = std::memchr(curr_buffer, '\0', bytes_read);
+    if (str_end != nullptr) {
+      total_bytes_read =
+          (size_t)(reinterpret_cast<char *>(str_end) - buffer + 1);
+      status.Clear();
+      break;
+    }
+
+    total_bytes_read += bytes_read;
+    curr_buffer += bytes_read;
+    curr_addr += bytes_read;
+    bytes_left -= bytes_read;
+  }
+
+  string_size = total_bytes_read - 1;
+
+  // Make sure we return a null terminated string.
+  if (bytes_left == 0 && max_size > 0 && buffer[max_size - 1] != '\0') {
+    buffer[max_size - 1] = '\0';
+    total_bytes_read--;
+  }
+
+  if (!status.Success())
+    return status.ToError();
+
+  return llvm::StringRef(buffer, string_size);
+}
+
 lldb::StateType NativeProcessProtocol::GetState() const {
   std::lock_guard<std::recursive_mutex> guard(m_state_mutex);
   return m_state;

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=363750&r1=363749&r2=363750&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Tue Jun 18 16:27:57 2019
@@ -2076,4 +2076,4 @@ Status NativeProcessLinux::StopProcessor
   m_processor_trace_monitor.erase(iter);
 
   return error;
-}
\ No newline at end of file
+}

Modified: lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp?rev=363750&r1=363749&r2=363750&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp (original)
+++ lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp Tue Jun 18 16:27:57 2019
@@ -118,14 +118,13 @@ NativeProcessELF::ReadSVR4LibraryInfo(ll
     return error.ToError();
 
   char name_buffer[PATH_MAX];
-  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
-                     bytes_read);
-  if (!error.Success())
-    return error.ToError();
-  name_buffer[PATH_MAX - 1] = '\0';
+  llvm::Expected<llvm::StringRef> string_or_error = ReadCStringFromMemory(
+      link_map.l_name, &name_buffer[0], sizeof(name_buffer), bytes_read);
+  if (!string_or_error)
+    return string_or_error.takeError();
 
   SVR4LibraryInfo info;
-  info.name = std::string(name_buffer);
+  info.name = string_or_error->str();
   info.link_map = link_map_addr;
   info.base_addr = link_map.l_addr;
   info.ld_addr = link_map.l_ld;

Modified: lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp?rev=363750&r1=363749&r2=363750&view=diff
==============================================================================
--- lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp (original)
+++ lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp Tue Jun 18 16:27:57 2019
@@ -9,6 +9,7 @@
 #include "TestingSupport/Host/NativeProcessTestUtils.h"
 
 #include "lldb/Host/common/NativeProcessProtocol.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 
@@ -96,3 +97,53 @@ TEST(NativeProcessProtocolTest, ReadMemo
   EXPECT_THAT_EXPECTED(Process.ReadMemoryWithoutTrap(4, 2),
                        llvm::HasValue(std::vector<uint8_t>{4, 5}));
 }
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory) {
+  NiceMock<MockDelegate> DummyDelegate;
+  MockProcess<NativeProcessProtocol> Process(DummyDelegate,
+                                             ArchSpec("aarch64-pc-linux"));
+  FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'});
+  EXPECT_CALL(Process, ReadMemory(_, _))
+      .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+
+  char string[1024];
+  size_t bytes_read;
+  EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory(
+                           0x0, &string[0], sizeof(string), bytes_read),
+                       llvm::HasValue(llvm::StringRef("hello")));
+  EXPECT_EQ(bytes_read, 6UL);
+}
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory_MaxSize) {
+  NiceMock<MockDelegate> DummyDelegate;
+  MockProcess<NativeProcessProtocol> Process(DummyDelegate,
+                                             ArchSpec("aarch64-pc-linux"));
+  FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'});
+  EXPECT_CALL(Process, ReadMemory(_, _))
+      .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+
+  char string[4];
+  size_t bytes_read;
+  EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory(
+                           0x0, &string[0], sizeof(string), bytes_read),
+                       llvm::HasValue(llvm::StringRef("hel")));
+  EXPECT_EQ(bytes_read, 3UL);
+}
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory_CrossPageBoundary) {
+  NiceMock<MockDelegate> DummyDelegate;
+  MockProcess<NativeProcessProtocol> Process(DummyDelegate,
+                                             ArchSpec("aarch64-pc-linux"));
+  unsigned string_start = llvm::sys::Process::getPageSizeEstimate() - 3;
+  FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'}, string_start);
+  EXPECT_CALL(Process, ReadMemory(_, _))
+      .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+
+  char string[1024];
+  size_t bytes_read;
+  EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory(string_start, &string[0],
+                                                     sizeof(string),
+                                                     bytes_read),
+                       llvm::HasValue(llvm::StringRef("hello")));
+  EXPECT_EQ(bytes_read, 6UL);
+}
\ No newline at end of file




More information about the lldb-commits mailing list