labath created this revision.
labath added reviewers: teemperor, vsk.
labath requested review of this revision.
Herald added a project: LLDB.

The StringPrinter class was using a Process instance to read memory.
This automatically prevented it from working before starting the
program.

This patch changes the class to use the Target object for reading
memory, as targets are always available. This required moving
ReadStringFromMemory from Process to Target.

This is sufficient to make frame/target variable work, but further
changes are necessary for the expression evaluator. Preliminary analysis
indicates the failures are due to the expression result ValueObjects
failing to provide an address, presumably because we're operating on
file addresses before starting. I haven't looked into what would it take
to make that work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113098

Files:
  lldb/include/lldb/DataFormatters/StringPrinter.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
  lldb/source/Plugins/Language/ObjC/NSString.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py

Index: lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
===================================================================
--- lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
+++ lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py
@@ -18,8 +18,20 @@
     def test(self):
         """Test that C++ supports char8_t correctly."""
         self.build()
-        lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.cpp"))
 
+        lldbutil.run_to_breakpoint_make_target(self)
+
+        # Make sure the variables can be printed without a running process.
+        self.expect("target variable a", substrs=["char8_t", "0x61 u8'a'"])
+        self.expect("target variable ab",
+                substrs=["const char8_t *", 'u8"你好"'])
+        self.expect("target variable abc", substrs=["char8_t[9]", 'u8"你好"'])
+        # TODO: Make this work through expression evaluation as well
+
+        lldbutil.run_break_set_by_source_regexp(self, "// break here", "-f main.cpp")
+        self.runCmd("run")
+
+        # As well as with it
         self.expect_expr("a", result_type="char8_t", result_summary="0x61 u8'a'")
         self.expect_expr("ab", result_type="const char8_t *", result_summary='u8"你好"')
         self.expect_expr("abc", result_type="char8_t[9]", result_summary='u8"你好"')
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1904,6 +1904,61 @@
   return total_cstr_len;
 }
 
+size_t Target::ReadStringFromMemory(const Address &addr, char *dst,
+                                    size_t max_bytes, Status &error,
+                                    size_t type_width, bool force_live_memory) {
+  size_t total_bytes_read = 0;
+  if (dst && max_bytes && type_width && max_bytes >= type_width) {
+    // Ensure a null terminator independent of the number of bytes that is
+    // read.
+    memset(dst, 0, max_bytes);
+    size_t bytes_left = max_bytes - type_width;
+
+    const char terminator[4] = {'\0', '\0', '\0', '\0'};
+    assert(sizeof(terminator) >= type_width && "Attempting to validate a "
+                                               "string with more than 4 bytes "
+                                               "per character!");
+
+    addr_t curr_addr = addr.GetLoadAddress(this);
+    Address address = addr;
+    const size_t cache_line_size = 512;
+    char *curr_dst = dst;
+
+    error.Clear();
+    while (bytes_left > 0 && error.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);
+      size_t bytes_read = ReadMemory(address, curr_dst, bytes_to_read, error,
+                                     force_live_memory);
+
+      if (bytes_read == 0)
+        break;
+
+      // Search for a null terminator of correct size and alignment in
+      // bytes_read
+      size_t aligned_start = total_bytes_read - total_bytes_read % type_width;
+      for (size_t i = aligned_start;
+           i + type_width <= total_bytes_read + bytes_read; i += type_width)
+        if (::memcmp(&dst[i], terminator, type_width) == 0) {
+          error.Clear();
+          return i;
+        }
+
+      total_bytes_read += bytes_read;
+      curr_dst += bytes_read;
+      curr_addr += bytes_read;
+      address = Address(curr_addr);
+      bytes_left -= bytes_read;
+    }
+  } else {
+    if (max_bytes)
+      error.SetErrorString("invalid arguments");
+  }
+  return total_bytes_read;
+}
+
 size_t Target::ReadScalarIntegerFromMemory(const Address &addr, uint32_t byte_size,
                                            bool is_signed, Scalar &scalar,
                                            Status &error,
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -1983,57 +1983,6 @@
   return out_str.size();
 }
 
