teemperor created this revision.
teemperor added reviewers: aprantl, JDevlieghere, shafik.
Herald added subscribers: lldb-commits, abidh, christof.
Herald added a project: LLDB.

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). As this code is shared between 
all string reading, this patch also adds tests for printing empty strings
to all string type tests that seem to use this code path.

Note that I'm aware that some of the surrounding code that I touch here is 
spaghetti which should be refactored. However, I would prefer if we could
backport this patch without also having to backport a series of refactoring 
patches, so let's just add sauce to that pasta for now and I'll do the 
refactoring
once this has landed.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D68010

Files:
  lldb/include/lldb/DataFormatters/StringPrinter.h
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
  lldb/source/DataFormatters/StringPrinter.cpp
  lldb/source/Plugins/Language/ObjC/NSString.cpp

Index: lldb/source/Plugins/Language/ObjC/NSString.cpp
===================================================================
--- lldb/source/Plugins/Language/ObjC/NSString.cpp
+++ lldb/source/Plugins/Language/ObjC/NSString.cpp
@@ -171,6 +171,7 @@
       options.SetStream(&stream);
       options.SetQuote('"');
       options.SetSourceSize(explicit_length);
+      options.SetHasSourceSize(has_explicit_length);
       options.SetNeedsZeroTermination(false);
       options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                  TypeSummaryCapping::eTypeSummaryUncapped);
@@ -183,6 +184,7 @@
       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);
@@ -200,6 +202,7 @@
     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());
@@ -222,6 +225,7 @@
     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);
@@ -242,6 +246,7 @@
     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);
@@ -264,6 +269,7 @@
     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);
@@ -287,6 +293,7 @@
     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());
Index: lldb/source/DataFormatters/StringPrinter.cpp
===================================================================
--- lldb/source/DataFormatters/StringPrinter.cpp
+++ lldb/source/DataFormatters/StringPrinter.cpp
@@ -543,7 +543,7 @@
   bool is_truncated = false;
   const auto max_size = process_sp->GetTarget().GetMaximumSizeOfStringSummary();
 
