[lldb/StringPrinter] Avoid reading garbage in uninitialized strings

[vsk]

Summary

This patch fixes a few related out-of-bounds read bugs in the
string data formatters. These issues have to do with mishandling of un-
initialized strings. These manifest as ASan exceptions when debugging a
clang binary.

The first issue was that the std::string formatter treated strings in
"short mode" with length greater than the size of the inline buffer as
valid.

The second issue was that the StringPrinter facility did not check that
a full utf8 codepoint sequence can be read from the buffer (i.e. there
are some missing range checks). I took the opportunity here to delete
some untested code that was meant to deal with invalid input and replace
it with fail-on-invalid logic ([1][2][3]). This means we'll give up on
formatting an invalid string instead of guessing our way through it.

The third issue is that StringPrinter did not check that a utf8 sequence
could actually be fully read from the string payload. This one is especially
tricky as we may overflow the buffer pointer while reading the sequence.

I also noticed that the std::string formatter would spew the raw version of
the underlying ValueObject when garbage is detected. I've changed this to
just print "Summary Unavailable" instead, as we do elsewhere.

I've added regression tests for these issues to
test/functionalities/data-formatter/data-formatter-stl/libcxx/string.

[1]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L136
[2]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L163
[3]
http://lab.llvm.org:8080/coverage/coverage-reports/coverage/Users/buildslave/jenkins/workspace/coverage/llvm-project/lldb/source/DataFormatters/StringPrinter.cpp.html#L357

rdar://59080026

[shafik]

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

76

While I get what you are doing here, we know he structure of libc++ SSO implementation and we are manually building a corrupt one, this is fragile to changes in the implementation.

I don't have an immediate suggestion for an alternative approach but if we stick with this we should stick a big comment explaining this, perhaps laying out the assumptions of the internal layout we are assuming and maybe some sanity checks maybe using offsetof to verify fields exist and are where we expect them to be.

lldb/source/DataFormatters/StringPrinter.cpp

65

can we use reinterpret_cast as opposed to what is basically a C-style cast. This also has the advantage of pointing out potentially dangerous code for future persons refactoring this code.

[shafik]

Is this missing additions to TestDataFormatterLibcxxString.py?

[vsk]

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

76

I don't see how this is fragile. The structure of libc++'s SSO implementation is ABI, and is unlikely to change (esp. not in a way that turns either one of the garbage strings into a valid string). I've left comments explaining what's wrong with both of the garbage strings, but can leave a pointer to https://joellaity.com/2020/01/31/string.html for more info?

lldb/source/DataFormatters/StringPrinter.cpp

65

As written, I think the casts to uintptr_t convey the assumptions being made here.

[vsk]

I'll update this tomorrow with checks in TestDataFormatterLibcxxString.py for how lldb formats garbage{1,2}.

[shafik]

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

76

Sure, that note would be fine.

lldb/source/DataFormatters/StringPrinter.cpp

65

Perhaps but I can not easily search for C-style casts but I can easily audit for reinterpret_cast and reinterpret_cast sometimes has stronger semantics than C-style casts e.g. this example

So while this does not apply to this case it is best to get used to using reinterpret_cast instead saving some character by using a C-style cast.

[teemperor]

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

62

I think those byte arrays need a quick comment about which elements mean what (or how they trigger the respective code paths). Just pointing out which bytes are supposed to overwrite which std::string members is good enough. Something like a macro maybe? #define STD_STRING_BYTES(cap, size, length) {cap, size, length}

76

Can you instead do a #if _LIBCPP_ABI_VERSION == 1 and have the #else as an #error that this test needs updating. We don't support any other libc++ ABI beside 1 in LLDB but if we ever do then this should not silently pass.

lldb/source/DataFormatters/StringPrinter.cpp

68

Isn't this just assert(buffer<buffer_end)? That's less confusing IMHO (and I think in general this check can be in GetPrintable as this should always be true for all GetPrintableImpl).

133

Same as above.

140

