george.burgess.iv created this revision.
george.burgess.iv added a subscriber: cfe-commits.

This keeps the ICE in https://llvm.org/bugs/show_bug.cgi?id=25836 from 
happening. Long story short, the following C code will make clang overflow its 
stack:

```
int Foo(struct A*) __attribute__((overloadable)) {}
```

...Because the mangling of `struct A` (which is declared inside of `Foo`, and 
therefore nested in `Foo`) depends on the mangling of `Foo`, and the mangling 
of `Foo` depends on the mangling of `struct A`. It's a vicious cycle.

This patch fixes the ICE by simply not allowing the user to declare new named 
types in the parameter list of an overloadable function. See the bug for 

If no one wants to pick this up, I'll find a victim at some point. :)

http://reviews.llvm.org/D15621

Files:
  include/clang/AST/Decl.h
  include/clang/Basic/AttrDocs.td
  include/clang/Sema/Sema.h
  lib/AST/ASTDumper.cpp
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  test/CodeGen/overloadable.c
  test/Sema/overloadable.c

Index: test/Sema/overloadable.c
===================================================================
--- test/Sema/overloadable.c
+++ test/Sema/overloadable.c
@@ -99,3 +99,8 @@
   unsigned char *c;
   multi_type(c);
 }
+
+void pr25836() {
+  void foo(struct Declaring *) __attribute__((overloadable)); // expected-error{{'Declaring' cannot be defined in a parameter type}}
+  void foo(struct Foo {int foo; int bar;} a) __attribute__((overloadable)); // expected-error{{'Foo' cannot be defined in a parameter type}}
+}
Index: test/CodeGen/overloadable.c
===================================================================
--- test/CodeGen/overloadable.c
+++ test/CodeGen/overloadable.c
@@ -16,6 +16,8 @@
 
 void __attribute__((overloadable)) f(void (*x)()) {}
 
+void anon_struct(struct {int foo; int bar;} *a) __attribute__((overloadable)) { }
+
 int main() {
   int iv = 17;
   float fv = 3.0f;
@@ -28,4 +30,5 @@
   dv = f(dv);
   cdv = f(cdv);
   vv = f(vv);
+  anon_struct((void*)0);
 }
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8289,6 +8289,16 @@
   return NewFD;
 }
 
+/// We complain every time a user declares an enum/struct/union/class inside of
+/// a parameter list. In the case where the user is doing so in a function that
+/// can be overloaded, our complaint is an error; otherwise, it's a warning. We
+/// don't always know if a function is overloadable when we would normally
+/// complain, so we need to defer complaints where we're not sure until later.
+static bool
+needToDeferDeclInParamListComplaint(const clang::LangOptions &Opts) {
+  return !Opts.CPlusPlus && !Opts.ObjC1 && !Opts.ObjC2;
+}
+
 /// \brief Perform semantic checking of a new function declaration.
 ///
 /// Performs semantic analysis of the new function declaration
@@ -8394,6 +8404,22 @@
     }
   }
 
+  // If we have a decl in our param list, and the complaint about it was
+  // deferred, now is a good time to complain.
+  if (!NewFD->getDeclsInPrototypeScope().empty() &&
+      needToDeferDeclInParamListComplaint(Context.getLangOpts())) {
+    if (NewFD->hasAttr<OverloadableAttr>()) {
+      for (TagDecl *Tag : NewFD->getDeclsInPrototypeScope())
+        Diag(Tag->getLocStart(), diag::err_type_defined_in_param_type) << Tag;
+      NewFD->setInvalidDecl();
+      return Redeclaration;
+    }
+
+    for (TagDecl *Tag : NewFD->getDeclsInPrototypeScope())
+      Diag(Tag->getLocStart(), diag::warn_decl_in_param_list) <<
+        Context.getTagDeclType(Tag);
+  }
+
   // C++11 [dcl.constexpr]p8:
   //   A constexpr specifier for a non-static member function that is not
   //   a constructor declares that member function to be const.
