Limit types of builtins that can be redeclared.

As reported here: https://bugs.llvm.org/show_bug.cgi?id=37033
Any usage of a builtin function that uses a va_list by reference
will cause an assertion when redeclaring it.

After discussion in the review, it was concluded that the correct
way of accomplishing this fix is to make attempts to redeclare certain
builtins an error. Unfortunately, doing this limitation for all builtins
is likely a breaking change, so this commit simply limits it to
types with custom type checking and those that take a reference.

Two tests needed to be updated to make this work.

Differential Revision: https://reviews.llvm.org/D45383

llvm-svn: 330160

diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 792766daf41c6eaa01c400caf6b3c9a2cdb37e34..40561b39d11091ad1f89f872b48e68a60baa5b61 100644 (file)
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1881,6 +1881,10 @@ public:
     return getTypeDeclType(getBuiltinMSVaListDecl());
   }
 
+  /// Return whether a declaration to a builtin is allowed to be
+  /// overloaded/redeclared.
+  bool canBuiltinBeRedeclared(const FunctionDecl *) const;
+
   /// \brief Return a type with additional \c const, \c volatile, or
   /// \c restrict qualifiers.
   QualType getCVRQualifiedType(QualType T, unsigned CVR) const {

diff --git a/clang/include/clang/Basic/Builtins.h b/clang/include/clang/Basic/Builtins.h
index 963c72ea82e0e3bebedf2f49165c20e5ceef815f..a3f42a5ab758c71872c4e0de8f97aa76ae247220 100644 (file)
--- a/clang/include/clang/Basic/Builtins.h
+++ b/clang/include/clang/Basic/Builtins.h
@@ -167,6 +167,13 @@ public:
     return strchr(getRecord(ID).Type, '*') != nullptr;
   }
 
+  /// \brief Return true if this builtin has a result or any arguments which are
+  /// reference types.
+  bool hasReferenceArgsOrResult(unsigned ID) const {
+    return strchr(getRecord(ID).Type, '&') != nullptr ||
+           strchr(getRecord(ID).Type, 'A') != nullptr;
+  }
+
   /// \brief Completely forget that the given ID was ever considered a builtin,
   /// e.g., because the user provided a conflicting signature.
   void forgetBuiltin(unsigned ID, IdentifierTable &Table);
@@ -212,6 +219,10 @@ public:
   /// prefix.
   static bool isBuiltinFunc(const char *Name);
 
+  /// Returns true if this is a builtin that can be redeclared.  Returns true
+  /// for non-builtins.
+  bool canBeRedeclared(unsigned ID) const;
+
 private:
   const Info &getRecord(unsigned ID) const;
 

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d68afa181bc3217ccab85480bf3ea34b6f49bff6..6e9acda22a1239b9cc32007910b8df15dd390768 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -602,6 +602,7 @@ def warn_redecl_library_builtin : Warning<
   "incompatible redeclaration of library function %0">,
   InGroup<DiagGroup<"incompatible-library-redeclaration">>;
 def err_builtin_definition : Error<"definition of builtin function %0">;
+def err_builtin_redeclare : Error<"cannot redeclare builtin function %0">;
 def err_arm_invalid_specialreg : Error<"invalid special register for builtin">;
 def err_invalid_cpu_supports : Error<"invalid cpu feature string for builtin">;
 def err_invalid_cpu_is : Error<"invalid cpu name for builtin">;

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 4b095ce45ab4e42bbfa29bcb3e246822e61cb1e4..d61ca589d5bd4b584a8ec43c31dcb72e17ca4604 100644 (file)
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -7244,6 +7244,10 @@ TypedefDecl *ASTContext::getBuiltinMSVaListDecl() const {
   return BuiltinMSVaListDecl;
 }
 