Isnt' !isInHalfOpenRange(buffer + (utf8_encoded_len - 1), buffer, buffer_end)) just buffer + (utf8_encoded_len - 1U) < buffer_end? utf8_encoded_len is always positive so the check if it adding it to buffer makes it smaller than buffer can only happen with an integer overflow IIUC (which we probably should check against more explicitly then).

[vsk]

Address review feedback.

[vsk]

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

76

Sure, but the size check is not primarily about the ABI. The garbage examples presuppose 64-bit pointer & size types, which is not true on some watches.

lldb/source/DataFormatters/StringPrinter.cpp

140

I've lifted the buffer < buffer_end check into GetPrintable, and made the overflow check here explicit.

[vsk]

• Enhance test coverage with several more examples of garbage long-mode std::strings.
• In several cases, when we detected an invalid string, we would fall back to spewing the "raw mode" representation of the ValueObject to the user. Use a ScopeExit to make sure we never do that.
• Detect when the GetPointeeData call for the long-mode payload fails, and do not attempt to read the full payload when this happens.

[vsk]

Per offline feedback from Jonas, label variables const where applicable and get rid of the ScopeExits.

[JDevlieghere]

lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

474

Definitely something for another patch, but this is really bad, looking at the signature I have no idea what's in or output. This should return an optional<struct/pair/tuple>.

611

This is so much better.

656–658

Can we move the extractor closer to where it's used, so basically after the capping check?

663

Not code you touched, but...

options.SetPrefixToken(prefix_token.empty() ? nullptr : prefix_token);

might prevent this visual break in setting the options.

679

const bool

[vsk]

• Make a bool const.
• Split off other refactoring into https://reviews.llvm.org/D74018.

[JDevlieghere]

LGTM, but Raphael should have a chance to take another look.

[shafik]

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

23

This is much nicer then the previous raw arrays and self-documents with the fields, nice!

lldb/source/DataFormatters/StringPrinter.cpp

144

Wouldn't we want checkedAddUnsigned? This would also mean casting to uintptr_t.

474

Not your code but these auto seem unnecessary. Especially printable is that just a bool? Same comment in similar code above.

lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

530

So short_mode means SSO?

555

What does cap represent? It is not obvious in this context.

[vsk]

• s/cap/capacity/
• Remove the checkedAdd pointer overflow check as it's not necessary.

lldb/source/DataFormatters/StringPrinter.cpp

144

Not sure about that, as a change in the MSB seems significant to me.

Stepping back a bit though, it seems like this overflow check isn't useful. The data formatter must have called GetPointeeData to transfer the string payload into a host buffer, and we validate the number of bytes read against the size of the payload. So, if pointer overflow occurs, the bytes_read validation would catch it.

I'll just delete this.

474

I'll leave this as a follow-up. printable is a StringPrinter::StringPrinterBufferPointer.

lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

530

Yes.

555

"capacity". I'm using the same jargon as the comment above LibcxxStringLayoutMode does, but it doesn't hurt to write this out.

[vsk]

Ping, @teemperor do you have any concerns about this one?

[teemperor]

In D73860#1872746, @vsk wrote:

Ping, @teemperor do you have any concerns about this one?

Didn't see Jonas comment when he accepted, sorry. LGTM.

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

76

Actually the #if *and* a static_assert comparing the size would be best IMHO.

[vsk]

Thanks everyone for the reviews!

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

76

I'm not sure how to write a static assert that isn't a little brittle. Maybe static_assert(sizeof(void *) != 8 || sizeof(std::string) == 24, "unknown std::string layout")?

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py

def test_with_run_command(self):
             self.runCmd('type format clear', check=False)
             self.runCmd('type summary clear', check=False)
             self.runCmd('type filter clear', check=False)
             self.runCmd('type synth clear', check=False)
             self.runCmd(
                 "settings set target.max-children-count 256",
                 check=False)
 