-size_t Process::ReadStringFromMemory(addr_t addr, char *dst, size_t max_bytes,
-                                     Status &error, size_t type_width) {
-  size_t total_bytes_read = 0;
-  if (dst && max_bytes && type_width && max_bytes >= type_width) {
-    // Ensure a null terminator independent of the number of bytes that is
-    // read.
-    memset(dst, 0, max_bytes);
-    size_t bytes_left = max_bytes - type_width;
-
-    const char terminator[4] = {'\0', '\0', '\0', '\0'};
-    assert(sizeof(terminator) >= type_width && "Attempting to validate a "
-                                               "string with more than 4 bytes "
-                                               "per character!");
-
-    addr_t curr_addr = addr;
-    const size_t cache_line_size = m_memory_cache.GetMemoryCacheLineSize();
-    char *curr_dst = dst;
-
-    error.Clear();
-    while (bytes_left > 0 && error.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);
-      size_t bytes_read = ReadMemory(curr_addr, curr_dst, bytes_to_read, error);
-
-      if (bytes_read == 0)
-        break;
-
-      // Search for a null terminator of correct size and alignment in
-      // bytes_read
-      size_t aligned_start = total_bytes_read - total_bytes_read % type_width;
-      for (size_t i = aligned_start;
-           i + type_width <= total_bytes_read + bytes_read; i += type_width)
-        if (::memcmp(&dst[i], terminator, type_width) == 0) {
-          error.Clear();
-          return i;
-        }
-
-      total_bytes_read += bytes_read;
-      curr_dst += bytes_read;
-      curr_addr += bytes_read;
-      bytes_left -= bytes_read;
-    }
-  } else {
-    if (max_bytes)
-      error.SetErrorString("invalid arguments");
-  }
-  return total_bytes_read;
-}
-
 // Deprecated in favor of ReadStringFromMemory which has wchar support and
 // correct code to find null terminators.
 size_t Process::ReadCStringFromMemory(addr_t addr, char *dst,
Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===================================================================
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -166,7 +166,7 @@
       return false;
     if (has_explicit_length && is_unicode) {
       options.SetLocation(location);
-      options.SetProcessSP(process_sp);
+      options.SetTargetSP(valobj.GetTargetSP());
       options.SetStream(&stream);
       options.SetQuote('"');
       options.SetSourceSize(explicit_length);
@@ -179,7 +179,7 @@
           StringPrinter::StringElementType::UTF16>(options);
     } else {
       options.SetLocation(location + 1);
-      options.SetProcessSP(process_sp);
+      options.SetTargetSP(valobj.GetTargetSP());
       options.SetStream(&stream);
       options.SetSourceSize(explicit_length);
       options.SetHasSourceSize(has_explicit_length);
@@ -195,7 +195,7 @@
     uint64_t location = 3 * ptr_size + valobj_addr;
 
     options.SetLocation(location);
-    options.SetProcessSP(process_sp);
+    options.SetTargetSP(valobj.GetTargetSP());
     options.SetStream(&stream);
     options.SetQuote('"');
     options.SetSourceSize(explicit_length);
@@ -217,7 +217,7 @@
         return false;
     }
     options.SetLocation(location);
-    options.SetProcessSP(process_sp);
+    options.SetTargetSP(valobj.GetTargetSP());
     options.SetStream(&stream);
     options.SetQuote('"');
     options.SetSourceSize(explicit_length);
@@ -237,7 +237,7 @@
     lldb::addr_t location = valobj.GetValueAsUnsigned(0) + ptr_size + 4;
 
     options.SetLocation(location);
-    options.SetProcessSP(process_sp);
+    options.SetTargetSP(valobj.GetTargetSP());
     options.SetStream(&stream);
     options.SetQuote('"');
     options.SetSourceSize(explicit_length);
@@ -260,7 +260,7 @@
       location++;
     }
     options.SetLocation(location);
-    options.SetProcessSP(process_sp);
+    options.SetTargetSP(valobj.GetTargetSP());
     options.SetStream(&stream);
     options.SetSourceSize(explicit_length);
     options.SetHasSourceSize(has_explicit_length);
@@ -283,7 +283,7 @@
       explicit_length++; // account for the fact that there is no NULL and we
                          // need to have one added
     options.SetLocation(location);
-    options.SetProcessSP(process_sp);
+    options.SetTargetSP(valobj.GetTargetSP());
     options.SetStream(&stream);
     options.SetSourceSize(explicit_length);
     options.SetHasSourceSize(has_explicit_length);
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -250,7 +250,7 @@
           addr_of_data == LLDB_INVALID_ADDRESS)
         return false;
       options.SetLocation(addr_of_data);