+bool ASTContext::canBuiltinBeRedeclared(const FunctionDecl *FD) const {
+  return BuiltinInfo.canBeRedeclared(FD->getBuiltinID());
+}
+
 void ASTContext::setObjCConstantStringInterface(ObjCInterfaceDecl *Decl) {
   assert(ObjCConstantStringType.isNull() &&
          "'NSConstantString' type already set!");

diff --git a/clang/lib/Basic/Builtins.cpp b/clang/lib/Basic/Builtins.cpp
index ed7f87c9b95c7f565174311a1f89ebb399453eda..bb04384fd65e54b320866eabfd7a67f4d0888947 100644 (file)
--- a/clang/lib/Basic/Builtins.cpp
+++ b/clang/lib/Basic/Builtins.cpp
@@ -139,3 +139,10 @@ bool Builtin::Context::isScanfLike(unsigned ID, unsigned &FormatIdx,
                                    bool &HasVAListArg) {
   return isLike(ID, FormatIdx, HasVAListArg, "sS");
 }
+
+bool Builtin::Context::canBeRedeclared(unsigned ID) const {
+  return ID == Builtin::NotBuiltin ||
+         ID == Builtin::BI__va_start ||
+         (!hasReferenceArgsOrResult(ID) &&
+          !hasCustomTypechecking(ID));
+}

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 9228d65250cd56d01169e6af2394fa9db6d98d17..e692b35705bfa2c9e40d4c3572c4666339fd935d 100644 (file)
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3011,6 +3011,14 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD,
   if (Old->isInvalidDecl())
     return true;
 
+  // Disallow redeclaration of some builtins.
+  if (!getASTContext().canBuiltinBeRedeclared(Old)) {
+    Diag(New->getLocation(), diag::err_builtin_redeclare) << Old->getDeclName();
+    Diag(Old->getLocation(), diag::note_previous_builtin_declaration)
+        << Old << Old->getType();
+    return true;
+  }
+
   diag::kind PrevDiag;
   SourceLocation OldLocation;
   std::tie(PrevDiag, OldLocation) =

diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 7905f5e6228a2a77b8606ce51a45346407298560..9cce75c71977697a188e1bf0090117efe4ff3c0e 100644 (file)
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -998,6 +998,13 @@ Sema::CheckOverload(Scope *S, FunctionDecl *New, const LookupResult &Old,
         Match = *I;
         return Ovl_Match;
       }
+
+      // Builtins that have custom typechecking or have a reference should
+      // not be overloadable or redeclarable.
+      if (!getASTContext().canBuiltinBeRedeclared(OldF)) {
+        Match = *I;
+        return Ovl_NonFunction;
+      }
     } else if (isa<UsingDecl>(OldD) || isa<UsingPackDecl>(OldD)) {
       // We can overload with these, which can show up when doing
       // redeclaration checks for UsingDecls.

diff --git a/clang/test/Sema/builtin-redecl.cpp b/clang/test/Sema/builtin-redecl.cpp
new file mode 100644 (file)
index 0000000..9d1a7ff
--- /dev/null
+++ b/clang/test/Sema/builtin-redecl.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify 
+// RUN: %clang_cc1 %s -fsyntax-only -verify -x c
+// RUN: %clang_cc1 %s -fsyntax-only -verify -fms-compatibility
+
+// Redeclaring library builtins is OK.
+void exit(int);
+
+// expected-error@+2 {{cannot redeclare builtin function '__builtin_va_copy'}}
+// expected-note@+1 {{'__builtin_va_copy' is a builtin with type}}
+void __builtin_va_copy(double d);
+
+// expected-error@+2 {{cannot redeclare builtin function '__builtin_va_end'}}
+// expected-note@+1 {{'__builtin_va_end' is a builtin with type}}
+void __builtin_va_end(__builtin_va_list);
+// RUN: %clang_cc1 %s -fsyntax-only -verify 
+// RUN: %clang_cc1 %s -fsyntax-only -verify -x c
+
+void __va_start(__builtin_va_list*, ...);