[MS Demangler] Properly handle function parameter list back-references.

[zturner]

Summary

Properly demangle function parameter back-references.

Previously we treated lists of function parameters and template parameters the same. There are some important differences with regards to back-references, and some less important differences regarding which characters can appear before or after the name.

The important differences are that with a given type T, all instances of a function parameter list share the same global back-ref table. Specifically, if X and Y are function pointers, then there are 3 entities in the declaration X func(Y) which all affect and are affected by the master parameter back-ref table. 1) The parameter list of X's function type, 2) the parameter list of func itself, and 3) The parameter list of Y's function type.

The previous code would create a back-reference table that was local to a single parameter list, so it would not be shared across parameter lists.

This was discovered when porting ms-back-references.test from clang's mangling tests. All of these tests should now pass with the new changes.

In doing so, I split the function for parsing template and function parameters into two separate functions. This makes the template parameter list parsing code in particular very small and easy to understand now.

[rnk]

lgtm

llvm/lib/Demangle/MicrosoftDemangle.cpp

1595 ↗

(On Diff #157579)

s/memorizing/memoizing/

[zturner]

llvm/lib/Demangle/MicrosoftDemangle.cpp

1595 ↗

(On Diff #157579)

Yea I should fix this, but it's spelled memorize throughout in many places throughout the file. So it should probably be a follow-up that fixes all places at once.

llvm/trunk/lib/Demangle/MicrosoftDemangle.cpp

private:
   Type *demangleBasicType();
   UdtType *demangleClassType();
   PointerType *demanglePointerType();
   MemberPointerType *demangleMemberPointerType();
   FunctionType *demangleFunctionType(bool HasThisQuals, bool IsFunctionPointer);
 
   ArrayType *demangleArrayType();
 
-  ParamList demangleParameterList();
+  ParamList demangleTemplateParameterList();
+  ParamList demangleFunctionParameterList();
 
   int demangleNumber();
   void demangleNamePiece(Name &Node, bool IsHead);
 
   StringView demangleString(bool memorize);
   void memorizeString(StringView s);
   Name *demangleName();
   void demangleOperator(Name *);
private:
   Type *SymbolType = nullptr;
 
   // The main symbol name. (e.g. "ns::foo" in "int ns::foo()".)
   Name *SymbolName = nullptr;
 
   // Memory allocator.
   ArenaAllocator Arena;
 
+  // A single type uses one global back-ref table for all function params.
+  // This means back-refs can even go "into" other types.  Examples:
+  //
+  //  // Second int* is a back-ref to first.
+  //  void foo(int *, int*);
+  //
+  //  // Second int* is not a back-ref to first (first is not a function param).
+  //  int* foo(int*);
+  //
+  //  // Second int* is a back-ref to first (ALL function types share the same
+  //  // back-ref map.
+  //  using F = void(*)(int*);
+  //  F G(int *);
+  Type *FunctionParamBackRefs[10];
+  size_t FunctionParamBackRefCount = 0;
+
   // The first 10 BackReferences in a mangled name can be back-referenced by
   // special name @[0-9]. This is a storage for the first 10 BackReferences.
   StringView BackReferences[10];
   size_t BackRefCount = 0;
 };
 } // namespace
 
 // Parser entry point.
