Fix pretty printing the unspecified param of a variadic function

[asmith]

Summary

• Fix a bug in PrettyBuiltinDumper that returns "void" as the name for an unspecified builtin type. Since the unspecified param of a variadic function is considered a builtin of unspecified type in PDBs, we set "..." for its name.
  • Provide a method to determine if a PDBSymbolFunc is variadic in PrettyFunctionDumper since PDBSymbolFunc::getArgument() doesn't return the last unspecified-type param.
  • Add a pretty-func-dumper.test to test pretty dumping of variadic functions.

[zturner]

Thanks for finding this and adding a test. Only one comment, but after that this will be good.

tools/llvm-pdbutil/PrettyFunctionDumper.cpp

193–194

This check is pretty unintuitive. Is it possible to implement just by looking at the last argument and checking if it's a builtin with type unspecified? Something like

auto Last = SigArguments->getLast();
if (Last->getType() == PDB_Type::Builtin && Last->getBuiltinType() == PDB_BuiltinType::None)

? Either way, to make sure nobody else has to rediscover this, it would be nice if whatever check we do decide on can be put into a function in PDBFunctionType and PDBFunctionSigType, so that we can just say if (Signature->isCVarArgs()) or if (Func->isCVarArgs())?

[asmith]

I've added the test and were there any other comments?

Would like to close this review out as it's blocking D41427.

Thanks!

[zturner]

LGTM, sorry for the delay, thanks for reminding me.

test/tools/llvm-pdbdump/Inputs/PrettyFuncDumperTest.cpp

+// Compile with "cl /c /Zi /GR- PrettyFuncDumperTest.cpp"
+// Link with "link PrettyFuncDumperTest.obj /debug /nodefaultlib /entry:main"
+
+typedef void (*FuncPtrA)();
+FuncPtrA FuncVarA;
+
+typedef float (*FuncPtrB)(void);
+FuncPtrB FuncVarB;
+
+typedef int(*VariadicFuncPtrTypedef)(char, double, ...);
+VariadicFuncPtrTypedef VariadicFuncVar;
+
+void Func(int array[]) { return; }
+
+template <int N=1, class ...T>
+void TemplateFunc(T ...Arg) {
+  return;
+}
+
+namespace {
+  void Func(int& a, const double b, volatile bool c) { return; }
+}
+
+namespace NS {
+  void Func(char a, int b, ...) {
+    return;
+  }
+}
+
+namespace MemberFuncsTest {
+  class A {
+  public:
+    int FuncA() { return 1; }
+  };
+}
+
+int main() {
+  MemberFuncsTest::A v1;
+  v1.FuncA();
+
+  NS::Func('c', 2, 10, 100);
+
+  TemplateFunc(10);
+  TemplateFunc(10, 11, 88);
+  return 0;
+}

test/tools/llvm-pdbdump/pretty-func-dumper.test

+; RUN: llvm-pdbutil pretty -all -class-recurse-depth=1 \
+; RUN:   %p/Inputs/PrettyFuncDumperTest.pdb > %t
+; RUN: FileCheck -input-file=%t %s -check-prefix=GLOBALS_FUNC
+; RUN: FileCheck -input-file=%t %s -check-prefix=GLOBALS_DATA
+; RUN: FileCheck -input-file=%t %s -check-prefix=TYPEDEF
+
+; GLOBALS_FUNC: ---GLOBALS---
+; GLOBALS_FUNC-DAG: void __cdecl NS::Func(char a, int b, ...)
+; GLOBALS_FUNC-DAG: void __cdecl TemplateFunc<1,int>(int <Arg_0>)
+; GLOBALS_FUNC-DAG: void __cdecl TemplateFunc<1,int,int,int>(int <Arg_0>, int <Arg_1>, int <Arg_2>)
+; GLOBALS_FUNC-DAG: void __cdecl `anonymous namespace'::Func(int& a, const double b, volatile bool c)
+; GLOBALS_FUNC-DAG: void __cdecl Func(int* array)
+; GLOBALS_FUNC-DAG: int MemberFuncsTest::A::FuncA()
+
+; GLOBALS_DATA: ---GLOBALS---
+; GLOBALS_DATA-DAG: void (__cdecl * FuncVarA)()
+; GLOBALS_DATA-DAG: float (__cdecl * FuncVarB)()
+; GLOBALS_DATA-DAG: int (__cdecl * VariadicFuncVar)(char, double, ...)
+
+; TYPEDEF: ---TYPES---
+; TYPEDEF: Typedefs:
+; TYPEDEF-DAG: typedef void (__cdecl *)() FuncPtrA
+; TYPEDEF-DAG: typedef int (__cdecl *)(char, double, ...) VariadicFuncPtrTypedef
+; TYPEDEF-DAG: typedef float (__cdecl *)() FuncPtrB

tools/llvm-pdbutil/PrettyBuiltinDumper.cpp

     return "HRESULT";
   case PDB_BuiltinType::BCD:
     return "HRESULT";
+  case PDB_BuiltinType::None:
+    return "...";
   default:
     return "void";
   }

tools/llvm-pdbutil/PrettyFunctionDumper.cpp

       if (++Index < Arguments->getChildCount())
         Printer << ", ";
     }
+    if (auto SigArguments = Signature->getArguments()) {
+      if (SigArguments->getChildCount() > Arguments->getChildCount())
+        Printer << ", ...";
+    }
   }
   Printer << ")";
   if (Symbol.isConstType())