llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Nerixyz (Nerixyz) <details> <summary>Changes</summary> `StringPrinter` had a special case for ASCII strings that indicated they'd know their size (`SetHasSourceSize(true)`), supplied zero as the size, but actually wanted the printer to search for a null terminator. The only user doing this was the `NSString` summary provider. The C++ formatters only use `SetHasSourceSize(true)` if they're formatting an array (https://github.com/llvm/llvm-project/pull/195514). I don't know much about `NSString` as I couldn't find good documentation on its layout and couldn't test it. The following is based on the code here. `NSString` used the `has_explicit_length` flag, however if that was set the formatter didn't always read that length. For example, if the info bits indicated that the string is null terminated (`has_null`), then the length wasn't read even if `has_explicit_length` was set. However, the formatter still used `SetHasSourceSize(true)` in that case. So without the previous workaround, we'd read zero bytes. This PR adds a `has_source_size` to indicate that we have actually read the size. --- Full diff: https://github.com/llvm/llvm-project/pull/196311.diff 2 Files Affected: - (modified) lldb/source/DataFormatters/StringPrinter.cpp (+1-11) - (modified) lldb/source/Plugins/Language/ObjC/NSString.cpp (+16-10) ``````````diff diff --git a/lldb/source/DataFormatters/StringPrinter.cpp b/lldb/source/DataFormatters/StringPrinter.cpp index 3a6a55e9d4e02..5c556bede3ae4 100644 --- a/lldb/source/DataFormatters/StringPrinter.cpp +++ b/lldb/source/DataFormatters/StringPrinter.cpp @@ -440,17 +440,7 @@ static bool ReadEncodedBufferAndDumpToStream( 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 SetZeroTermination(Ignore). IIUC the - // C++ formatter also sets SetZeroTermination(Ignore) when it doesn't - // mean to. I don't see how this makes sense: we should fix the formatters. - // - // Until then, the behavior that's expected for ASCII strings with unknown - // lengths is to read up to the max size and then null-terminate. Do that. - sourceSize = max_size; - needs_zero_terminator = true; - } else if (options.HasSourceSize()) { + if (options.HasSourceSize()) { sourceSize = options.GetSourceSize(); if (!options.GetIgnoreMaxLength()) { if (sourceSize > max_size) { diff --git a/lldb/source/Plugins/Language/ObjC/NSString.cpp b/lldb/source/Plugins/Language/ObjC/NSString.cpp index 7a295119bd031..7d1dbf2f7afcd 100644 --- a/lldb/source/Plugins/Language/ObjC/NSString.cpp +++ b/lldb/source/Plugins/Language/ObjC/NSString.cpp @@ -100,6 +100,10 @@ bool lldb_private::formatters::NSStringSummaryProvider( bool has_null = (info_bits & 8) == 8; size_t explicit_length = 0; + // Even though the explicit length flag is set, we might only want to print + // the string until the first null terminator. `has_source_size` indicates if + // the string printer should use the size we gave it. + bool has_source_size = false; if (!has_null && has_explicit_length && !is_path_store) { lldb::addr_t explicit_length_offset = 2 * ptr_size; if (is_mutable && !is_inline) @@ -117,6 +121,7 @@ bool lldb_private::formatters::NSStringSummaryProvider( explicit_length_offset = valobj_addr + explicit_length_offset; explicit_length = process_sp->ReadUnsignedIntegerFromMemory( explicit_length_offset, 4, 0, error); + has_source_size = true; } } @@ -150,7 +155,7 @@ bool lldb_private::formatters::NSStringSummaryProvider( options.SetStream(&stream); options.SetQuote('"'); options.SetSourceSize(explicit_length); - options.SetHasSourceSize(has_explicit_length); + options.SetHasSourceSize(has_source_size); options.SetZeroTermination(StringPrinter::ZeroTermination::Ignore); options.SetIgnoreMaxLength(summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryUncapped); @@ -161,7 +166,7 @@ bool lldb_private::formatters::NSStringSummaryProvider( options.SetTargetSP(valobj.GetTargetSP()); options.SetStream(&stream); options.SetSourceSize(explicit_length); - options.SetHasSourceSize(has_explicit_length); + options.SetHasSourceSize(has_source_size); options.SetZeroTermination(StringPrinter::ZeroTermination::Ignore); options.SetIgnoreMaxLength(summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryUncapped); @@ -177,7 +182,7 @@ bool lldb_private::formatters::NSStringSummaryProvider( options.SetStream(&stream); options.SetQuote('"'); options.SetSourceSize(explicit_length); - options.SetHasSourceSize(has_explicit_length); + options.SetHasSourceSize(has_source_size); options.SetIgnoreMaxLength(summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryUncapped); return StringPrinter::ReadStringAndDumpToStream< @@ -199,8 +204,8 @@ bool lldb_private::formatters::NSStringSummaryProvider( options.SetStream(&stream); options.SetQuote('"'); options.SetSourceSize(explicit_length); - options.SetHasSourceSize(has_explicit_length); - if (has_explicit_length) + options.SetHasSourceSize(has_source_size); + if (has_source_size) options.SetZeroTermination(StringPrinter::ZeroTermination::Ignore); else options.SetZeroTermination(StringPrinter::ZeroTermination::ZeroTerminate); @@ -227,8 +232,8 @@ bool lldb_private::formatters::NSStringSummaryProvider( options.SetStream(&stream); options.SetQuote('"'); options.SetSourceSize(explicit_length); - options.SetHasSourceSize(has_explicit_length); - if (has_explicit_length) + options.SetHasSourceSize(has_source_size); + if (has_source_size) options.SetZeroTermination(StringPrinter::ZeroTermination::Ignore); else options.SetZeroTermination(StringPrinter::ZeroTermination::ZeroTerminate); @@ -245,20 +250,21 @@ bool lldb_private::formatters::NSStringSummaryProvider( explicit_length = process_sp->ReadUnsignedIntegerFromMemory(location, 1, 0, error); has_explicit_length = !(error.Fail() || explicit_length == 0); + has_source_size = has_explicit_length; location++; } options.SetLocation(Address(location)); options.SetTargetSP(valobj.GetTargetSP()); options.SetStream(&stream); options.SetSourceSize(explicit_length); - options.SetHasSourceSize(has_explicit_length); - if (has_explicit_length) + options.SetHasSourceSize(has_source_size); + if (has_source_size) options.SetZeroTermination(StringPrinter::ZeroTermination::Ignore); else options.SetZeroTermination(StringPrinter::ZeroTermination::ZeroTerminate); options.SetIgnoreMaxLength(summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryUncapped); - if (has_explicit_length) + if (has_source_size) return StringPrinter::ReadStringAndDumpToStream< StringPrinter::StringElementType::UTF8>(options); else `````````` </details> https://github.com/llvm/llvm-project/pull/196311 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
