[lldb] (Partially) enable formatting of utf strings before the program is started

[labath]

Summary

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.

[labath]

bug reported in PR45856.

[ljmf00]

In D113098#3105809, @labath wrote:

bug reported in PR45856.

Thanks for your time fixing this. So, for better understanding, does the infrastructure behind LLDB, Target and Process have similar APIs but Process is just for the running state?

[labath]

In D113098#3105906, @ljmf00 wrote:

In D113098#3105809, @labath wrote:

bug reported in PR45856.

Thanks for your time fixing this. So, for better understanding, does the infrastructure behind LLDB, Target and Process have similar APIs but Process is just for the running state?

Not exactly. It's true that a Process represents the running state of the application, and that both of them can read memory (process always reads live memory while a target can optionally return data from the object file), but that's pretty much where the similarities end. Process' methods generally concern the running of the application, so you have things that deal with stdio, signals, threads, etc. OTOH, a target works with the object files, and the debug info contained therein (breakpoints, modules, symbols, ...).

[labath]

Jim, could you take look at this?

[jingham]

It makes sense for the string formatters to try reading from the target if there's no process. So the general idea is fine.

But one of the other major differences between the Target before a process has been launched and attached to; and its state after attachment; is that before running there isn't a unique mapping from a scalar address (lldb::addr_t) to a specific byte in a specific Object File. For instance, on the systems where lldb can figure it out the dependent libraries before execution, lldb will determine & load into the Target not just the binary handed to it by the "file" command, but also the closure of that binary's direct dependents. That's handy, it means that you can inspect disassembly, set breakpoints, etc. seeing all the libraries your binary depends on before launch. However, on many systems libraries aren't given unique load addresses. They generally just get set to load at 0x0, and the loader is the one that gives them all unique slots in the memory space when the process runs. So Modules in the Target might and often do overlap each other in the "before running" address space.

When you are asking a process for memory, there's only one value at any given address, so all those API's can take an lldb::addr_t, the Process will always be able to unambiguously turn that back into either a plain address (if the memory is on the heap or stack) or a section relative address if the address is in TEXT/DATA, etc. of some loaded binary. But when you start asking the Target questions, you can't rely on this fact.

The lldb_private::Address class handles that by recording the address as section in a Module + offset, inherently picking the right binary. So using Addresses we can always record the user's intentions correctly. For instance, if you get the Address from a ValueObject representing a global variable which is in the DATA section of a target, that Address will record both the data section, and the variable's offset in that sections. But for that to work, you have to be careful to preserve the Address throughout the process, and never go back & forth from lldb_private::Address to lldb::addr_t until you actually have to request bytes of something (process, file, etc). Otherwise you might start out trying to read a string in library A, but end up reading the memory for it from library B...

Preserving the Address as long as is practicable is actually a general good practice in lldb. Even if you have a live process, when you go from section-relative Address to addr_t and back to Address, we have to re-lookup that address in the section load list, which is inefficient.

You make this mistake a few places in this patch, which I've pointed out in the detailed comments.

lldb/include/lldb/Target/Target.h

1028

ReadCStringFromMemory (both flavors) set "force_live_memory" to be true. Note that doesn't mean "don't read from the target" it means "always read from the process first." It does fall back to the Target if there's no process, AND if your Address is section relative.

OTOH, you make it a default argument and then set it to "false" - the opposite behavior. Is there a reason for that? That seems confusing.

force_live_memory is really an efficiency thing. It will only make a difference if the data to be read is in a read-only section of a binary, in which case it will read from there. lldb always used to prefer reading locally if it can, but if someone has been able to flip permissions and overwrite read-only DATA through some kind of attack, you wouldn't be able to see that if this is the default. So in actual fact, we pass force_live_memory -> true most of the places we use these Target Memory Reading API's.

So it seems better to follow ReadCStringFromMemory and just force this to true.

lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

58

This should take some form of lldb_private::Address not an lldb::addr_t or it won't work pre-run. See the overall comment for details.

lldb/source/Target/Target.cpp

1911

