llvm-undname: Fix more crashes and asserts on invalid inputs

[thakis]

Summary

For functions whose callers don't check that enough input is present,
add checks at the start of the function that enough input is there and
set Error otherwise.

For functions that return AST objects, return nullptr instead of
incomplete AST objects with nullptr fields if an error occurred during
the function.

Introduce a new function demangleDeclarator() for the sequence
demangleFullyQualifiedSymbolName(); demangleEncodedSymbol() and
use it in the two places that had this sequence. Let this new function
check that ConversionOperatorIdentifiers have a valid TargetType.

Some of the bad inputs found by oss-fuzz, others by inspection.

[thakis]

Add test coverage for each modification -- removing each green block now makes some test fail. Found another bug this way, fix that too. Found a few tests I had added that aren't reachable, removed those.

[zturner]

llvm/lib/Demangle/MicrosoftDemangle.cpp

722–724 ↗

(On Diff #194047)

How about

if (Error)
  return nullptr;

assert(Symbol);
Symbol->Name = QN;

1955 ↗

(On Diff #194047)

s/guarantess/guarantees/

That said, I would just change this to:

// isMemberPointer() only returns true if there is at least one character after the qualifiers.
assert(!MangledName.empty());

[thakis]

Address comments (but don't add the assert()s because they're implied by the line following the assert in both cases).

llvm/lib/Demangle/MicrosoftDemangle.cpp

722–724 ↗

(On Diff #194047)

That's fine too. I just moved this code around though, see line 732 on the lhs.

1955 ↗

(On Diff #194047)

Thanks, that's better.

llvm/trunk/include/llvm/Demangle/MicrosoftDemangle.h

public:
   // True if an error occurred.
   bool Error = false;
 
   void dumpBackReferences();
 
 private:
   SymbolNode *demangleEncodedSymbol(StringView &MangledName,
                                     QualifiedNameNode *QN);
+  SymbolNode *demangleDeclarator(StringView &MangledName);
 
   VariableSymbolNode *demangleVariableEncoding(StringView &MangledName,
                                                StorageClass SC);
   FunctionSymbolNode *demangleFunctionEncoding(StringView &MangledName);
 
   Qualifiers demanglePointerExtQualifiers(StringView &MangledName);
 
   // Parser functions. This is a recursive-descent parser.

llvm/trunk/lib/Demangle/MicrosoftDemangle.cpp

static bool isMemberPointer(StringView MangledName, bool &Error) {
   default:
     Error = true;
     return false;
   }
 
   // If it starts with a number, then 6 indicates a non-member function
   // pointer, and 8 indicates a member function pointer.
   if (startsWithDigit(MangledName)) {
-    assert(MangledName[0] == '6' || MangledName[0] == '8');
+    if (MangledName[0] != '6' && MangledName[0] != '8') {
+      Error = true;
+      return false;
+    }
     return (MangledName[0] == '8');
   }
 
   // Remove ext qualifiers since those can appear on either type and are
   // therefore not indicative.
   MangledName.consumeFront('E'); // 64-bit
   MangledName.consumeFront('I'); // restrict
   MangledName.consumeFront('F'); // unaligned
 
-  assert(!MangledName.empty());
+  if (MangledName.empty()) {
+    Error = true;
+    return false;
+  }
 
   // The next value should be either ABCD (non-member) or QRST (member).
   switch (MangledName.front()) {
   case 'A':
   case 'B':
   case 'C':
   case 'D':
     return false;
FunctionSymbolNode *Demangler::demangleInitFiniStub(StringView &MangledName,
   DynamicStructorIdentifierNode *DSIN =
       Arena.alloc<DynamicStructorIdentifierNode>();
   DSIN->IsDestructor = IsDestructor;
 
   bool IsKnownStaticDataMember = false;
   if (MangledName.consumeFront('?'))
     IsKnownStaticDataMember = true;
 
-  QualifiedNameNode *QN = demangleFullyQualifiedSymbolName(MangledName);
+  SymbolNode *Symbol = demangleDeclarator(MangledName);
+  if (Error)
+    return nullptr;
 
-  SymbolNode *Symbol = demangleEncodedSymbol(MangledName, QN);
   FunctionSymbolNode *FSN = nullptr;
-  Symbol->Name = QN;
 
   if (Symbol->kind() == NodeKind::VariableSymbol) {
     DSIN->Variable = static_cast<VariableSymbolNode *>(Symbol);
 
     // Older versions of clang mangled this type of symbol incorrectly.  They
     // would omit the leading ? and they would only emit a single @ at the end.
     // The correct mangling is a leading ? and 2 trailing @ signs.  Handle
     // both cases.
     int AtCount = IsKnownStaticDataMember ? 2 : 1;
     for (int I = 0; I < AtCount; ++I) {
       if (MangledName.consumeFront('@'))
         continue;
       Error = true;
       return nullptr;
     }
 
     FSN = demangleFunctionEncoding(MangledName);
+    if (FSN)
       FSN->Name = synthesizeQualifiedName(Arena, DSIN);
   } else {
     if (IsKnownStaticDataMember) {
       // This was supposed to be a static data member, but we got a function.
       Error = true;
       return nullptr;
     }
 
     FSN = static_cast<FunctionSymbolNode *>(Symbol);
Demangler::demangleFunctionIdentifierCode(StringView &MangledName,
   }
   // No Mangling Yet:      Spaceship,                    // operator<=>
 
   return nullptr;
 }
 
 SymbolNode *Demangler::demangleEncodedSymbol(StringView &MangledName,
                                              QualifiedNameNode *Name) {
+  if (MangledName.empty()) {
+    Error = true;
+    return nullptr;
+  }
+
   // Read a variable.
   switch (MangledName.front()) {
   case '0':
   case '1':
   case '2':
   case '3':
   case '4': {
     StorageClass SC = demangleVariableStorageClass(MangledName);
     return demangleVariableEncoding(MangledName, SC);
   }
   case '8':
     return nullptr;
   }
   FunctionSymbolNode *FSN = demangleFunctionEncoding(MangledName);
 
   IdentifierNode *UQN = Name->getUnqualifiedIdentifier();
   if (UQN->kind() == NodeKind::ConversionOperatorIdentifier) {
     ConversionOperatorIdentifierNode *COIN =
         static_cast<ConversionOperatorIdentifierNode *>(UQN);
+    if (FSN)
       COIN->TargetType = FSN->Signature->ReturnType;
   }
   return FSN;
 }
 
+SymbolNode *Demangler::demangleDeclarator(StringView &MangledName) {
+  // What follows is a main symbol name. This may include namespaces or class
+  // back references.
+  QualifiedNameNode *QN = demangleFullyQualifiedSymbolName(MangledName);
+  if (Error)
+    return nullptr;
+
+  SymbolNode *Symbol = demangleEncodedSymbol(MangledName, QN);
+  if (Error)
+    return nullptr;
+  Symbol->Name = QN;
+
+  IdentifierNode *UQN = QN->getUnqualifiedIdentifier();
+  if (UQN->kind() == NodeKind::ConversionOperatorIdentifier) {
+    ConversionOperatorIdentifierNode *COIN =
+        static_cast<ConversionOperatorIdentifierNode *>(UQN);
+    if (!COIN->TargetType) {
+      Error = true;
+      return nullptr;
+    }
+  }
+  return Symbol;
+}
+
 // Parser entry point.
 SymbolNode *Demangler::parse(StringView &MangledName) {
   // We can't demangle MD5 names, just output them as-is.
   // Also, MSVC-style mangled symbols must start with '?'.
   if (MangledName.startsWith("??@")) {
     // This is an MD5 mangled name.  We can't demangle it, just return the
     // mangled name.
     SymbolNode *S = Arena.alloc<SymbolNode>(NodeKind::Md5Symbol);
     S->Name = synthesizeQualifiedName(Arena, MangledName);
     return S;
   }
 
   if (!MangledName.startsWith('?')) {
     Error = true;
     return nullptr;
   }
 
   MangledName.consumeFront('?');
 
   // ?$ is a template instantiation, but all other names that start with ? are
   // operators / special names.
   if (SymbolNode *SI = demangleSpecialIntrinsic(MangledName))
     return SI;
 
-  // What follows is a main symbol name. This may include namespaces or class
-  // back references.
-  QualifiedNameNode *QN = demangleFullyQualifiedSymbolName(MangledName);
-  if (Error)
-    return nullptr;
-
-  SymbolNode *Symbol = demangleEncodedSymbol(MangledName, QN);
-  if (Symbol) {
-    Symbol->Name = QN;
-  }
-
-  if (Error)
-    return nullptr;
-
-  return Symbol;
+  return demangleDeclarator(MangledName);
 }
 
 TagTypeNode *Demangler::parseTagUniqueName(StringView &MangledName) {
   if (!MangledName.consumeFront(".?A"))
     return nullptr;
   MangledName.consumeFront(".?A");
   if (MangledName.empty())
     return nullptr;

 
 VariableSymbolNode *Demangler::demangleVariableEncoding(StringView &MangledName,
                                                         StorageClass SC) {
   VariableSymbolNode *VSN = Arena.alloc<VariableSymbolNode>();
 
   VSN->Type = demangleType(MangledName, QualifierMangleMode::Drop);
   VSN->SC = SC;
 
+  if (Error)
+    return nullptr;
+
   // <variable-type> ::= <type> <cvr-qualifiers>
   //                 ::= <type> <pointee-cvr-qualifiers> # pointers, references
   switch (VSN->Type->kind()) {
   case NodeKind::PointerType: {
     PointerTypeNode *PTN = static_cast<PointerTypeNode *>(VSN->Type);
 
     Qualifiers ExtraChildQuals = Q_None;
     PTN->Quals = Qualifiers(VSN->Type->Quals |
FuncClass Demangler::demangleFunctionClass(StringView &MangledName) {
   }
   }
 
   Error = true;
   return FC_Public;
 }
 
 CallingConv Demangler::demangleCallingConvention(StringView &MangledName) {
+  if (MangledName.empty()) {
+    Error = true;
+    return CallingConv::None;
+  }
+
   switch (MangledName.popFront()) {
   case 'A':
   case 'B':
     return CallingConv::Cdecl;
   case 'C':
   case 'D':
     return CallingConv::Pascal;
   case 'E':
case '4':
     return StorageClass::FunctionLocalStatic;
   }
   Error = true;
   return StorageClass::None;
 }
 
 std::pair<Qualifiers, bool>
 Demangler::demangleQualifiers(StringView &MangledName) {
+  if (MangledName.empty()) {
+    Error = true;
+    return std::make_pair(Q_None, false);
+  }
 
   switch (MangledName.popFront()) {
   // Member qualifiers
   case 'Q':
     return std::make_pair(Q_None, true);
   case 'R':
     return std::make_pair(Q_Const, true);
   case 'S':
TypeNode *Demangler::demangleType(StringView &MangledName,
   bool IsMember = false;
   if (QMM == QualifierMangleMode::Mangle) {
     std::tie(Quals, IsMember) = demangleQualifiers(MangledName);
   } else if (QMM == QualifierMangleMode::Result) {
     if (MangledName.consumeFront('?'))
       std::tie(Quals, IsMember) = demangleQualifiers(MangledName);
   }
 
+  if (MangledName.empty()) {
+    Error = true;
+    return nullptr;
+  }
+
   TypeNode *Ty = nullptr;
   if (isTagType(MangledName))
     Ty = demangleClassType(MangledName);
   else if (isPointerType(MangledName)) {
     if (isMemberPointer(MangledName, Error))
       Ty = demangleMemberPointerType(MangledName);
     else if (!Error)
       Ty = demanglePointerType(MangledName);

 }
 
 FunctionSymbolNode *
 Demangler::demangleFunctionEncoding(StringView &MangledName) {
   FuncClass ExtraFlags = FC_None;
   if (MangledName.consumeFront("$$J0"))
     ExtraFlags = FC_ExternC;
 
+  if (MangledName.empty()) {
+    Error = true;
+    return nullptr;
+  }
+
   FuncClass FC = demangleFunctionClass(MangledName);
   FC = FuncClass(ExtraFlags | FC);
 
   FunctionSignatureNode *FSN = nullptr;
   ThunkSignatureNode *TTN = nullptr;
   if (FC & FC_StaticThisAdjust) {
     TTN = Arena.alloc<ThunkSignatureNode>();
     TTN->ThisAdjust.StaticOffset = demangleSigned(MangledName);
if (FC & FC_NoParameterList) {
     // This is an extern "C" function whose full signature hasn't been mangled.
     // This happens when we need to mangle a local symbol inside of an extern
     // "C" function.
     FSN = Arena.alloc<FunctionSignatureNode>();
   } else {
     bool HasThisQuals = !(FC & (FC_Global | FC_Static));
     FSN = demangleFunctionType(MangledName, HasThisQuals);
   }
+
+  if (Error)
+    return nullptr;
+
   if (TTN) {
     *static_cast<FunctionSignatureNode *>(TTN) = *FSN;
     FSN = TTN;
   }
   FSN->FunctionClass = FC;
 
   FunctionSymbolNode *Symbol = Arena.alloc<FunctionSymbolNode>();
   Symbol->Signature = FSN;
PointerTypeNode *Demangler::demangleMemberPointerType(StringView &MangledName) {
 
   std::tie(Pointer->Quals, Pointer->Affinity) =
       demanglePointerCVQualifiers(MangledName);
   assert(Pointer->Affinity == PointerAffinity::Pointer);
 
   Qualifiers ExtQuals = demanglePointerExtQualifiers(MangledName);
   Pointer->Quals = Qualifiers(Pointer->Quals | ExtQuals);
 
+  // isMemberPointer() only returns true if there is at least one character
+  // after the qualifiers.
   if (MangledName.consumeFront("8")) {
     Pointer->ClassParent = demangleFullyQualifiedTypeName(MangledName);
     Pointer->Pointee = demangleFunctionType(MangledName, true);
   } else {
     Qualifiers PointeeQuals = Q_None;
     bool IsMember = false;
     std::tie(PointeeQuals, IsMember) = demangleQualifiers(MangledName);
-    assert(IsMember);
+    assert(IsMember || Error);
     Pointer->ClassParent = demangleFullyQualifiedTypeName(MangledName);
 
     Pointer->Pointee = demangleType(MangledName, QualifierMangleMode::Drop);
+    if (Pointer->Pointee)
       Pointer->Pointee->Quals = PointeeQuals;
   }
 
   return Pointer;
 }
 
 Qualifiers Demangler::demanglePointerExtQualifiers(StringView &MangledName) {
   Qualifiers Quals = Q_None;
   if (MangledName.consumeFront('E'))

llvm/trunk/test/Demangle/invalid-manglings.test

 ; CHECK-EMPTY:
 ; CHECK-NEXT: ??0@
 ; CHECK-NEXT: error: Invalid mangled name
 
 ? @@   YC@
 ; CHECK-EMPTY:
 ; CHECK-NEXT: ? @@   YC@
 ; CHECK-NEXT: error: Invalid mangled name
+
+??B@$$J0
+; CHECK-EMPTY:
+; CHECK-NEXT: ??B@$$J0
+; CHECK-NEXT: error: Invalid mangled name
+
+??B@4
+; CHECK-EMPTY:
+; CHECK-NEXT: ??B@4
+; CHECK-NEXT: error: Invalid mangled name
+
+?A?@?@???B@4D
+; CHECK-EMPTY:
+; CHECK-NEXT: ?A?@?@???B@4D
+; CHECK-NEXT: error: Invalid mangled name
+
+?A?@?@???B@4DD
+; CHECK-EMPTY:
+; CHECK-NEXT: ?A?@?@???B@4DD
+; CHECK-NEXT: error: Invalid mangled name
+
+??$A@P15@
+; CHECK-EMPTY:
+; CHECK-NEXT: ??$A@P15@
+; CHECK-NEXT: error: Invalid mangled name
+
+??$A@P
+; CHECK-EMPTY:
+; CHECK-NEXT: ??$A@P
+; CHECK-NEXT: error: Invalid mangled name
+
+?A@@
+; CHECK-EMPTY:
+; CHECK-NEXT: ?A@@
+; CHECK-NEXT: error: Invalid mangled name
+
+?A@@P
+; CHECK-EMPTY:
+; CHECK-NEXT: ?A@@P
+; CHECK-NEXT: error: Invalid mangled name
+
+?A@@4PQA@@
+; CHECK-EMPTY:
+; CHECK-NEXT: ?A@@4PQA@@
+; CHECK-NEXT: error: Invalid mangled name
+
+??__E
+; CHECK-EMPTY:
+; CHECK-NEXT: ??__E
+; CHECK-NEXT: error: Invalid mangled name
+
+??__E@@
+; CHECK-EMPTY:
+; CHECK-NEXT: ??__E@@
+; CHECK-NEXT: error: Invalid mangled name
+
+??__E?Foo@@0HA@@
+; CHECK-EMPTY:
+; CHECK-NEXT: ??__E?Foo@@0HA@@
+; CHECK-NEXT: error: Invalid mangled name