+        is_64_bit = self.process().GetAddressByteSize() == 8
+
         # Execute the cleanup function during test case tear down.
         self.addTearDownHook(cleanup)
 
         ns = self.namespace
         self.expect(
             "frame variable",
             substrs=[
                 '(%s::wstring) wempty = L""'%ns,
def test_with_run_command(self):
                 '(%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,
                 '(%s::u32string) u32_empty = ""'%ns,
                 '(%s::basic_string<unsigned char, %s::char_traits<unsigned char>, '
                 '%s::allocator<unsigned char> >) uchar = "aaaaa"'%(ns,ns,ns),
         ])
+
+        if is_64_bit:
+            self.expect("frame variable garbage1", substrs=['garbage1 = Summary Unavailable'])
+            self.expect("frame variable garbage2", substrs=['garbage2 = Summary Unavailable'])
+            self.expect("frame variable garbage3", substrs=['garbage3 = Summary Unavailable'])
+            self.expect("frame variable garbage4", substrs=['garbage4 = Summary Unavailable'])
+            self.expect("frame variable garbage5", substrs=['garbage5 = Summary Unavailable'])

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

 #include <string>
+#include <stdint.h>
+
+// For more information about libc++'s std::string ABI, see:
+//
+//   https://joellaity.com/2020/01/31/string.html
+
+// A corrupt string which hits the SSO code path, but has an invalid size.
+static struct {
+  // Set the size of this short-mode string to 116. Note that in short mode,
+  // the size is encoded as `size << 1`.
+  unsigned char size = 232;
+
+  // 23 garbage bytes for the inline string payload.
+  char inline_buf[23] = {0};
+} garbage_string_short_mode;
+
+// A corrupt libcxx string in long mode with a payload that contains a utf8
+// sequence that's inherently too long.
+static unsigned char garbage_utf8_payload1[] = {
+  250 // This means that we expect a 5-byte sequence, this is invalid.
+};
+static struct {
+  uint64_t cap = 5;
+  uint64_t size = 4;
+  unsigned char *data = &garbage_utf8_payload1[0];
+} garbage_string_long_mode1;
+
+// A corrupt libcxx string in long mode with a payload that contains a utf8
+// sequence that's too long to fit in the buffer.
+static unsigned char garbage_utf8_payload2[] = {
+  240 // This means that we expect a 4-byte sequence, but the buffer is too
+      // small for this.
+};
+static struct {
+  uint64_t cap = 3;
+  uint64_t size = 2;
+  unsigned char *data = &garbage_utf8_payload1[0];
+} garbage_string_long_mode2;
+
+// A corrupt libcxx string which has an invalid size (i.e. a size greater than
+// the capacity of the string).
+static struct {
+  uint64_t cap = 5;
+  uint64_t size = 7;
+  const char *data = "foo";
+} garbage_string_long_mode3;
+
+// A corrupt libcxx string in long mode with a payload that would trigger a
+// buffer overflow.
+static struct {
+  uint64_t cap = 5;
+  uint64_t size = 2;
+  uint64_t data = 0xfffffffffffffffeULL;
+} garbage_string_long_mode4;
 
 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"");
     std::basic_string<unsigned char> uchar(5, 'a');
+
+#if _LIBCPP_ABI_VERSION == 1
+    std::string garbage1, garbage2, garbage3, garbage4, garbage5;
+    if (sizeof(std::string) == sizeof(garbage_string_short_mode))
+      memcpy((void *)&garbage1, &garbage_string_short_mode, sizeof(std::string));
+    if (sizeof(std::string) == sizeof(garbage_string_long_mode1))
+      memcpy((void *)&garbage2, &garbage_string_long_mode1, sizeof(std::string));
+    if (sizeof(std::string) == sizeof(garbage_string_long_mode2))
+      memcpy((void *)&garbage3, &garbage_string_long_mode2, sizeof(std::string));
+    if (sizeof(std::string) == sizeof(garbage_string_long_mode3))
+      memcpy((void *)&garbage4, &garbage_string_long_mode3, sizeof(std::string));
+    if (sizeof(std::string) == sizeof(garbage_string_long_mode4))
+      memcpy((void *)&garbage5, &garbage_string_long_mode4, sizeof(std::string));
+#else
+#error "Test potentially needs to be updated for a new std::string ABI."
+#endif
+
     S.assign(L"!!!!!"); // Set break point at this line.
     return 0;
 }

