[Lldb-commits] [lldb] 7b24425 - Reland [lldb] Fix string summary of an empty NSPathStore2
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 19 10:50:57 PDT 2020
Author: Raphael Isemann
Date: 2020-03-19T18:50:26+01:00
New Revision: 7b2442584e40f97693c38c0d79b83f770d557039
URL: https://github.com/llvm/llvm-project/commit/7b2442584e40f97693c38c0d79b83f770d557039
DIFF: https://github.com/llvm/llvm-project/commit/7b2442584e40f97693c38c0d79b83f770d557039.diff
LOG: Reland [lldb] Fix string summary of an empty NSPathStore2
(This is D68010 but I also set the new parameter in LibStdcpp.cpp to fix
the Debian tests).
Summary:
Printing a summary for an empty NSPathStore2 string currently prints random bytes behind the empty string pointer from memory (rdar://55575888).
It seems the reason for this is that the SourceSize parameter in the `ReadStringAndDumpToStreamOptions` - which is supposed to contain the string
length - actually uses the length 0 as a magic value for saying "read as much as possible from the buffer" which is clearly wrong for empty strings.
This patch adds another flag that indicates if we have know the string length or not and makes this behaviour dependent on that (which seemingly
was the original purpose of this magic value).
Reviewers: aprantl, JDevlieghere, shafik
Reviewed By: aprantl
Subscribers: christof, abidh, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D68010
Added:
Modified:
lldb/include/lldb/DataFormatters/StringPrinter.h
lldb/source/DataFormatters/StringPrinter.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/ObjC/NSString.cpp
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
Removed:
################################################################################
diff --git a/lldb/include/lldb/DataFormatters/StringPrinter.h b/lldb/include/lldb/DataFormatters/StringPrinter.h
index 6f8869cc2a1e..5842cde893d8 100644
--- a/lldb/include/lldb/DataFormatters/StringPrinter.h
+++ b/lldb/include/lldb/DataFormatters/StringPrinter.h
@@ -115,9 +115,15 @@ class StringPrinter {
lldb::ProcessSP GetProcessSP() const { return m_process_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;
+ /// True iff we know the source size of the string.
+ bool m_has_source_size = false;
};
class ReadBufferAndDumpToStreamOptions : public DumpToStreamOptions {
diff --git a/lldb/source/DataFormatters/StringPrinter.cpp b/lldb/source/DataFormatters/StringPrinter.cpp
index 92dd71d17b8c..4515b67b2adf 100644
--- a/lldb/source/DataFormatters/StringPrinter.cpp
+++ b/lldb/source/DataFormatters/StringPrinter.cpp
@@ -525,27 +525,33 @@ static bool ReadUTFBufferAndDumpToStream(
if (!options.GetStream())
return false;
- uint32_t sourceSize = options.GetSourceSize();
+ uint32_t sourceSize;
bool needs_zero_terminator = options.GetNeedsZeroTermination();
bool is_truncated = false;
const auto max_size = process_sp->GetTarget().GetMaximumSizeOfStringSummary();
- if (!sourceSize) {
+ if (options.HasSourceSize()) {
+ sourceSize = options.GetSourceSize();
+ if (!options.GetIgnoreMaxLength()) {
+ if (sourceSize > max_size) {
+ sourceSize = max_size;
+ is_truncated = true;
+ }
+ }
+ } else {
sourceSize = max_size;
needs_zero_terminator = true;
- } else if (!options.GetIgnoreMaxLength()) {
- if (sourceSize > max_size) {
- sourceSize = max_size;
- is_truncated = true;
- }
}
const int bufferSPSize = sourceSize * type_width;
lldb::DataBufferSP buffer_sp(new DataBufferHeap(bufferSPSize, 0));
- if (!buffer_sp->GetBytes())
+ // Check if we got bytes. We never get any bytes if we have an empty
+ // string, but we still continue so that we end up actually printing
+ // an empty string ("").
+ if (sourceSize != 0 && !buffer_sp->GetBytes())
return false;
Status error;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 78f58754cc31..b4af67ecee0d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -259,6 +259,7 @@ bool lldb_private::formatters::LibStdcppStringSummaryProvider(
if (error.Fail())
return false;
options.SetSourceSize(size_of_data);
+ options.SetHasSourceSize(true);
if (!StringPrinter::ReadStringAndDumpToStream<
StringPrinter::StringElementType::UTF8>(options)) {
@@ -319,6 +320,7 @@ bool lldb_private::formatters::LibStdcppWStringSummaryProvider(
if (error.Fail())
return false;
options.SetSourceSize(size_of_data);
+ options.SetHasSourceSize(true);
options.SetPrefixToken("L");
switch (wchar_size) {
diff --git a/lldb/source/Plugins/Language/ObjC/NSString.cpp b/lldb/source/Plugins/Language/ObjC/NSString.cpp
index 65256dc7acbd..7c4afb36b588 100644
--- a/lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -170,6 +170,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
options.SetStream(&stream);
options.SetQuote('"');
options.SetSourceSize(explicit_length);
+ options.SetHasSourceSize(has_explicit_length);
options.SetNeedsZeroTermination(false);
options.SetIgnoreMaxLength(summary_options.GetCapping() ==
TypeSummaryCapping::eTypeSummaryUncapped);
@@ -182,6 +183,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
options.SetProcessSP(process_sp);
options.SetStream(&stream);
options.SetSourceSize(explicit_length);
+ options.SetHasSourceSize(has_explicit_length);
options.SetNeedsZeroTermination(false);
options.SetIgnoreMaxLength(summary_options.GetCapping() ==
TypeSummaryCapping::eTypeSummaryUncapped);
@@ -199,6 +201,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
options.SetStream(&stream);
options.SetQuote('"');
options.SetSourceSize(explicit_length);
+ options.SetHasSourceSize(has_explicit_length);
options.SetIgnoreMaxLength(summary_options.GetCapping() ==
TypeSummaryCapping::eTypeSummaryUncapped);
options.SetLanguage(summary_options.GetLanguage());
@@ -221,6 +224,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
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);
@@ -241,6 +245,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
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);
@@ -263,6 +268,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
options.SetProcessSP(process_sp);
options.SetStream(&stream);
options.SetSourceSize(explicit_length);
+ options.SetHasSourceSize(has_explicit_length);
options.SetNeedsZeroTermination(!has_explicit_length);
options.SetIgnoreMaxLength(summary_options.GetCapping() ==
TypeSummaryCapping::eTypeSummaryUncapped);
@@ -286,6 +292,7 @@ bool lldb_private::formatters::NSStringSummaryProvider(
options.SetProcessSP(process_sp);
options.SetStream(&stream);
options.SetSourceSize(explicit_length);
+ options.SetHasSourceSize(has_explicit_length);
options.SetIgnoreMaxLength(summary_options.GetCapping() ==
TypeSummaryCapping::eTypeSummaryUncapped);
options.SetLanguage(summary_options.GetLanguage());
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
index 4ef0a5957503..5b323f5614b2 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
@@ -76,8 +76,8 @@ def rdar11106605_commands(self):
self.expect('frame variable hebrew', substrs=['לילה טוב'])
def nsstring_data_formatter_commands(self):
- self.expect('frame variable str0 str1 str2 str3 str4 str5 str6 str8 str9 str10 str11 label1 label2 processName str12',
- substrs=[
+ self.expect('frame variable empty str0 str1 str2 str3 str4 str5 str6 str8 str9 str10 str11 label1 label2 processName str12',
+ substrs=['(NSString *) empty = ', ' @""',
# '(NSString *) str0 = ',' @"255"',
'(NSString *) str1 = ', ' @"A rather short ASCII NSString object is here"',
'(NSString *) str2 = ', ' @"A rather short UTF8 NSString object is here"',
@@ -104,6 +104,8 @@ def nsstring_data_formatter_commands(self):
self.expect('expr -d run-target -- path', substrs=['usr/blah/stuff'])
self.expect('frame variable path', substrs=['usr/blah/stuff'])
+ self.expect('expr -d run-target -- empty_path', substrs=['@""'])
+ self.expect('frame variable empty_path', substrs=['@""'])
def nsstring_withNULs_commands(self):
"""Check that the NSString formatter supports embedded NULs in the text"""
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
index 576e091db1bc..0787561e4da3 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
@@ -17,6 +17,7 @@ int main (int argc, const char * argv[])
NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+ NSString *empty = @"";
NSString *str0 = [[NSNumber numberWithUnsignedLongLong:0xFF] stringValue];
NSString *str1 = [NSString stringWithCString:"A rather short ASCII NSString object is here" encoding:NSASCIIStringEncoding];
NSString *str2 = [NSString stringWithUTF8String:"A rather short UTF8 NSString object is here"];
@@ -69,6 +70,7 @@ int main (int argc, const char * argv[])
NSArray *components = @[@"usr", @"blah", @"stuff"];
NSString *path = [NSString pathWithComponents: components];
+ NSString *empty_path = [empty stringByDeletingPathExtension];
const unichar someOfTheseAreNUL[] = {'a',' ', 'v','e','r','y',' ',
'm','u','c','h',' ','b','o','r','i','n','g',' ','t','a','s','k',
More information about the lldb-commits
mailing list