-  if (!sourceSize) {
+  if (!options.GetHasSourceSize()) {
     sourceSize = max_size;
     needs_zero_terminator = true;
   } else if (!options.GetIgnoreMaxLength()) {
@@ -557,7 +557,7 @@
 
   lldb::DataBufferSP buffer_sp(new DataBufferHeap(bufferSPSize, 0));
 
-  if (!buffer_sp->GetBytes())
+  if (!buffer_sp->GetBytes() && sourceSize != 0)
     return false;
 
   Status error;
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -2,9 +2,11 @@
 
 int main()
 {
+    std::wstring wempty(L"");
     std::wstring s(L"hello world! מזל טוב!");
     std::wstring S(L"!!!!");
     const wchar_t *mazeltov = L"מזל טוב";
+    std::string empty("");
     std::string q("hello world");
     std::string Q("quite a long std::strin with lots of info inside it");
     S.assign(L"!!!!!"); // Set break point at this line.
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -52,12 +52,15 @@
         # Execute the cleanup function during test case tear down.
         self.addTearDownHook(cleanup)
 
+        var_wempty = self.frame().FindVariable('wempty')
         var_s = self.frame().FindVariable('s')
         var_S = self.frame().FindVariable('S')
         var_mazeltov = self.frame().FindVariable('mazeltov')
+        var_empty = self.frame().FindVariable('empty')
         var_q = self.frame().FindVariable('q')
         var_Q = self.frame().FindVariable('Q')
 
+        self.assertTrue(var_wempty.GetSummary() == 'L""', "wempty summary wrong")
         self.assertTrue(
             var_s.GetSummary() == 'L"hello world! מזל טוב!"',
             "s summary wrong")
@@ -65,6 +68,7 @@
         self.assertTrue(
             var_mazeltov.GetSummary() == 'L"מזל טוב"',
             "mazeltov summary wrong")
+        self.assertTrue(var_empty.GetSummary() == '""', "empty summary wrong")
         self.assertTrue(
             var_q.GetSummary() == '"hello world"',
             "q summary wrong")
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -2,16 +2,20 @@
 
 int main()
 {
+    std::wstring wempty(L"");
     std::wstring s(L"hello world! מזל טוב!");
     std::wstring S(L"!!!!");
     const wchar_t *mazeltov = L"מזל טוב";
+    std::string empty("");
     std::string q("hello world");
     std::string Q("quite a long std::strin with lots of info inside it");
     std::string TheVeryLongOne("1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890someText1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890");
     std::string IHaveEmbeddedZeros("a\0b\0c\0d",7);
     std::wstring IHaveEmbeddedZerosToo(L"hello world!\0てざ ル゜䋨ミ㠧槊 きゅへ狦穤襩 じゃ馩リョ 䤦監", 38);
     std::u16string u16_string(u"ß水氶");
+    std::u16string u16_empty(u"");
     std::u32string u32_string(U"🍄🍅🍆🍌");
+    std::u32string u32_empty(U"");
     S.assign(L"!!!!!"); // Set break point at this line.
     return 0;
 }
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -59,16 +59,23 @@
         self.expect(
             "frame variable",
             substrs=[
+                '(%s::wstring) wempty = L""'%ns,
                 '(%s::wstring) s = L"hello world! מזל טוב!"'%ns,
                 '(%s::wstring) S = L"!!!!"'%ns,
                 '(const wchar_t *) mazeltov = 0x',
                 'L"מזל טוב"',
+                '(%s::string) empty = ""'%ns,
                 '(%s::string) q = "hello world"'%ns,
                 '(%s::string) Q = "quite a long std::strin with lots of info inside it"'%ns,
                 '(%s::string) IHaveEmbeddedZeros = "a\\0b\\0c\\0d"'%ns,
                 '(%s::wstring) IHaveEmbeddedZerosToo = L"hello world!\\0てざ ル゜䋨ミ㠧槊 きゅへ狦穤襩 じゃ馩リョ 䤦監"'%ns,
                 '(%s::u16string) u16_string = u"ß水氶"'%ns,
-                '(%s::u32string) u32_string = U"🍄🍅🍆🍌"'%ns])
+                # FIXME: This should have a 'u' prefix.
+                '(%s::u16string) u16_empty = ""'%ns,
+                '(%s::u32string) u32_string = U"🍄🍅🍆🍌"'%ns,
+                # FIXME: This should have a 'U' prefix.
+                '(%s::u32string) u32_empty = ""'%ns
+])
 
         self.runCmd("n")
 
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsstring/main.m
@@ -24,7 +24,7 @@
 {
     
     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"];
@@ -77,6 +77,7 @@
 
 	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',
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py
@@ -77,8 +77,9 @@
         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=['(NSString *) str1 = ', ' @"A rather short ASCII NSString object is here"',
+        self.expect('frame variable empty str0 str1 str2 str3 str4 str5 str6 str8 str9 str10 str11 label1 label2 processName str12',
+                    substrs=['(NSString *) empty = ', ' @""',
+                             '(NSString *) str1 = ', ' @"A rather short ASCII NSString object is here"',
                              # '(NSString *) str0 = ',' @"255"',
                              '(NSString *) str1 = ', ' @"A rather short ASCII NSString object is here"',
                              '(NSString *) str2 = ', ' @"A rather short UTF8 NSString object is here"',
@@ -105,6 +106,8 @@
 
         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"""
Index: lldb/include/lldb/DataFormatters/StringPrinter.h
===================================================================
--- lldb/include/lldb/DataFormatters/StringPrinter.h
+++ lldb/include/lldb/DataFormatters/StringPrinter.h
@@ -89,6 +89,13 @@
 
     uint32_t GetSourceSize() const { return m_source_size; }
 
+    ReadStringAndDumpToStreamOptions &SetHasSourceSize(bool e) {
+      m_has_source_size = e;
+      return *this;
+    }
+
+    bool GetHasSourceSize() const { return m_has_source_size; }
+
     ReadStringAndDumpToStreamOptions &SetNeedsZeroTermination(bool z) {
       m_needs_zero_termination = z;
       return *this;
@@ -135,6 +142,8 @@
     std::string m_suffix_token;
     char m_quote = '"';
     uint32_t m_source_size = 0;
+    /// True if we know the source size of the string.
+    bool m_has_source_size = false;
     bool m_needs_zero_termination = true;
     bool m_escape_non_printables = true;
     bool m_ignore_max_length = false;
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to