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