From 77d451a53d8c57ffc3f7d90c0f8d4b0c045579e7 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron@aaronballman.com>
Date: Fri, 1 Dec 2017 16:53:49 +0000
Subject: [PATCH] Disallow a cleanup attribute from appertaining to a parameter
 (the attribute only appertains to local variables and is silently a noop on
 parameters). This repurposes the unused (and syntactically incorrect)
 NormalVar attribute subject.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@319555 91177308-0d34-0410-b5e6-96231b3b80d8
---
 include/clang/Basic/Attr.td | 12 +++---------
 lib/Sema/SemaDeclAttr.cpp   |  8 +-------
 test/Sema/attr-cleanup.c    | 10 ++++++----
 3 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index a13052a3b8..0ee89e4fd0 100644
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -77,13 +77,8 @@ class SubsetSubject<AttrSubject base, code check, string diag> : AttrSubject {
   string DiagSpelling = diag;
 }
 
-// This is the type of a variable which C++11 allows alignas(...) to appertain
-// to.
-def NormalVar : SubsetSubject<Var,
-                              [{S->getStorageClass() != VarDecl::Register &&
-                                S->getKind() != Decl::ImplicitParam &&
-                                S->getKind() != Decl::ParmVar &&
-                                S->getKind() != Decl::NonTypeTemplateParm}],
+def LocalVar : SubsetSubject<Var,
+                              [{S->hasLocalStorage() && !isa<ParmVarDecl>(S)}],
                               "local variables">;
 def NonParmVar : SubsetSubject<Var,
                                [{S->getKind() != Decl::ParmVar}],
@@ -533,7 +528,6 @@ def Alias : Attr {
 def Aligned : InheritableAttr {
   let Spellings = [GCC<"aligned">, Declspec<"align">, Keyword<"alignas">,
                    Keyword<"_Alignas">];
-//  let Subjects = SubjectList<[NonBitField, NormalVar, Tag]>;
   let Args = [AlignedArgument<"Alignment", 1>];
   let Accessors = [Accessor<"isGNU", [GCC<"aligned">]>,
                    Accessor<"isC11", [Keyword<"_Alignas">]>,
@@ -768,7 +762,7 @@ def CFConsumed : InheritableParamAttr {
 def Cleanup : InheritableAttr {
   let Spellings = [GCC<"cleanup">];
   let Args = [FunctionArgument<"FunctionDecl">];
-  let Subjects = SubjectList<[Var]>;
+  let Subjects = SubjectList<[LocalVar]>;
   let Documentation = [Undocumented];
 }
 
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index 655d6e90e2..7698fb590a 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -3067,12 +3067,6 @@ static void handleTargetAttr(Sema &S, Decl *D, const AttributeList &Attr) {
 }
 
 static void handleCleanupAttr(Sema &S, Decl *D, const AttributeList &Attr) {
-  VarDecl *VD = cast<VarDecl>(D);
-  if (!VD->hasLocalStorage()) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
-    return;
-  }
-
   Expr *E = Attr.getArgAsExpr(0);
   SourceLocation Loc = E->getExprLoc();
   FunctionDecl *FD = nullptr;
@@ -3115,7 +3109,7 @@ static void handleCleanupAttr(Sema &S, Decl *D, const AttributeList &Attr) {
 
   // We're currently more strict than GCC about what function types we accept.
   // If this ever proves to be a problem it should be easy to fix.
-  QualType Ty = S.Context.getPointerType(VD->getType());
+  QualType Ty = S.Context.getPointerType(cast<VarDecl>(D)->getType());
   QualType ParamTy = FD->getParamDecl(0)->getType();
   if (S.CheckAssignmentConstraints(FD->getParamDecl(0)->getLocation(),
                                    ParamTy, Ty) != Sema::Compatible) {
diff --git a/test/Sema/attr-cleanup.c b/test/Sema/attr-cleanup.c
index 26f283a1a4..36692898c1 100644
--- a/test/Sema/attr-cleanup.c
+++ b/test/Sema/attr-cleanup.c
@@ -2,16 +2,16 @@
 
 void c1(int *a);
 
-extern int g1 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute ignored}}
-int g2 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute ignored}}
-static int g3 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute ignored}}
+extern int g1 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute only applies to local variables}}
+int g2 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute only applies to local variables}}
+static int g3 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute only applies to local variables}}
 
 void t1()
 {
     int v1 __attribute((cleanup)); // expected-error {{'cleanup' attribute takes one argument}}
     int v2 __attribute((cleanup(1, 2))); // expected-error {{'cleanup' attribute takes one argument}}
 
-    static int v3 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute ignored}}
+    static int v3 __attribute((cleanup(c1))); // expected-warning {{'cleanup' attribute only applies to local variables}}
 
     int v4 __attribute((cleanup(h))); // expected-error {{use of undeclared identifier 'h'}}
 
@@ -46,3 +46,5 @@ void t5() {
 void t6(void) {
   int i __attribute__((cleanup((void *)0)));  // expected-error {{'cleanup' argument is not a function}}
 }
+
+void t7(__attribute__((cleanup(c4))) int a) {} // expected-warning {{'cleanup' attribute only applies to local variables}}
-- 
2.50.1