-      options.SetProcessSP(process_sp);
+      options.SetTargetSP(valobj.GetTargetSP());
       options.SetStream(&stream);
       options.SetNeedsZeroTermination(false);
       options.SetBinaryZeroIsTerminator(true);
@@ -311,7 +311,7 @@
           addr_of_data == LLDB_INVALID_ADDRESS)
         return false;
       options.SetLocation(addr_of_data);
-      options.SetProcessSP(process_sp);
+      options.SetTargetSP(valobj.GetTargetSP());
       options.SetStream(&stream);
       options.SetNeedsZeroTermination(false);
       options.SetBinaryZeroIsTerminator(false);
Index: lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp
@@ -50,17 +50,13 @@
 
 template <StringElementType ElemType>
 static bool CharStringSummaryProvider(ValueObject &valobj, Stream &stream) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-    return false;
-
   lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
   if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
     return false;
 
   StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
   options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
+  options.SetTargetSP(valobj.GetTargetSP());
   options.SetStream(&stream);
   options.SetPrefixToken(getElementTraits(ElemType).first);
 
@@ -115,10 +111,6 @@
 
 bool lldb_private::formatters::WCharStringSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
-  ProcessSP process_sp = valobj.GetProcessSP();
-  if (!process_sp)
-    return false;
-
   lldb::addr_t valobj_addr = GetArrayAddressOrPointerValue(valobj);
   if (valobj_addr == 0 || valobj_addr == LLDB_INVALID_ADDRESS)
     return false;
@@ -138,7 +130,7 @@
 
   StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
   options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
+  options.SetTargetSP(valobj.GetTargetSP());
   options.SetStream(&stream);
   options.SetPrefixToken("L");
 
Index: lldb/source/DataFormatters/StringPrinter.cpp
===================================================================
--- lldb/source/DataFormatters/StringPrinter.cpp
+++ lldb/source/DataFormatters/StringPrinter.cpp
@@ -408,8 +408,8 @@
       options.GetLocation() == LLDB_INVALID_ADDRESS)
     return false;
 
-  lldb::ProcessSP process_sp(options.GetProcessSP());
-  if (!process_sp)
+  lldb::TargetSP target_sp = options.GetTargetSP();
+  if (!target_sp)
     return false;
 
   constexpr int type_width = sizeof(SourceDataType);
@@ -423,7 +423,7 @@
   bool needs_zero_terminator = options.GetNeedsZeroTermination();
 
   bool is_truncated = false;