Process::ReadStringFromMemory is a pretty old function, and hasn't been fully converted to llvm style, which prefers to check things like dst != nullptr up front and do an early return if anything fails. That reduces indentation, and spaces are a precious commodity if you restrict yourself to 80 characters on a line.

Also, the error message was originally pretty useless, but there's no reason to propagate that now that you've touched it ;-)

1922

This is wrong. If the incoming address was section relative, you've thrown that information away. Just pass Target::ReadMemory the full Address (addr), it will know what to do with it. And you can use:

addr.Slide(bytes_read);

instead of lines 1951-1952 to update the address.

1924

This is a seemingly arbitrary choice.

From what I can tell you are copying this from GetCStringFromMemory, so the decision to do this isn't yours...

IIUC the whole point of this paging so we don't read a big chunk of memory and then find the terminator a couple of characters in. The optimal size for that doesn't have to line up with the process cache size, but I don't see why you wouldn't use that if it was handy.

Anyway, having this here with no comment is confusing. Either repeat the original justification or get the process cache line size if there's a process.

lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py

25

It's a shame we don't have SBTarget.GetValueForVariablePath() - the equivalent of SBFrame.GetValueForVariablePath. If we did we could make a global variable version of lldbtest.expect_var_path, which would make this test more precise. But we don't...

29

Rather than a TODO, it's better to put in a test here, then expect it to fail. That way if someone comes upon this comment later on, they won't have to guess what didn't work for you. And if somebody fixes this while doing something else, the unexpected pass will let them know.

[labath]

Thanks for the review.

lldb/include/lldb/Target/Target.h

1028

I changed the value to true.

However, I've have copied this pattern from ReadMemory, ReadScalarIntegerFromMemory, etc. Should we be setting the value to true for those functions as well?

lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

58

I believe I've found the right incantation to do that. Please have a look at the new version.

lldb/source/Target/Target.cpp

1922

I've removed it. BTW, this was cargo-culted from Target::ReadCStringFromMemory, so it has the same bug. I'll create a patch to just delete that whole function

1924

I've created a helper function which returns a (properly alinged/rounded) size if we have a process, and a fixed value if we don't. Let me know what you think.

[labath]

New version.

[jingham]

Something seems to have happened to reviews.llvm.org. I can sort of see patches, but the interface isn't responding, and I can't actually make comments there, so we'll try this way instead.

Your changes look good, the "ReasonableSizeForReading" reasonable.

About the "force_live_memory" thing. As I understand it, this is really one of those "why we can't have nice things" deals that I think we haven't fully embraced. It would be very nice to not have to go to the device for read-only loaded sections, since in some situations it can be much faster. OTOH, the security folks are right, silently suppressing these differences hides something that, while it almost never happens, matters a lot when it does. There's no other reason that I can think of to choose to read read-only sections from memory rather than from the binary.

Maybe this is more the sort of thing that should be a policy controlled by a Target setting instead of variables passed around and set different ways by different functions? That way security conscious folks can force the always read behavior, and everybody else can work a little faster?

Anyway, the patch looks good, and if I could get Phabricator to allow me to check "Accept Revision" and click the Submit button, I would do so.

Jim

lldb/include/lldb/DataFormatters/FormattersHelpers.h

 
 void AddFilter(TypeCategoryImpl::SharedPointer category_sp,
                std::vector<std::string> children, const char *description,
                ConstString type_name, ScriptedSyntheticChildren::Flags flags,
                bool regex = false);
 
 size_t ExtractIndexFromString(const char *item_name);
 
