This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4c56f734b35d: [lldb] (Partially) enable formatting of utf 
strings before the program is… (authored by labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113098/new/

https://reviews.llvm.org/D113098

Files:
  lldb/include/lldb/DataFormatters/FormattersHelpers.h
  lldb/include/lldb/DataFormatters/StringPrinter.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Target.h
  lldb/source/DataFormatters/FormattersHelpers.cpp
  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,24 @@
     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"你好"'])
+
+        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"你好"')
+        # FIXME: This should work too.
+        self.expect("expr abc", substrs=['u8"你好"'], matching=False)
+
+        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,68 @@
   return total_cstr_len;
 }
 
+addr_t Target::GetReasonableReadSize(const Address &addr) {
+  addr_t load_addr = addr.GetLoadAddress(this);
+  if (load_addr != LLDB_INVALID_ADDRESS && m_process_sp) {
+    // Avoid crossing cache line boundaries.
+    addr_t cache_line_size = m_process_sp->GetMemoryCacheLineSize();
+    return cache_line_size - (load_addr % cache_line_size);
+  }
+
+  // The read is going to go to the file cache, so we can just pick a largish
+  // value.
+  return 0x1000;
+}
+
+size_t Target::ReadStringFromMemory(const Address &addr, char *dst,
+                                    size_t max_bytes, Status &error,
+                                    size_t type_width, bool force_live_memory) {
+  if (!dst || !max_bytes || !type_width || max_bytes < type_width)
+    return 0;
+
+  size_t total_bytes_read = 0;
+
+  // 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!");
+
+  Address address = addr;
+  char *curr_dst = dst;
+
+  error.Clear();
+  while (bytes_left > 0 && error.Success()) {
+    addr_t bytes_to_read =
+        std::min<addr_t>(bytes_left, GetReasonableReadSize(address));
+    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;
+    address.Slide(bytes_read);
+    bytes_left -= bytes_read;
+  }
+  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)
+  Address valobj_addr = GetArrayAddressOrPointerValue(valobj);
+  if (!valobj_addr.IsValid())
     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,12 +111,8 @@
 
 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)
+  Address valobj_addr = GetArrayAddressOrPointerValue(valobj);
+  if (!valobj_addr.IsValid())
     return false;
 
   // Get a wchar_t basic type from the current type system
@@ -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/source/DataFormatters/FormattersHelpers.cpp
===================================================================
--- lldb/source/DataFormatters/FormattersHelpers.cpp
+++ lldb/source/DataFormatters/FormattersHelpers.cpp
@@ -10,7 +10,7 @@
 
 
 #include "lldb/DataFormatters/FormattersHelpers.h"
-
+#include "lldb/Core/Module.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
@@ -131,14 +131,17 @@
   return idx;
 }
 
-lldb::addr_t
+Address
 lldb_private::formatters::GetArrayAddressOrPointerValue(ValueObject &valobj) {
   lldb::addr_t data_addr = LLDB_INVALID_ADDRESS;
+  AddressType type;
 
   if (valobj.IsPointerType())
-    data_addr = valobj.GetValueAsUnsigned(0);
+    data_addr = valobj.GetPointerValue(&type);
   else if (valobj.IsArrayType())
-    data_addr = valobj.GetAddressOf();
+    data_addr = valobj.GetAddressOf(/*scalar_is_load_address=*/true, &type);
+  if (data_addr != LLDB_INVALID_ADDRESS && type == eAddressTypeFile)
+    return Address(data_addr, valobj.GetModule()->GetSectionList());
 
   return data_addr;
 }
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 = true);
+
   size_t ReadScalarIntegerFromMemory(const Address &addr, uint32_t byte_size,
                                      bool is_signed, Scalar &scalar,
                                      Status &error,
@@ -1490,6 +1521,10 @@
 
   void FinalizeFileActions(ProcessLaunchInfo &info);
 
+  /// Return a recommended size for memory reads at \a addr, optimizing for
+  /// cache usage.
+  lldb::addr_t GetReasonableReadSize(const Address &addr);
+
   Target(const Target &) = delete;
   const Target &operator=(const Target &) = delete;
 };
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
@@ -12,9 +12,9 @@
 #include <functional>
 #include <string>
 
-#include "lldb/lldb-forward.h"
-
+#include "lldb/Core/Address.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/lldb-forward.h"
 
 namespace lldb_private {
 namespace formatters {
@@ -105,21 +105,21 @@
 
     ReadStringAndDumpToStreamOptions(ValueObject &valobj);
 
-    void SetLocation(uint64_t l) { m_location = l; }
+    void SetLocation(Address l) { m_location = std::move(l); }
 
-    uint64_t GetLocation() const { return m_location; }
+    const Address &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; }
 
     bool HasSourceSize() const { return m_has_source_size; }
 
   private:
-    uint64_t m_location = 0;
-    lldb::ProcessSP m_process_sp;
+    Address m_location;
+    lldb::TargetSP m_target_sp;
     /// True iff we know the source size of the string.
     bool m_has_source_size = false;
   };
Index: lldb/include/lldb/DataFormatters/FormattersHelpers.h
===================================================================
--- lldb/include/lldb/DataFormatters/FormattersHelpers.h
+++ lldb/include/lldb/DataFormatters/FormattersHelpers.h
@@ -56,7 +56,7 @@
 
 size_t ExtractIndexFromString(const char *item_name);
 
-lldb::addr_t GetArrayAddressOrPointerValue(ValueObject &valobj);
+Address GetArrayAddressOrPointerValue(ValueObject &valobj);
 
 lldb::ValueObjectSP GetValueOfLibCXXCompressedPair(ValueObject &pair);
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to