[lldb] Fix logic error in AppleObjCTypeEncodingParser

[kastiglione]

Fixes parsing of an ObjC type encoding such as {?="a""b"}. Parsing of such a type
encoding would lead to an assert. This was observed when running language objc class-table dump.

The function ReadQuotedString consumes the closing quote, however one of its two
callers (ReadStructElement) was also consuming a quote. For the above type encoding,
where two quoted strings occur back to back, the parser would unintentionally consume
the opening quote of the second quoted string - leaving the remaining text with an
unbalanced quote.

This changes fixes ReadStructElement to not consume a quote after calling
ReadQuotedString.

For callers to know whether a string was successfully parsed, ReadQuotedString now returns an optional string.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Dave Lee (kastiglione)

Changes

Fixes parsing of an ObjC type encoding such as {?="a""b"}. Parsing of such a type
encoding would lead to an assert. This was observed when running language objc class-table dump.

The function ReadQuotedString consumes the closing quote, however one of its two
callers (ReadStructElement) was also consuming a quote. For the above type encoding,
where two quoted strings occur back to back, the parser would unintentionally consume
the opening quote of the second quoted string - leaving the remaining text with an
unbalanced quote.

This changes fixes ReadStructElement to not consume a quote after calling
ReadQuotedString.

In order for the ReadStructElement to know whether a string was successfully parsed,
ReadQuotedString now returns an optional string.

---

Full diff: https://github.com/llvm/llvm-project/pull/137067.diff

2 Files Affected:

• (modified) lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp (+20-10)
• (modified) lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.h (+1-1)

diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
index ddaa7a8a597b4..f29a876ba2f24 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.cpp
@@ -19,6 +19,7 @@
 
 #include "clang/Basic/TargetInfo.h"
 
+#include <optional>
 #include <vector>
 
 using namespace lldb_private;
@@ -41,13 +42,17 @@ std::string AppleObjCTypeEncodingParser::ReadStructName(StringLexer &type) {
   return std::string(buffer.GetString());
 }
 
-std::string AppleObjCTypeEncodingParser::ReadQuotedString(StringLexer &type) {
+std::optional<std::string>
+AppleObjCTypeEncodingParser::ReadQuotedString(StringLexer &type) {
+  if (!type.HasAtLeast(1))
+    return std::nullopt;
+
   StreamString buffer;
-  while (type.HasAtLeast(1) && type.Peek() != '"')
+  while (type.Peek() != '"') {
     buffer.Printf("%c", type.Next());
-  StringLexer::Character next = type.Next();
-  UNUSED_IF_ASSERT_DISABLED(next);
-  assert(next == '"');
+    if (!type.HasAtLeast(1))
+      return std::nullopt;
+  }
   return std::string(buffer.GetString());
 }
 
@@ -70,10 +75,12 @@ AppleObjCTypeEncodingParser::ReadStructElement(TypeSystemClang &ast_ctx,
                                                StringLexer &type,
                                                bool for_expression) {
   StructElement retval;
-  if (type.NextIf('"'))
-    retval.name = ReadQuotedString(type);
-  if (!type.NextIf('"'))
-    return retval;
+  if (type.NextIf('"')) {
+    if (auto maybe_name = ReadQuotedString(type))
+      retval.name = *maybe_name;
+    else
+      return retval;
+  }
   uint32_t bitfield_size = 0;
   retval.type = BuildType(ast_ctx, type, for_expression, &bitfield_size);
   retval.bitfield = bitfield_size;
@@ -198,7 +205,10 @@ clang::QualType AppleObjCTypeEncodingParser::BuildObjCObjectPointerType(
     // quoted string is a class name. - If we see anything else, the quoted
     // string is a field name and we push it back onto type.
 
-    name = ReadQuotedString(type);
+    if (auto maybe_name = ReadQuotedString(type))
+      name = *maybe_name;
+    else
+      return clang::QualType();
 
     if (type.HasAtLeast(1)) {
       switch (type.Peek()) {
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.h b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.h
index 57ed9c21fabad..3058514f38ba1 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTypeEncodingParser.h
@@ -63,7 +63,7 @@ class AppleObjCTypeEncodingParser : public ObjCLanguageRuntime::EncodingToType {
 
   uint32_t ReadNumber(StringLexer &type);
 
-  std::string ReadQuotedString(StringLexer &type);
+  std::optional<std::string> ReadQuotedString(StringLexer &type);
 
   ObjCLanguageRuntime &m_runtime;
 };

[bulbazord]

In order for the ReadStructElement to know whether a string was successfully parsed, ReadQuotedString now returns an optional string.

The only times a string isn't successfully parsed are if you run out of characters in the stream right? This lets you differentiate between "I ran out of characters" and "I saw an empty string".

[kastiglione]

This lets you differentiate between "I ran out of characters" and "I saw an empty string".

@bulbazord correct