-lldb::addr_t GetArrayAddressOrPointerValue(ValueObject &valobj);
+Address GetArrayAddressOrPointerValue(ValueObject &valobj);
 
 lldb::ValueObjectSP GetValueOfLibCXXCompressedPair(ValueObject &pair);
 
 time_t GetOSXEpoch();
 
 struct InferiorSizedWord {
 
   InferiorSizedWord(const InferiorSizedWord &word) : ptr_size(word.ptr_size) {

lldb/include/lldb/DataFormatters/StringPrinter.h

 //===-- StringPrinter.h -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_DATAFORMATTERS_STRINGPRINTER_H
 #define LLDB_DATAFORMATTERS_STRINGPRINTER_H
 
 #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 {
 class StringPrinter {
 public:
   enum class StringElementType { ASCII, UTF8, UTF16, UTF32 };
 
   enum class GetPrintableElementType { ASCII, UTF8 };
public:
   };
 
   class ReadStringAndDumpToStreamOptions : public DumpToStreamOptions {
   public:
     ReadStringAndDumpToStreamOptions() = default;
 
     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;
   };
 
   class ReadBufferAndDumpToStreamOptions : public DumpToStreamOptions {
   public:
     ReadBufferAndDumpToStreamOptions() = default;
 

lldb/include/lldb/Target/Process.h

/// \see Process::Notifications
   ///     The number of bytes that were actually read into \a buf. If
   ///     the returned number is greater than zero, yet less than \a
   ///     size, then this function will get called again with \a
   ///     vm_addr, \a buf, and \a size updated appropriately. Zero is
   ///     returned in the case of an error.
   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
   /// C string terminator is found. It will stop reading if the NULL
   /// termination byte isn't found before reading \a cstr_max_len bytes, and
   /// the results are always guaranteed to be NULL terminated (at most
   /// cstr_max_len - 1 bytes will be read).
   size_t ReadCStringFromMemory(lldb::addr_t vm_addr, char *cstr,

lldb/include/lldb/Target/Target.h

size_t ReadMemory(const Address &addr, void *dst, size_t dst_len,
                     lldb::addr_t *load_addr_ptr = nullptr);
 
   size_t ReadCStringFromMemory(const Address &addr, std::string &out_str,
                                Status &error);
 
   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,
                                      bool force_live_memory = false);
 
   uint64_t ReadUnsignedIntegerFromMemory(const Address &addr,
                                          size_t integer_byte_size,
                                          uint64_t fail_value, Status &error,
private:
 
   // Copy breakpoints, stop hooks and so forth from the dummy target:
   void PrimeFromDummyTarget(Target &target);
 
   void AddBreakpoint(lldb::BreakpointSP breakpoint_sp, bool internal);
 
   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;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_TARGET_H

lldb/source/DataFormatters/FormattersHelpers.cpp

 //===-- FormattersHelpers.cpp ---------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 
 
 
 #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"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/RegularExpression.h"
 
 using namespace lldb;
 using namespace lldb_private;
size_t lldb_private::formatters::ExtractIndexFromString(const char *item_name) {
   unsigned long int idx = ::strtoul(item_name, &endptr, 0);
   if (idx == 0 && endptr == item_name)
     return UINT32_MAX;
   if (idx == ULONG_MAX)
     return UINT32_MAX;
   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;
 }
 
 lldb::ValueObjectSP
 lldb_private::formatters::GetValueOfLibCXXCompressedPair(ValueObject &pair) {
   ValueObjectSP value =
       pair.GetChildMemberWithName(ConstString("__value_"), true);
   if (!value) {
     // pre-r300140 member name
     value = pair.GetChildMemberWithName(ConstString("__first_"), true);
   }
   return value;
 }

lldb/source/DataFormatters/StringPrinter.cpp

static bool ReadEncodedBufferAndDumpToStream(
   assert(options.GetStream() && "need a Stream to print the string to");
   if (!options.GetStream())
     return false;
 
   if (options.GetLocation() == 0 ||
       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);
   constexpr int origin_encoding = 8 * type_width;
   if (origin_encoding != 8 && origin_encoding != 16 && origin_encoding != 32)
     return false;
   // If not UTF8 or ASCII, conversion to UTF8 is necessary.
   if (origin_encoding != 8 && !ConvertFunction)
     return false;
 
   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()) {
     // FIXME: The NSString formatter sets HasSourceSize(true) when the size is
     // actually unknown, as well as SetBinaryZeroIsTerminator(false). IIUC the
     // C++ formatter also sets SetBinaryZeroIsTerminator(false) when it doesn't
     // mean to. I don't see how this makes sense: we should fix the formatters.
     //
static bool ReadEncodedBufferAndDumpToStream(
   // an empty string ("").
   if (sourceSize != 0 && !buffer_sp->GetBytes())
     return false;
 
   Status error;
   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);
   if (needs_zero_terminator)
     dump_options.SetBinaryZeroIsTerminator(true);
 
   GetPrintableElementType print_style = (elem_type == StringElementType::ASCII)
                                             ? GetPrintableElementType::ASCII

lldb/source/Plugins/Language/CPlusPlus/CxxStringTypes.cpp

case StringElementType::UTF32:
     return std::make_pair("U", lldb::eFormatUnicode32);
   default:
     return std::make_pair(nullptr, lldb::eFormatInvalid);
   }
 }
 
 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);
 
   if (!StringPrinter::ReadStringAndDumpToStream<ElemType>(options))
     stream.Printf("Summary Unavailable");
 
   return true;
 }

 
 bool lldb_private::formatters::Char32StringSummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &) {
   return CharStringSummaryProvider<StringElementType::UTF32>(valobj, stream);
 }
 
 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
   CompilerType wchar_compiler_type =
       valobj.GetCompilerType().GetBasicTypeFromAST(lldb::eBasicTypeWChar);
 
   if (!wchar_compiler_type)
     return false;
 
   // Safe to pass nullptr for exe_scope here.
   llvm::Optional<uint64_t> size = wchar_compiler_type.GetBitSize(nullptr);
   if (!size)
     return false;
   const uint32_t wchar_size = *size;
 
   StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
   options.SetLocation(valobj_addr);
