Fix completely bogus types for some builtins:

 * In C++, never create a FunctionNoProtoType for a builtin (fixes C++1z
   crasher from r289754).

 * Fix type of __sync_synchronize to be a no-parameter function rather than a
   varargs function. This matches GCC.

 * Fix type of vfprintf to match its actual type. We gave it a wrong type due
   to PR4290 (apparently autoconf generates invalid code and expects compilers
   to choke it down or it miscompiles the program; the relevant error in clang
   was downgraded to a warning in r122744 to fix other occurrences of this
   autoconf brokenness, so we don't need this workaround any more).

 * Turn off vararg argument checking for __noop, since it's not *really* a
   varargs function. Alternatively we could add custom type checking for it
   and synthesize parameter types matching the actual arguments in each call,
   but that seemed like overkill.

llvm-svn: 290146

7 files changed, 30 insertions, 16 deletions

diff --git a/clang/include/clang/Basic/Builtins.def b/clang/include/clang/Basic/Builtins.def
index 731d8565486..ec0a2796ec0 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -702,7 +702,7 @@ BUILTIN(__atomic_is_lock_free, "izvCD*", "n")
 #undef ATOMIC_BUILTIN
 
 // Non-overloaded atomic builtins.
-BUILTIN(__sync_synchronize, "v.", "n")
+BUILTIN(__sync_synchronize, "v", "n")
 // GCC does not support these, they are a Clang extension.
 BUILTIN(__sync_fetch_and_min, "iiD*i", "n")
 BUILTIN(__sync_fetch_and_max, "iiD*i", "n")
@@ -813,7 +813,7 @@ LIBBUILTIN(fprintf, "iP*cC*.",    "fp:1:", "stdio.h", ALL_LANGUAGES)
 LIBBUILTIN(snprintf, "ic*zcC*.",  "fp:2:", "stdio.h", ALL_LANGUAGES)
 LIBBUILTIN(sprintf, "ic*cC*.",    "fp:1:", "stdio.h", ALL_LANGUAGES)
 LIBBUILTIN(vprintf, "icC*a",      "fP:0:", "stdio.h", ALL_LANGUAGES)
-LIBBUILTIN(vfprintf, "i.",        "fP:1:", "stdio.h", ALL_LANGUAGES)
+LIBBUILTIN(vfprintf, "iP*cC*a",   "fP:1:", "stdio.h", ALL_LANGUAGES)
 LIBBUILTIN(vsnprintf, "ic*zcC*a", "fP:2:", "stdio.h", ALL_LANGUAGES)
 LIBBUILTIN(vsprintf, "ic*cC*a",   "fP:1:", "stdio.h", ALL_LANGUAGES)
 LIBBUILTIN(scanf, "icC*R.",       "fs:0:", "stdio.h", ALL_LANGUAGES)
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 0ad663fdf96..4abca4ce5d1 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -8729,8 +8729,8 @@ QualType ASTContext::GetBuiltinType(unsigned Id,
 
   bool Variadic = (TypeStr[0] == '.');
 
-  // We really shouldn't be making a no-proto type here, especially in C++.
-  if (ArgTypes.empty() && Variadic)
+  // We really shouldn't be making a no-proto type here.
+  if (ArgTypes.empty() && Variadic && !getLangOpts().CPlusPlus)
     return getFunctionNoProtoType(ResType, EI);
 
   FunctionProtoType::ExtProtoInfo EPI;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 450e2789c79..9c902959233 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -2448,7 +2448,9 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
 
   // Refuse POD arguments that weren't caught by the format string
   // checks above.
-  if (CallType != VariadicDoesNotApply) {
+  auto *FD = dyn_cast_or_null<FunctionDecl>(FDecl);
+  if (CallType != VariadicDoesNotApply &&
+      (!FD || FD->getBuiltinID() != Builtin::BI__noop)) {
     unsigned NumParams = Proto ? Proto->getNumParams()
                        : FDecl && isa<FunctionDecl>(FDecl)
                            ? cast<FunctionDecl>(FDecl)->getNumParams()
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 781f78018b4..2ac2aca6f66 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -288,14 +288,15 @@ bool Sema::CheckEquivalentExceptionSpec(FunctionDecl *Old, FunctionDecl *New) {
   // The new function declaration is only missing an empty exception
   // specification "throw()". If the throw() specification came from a
   // function in a system header that has C linkage, just add an empty
-  // exception specification to the "new" declaration. This is an
-  // egregious workaround for glibc, which adds throw() specifications
-  // to many libc functions as an optimization. Unfortunately, that
-  // optimization isn't permitted by the C++ standard, so we're forced
-  // to work around it here.
+  // exception specification to the "new" declaration. Note that C library
+  // implementations are permitted to add these nothrow exception
+  // specifications.
+  //
+  // Likewise if the old function is a builtin.
   if (MissingEmptyExceptionSpecification && NewProto &&
       (Old->getLocation().isInvalid() ||
-       Context.getSourceManager().isInSystemHeader(Old->getLocation())) &&
+       Context.getSourceManager().isInSystemHeader(Old->getLocation()) ||
+       Old->getBuiltinID()) &&
       Old->isExternC()) {
     New->setType(Context.getFunctionType(
         NewProto->getReturnType(), NewProto->getParamTypes(),
diff --git a/clang/test/Sema/vfprintf-invalid-redecl.c b/clang/test/Sema/vfprintf-invalid-redecl.c
index cbf47a69a9a..f06b1b6d214 100644
--- a/clang/test/Sema/vfprintf-invalid-redecl.c
+++ b/clang/test/Sema/vfprintf-invalid-redecl.c
@@ -3,4 +3,4 @@
 
 // The following declaration is not compatible with vfprintf(), but make
 // sure this isn't an error: autoconf expects this to build.
-char vfprintf(); // expected-warning {{incompatible redeclaration of library function 'vfprintf'}} expected-note {{'vfprintf' is a builtin}}
+char vfprintf(); // expected-warning {{declaration of built-in function 'vfprintf'}}
diff --git a/clang/test/Sema/vfprintf-valid-redecl.c b/clang/test/Sema/vfprintf-valid-redecl.c
index 8ed18786ab7..eb5c4d33071 100644
--- a/clang/test/Sema/vfprintf-valid-redecl.c
+++ b/clang/test/Sema/vfprintf-valid-redecl.c
@@ -1,16 +1,18 @@
 // RUN: %clang_cc1 %s -fsyntax-only -pedantic -verify
 // RUN: %clang_cc1 %s -fsyntax-only -pedantic -verify -DPREDECLARE
-// expected-no-diagnostics
 
 #ifdef PREDECLARE
 // PR16344
 // Clang has defined 'vfprint' in builtin list. If the following line occurs before any other
 // `vfprintf' in this file, and we getPreviousDecl()->getTypeSourceInfo() on it, then we will
 // get a null pointer since the one in builtin list doesn't has valid TypeSourceInfo.
-int vfprintf(void) { return 0; }
+int vfprintf(void) { return 0; } // expected-warning {{requires inclusion of the header <stdio.h>}}
 #endif
 
 // PR4290
 // The following declaration is compatible with vfprintf, so we shouldn't
-// warn.
+// reject.
 int vfprintf();
+#ifndef PREDECLARE
+// expected-warning@-2 {{requires inclusion of the header <stdio.h>}}
+#endif
diff --git a/clang/test/SemaCXX/builtins.cpp b/clang/test/SemaCXX/builtins.cpp
index 69bdfa61451..f26931b55bd 100644
--- a/clang/test/SemaCXX/builtins.cpp
+++ b/clang/test/SemaCXX/builtins.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++11
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++11 -fcxx-exceptions
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++1z -fcxx-exceptions
 typedef const struct __CFString * CFStringRef;
 #define CFSTR __builtin___CFStringMakeConstantString
 
@@ -44,3 +45,11 @@ void no_ms_builtins() {
   __noop(1); // expected-error {{use of undeclared}}
   __debugbreak(); // expected-error {{use of undeclared}}
 }
+
+struct FILE;
+extern "C" int vfprintf(FILE *__restrict, const char *__restrict,
+                        __builtin_va_list va);
+
+void synchronize_args() {
+  __sync_synchronize(0); // expected-error {{too many arguments}}
+}