-  const auto max_size = process_sp->GetTarget().GetMaximumSizeOfStringSummary();
+  const auto max_size = target_sp->GetMaximumSizeOfStringSummary();
 
   uint32_t sourceSize;
   if (elem_type == StringElementType::ASCII && !options.GetSourceSize()) {
@@ -462,22 +462,22 @@
   char *buffer = reinterpret_cast<char *>(buffer_sp->GetBytes());
 
   if (elem_type == StringElementType::ASCII)
-    process_sp->ReadCStringFromMemory(options.GetLocation(), buffer,
+    target_sp->ReadCStringFromMemory(options.GetLocation(), buffer,
                                       bufferSPSize, error);
   else if (needs_zero_terminator)
-    process_sp->ReadStringFromMemory(options.GetLocation(), buffer,
+    target_sp->ReadStringFromMemory(options.GetLocation(), buffer,
                                      bufferSPSize, error, type_width);
   else
-    process_sp->ReadMemoryFromInferior(options.GetLocation(), buffer,
-                                       bufferSPSize, error);
+    target_sp->ReadMemory(options.GetLocation(), buffer, bufferSPSize, error);
   if (error.Fail()) {
     options.GetStream()->Printf("unable to read data");
     return true;
   }
 
   StringPrinter::ReadBufferAndDumpToStreamOptions dump_options(options);
-  dump_options.SetData(DataExtractor(buffer_sp, process_sp->GetByteOrder(),
-                                     process_sp->GetAddressByteSize()));
+  dump_options.SetData(
+      DataExtractor(buffer_sp, target_sp->GetArchitecture().GetByteOrder(),
+                    target_sp->GetArchitecture().GetAddressByteSize()));
   dump_options.SetSourceSize(sourceSize);
   dump_options.SetIsTruncated(is_truncated);
   dump_options.SetNeedsZeroTermination(needs_zero_terminator);
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -1023,6 +1023,37 @@
   size_t ReadCStringFromMemory(const Address &addr, char *dst,
                                size_t dst_max_len, Status &result_error);
 
+  /// Read a NULL terminated string from memory
+  ///
+  /// This function will read a cache page at a time until a NULL string
+  /// terminator is found. It will stop reading if an aligned sequence of NULL
+  /// termination \a type_width bytes is not found before reading \a
+  /// cstr_max_len bytes.  The results are always guaranteed to be NULL
+  /// terminated, and that no more than (max_bytes - type_width) bytes will be
+  /// read.
+  ///
+  /// \param[in] addr
+  ///     The address to start the memory read.
+  ///
+  /// \param[in] dst
+  ///     A character buffer containing at least max_bytes.
+  ///
+  /// \param[in] max_bytes
+  ///     The maximum number of bytes to read.
+  ///
+  /// \param[in] error
+  ///     The error status of the read operation.
+  ///
+  /// \param[in] type_width
+  ///     The size of the null terminator (1 to 4 bytes per
+  ///     character).  Defaults to 1.
+  ///
+  /// \return
+  ///     The error status or the number of bytes prior to the null terminator.
+  size_t ReadStringFromMemory(const Address &addr, char *dst, size_t max_bytes,
+                              Status &error, size_t type_width,
+                              bool force_live_memory = false);
+
   size_t ReadScalarIntegerFromMemory(const Address &addr, uint32_t byte_size,
                                      bool is_signed, Scalar &scalar,
                                      Status &error,
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -1482,36 +1482,6 @@
   size_t ReadMemoryFromInferior(lldb::addr_t vm_addr, void *buf, size_t size,
                                 Status &error);
 
-  /// Read a NULL terminated string from memory
-  ///
-  /// This function will read a cache page at a time until a NULL string
-  /// terminator is found. It will stop reading if an aligned sequence of NULL
-  /// termination \a type_width bytes is not found before reading \a
-  /// cstr_max_len bytes.  The results are always guaranteed to be NULL
-  /// terminated, and that no more than (max_bytes - type_width) bytes will be
-  /// read.
-  ///
-  /// \param[in] vm_addr
-  ///     The virtual load address to start the memory read.
-  ///
-  /// \param[in] str
-  ///     A character buffer containing at least max_bytes.
-  ///
-  /// \param[in] max_bytes
-  ///     The maximum number of bytes to read.
-  ///
-  /// \param[in] error
-  ///     The error status of the read operation.
-  ///
-  /// \param[in] type_width
-  ///     The size of the null terminator (1 to 4 bytes per
-  ///     character).  Defaults to 1.
-  ///
-  /// \return
-  ///     The error status or the number of bytes prior to the null terminator.
-  size_t ReadStringFromMemory(lldb::addr_t vm_addr, char *str, size_t max_bytes,
-                              Status &error, size_t type_width = 1);
-
   /// Read a NULL terminated C string from memory
   ///
   /// This function will read a cache page at a time until the NULL
Index: lldb/include/lldb/DataFormatters/StringPrinter.h
===================================================================
--- lldb/include/lldb/DataFormatters/StringPrinter.h
+++ lldb/include/lldb/DataFormatters/StringPrinter.h
@@ -109,9 +109,9 @@
 
     uint64_t GetLocation() const { return m_location; }
 
-    void SetProcessSP(lldb::ProcessSP p) { m_process_sp = std::move(p); }
+    void SetTargetSP(lldb::TargetSP t) { m_target_sp = std::move(t); }
 
-    lldb::ProcessSP GetProcessSP() const { return m_process_sp; }
+    lldb::TargetSP GetTargetSP() const { return m_target_sp; }
 
     void SetHasSourceSize(bool e) { m_has_source_size = e; }
 
@@ -119,7 +119,7 @@
 
   private:
     uint64_t m_location = 0;
-    lldb::ProcessSP m_process_sp;
+    lldb::TargetSP m_target_sp;
     /// True iff we know the source size of the string.
     bool m_has_source_size = false;
   };
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to