[lldb] Print embedded nuls in char arrays (PR44649)

[labath]

Summary

When we know the bounds of the array, print any embedded nuls instead of
treating them as terminators. An exception to this rule is made for the
nul character at the very end of the string. We don't print that, as
otherwise 99% of the strings would end in \0. This way the strings
usually come out the same as how the user typed it into the compiler
(char foo[] = "with\0nuls"). It also matches how they come out in gdb.

This resolves a FIXME left from D111399, and leaves another FIXME for dealing
with nul characters in "escape-non-printables=false" mode. In this mode the
characters cause the entire summary string to be terminated prematurely.

[teemperor]

LGTM

lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp

41

I think the whole 'empty lines break inline tests' thing has been fixed at some point, so could you drop a new line between this and the printable thing when you land?

[labath]

lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp

41

sounds good

[kastiglione]

An exception to this rule is made for the nul character at the very end of the string.

@labath What do you think of hiding all trailing nul characters, not just the final one? For example consider:

char buf[16] = "hello world";
# v buf
# (char[16]) buf = "hello world\0\0\0\0"

1. This shows 4 out of 5 nuls, which strikes me as odd
2. None of the nuls hide any further data, so it seems friendly enough to print up to the last nul

thanks

[labath]

Yeah, I suppose that makes sense. Like, I don't think we can ever have a single solution that does exactly what people would expect in every possible situation. The only counterargument I can think of is "but gdb does it that way", but I don't think that's a particularly strong one in this case.

lldb/source/Core/ValueObject.cpp

