[lldb] Fix string summary of an empty NSPathStore2

[teemperor]

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).

[teemperor]

• Land the NFC test additions to other string classes as separate NFC commits.
• Rebase on the already landed refactoring.

[aprantl]

lldb/include/lldb/DataFormatters/StringPrinter.h

120

I don't know the LLDB rules that well: is the convention really GetHas instead of Has?

lldb/source/DataFormatters/StringPrinter.cpp

534–542

why unconditionally call GetSourceSize? Should we move it into the else?

554

Why call GetBytes() if sourceSize is 0? Should we reverse the two conditions?

[teemperor]

• GetHasSourceSize -> HasSourceSize
• Moved some code around according to feedback.

[shafik]

lldb/source/DataFormatters/StringPrinter.cpp

532

Why not just make this uint32_t making this auto gains us nothing but now I am left wondering do the types match later on when we use max_size.

[teemperor]

• Rebased the patch.

This has been (really) delayed because of the whole unit test I wanted to write, but I'll just land this as-is (as having this broken even longer just because I haven't written that new unit test seems silly).

lldb/include/lldb/DataFormatters/StringPrinter.h

public:
     void SetLocation(uint64_t l) { m_location = l; }
 
     uint64_t GetLocation() const { return m_location; }
 
     void SetProcessSP(lldb::ProcessSP p) { m_process_sp = p; }
 
     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 {
   public:
     ReadBufferAndDumpToStreamOptions() = default;
 
     ReadBufferAndDumpToStreamOptions(ValueObject &valobj);
 

lldb/source/DataFormatters/StringPrinter.cpp

if (origin_encoding != 8 && origin_encoding != 16 && origin_encoding != 32)
     return false;
   // if not UTF8, I need a conversion function to return proper UTF8
   if (origin_encoding != 8 && !ConvertFunction)
     return false;
 
   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) {
-    sourceSize = max_size;
-    needs_zero_terminator = true;
-  } else if (!options.GetIgnoreMaxLength()) {
+  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;
+  }
 
   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;
   char *buffer = reinterpret_cast<char *>(buffer_sp->GetBytes());
 
   if (needs_zero_terminator)
     process_sp->ReadStringFromMemory(options.GetLocation(), buffer,
                                      bufferSPSize, error, type_width);

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

if (is_mutable) {
     if (error.Fail())
       return false;
     if (has_explicit_length && is_unicode) {
       options.SetLocation(location);
       options.SetProcessSP(process_sp);
       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);
       options.SetLanguage(summary_options.GetLanguage());
       return StringPrinter::ReadStringAndDumpToStream<
           StringPrinter::StringElementType::UTF16>(options);
     } else {
       options.SetLocation(location + 1);
       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);
       options.SetBinaryZeroIsTerminator(false);
       options.SetLanguage(summary_options.GetLanguage());
       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.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());
     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.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);
     options.SetLanguage(summary_options.GetLanguage());
     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.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);
     options.SetLanguage(summary_options.GetLanguage());
     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.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);
     options.SetLanguage(summary_options.GetLanguage());
     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.SetStream(&stream);
     options.SetSourceSize(explicit_length);
+    options.SetHasSourceSize(has_explicit_length);
     options.SetIgnoreMaxLength(summary_options.GetCapping() ==
                                TypeSummaryCapping::eTypeSummaryUncapped);
     options.SetLanguage(summary_options.GetLanguage());
     return StringPrinter::ReadStringAndDumpToStream<
         StringPrinter::StringElementType::ASCII>(options);
   }
 }
 

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/TestDataFormatterNSString.py

def rdar11106605_commands(self):
         self.expect('frame variable french', substrs=[
                     'Que veut cette horde d\'esclaves, De traîtres, de rois conjurés?'])
         self.expect('frame variable german', substrs=[
                     'Über-Ich und aus den Ansprüchen der sozialen Umwelt'])
         self.expect('frame variable japanese', substrs=['色は匂へど散りぬるを'])
         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"',
                              '(NSString *) str3 = ', ' @"A string made with the at sign is here"',
                              '(NSString *) str4 = ', ' @"This is string number 4 right here"',
                              '(NSString *) str5 = ', ' @"{{1, 1}, {5, 5}}"',
                              '(NSString *) str6 = ', ' @"1ST"',
                              '(NSString *) str8 = ', ' @"hasVeryLongExtensionThisTimehasVeryLongExtensionThisTimehasVeryLongExtensionThisTimehasVeryLongExtensionThisTimehasVeryLongExtensionThisTimehasVeryLongExtensionThisTimehasVeryLongExtensionThisTimehasVeryLongExtensionThisTimehasVeryLongExtensionThisTime',
def nsstring_data_formatter_commands(self):
                 ' @"hello world from foo"',
                 '(NSAttributedString *) mutableAttrString = ',
                 ' @"hello world from foo"',
                 '(NSString *) mutableGetConst = ',
                 ' @"foo said this string needs to be very long so much longer than whatever other string has been seen ever before by anyone of the mankind that of course this is still not long enough given what foo our friend foo our lovely dearly friend foo desired of us so i am adding more stuff here for the sake of it and for the joy of our friend who is named guess what just foo. hence, dear friend foo, stay safe, your string is now  long enough to accommodate your testing need and I will make sure that if not we extend it with even more fuzzy random meaningless words pasted one after the other from a long tiresome friday evening spent working in my office. my office mate went home but I am still randomly typing just for the fun of seeing what happens of the length of a Mutable String in Cocoa if it goes beyond one byte.. so be it, dear foo"'])
 
         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"""
         self.expect(
             'po strwithNULs',
             substrs=['a very much boring task to write'])
         self.expect('expr [strwithNULs length]', substrs=['54'])
         self.expect('frame variable strwithNULs', substrs=[
                     '@"a very much boring task to write\\0a string this way!!'])
         self.expect('po strwithNULs2', substrs=[
                     'a very much boring task to write'])
         self.expect('expr [strwithNULs2 length]', substrs=['52'])
         self.expect('frame variable strwithNULs2', substrs=[
                     '@"a very much boring task to write\\0a string this way!!'])

lldb/test/API/functionalities/data-formatter/data-formatter-objc/nsstring/main.m

 #import <Carbon/Carbon.h>
 #endif
 
 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"];
       NSString *str3 = @"A string made with the at sign is here";
       NSString *str4 = [NSString stringWithFormat:@"This is string number %ld right here", (long)4];
       NSRect ns_rect_4str = {{1,1},{5,5}};
       NSString* str5 = NSStringFromRect(ns_rect_4str);
       NSString* str6 = [@"/usr/doc/README.1ST" pathExtension];
NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
 
       [mutableGetConst length];
       CFMutableStringRef mutable_string_ref = CFStringCreateMutable(NULL,100);
       CFStringAppend(mutable_string_ref, CFSTR("Wish ya knew"));
       CFStringRef cfstring_ref = CFSTR("HELLO WORLD");
 
   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',
       ' ','t','o',' ','w','r','i','t','e', 0, 'a', ' ', 's', 't', 'r', 'i', 'n', 'g', ' ',
       't','h','i','s',' ','w','a','y','!','!', 0x03C3, 0};
   NSString *strwithNULs = [NSString stringWithCharacters: someOfTheseAreNUL
                                            length: sizeof someOfTheseAreNUL / sizeof *someOfTheseAreNUL];