lldb/source/DataFormatters/StringPrinter.cpp

 
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Target/Language.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Status.h"
 
+#include "llvm/Support/CheckedArithmetic.h"
 #include "llvm/Support/ConvertUTF.h"
 
 #include <ctype.h>
 #include <locale>
 #include <memory>
 
 using namespace lldb;
 using namespace lldb_private;
static bool isprint(char32_t codepoint) {
   {
     return false;
   }
   return true;
 }
 
 template <>
 StringPrinter::StringPrinterBufferPointer<>
 GetPrintableImpl<StringPrinter::StringElementType::ASCII>(uint8_t *buffer,
                                                           uint8_t *buffer_end,
                                                           uint8_t *&next) {
   StringPrinter::StringPrinterBufferPointer<> retval = {nullptr};
 
   switch (*buffer) {
   case 0:
     retval = {"\\0", 2};
     break;
   case '\a':
     retval = {"\\a", 2};
     break;
static char32_t ConvertUTF8ToCodePoint(unsigned char c0, unsigned char c1,
   return (c0 - 240) * 262144 + (c2 - 128) * 4096 + (c2 - 128) * 64 + (c3 - 128);
 }
 
 template <>
 StringPrinter::StringPrinterBufferPointer<>
 GetPrintableImpl<StringPrinter::StringElementType::UTF8>(uint8_t *buffer,
                                                          uint8_t *buffer_end,
                                                          uint8_t *&next) {
   StringPrinter::StringPrinterBufferPointer<> retval{nullptr};
 
-  unsigned utf8_encoded_len = llvm::getNumBytesForUTF8(*buffer);
+  const unsigned utf8_encoded_len = llvm::getNumBytesForUTF8(*buffer);
 
-  if (1u + std::distance(buffer, buffer_end) < utf8_encoded_len) {
-    // I don't have enough bytes - print whatever I have left
-    retval = {buffer, static_cast<size_t>(1 + buffer_end - buffer)};
-    next = buffer_end + 1;
+  // If the utf8 encoded length is invalid, or if there aren't enough bytes to
+  // print, this is some kind of corrupted string.
+  if (utf8_encoded_len == 0 || utf8_encoded_len > 4)
+    return retval;
+  if ((buffer_end - buffer) < utf8_encoded_len)
+    // There's no room in the buffer for the utf8 sequence.
+    return retval;
+  if (!llvm::checkedAdd(reinterpret_cast<intptr_t>(buffer),
+                        static_cast<intptr_t>(utf8_encoded_len)))
+    // Accessing the utf8 sequence would cause a pointer overflow.
     return retval;
-  }
 
   char32_t codepoint = 0;
   switch (utf8_encoded_len) {
   case 1:
     // this is just an ASCII byte - ask ASCII
     return GetPrintableImpl<StringPrinter::StringElementType::ASCII>(
         buffer, buffer_end, next);
   case 2:
     codepoint = ConvertUTF8ToCodePoint((unsigned char)*buffer,
                                        (unsigned char)*(buffer + 1));
     break;
   case 3:
     codepoint = ConvertUTF8ToCodePoint((unsigned char)*buffer,
                                        (unsigned char)*(buffer + 1),
                                        (unsigned char)*(buffer + 2));
     break;
   case 4:
     codepoint = ConvertUTF8ToCodePoint(
         (unsigned char)*buffer, (unsigned char)*(buffer + 1),
         (unsigned char)*(buffer + 2), (unsigned char)*(buffer + 3));
     break;
-  default:
-    // this is probably some bogus non-character thing just print it as-is and
-    // hope to sync up again soon
-    retval = {buffer, 1};
-    next = buffer + 1;
-    return retval;
   }
 
   if (codepoint) {
     switch (codepoint) {
     case 0:
       retval = {"\\0", 2};
       break;
     case '\a':
default:
         break;
       }
     }
 
     next = buffer + utf8_encoded_len;
     return retval;
   }
 
-  // this should not happen - but just in case.. try to resync at some point
-  retval = {buffer, 1};
-  next = buffer + 1;
+  // We couldn't figure out how to print this string.
   return retval;
 }
 
 // Given a sequence of bytes, this function returns: a sequence of bytes to
 // actually print out + a length the following unscanned position of the buffer
 // is in next
 static StringPrinter::StringPrinterBufferPointer<>
 GetPrintable(StringPrinter::StringElementType type, uint8_t *buffer,
              uint8_t *buffer_end, uint8_t *&next) {
-  if (!buffer)
+  if (!buffer || buffer >= buffer_end)
     return {nullptr};
 
   switch (type) {
   case StringPrinter::StringElementType::ASCII:
     return GetPrintableImpl<StringPrinter::StringElementType::ASCII>(
         buffer, buffer_end, next);
   case StringPrinter::StringElementType::UTF8:
     return GetPrintableImpl<StringPrinter::StringElementType::UTF8>(
for (; utf8_data_ptr < utf8_data_end_ptr;) {
         break;
 
       if (escape_non_printables) {
         uint8_t *next_data = nullptr;
         auto printable =
             escaping_callback(utf8_data_ptr, utf8_data_end_ptr, next_data);
         auto printable_bytes = printable.GetBytes();
         auto printable_size = printable.GetSize();
-        if (!printable_bytes || !next_data) {
-          // GetPrintable() failed on us - print one byte in a desperate resync
-          // attempt
-          printable_bytes = utf8_data_ptr;
-          printable_size = 1;
-          next_data = utf8_data_ptr + 1;
-        }
+
+        // We failed to figure out how to print this string.
+        if (!printable_bytes || !next_data)
+          return false;
+
         for (unsigned c = 0; c < printable_size; c++)
           stream.Printf("%c", *(printable_bytes + c));
         utf8_data_ptr = (uint8_t *)next_data;
       } else {
         stream.Printf("%c", *utf8_data_ptr);
         utf8_data_ptr++;
       }
     }
bool StringPrinter::ReadStringAndDumpToStream<
   }
 
   // since we tend to accept partial data (and even partially malformed data)
   // we might end up with no NULL terminator before the end_ptr hence we need
   // to take a slower route and ensure we stay within boundaries
   for (uint8_t *data = buffer_sp->GetBytes(); *data && (data < data_end);) {
     if (escape_non_printables) {
       uint8_t *next_data = nullptr;
       auto printable = escaping_callback(data, data_end, next_data);
       auto printable_bytes = printable.GetBytes();
       auto printable_size = printable.GetSize();
-      if (!printable_bytes || !next_data) {
-        // GetPrintable() failed on us - print one byte in a desperate resync
-        // attempt
-        printable_bytes = data;
-        printable_size = 1;
-        next_data = data + 1;
-      }
+
+      // We failed to figure out how to print this string.
+      if (!printable_bytes || !next_data)
+        return false;
+
       for (unsigned c = 0; c < printable_size; c++)
         options.GetStream()->Printf("%c", *(printable_bytes + c));
       data = (uint8_t *)next_data;
     } else {
       options.GetStream()->Printf("%c", *data);
       data++;
     }
   }

lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

 //===-- LibCxx.cpp --------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "LibCxx.h"
 
-#include "llvm/ADT/ScopeExit.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/StringPrinter.h"
 #include "lldb/DataFormatters/TypeSummary.h"
 #include "lldb/DataFormatters/VectorIterator.h"
enum LibcxxStringLayoutMode {
   eLibcxxStringLayoutModeCSD = 0,
   eLibcxxStringLayoutModeDSC = 1,
   eLibcxxStringLayoutModeInvalid = 0xffff
 };
 
 // this function abstracts away the layout and mode details of a libc++ string
 // and returns the address of the data and the size ready for callers to
 // consume
 static bool ExtractLibcxxStringInfo(ValueObject &valobj,
                                     ValueObjectSP &location_sp,
                                     uint64_t &size) {
   ValueObjectSP D(valobj.GetChildAtIndexPath({0, 0, 0, 0}));
   if (!D)
     return false;
 
   ValueObjectSP layout_decider(
     D->GetChildAtIndexPath(llvm::ArrayRef<size_t>({0, 0})));
if (short_mode) {
     ValueObjectSP s(D->GetChildAtIndex(1, true));
     if (!s)
       return false;
     location_sp = s->GetChildAtIndex(
         (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
     size = (layout == eLibcxxStringLayoutModeDSC)
                ? size_mode_value
                : ((size_mode_value >> 1) % 256);
+
+    // When the small-string optimization takes place, the data must fit in the
+    // inline string buffer (23 bytes on x86_64/Darwin). If it doesn't, it's
+    // likely that the string isn't initialized and we're reading garbage.
+    ExecutionContext exe_ctx(location_sp->GetExecutionContextRef());
+    const llvm::Optional<uint64_t> max_bytes =
+        location_sp->GetCompilerType().GetByteSize(
+            exe_ctx.GetBestExecutionContextScope());
+    if (!max_bytes || size > *max_bytes)
+      return false;
+
     return (location_sp.get() != nullptr);
   } else {
     ValueObjectSP l(D->GetChildAtIndex(0, true));
     if (!l)
       return false;
     // we can use the layout_decider object as the data pointer
     location_sp = (layout == eLibcxxStringLayoutModeDSC)
                       ? layout_decider
                       : l->GetChildAtIndex(2, true);
     ValueObjectSP size_vo(l->GetChildAtIndex(1, true));
-    if (!size_vo || !location_sp)
+    const unsigned cap_index = (layout == eLibcxxStringLayoutModeDSC) ? 2 : 0;
+    ValueObjectSP cap_vo(l->GetChildAtIndex(cap_index, true));
+    if (!size_vo || !location_sp || !cap_vo)
+      return false;
+    size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+    const uint64_t cap = cap_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);
+    if (size == LLDB_INVALID_OFFSET || cap == LLDB_INVALID_OFFSET || cap < size)
       return false;
-    size = size_vo->GetValueAsUnsigned(0);
     return true;
   }
 }
 
 bool lldb_private::formatters::LibcxxWStringSummaryProvider(
     ValueObject &valobj, Stream &stream,
     const TypeSummaryOptions &summary_options) {
   uint64_t size = 0;
bool lldb_private::formatters::LibcxxWStringSummaryProvider(
 
   if (summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryCapped) {
     const auto max_size = valobj.GetTargetSP()->GetMaximumSizeOfStringSummary();
     if (size > max_size) {
       size = max_size;
       options.SetIsTruncated(true);
     }
   }
-  location_sp->GetPointeeData(extractor, 0, size);
+  const size_t bytes_read = location_sp->GetPointeeData(extractor, 0, size);
+  if (bytes_read < size)
+    return false;
 
   // std::wstring::size() is measured in 'characters', not bytes
   TypeSystemClang *ast_context =
       TypeSystemClang::GetScratch(*valobj.GetTargetSP());
   if (!ast_context)
     return false;
 
   auto wchar_t_size =
       ast_context->GetBasicType(lldb::eBasicTypeWChar).GetByteSize(nullptr);
   if (!wchar_t_size)
     return false;
 
   options.SetData(extractor);
   options.SetStream(&stream);
   options.SetPrefixToken("L");
   options.SetQuote('"');
   options.SetSourceSize(size);
   options.SetBinaryZeroIsTerminator(false);
 
   switch (*wchar_t_size) {
   case 1:
-    StringPrinter::ReadBufferAndDumpToStream<
+    return StringPrinter::ReadBufferAndDumpToStream<
         lldb_private::formatters::StringPrinter::StringElementType::UTF8>(
         options);
     break;
 
   case 2:
-    lldb_private::formatters::StringPrinter::ReadBufferAndDumpToStream<
+    return StringPrinter::ReadBufferAndDumpToStream<
         lldb_private::formatters::StringPrinter::StringElementType::UTF16>(
         options);
     break;
 
   case 4:
-    lldb_private::formatters::StringPrinter::ReadBufferAndDumpToStream<
+    return StringPrinter::ReadBufferAndDumpToStream<
         lldb_private::formatters::StringPrinter::StringElementType::UTF32>(
         options);
-    break;
-
-  default:
-    stream.Printf("size for wchar_t is not valid");
-    return true;
   }
-
-  return true;
+  return false;
 }
 
 template <StringPrinter::StringElementType element_type>
 bool LibcxxStringSummaryProvider(ValueObject &valobj, Stream &stream,
                                  const TypeSummaryOptions &summary_options,
-                                 std::string prefix_token = "") {
+                                 std::string prefix_token) {
   uint64_t size = 0;
   ValueObjectSP location_sp;
-
   if (!ExtractLibcxxStringInfo(valobj, location_sp, size))
     return false;
-
   if (size == 0) {
     stream.Printf("\"\"");
     return true;
   }
 
   if (!location_sp)
     return false;
 
   StringPrinter::ReadBufferAndDumpToStreamOptions options(valobj);
 
   DataExtractor extractor;
   if (summary_options.GetCapping() == TypeSummaryCapping::eTypeSummaryCapped) {
     const auto max_size = valobj.GetTargetSP()->GetMaximumSizeOfStringSummary();
     if (size > max_size) {
       size = max_size;
       options.SetIsTruncated(true);
     }
   }
-  location_sp->GetPointeeData(extractor, 0, size);
+  const size_t bytes_read = location_sp->GetPointeeData(extractor, 0, size);
+  if (bytes_read < size)
+    return false;
 
   options.SetData(extractor);
   options.SetStream(&stream);
 
   if (prefix_token.empty())
     options.SetPrefixToken(nullptr);
   else
     options.SetPrefixToken(prefix_token);
 
   options.SetQuote('"');
   options.SetSourceSize(size);
   options.SetBinaryZeroIsTerminator(false);
-  StringPrinter::ReadBufferAndDumpToStream<element_type>(options);
+  return StringPrinter::ReadBufferAndDumpToStream<element_type>(options);
+}
 
+template <StringPrinter::StringElementType element_type>
+static bool formatStringImpl(ValueObject &valobj, Stream &stream,
+                             const TypeSummaryOptions &summary_options,
+                             std::string prefix_token) {
+  StreamString scratch_stream;
+  const bool success = LibcxxStringSummaryProvider<element_type>(
+      valobj, scratch_stream, summary_options, prefix_token);
+  if (success)
+    stream << scratch_stream.GetData();
+  else
+    stream << "Summary Unavailable";
   return true;
 }
 
 bool lldb_private::formatters::LibcxxStringSummaryProviderASCII(
     ValueObject &valobj, Stream &stream,
     const TypeSummaryOptions &summary_options) {
-  return LibcxxStringSummaryProvider<StringPrinter::StringElementType::ASCII>(
-      valobj, stream, summary_options);
+  return formatStringImpl<StringPrinter::StringElementType::ASCII>(
+      valobj, stream, summary_options, "");
 }
 
 bool lldb_private::formatters::LibcxxStringSummaryProviderUTF16(
     ValueObject &valobj, Stream &stream,
     const TypeSummaryOptions &summary_options) {
-  return LibcxxStringSummaryProvider<StringPrinter::StringElementType::UTF16>(
+  return formatStringImpl<StringPrinter::StringElementType::UTF16>(
       valobj, stream, summary_options, "u");
 }
 
 bool lldb_private::formatters::LibcxxStringSummaryProviderUTF32(
     ValueObject &valobj, Stream &stream,
     const TypeSummaryOptions &summary_options) {
-  return LibcxxStringSummaryProvider<StringPrinter::StringElementType::UTF32>(
+  return formatStringImpl<StringPrinter::StringElementType::UTF32>(
       valobj, stream, summary_options, "U");
 }