bool ValueObject::SetData(DataExtractor &data, Status &error) {
   // If we have reached this point, then we have successfully changed the
   // value.
   SetNeedsUpdate();
   return true;
 }
 
 static bool CopyStringDataToBufferSP(const StreamString &source,
                                      lldb::DataBufferSP &destination) {
-  destination = std::make_shared<DataBufferHeap>(source.GetSize() + 1, 0);
-  memcpy(destination->GetBytes(), source.GetString().data(), source.GetSize());
+  llvm::StringRef src = source.GetString();
+  src.consume_back(llvm::StringRef("\0", 1));
+  destination = std::make_shared<DataBufferHeap>(src.size(), 0);
+  memcpy(destination->GetBytes(), src.data(), src.size());
   return true;
 }
 
 std::pair<size_t, bool>
 ValueObject::ReadPointedString(lldb::DataBufferSP &buffer_sp, Status &error,
                                uint32_t max_length, bool honor_array,
                                Format item_format) {
   bool was_capped = false;
if (cstr_address == 0 || cstr_address == LLDB_INVALID_ADDRESS) {
       if (cstr_address_type == eAddressTypeHost && is_array) {
         const char *cstr = GetDataExtractor().PeekCStr(0);
         if (cstr == nullptr) {
           s << "<invalid address>";
           error.SetErrorString("invalid address");
           CopyStringDataToBufferSP(s, buffer_sp);
           return {0, was_capped};
         }
-        buffer_sp = std::make_shared<DataBufferHeap>(cstr_len, 0);
-        memcpy(buffer_sp->GetBytes(), cstr, cstr_len);
+        s << llvm::StringRef(cstr, cstr_len);
+        CopyStringDataToBufferSP(s, buffer_sp);
         return {cstr_len, was_capped};
       } else {
         s << "<invalid address>";
         error.SetErrorString("invalid address");
         CopyStringDataToBufferSP(s, buffer_sp);
         return {0, was_capped};
       }
     }
if (flags.AnySet(eTypeIsArray | eTypeIsPointer) &&
         options.SetData(DataExtractor(
             buffer_sp, lldb::eByteOrderInvalid,
             8)); // none of this matters for a string - pass some defaults
         options.SetStream(&s);
         options.SetPrefixToken(nullptr);
         options.SetQuote('"');
         options.SetSourceSize(buffer_sp->GetByteSize());
         options.SetIsTruncated(read_string.second);
+        options.SetBinaryZeroIsTerminator(custom_format != eFormatVectorOfChar);
         formatters::StringPrinter::ReadBufferAndDumpToStream<
             lldb_private::formatters::StringPrinter::StringElementType::ASCII>(
             options);
         return !error.Fail();
       }
 
       if (custom_format == eFormatEnum)
         return false;

lldb/test/API/functionalities/data-formatter/builtin-formats/TestBuiltinFormats.py

def test(self):
 
         # binary
         self.assertIn("= 0b00000000000000000000000000000010\n", self.getFormatted("binary", "2"))
         self.assertIn("= 0b01100001\n", self.getFormatted("binary", "'a'"))
         self.assertIn(" = 0b10010001101000101011001111000\n", self.getFormatted("binary", "(__uint128_t)0x12345678ll"))
 
         # Different character arrays.
         # FIXME: Passing a 'const char *' will ignore any given format,
-        self.assertIn('= " \\U0000001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', self.getFormatted("character array", "cstring"))
-        self.assertIn('= " \\U0000001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', self.getFormatted("c-string", "cstring"))
+        self.assertIn(r'= " \U0000001b\a\b\f\n\r\t\vaA09\0"', self.getFormatted("character array", "cstring"))
+        self.assertIn(r'= " \U0000001b\a\b\f\n\r\t\vaA09\0"', self.getFormatted("c-string", "cstring"))
         self.assertIn(' = " \\e\\a\\b\\f\\n\\r\\t\\vaA09" " \\U0000001b\\a\\b\\f\\n\\r\\t\\vaA09"\n',
                       self.getFormatted("c-string", "(char *)cstring"))
         self.assertIn('=\n', self.getFormatted("c-string", "(__UINT64_TYPE__)0"))
 
         # Build a uint128_t that contains a series of characters in each byte.
         # First 8 byte of the uint128_t.
         cstring_chars1 = " \a\b\f\n\r\t\v"
         # Last 8 byte of the uint128_t.
def test(self):
         ostype_expr = "(__UINT64_TYPE__)0x"
         for c in cstring_chars2:
              ostype_expr += format(ord(c), "x").zfill(2)
         self.assertIn("= 'AZaz09\\e\\0'\n", self.getFormatted("OSType", ostype_expr))
 
         self.assertIn('= 0x2007080c0a0d090b415a617a30391b00\n', self.getFormatted("OSType", string_expr))
 
         # bytes
-        self.assertIn('= " \\U0000001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', self.getFormatted("bytes", "cstring"))
+        self.assertIn(r'= " \U0000001b\a\b\f\n\r\t\vaA09\0"', self.getFormatted("bytes", "cstring"))
 
         # bytes with ASCII
-        self.assertIn('= " \\U0000001b\\a\\b\\f\\n\\r\\t\\vaA09"\n', self.getFormatted("bytes with ASCII", "cstring"))
+        self.assertIn(r'= " \U0000001b\a\b\f\n\r\t\vaA09\0"', self.getFormatted("bytes with ASCII", "cstring"))
 
         # unicode16
         self.assertIn('= U+5678 U+1234\n', self.getFormatted("unicode16", "0x12345678"))
 
         # unicode32
         self.assertIn('= U+0x89abcdef U+0x01234567\n', self.getFormatted("unicode32", "(__UINT64_TYPE__)0x123456789ABCDEFll"))
 
         # address

lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp

 #include <string>
 #include <cstring>
 
 struct A {
   char data[4];
   char overflow[4];
 };
 
 int main (int argc, char const *argv[])
 {
-    A a, b;
+    A a, b, c;
     // Deliberately write past the end of data to test that the formatter stops
     // at the end of array.
     memcpy(a.data, "FOOBAR", 7);
     memcpy(b.data, "FO\0BAR", 7);
+    memcpy(c.data, "F\0O\0AR", 7);
     std::string stdstring("Hello\t\tWorld\nI am here\t\tto say hello\n"); //%self.addTearDownHook(lambda x: x.runCmd("setting set escape-non-printables true"))
     const char* constcharstar = stdstring.c_str();
     std::string longstring(
 "I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
 " is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
 " a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
 " for science, or something"
       "I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
       " is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
       " a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
       " for science, or something"
             "I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
             " is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
             " a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
             " for science, or something"
       );
     const char* longconstcharstar = longstring.c_str();
     return 0;     //% if self.TraceOn(): self.runCmd('frame variable')
     //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
     //% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+    //% self.expect_var_path("a.data", summary='"FOOB"')
+    //% self.expect_var_path("b.data", summary=r'"FO\0B"')
+    //% self.expect_var_path("c.data", summary=r'"F\0O"')
+    //%
     //% self.runCmd("setting set escape-non-printables false")
     //% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
     //% self.expect_var_path('constcharstar', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
     //% self.assertTrue(self.frame().FindVariable('longstring').GetSummary().endswith('"...'))
     //% self.assertTrue(self.frame().FindVariable('longconstcharstar').GetSummary().endswith('"...'))
-    //% self.expect_var_path("a.data", summary='"FOOB"')
-    // FIXME: Should this be "FO\0B" instead?
-    //% self.expect_var_path("b.data", summary='"FO"')
+    // FIXME: make "b.data" and "c.data" work sanely
 }
 

lldb/test/Shell/SymbolFile/DWARF/x86/DW_AT_const_value.s

 ## Variables specified via fixed-size forms. This is typical for gcc (9).
 # CHECK: (unsigned long) data1 = 47
 # CHECK: (unsigned long) data2 = 4742
 # CHECK: (unsigned long) data4 = 47424742
 # CHECK: (unsigned long) data8 = 4742474247424742
 ## Variables specified using string forms. This behavior purely speculative -- I
 ## don't know of any compiler that would represent character strings this way.
 # CHECK: (char [7]) string = "string"
-# CHECK: (char [7]) strp = "strp"
+# CHECK: (char [7]) strp = "strp\0\0"
 ## Bogus attribute form. Let's make sure we don't crash at least.
 # CHECK: (char [7]) ref4 = <empty constant data>
 ## A variable of pointer type.
 # CHECK: (unsigned long *) udata_ptr = 0xdeadbeefbaadf00d
 
         .section        .debug_abbrev,"",@progbits
         .byte   1                       # Abbreviation Code
         .byte   17                      # DW_TAG_compile_unit