if (I >= BackRefCount) {
       Error = true;
       return;
     }
     MangledName = MangledName.dropFront();
     Node.Str = BackReferences[I];
   } else if (MangledName.consumeFront("?$")) {
     // Class template.
     Node.Str = demangleString(false);
-    Node.TemplateParams = demangleParameterList();
+    Node.TemplateParams = demangleTemplateParameterList();
   } else if (!IsHead && MangledName.consumeFront("?A")) {
     // Anonymous namespace starts with ?A.  So does overloaded operator[],
     // but the distinguishing factor is that namespace themselves are not
     // mangled, only the variables and functions inside of them are.  So
     // an anonymous namespace will never occur as the first item in the
     // name.
     Node.Str = "`anonymous namespace'";
     if (!MangledName.consumeFront('@')) {
FunctionType *Demangler::demangleFunctionType(bool HasThisQuals,
   FTy->CallConvention = demangleCallingConvention();
 
   // <return-type> ::= <type>
   //               ::= @ # structors (they have no declared return type)
   bool IsStructor = MangledName.consumeFront('@');
   if (!IsStructor)
     FTy->ReturnType = demangleType(QualifierMangleMode::Result);
 
-  FTy->Params = demangleParameterList();
+  FTy->Params = demangleFunctionParameterList();
 
   demangleThrowSpecification();
 
   return FTy;
 }
 
 Type *Demangler::demangleFunctionEncoding() {
   FuncClass FC = demangleFunctionClass();
ArrayType *Demangler::demangleArrayType() {
   }
 
   ATy->ElementType = demangleType(QualifierMangleMode::Drop);
   Dim->ElementType = ATy->ElementType;
   return ATy;
 }
 
 // Reads a function or a template parameters.
-ParamList Demangler::demangleParameterList() {
-  // Within the same parameter list, you can backreference the first 10 types.
-  Type *BackRef[10];
-  int Idx = 0;
-
+ParamList Demangler::demangleFunctionParameterList() {
   // Empty parameter list.
-  // FIXME: Will this cause problems if demangleParameterList() is called in the
-  // context of a template parameter list?
   if (MangledName.consumeFront('X'))
     return {};
 
   ParamList *Head;
   ParamList **Current = &Head;
   while (!Error && !MangledName.startsWith('@') &&
          !MangledName.startsWith('Z')) {
+
     if (startsWithDigit(MangledName)) {
       int N = MangledName[0] - '0';
-      if (N >= Idx) {
+      if (N >= FunctionParamBackRefCount) {
         Error = true;
         return {};
       }
       MangledName = MangledName.dropFront();
 
       *Current = Arena.alloc<ParamList>();
-      (*Current)->Current = BackRef[N]->clone(Arena);
+      (*Current)->Current = FunctionParamBackRefs[N]->clone(Arena);
       Current = &(*Current)->Next;
       continue;
     }
 
-    size_t ArrayDimension = MangledName.size();
+    size_t OldSize = MangledName.size();
 
     *Current = Arena.alloc<ParamList>();
     (*Current)->Current = demangleType(QualifierMangleMode::Drop);
 
-    // Single-letter types are ignored for backreferences because
-    // memorizing them doesn't save anything.
-    if (Idx <= 9 && ArrayDimension - MangledName.size() > 1)
-      BackRef[Idx++] = (*Current)->Current;
+    size_t CharsConsumed = OldSize - MangledName.size();
+    assert(CharsConsumed != 0);
+
+    // Single-letter types are ignored for backreferences because memorizing
+    // them doesn't save anything.
+    if (FunctionParamBackRefCount <= 9 && CharsConsumed > 1)
+      FunctionParamBackRefs[FunctionParamBackRefCount++] = (*Current)->Current;
+
     Current = &(*Current)->Next;
   }
 
   if (Error)
     return {};
 
   // A non-empty parameter list is terminated by either 'Z' (variadic) parameter
   // list or '@' (non variadic).  Careful not to consume "@Z", as in that case
   // the following Z could be a throw specifier.
   if (MangledName.consumeFront('@'))
     return *Head;
 
   if (MangledName.consumeFront('Z')) {
     Head->IsVariadic = true;
     return *Head;
   }
 
   Error = true;
   return {};
 }
 
+ParamList Demangler::demangleTemplateParameterList() {
+  ParamList *Head;
+  ParamList **Current = &Head;
+  while (!Error && !MangledName.startsWith('@')) {
+
+    // Template parameter lists don't participate in back-referencing.
+    *Current = Arena.alloc<ParamList>();
+    (*Current)->Current = demangleType(QualifierMangleMode::Drop);
+
+    Current = &(*Current)->Next;
+  }
+
+  if (Error)
+    return {};
+
+  // Template parameter lists cannot be variadic, so it can only be terminated
+  // by @.
+  if (MangledName.consumeFront('@'))
+    return *Head;
+  Error = true;
+  return {};
+}
+
 void Demangler::output() {
   // Converts an AST to a string.
   //
   // Converting an AST representing a C++ type to a string is tricky due
   // to the bad grammar of the C++ declaration inherited from C. You have
   // to construct a string from inside to outside. For example, if a type
   // X is a pointer to a function returning int, the order you create a
   // string becomes something like this:

llvm/trunk/test/Demangle/ms-back-references.test

+; RUN: llvm-undname < %s | FileCheck %s
+
+; CHECK-NOT: Invalid mangled name
+
+?f1@@YAXPBD0@Z
+; CHECK: void __cdecl f1(char const *, char const *)
+
+?f2@@YAXPBDPAD@Z
+; CHECK: void __cdecl f2(char const *, char *)
+
+?f3@@YAXHPBD0@Z
+; CHECK: void __cdecl f3(int, char const *, char const *)
+
+?f4@@YAPBDPBD0@Z
+; CHECK: char const * __cdecl f4(char const *, char const *)
+
+?f5@@YAXPBDIDPBX0I@Z
+; CHECK: void __cdecl f5(char const *, unsigned int, char, void const *, char const *, unsigned int)
+
+?f6@@YAX_N0@Z
+; CHECK: void __cdecl f6(bool, bool)
+
+?f7@@YAXHPAHH0_N1PA_N@Z
+; CHECK: void __cdecl f7(int, int *, int, int *, bool, bool, bool *)
+
+; FIXME: tests for more than 10 types?
+
+?g1@@YAXUS@@@Z
+; CHECK: void __cdecl g1(struct S)
+
+?g2@@YAXUS@@0@Z
+; CHECK: void __cdecl g2(struct S, struct S)
+
+?g3@@YAXUS@@0PAU1@1@Z
+; CHECK: void __cdecl g3(struct S, struct S, struct S *, struct S *)
+
+?g4@@YAXPBDPAUS@@01@Z
+; CHECK: void __cdecl g4(char const *, struct S *, char const *, struct S *)
+
+?mbb@S@@QAEX_N0@Z
+; CHECK: void __thiscall S::mbb(bool, bool)
+
+?h1@@YAXPBD0P6AXXZ1@Z
+; CHECK: void __cdecl h1(char const *, char const *, void (__cdecl *)(void), void (__cdecl *)(void))
+
+?h2@@YAXP6AXPAX@Z0@Z
+; CHECK: void __cdecl h2(void (__cdecl *)(void *), void *)
+
+?h3@@YAP6APAHPAH0@ZP6APAH00@Z10@Z
+; CHECK: int * (__cdecl * __cdecl h3(int * (__cdecl *)(int *, int *), int * (__cdecl *)(int *, int *), int *))(int *, int *)
+
+?foo@0@YAXXZ
+; CHECK: void __cdecl foo::foo(void)