@@ -10901,12 +10927,7 @@
   // If we had any tags defined in the function prototype,
   // introduce them into the function scope.
   if (FnBodyScope) {
-    for (ArrayRef<NamedDecl *>::iterator
-             I = FD->getDeclsInPrototypeScope().begin(),
-             E = FD->getDeclsInPrototypeScope().end();
-         I != E; ++I) {
-      NamedDecl *D = *I;
-
+    for (TagDecl *D : FD->getDeclsInPrototypeScope()) {
       // Some of these decls (like enums) may have been pinned to the
       // translation unit for lack of a real context earlier. If so, remove
       // from the translation unit and reattach to the current context.
@@ -12581,18 +12602,20 @@
   // the list of decls to inject into the function definition scope.
   if ((Name || Kind == TTK_Enum) &&
       getNonFieldDeclScope(S)->isFunctionPrototypeScope()) {
-    if (getLangOpts().CPlusPlus) {
-      // C++ [dcl.fct]p6:
-      //   Types shall not be defined in return or parameter types.
-      if (TUK == TUK_Definition && !IsTypeSpecifier) {
-        Diag(Loc, diag::err_type_defined_in_param_type)
-            << Name;
-        Invalid = true;
+    DeclsInPrototypeScope.push_back(New);
+
+    if (!needToDeferDeclInParamListComplaint(Context.getLangOpts())) {
+      if (getLangOpts().CPlusPlus) {
+        // C++ [dcl.fct]p6:
+        //   Types shall not be defined in return or parameter types.
+        if (TUK == TUK_Definition && !IsTypeSpecifier) {
+          Diag(Loc, diag::err_type_defined_in_param_type) << Name;
+          Invalid = true;
+        }
+      } else {
+        Diag(Loc, diag::warn_decl_in_param_list) << Context.getTagDeclType(New);
       }
-    } else {
-      Diag(Loc, diag::warn_decl_in_param_list) << Context.getTagDeclType(New);
     }
-    DeclsInPrototypeScope.push_back(New);
   }
 
   if (Invalid)
Index: lib/AST/Decl.cpp
===================================================================
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2675,11 +2675,11 @@
   }
 }
 
-void FunctionDecl::setDeclsInPrototypeScope(ArrayRef<NamedDecl *> NewDecls) {
+void FunctionDecl::setDeclsInPrototypeScope(ArrayRef<TagDecl *> NewDecls) {
   assert(DeclsInPrototypeScope.empty() && "Already has prototype decls!");
 
   if (!NewDecls.empty()) {
-    NamedDecl **A = new (getASTContext()) NamedDecl*[NewDecls.size()];
+    TagDecl **A = new (getASTContext()) TagDecl*[NewDecls.size()];
     std::copy(NewDecls.begin(), NewDecls.end(), A);
     DeclsInPrototypeScope = llvm::makeArrayRef(A, NewDecls.size());
     // Move declarations introduced in prototype to the function context.
Index: lib/AST/ASTDumper.cpp
===================================================================
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -1121,10 +1121,8 @@
           D->getTemplateSpecializationInfo())
     dumpTemplateArgumentList(*FTSI->TemplateArguments);
 
-  for (ArrayRef<NamedDecl *>::iterator
-       I = D->getDeclsInPrototypeScope().begin(),
-       E = D->getDeclsInPrototypeScope().end(); I != E; ++I)
-    dumpDecl(*I);
+  for (const TagDecl *TD : D->getDeclsInPrototypeScope())
+    dumpDecl(TD);
 
   if (!D->param_begin() && D->getNumParams())
     dumpChild([=] { OS << "<<NULL params x " << D->getNumParams() << ">>"; });
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1471,7 +1471,7 @@
   /// that incorrectly end up in translation unit scope because there is no
   /// function to pin them on. ActOnFunctionDeclarator reads this list and patches
   /// them into the FunctionDecl.
-  std::vector<NamedDecl*> DeclsInPrototypeScope;
+  std::vector<TagDecl*> DeclsInPrototypeScope;
 
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr);
 
Index: include/clang/Basic/AttrDocs.td
===================================================================
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -440,6 +440,9 @@
   linkage specification, it's name *will* be mangled in the same way as it
   would in C.
 
+Declarations of named types inside of parameter lists are not allowed in
+overloaded functions.
+
 Query for this feature with ``__has_extension(attribute_overloadable)``.
   }];
 }
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -1554,7 +1554,7 @@
   /// DeclsInPrototypeScope - Array of pointers to NamedDecls for
   /// decls defined in the function prototype that are not parameters. E.g.
   /// 'enum Y' in 'void f(enum Y {AA} x) {}'.
-  ArrayRef<NamedDecl *> DeclsInPrototypeScope;
+  ArrayRef<TagDecl *> DeclsInPrototypeScope;
 
   LazyDeclStmtPtr Body;
 
@@ -1995,10 +1995,10 @@
     return llvm::makeArrayRef(ParamInfo, getNumParams());
   }
 
-  ArrayRef<NamedDecl *> getDeclsInPrototypeScope() const {
+  ArrayRef<TagDecl *> getDeclsInPrototypeScope() const {
     return DeclsInPrototypeScope;
   }
-  void setDeclsInPrototypeScope(ArrayRef<NamedDecl *> NewDecls);
+  void setDeclsInPrototypeScope(ArrayRef<TagDecl *> NewDecls);
 
   /// getMinRequiredArguments - Returns the minimum number of arguments
   /// needed to call this function. This may be fewer than the number of
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to