-  options.SetProcessSP(process_sp);
+  options.SetTargetSP(valobj.GetTargetSP());
   options.SetStream(&stream);
   options.SetPrefixToken("L");
 
   switch (wchar_size) {
   case 8:
     return StringPrinter::ReadStringAndDumpToStream<StringElementType::UTF8>(
         options);
   case 16:

lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp

case eAddressTypeLoad: {
       StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
       Status error;
       lldb::addr_t addr_of_data =
           process_sp->ReadPointerFromMemory(addr_of_string, error);
       if (error.Fail() || addr_of_data == 0 ||
           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);
       lldb::addr_t size_of_data = process_sp->ReadPointerFromMemory(
           addr_of_string + process_sp->GetAddressByteSize(), error);
       if (error.Fail())
         return false;
       options.SetSourceSize(size_of_data);
case eAddressTypeLoad: {
       StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
       Status error;
       lldb::addr_t addr_of_data =
           process_sp->ReadPointerFromMemory(addr_of_string, error);
       if (error.Fail() || addr_of_data == 0 ||
           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);
       lldb::addr_t size_of_data = process_sp->ReadPointerFromMemory(
           addr_of_string + process_sp->GetAddressByteSize(), error);
       if (error.Fail())
         return false;
       options.SetSourceSize(size_of_data);

lldb/source/Plugins/Language/ObjC/NSString.cpp

bool lldb_private::formatters::NSStringSummaryProvider(
 
   if (is_mutable) {
     uint64_t location = 2 * ptr_size + valobj_addr;
     location = process_sp->ReadPointerFromMemory(location, error);
     if (error.Fail())
       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);
       options.SetHasSourceSize(has_explicit_length);
       options.SetNeedsZeroTermination(false);
       options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                  TypeSummaryCapping::eTypeSummaryUncapped);
       options.SetBinaryZeroIsTerminator(false);
       return StringPrinter::ReadStringAndDumpToStream<
           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);
       options.SetNeedsZeroTermination(false);
       options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                  TypeSummaryCapping::eTypeSummaryUncapped);
       options.SetBinaryZeroIsTerminator(false);
       return StringPrinter::ReadStringAndDumpToStream<
           StringPrinter::StringElementType::ASCII>(options);
     }
   } else if (is_inline && has_explicit_length && !is_unicode &&
              !is_path_store && !is_mutable) {
     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);
     options.SetHasSourceSize(has_explicit_length);
     options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                TypeSummaryCapping::eTypeSummaryUncapped);
     return StringPrinter::ReadStringAndDumpToStream<
         StringPrinter::StringElementType::ASCII>(options);
   } else if (is_unicode) {
     uint64_t location = valobj_addr + 2 * ptr_size;
     if (is_inline) {
       if (!has_explicit_length) {
         return false;
       } else
         location += ptr_size;
     } else {
       location = process_sp->ReadPointerFromMemory(location, error);
       if (error.Fail())
         return false;
     }
     options.SetLocation(location);
-    options.SetProcessSP(process_sp);
+    options.SetTargetSP(valobj.GetTargetSP());
     options.SetStream(&stream);
     options.SetQuote('"');
     options.SetSourceSize(explicit_length);
     options.SetHasSourceSize(has_explicit_length);
     options.SetNeedsZeroTermination(!has_explicit_length);
     options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                TypeSummaryCapping::eTypeSummaryUncapped);
     options.SetBinaryZeroIsTerminator(!has_explicit_length);
     return StringPrinter::ReadStringAndDumpToStream<
         StringPrinter::StringElementType::UTF16>(options);
   } else if (is_path_store) {
     ProcessStructReader reader(valobj.GetProcessSP().get(),
                                valobj.GetValueAsUnsigned(0),
                                GetNSPathStore2Type(*valobj.GetTargetSP()));
     explicit_length =
         reader.GetField<uint32_t>(ConstString("lengthAndRef")) >> 20;
     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);
     options.SetHasSourceSize(has_explicit_length);
     options.SetNeedsZeroTermination(!has_explicit_length);
     options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                TypeSummaryCapping::eTypeSummaryUncapped);
     options.SetBinaryZeroIsTerminator(!has_explicit_length);
     return StringPrinter::ReadStringAndDumpToStream<
         StringPrinter::StringElementType::UTF16>(options);
   } else if (is_inline) {
     uint64_t location = valobj_addr + 2 * ptr_size;
     if (!has_explicit_length) {
       // in this kind of string, the byte before the string content is a length
       // byte so let's try and use it to handle the embedded NUL case
       Status error;
       explicit_length =
           process_sp->ReadUnsignedIntegerFromMemory(location, 1, 0, error);
       has_explicit_length = !(error.Fail() || explicit_length == 0);
       location++;
     }
     options.SetLocation(location);
-    options.SetProcessSP(process_sp);
+    options.SetTargetSP(valobj.GetTargetSP());
     options.SetStream(&stream);
     options.SetSourceSize(explicit_length);
     options.SetHasSourceSize(has_explicit_length);
     options.SetNeedsZeroTermination(!has_explicit_length);
     options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                TypeSummaryCapping::eTypeSummaryUncapped);
     options.SetBinaryZeroIsTerminator(!has_explicit_length);
     if (has_explicit_length)
       return StringPrinter::ReadStringAndDumpToStream<
           StringPrinter::StringElementType::UTF8>(options);
     else
       return StringPrinter::ReadStringAndDumpToStream<
           StringPrinter::StringElementType::ASCII>(options);
   } else {
     uint64_t location = valobj_addr + 2 * ptr_size;
     location = process_sp->ReadPointerFromMemory(location, error);
     if (error.Fail())
       return false;
     if (has_explicit_length && !has_null)
       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);
     options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                TypeSummaryCapping::eTypeSummaryUncapped);
     return StringPrinter::ReadStringAndDumpToStream<
         StringPrinter::StringElementType::ASCII>(options);
   }

lldb/source/Target/Process.cpp

while (true) {
     if (length == sizeof(buf) - 1)
       curr_addr += length;
     else
       break;
   }
   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,
                                       size_t dst_max_len,
                                       Status &result_error) {
   size_t total_cstr_len = 0;
   if (dst && dst_max_len) {
     result_error.Clear();

lldb/source/Target/Target.cpp

if (dst && dst_max_len) {
     if (dst == nullptr)
       result_error.SetErrorString("invalid arguments");
     else
       result_error.Clear();
   }
   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,
                                            bool force_live_memory) {
   uint64_t uval;
 
   if (byte_size <= sizeof(uval)) {
     size_t bytes_read =

lldb/test/API/lang/cpp/char8_t/TestCxxChar8_t.py

 class CxxChar8_tTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @skipIf(compiler="clang", compiler_version=['<', '7.0'])
     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"你好"')