r310698 - Revert "Thread Safety Analysis: warn on nonsensical attributes."

Josh Gao via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 11 00:54:35 PDT 2017


Author: jmgao
Date: Fri Aug 11 00:54:35 2017
New Revision: 310698

URL: http://llvm.org/viewvc/llvm-project?rev=310698&view=rev
Log:
Revert "Thread Safety Analysis: warn on nonsensical attributes."

This reverts commit rL310403, which caused spurious warnings in libc++,
because it didn't properly handle templated scoped lockable types.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaDeclAttr.cpp
    cfe/trunk/test/Sema/attr-capabilities.c
    cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=310698&r1=310697&r2=310698&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Aug 11 00:54:35 2017
@@ -2935,16 +2935,6 @@ def warn_thread_attribute_decl_not_locka
   "%0 attribute can only be applied in a context annotated "
   "with 'capability(\"mutex\")' attribute">,
   InGroup<ThreadSafetyAttributes>, DefaultIgnore;
-def warn_thread_attribute_noargs_not_lockable : Warning<
-  "%0 attribute requires type annotated with 'capability' attribute; "
-  "type here is %1">,
-  InGroup<ThreadSafetyAttributes>, DefaultIgnore;
-def warn_thread_attribute_noargs_not_method : Warning<
-  "%0 attribute without arguments can only be applied to a method of a class">,
-  InGroup<ThreadSafetyAttributes>, DefaultIgnore;
-def warn_thread_attribute_noargs_static_method : Warning<
-  "%0 attribute without arguments cannot be applied to a static method">,
-  InGroup<ThreadSafetyAttributes>, DefaultIgnore;
 def warn_thread_attribute_decl_not_pointer : Warning<
   "%0 only applies to pointer types; type here is %1">,
   InGroup<ThreadSafetyAttributes>, DefaultIgnore;

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=310698&r1=310697&r2=310698&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Aug 11 00:54:35 2017
@@ -480,7 +480,7 @@ static const RecordType *getRecordType(Q
   return nullptr;
 }
 
-template <typename T> static bool checkRecordTypeForAttr(Sema &S, QualType Ty) {
+static bool checkRecordTypeForCapability(Sema &S, QualType Ty) {
   const RecordType *RT = getRecordType(Ty);
 
   if (!RT)
@@ -497,7 +497,7 @@ template <typename T> static bool checkR
 
   // Check if the record itself has a capability.
   RecordDecl *RD = RT->getDecl();
-  if (RD->hasAttr<T>())
+  if (RD->hasAttr<CapabilityAttr>())
     return true;
 
   // Else check if any base classes have a capability.
@@ -505,14 +505,14 @@ template <typename T> static bool checkR
     CXXBasePaths BPaths(false, false);
     if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) {
           const auto *Type = BS->getType()->getAs<RecordType>();
-          return Type->getDecl()->hasAttr<T>();
+          return Type->getDecl()->hasAttr<CapabilityAttr>();
         }, BPaths))
       return true;
   }
   return false;
 }
 
-template <typename T> static bool checkTypedefTypeForAttr(QualType Ty) {
+static bool checkTypedefTypeForCapability(QualType Ty) {
   const auto *TD = Ty->getAs<TypedefType>();
   if (!TD)
     return false;
@@ -521,27 +521,19 @@ template <typename T> static bool checkT
   if (!TN)
     return false;
 
-  return TN->hasAttr<T>();
+  return TN->hasAttr<CapabilityAttr>();
 }
 
-template <typename T> static bool typeHasAttr(Sema &S, QualType Ty) {
-  if (checkTypedefTypeForAttr<T>(Ty))
+static bool typeHasCapability(Sema &S, QualType Ty) {
+  if (checkTypedefTypeForCapability(Ty))
     return true;
 
-  if (checkRecordTypeForAttr<T>(S, Ty))
+  if (checkRecordTypeForCapability(S, Ty))
     return true;
 
   return false;
 }
 
-static bool typeHasCapability(Sema &S, QualType Ty) {
-  return typeHasAttr<CapabilityAttr>(S, Ty);
-}
-
-static bool typeHasScopedLockable(Sema &S, QualType Ty) {
-  return typeHasAttr<ScopedLockableAttr>(S, Ty);
-}
-
 static bool isCapabilityExpr(Sema &S, const Expr *Ex) {
   // Capability expressions are simple expressions involving the boolean logic
   // operators &&, || or !, a simple DeclRefExpr, CastExpr or a ParenExpr. Once
@@ -578,8 +570,6 @@ static void checkAttrArgsAreCapabilityOb
                                            SmallVectorImpl<Expr *> &Args,
                                            int Sidx = 0,
                                            bool ParamIdxOk = false) {
-  bool TriedParam = false;
-
   for (unsigned Idx = Sidx; Idx < Attr.getNumArgs(); ++Idx) {
     Expr *ArgExp = Attr.getArgAsExpr(Idx);
 
@@ -620,18 +610,15 @@ static void checkAttrArgsAreCapabilityOb
     const RecordType *RT = getRecordType(ArgTy);
 
     // Now check if we index into a record type function param.
-    if (!RT && ParamIdxOk) {
+    if(!RT && ParamIdxOk) {
       FunctionDecl *FD = dyn_cast<FunctionDecl>(D);
       IntegerLiteral *IL = dyn_cast<IntegerLiteral>(ArgExp);
-      if (FD && IL) {
-        // Don't emit free function warnings if an index was given.
-        TriedParam = true;
-
+      if(FD && IL) {
         unsigned int NumParams = FD->getNumParams();
         llvm::APInt ArgValue = IL->getValue();
         uint64_t ParamIdxFromOne = ArgValue.getZExtValue();
         uint64_t ParamIdxFromZero = ParamIdxFromOne - 1;
-        if (!ArgValue.isStrictlyPositive() || ParamIdxFromOne > NumParams) {
+        if(!ArgValue.isStrictlyPositive() || ParamIdxFromOne > NumParams) {
           S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_range)
             << Attr.getName() << Idx + 1 << NumParams;
           continue;
@@ -650,28 +637,6 @@ static void checkAttrArgsAreCapabilityOb
 
     Args.push_back(ArgExp);
   }
-
-  // If we don't have any lockable arguments, verify that we're an instance
-  // method on a lockable type.
-  if (Args.empty() && !TriedParam) {
-    if (auto *MD = dyn_cast<CXXMethodDecl>(D)) {
-      if (MD->isStatic()) {
-        S.Diag(Attr.getLoc(), diag::warn_thread_attribute_noargs_static_method)
-            << Attr.getName();
-        return;
-      }
-
-      QualType ThisType = MD->getThisType(MD->getASTContext());
-      if (!typeHasCapability(S, ThisType) &&
-          !typeHasScopedLockable(S, ThisType)) {
-        S.Diag(Attr.getLoc(), diag::warn_thread_attribute_noargs_not_lockable)
-            << Attr.getName() << ThisType;
-      }
-    } else {
-      S.Diag(Attr.getLoc(), diag::warn_thread_attribute_noargs_not_method)
-          << Attr.getName();
-    }
-  }
 }
 
 //===----------------------------------------------------------------------===//

Modified: cfe/trunk/test/Sema/attr-capabilities.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-capabilities.c?rev=310698&r1=310697&r2=310698&view=diff
==============================================================================
--- cfe/trunk/test/Sema/attr-capabilities.c (original)
+++ cfe/trunk/test/Sema/attr-capabilities.c Fri Aug 11 00:54:35 2017
@@ -37,25 +37,15 @@ void Func6(void) __attribute__((requires
 void Func7(void) __attribute__((assert_capability(GUI))) {}
 void Func8(void) __attribute__((assert_shared_capability(GUI))) {}
 
-void Func9(void) __attribute__((assert_capability())) {} // expected-warning {{'assert_capability' attribute without arguments can only be applied to a method of a class}}
-void Func10(void) __attribute__((assert_shared_capability())) {} // expected-warning {{'assert_shared_capability' attribute without arguments can only be applied to a method of a class}}
-
 void Func11(void) __attribute__((acquire_capability(GUI))) {}
 void Func12(void) __attribute__((acquire_shared_capability(GUI))) {}
 
-void Func13(void) __attribute__((acquire_capability())) {} // expected-warning {{'acquire_capability' attribute without arguments can only be applied to a method of a class}}
-void Func14(void) __attribute__((acquire_shared_capability())) {} // expected-warning {{'acquire_shared_capability' attribute without arguments can only be applied to a method of a class}}
-
 void Func15(void) __attribute__((release_capability(GUI))) {}
 void Func16(void) __attribute__((release_shared_capability(GUI))) {}
 void Func17(void) __attribute__((release_generic_capability(GUI))) {}
 
-void Func18(void) __attribute__((release_capability())) {} // expected-warning {{'release_capability' attribute without arguments can only be applied to a method of a class}}
-void Func19(void) __attribute__((release_shared_capability())) {} // expected-warning {{'release_shared_capability' attribute without arguments can only be applied to a method of a class}}
-void Func20(void) __attribute__((release_generic_capability())) {} // expected-warning {{'release_generic_capability' attribute without arguments can only be applied to a method of a class}}
-
-void Func21(void) __attribute__((try_acquire_capability(1))) {} // expected-warning {{'try_acquire_capability' attribute without arguments can only be applied to a method of a class}}
-void Func22(void) __attribute__((try_acquire_shared_capability(1))) {} // expected-warning {{'try_acquire_shared_capability' attribute without arguments can only be applied to a method of a class}}
+void Func21(void) __attribute__((try_acquire_capability(1))) {}
+void Func22(void) __attribute__((try_acquire_shared_capability(1))) {}
 
 void Func23(void) __attribute__((try_acquire_capability(1, GUI))) {}
 void Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {}

Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp?rev=310698&r1=310697&r2=310698&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-thread-safety-parsing.cpp Fri Aug 11 00:54:35 2017
@@ -571,11 +571,11 @@ UnlockableMu ab_var_arg_bad_5 ACQUIRED_B
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void elf_function() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without arguments can only be applied to a method of a class}}
+void elf_function() EXCLUSIVE_LOCK_FUNCTION();
 
 void elf_function_args() EXCLUSIVE_LOCK_FUNCTION(mu1, mu2);
 
-int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute without arguments can only be applied to a method of a class}}
+int elf_testfn(int y) EXCLUSIVE_LOCK_FUNCTION();
 
 int elf_testfn(int y) {
   int x EXCLUSIVE_LOCK_FUNCTION() = y; // \
@@ -590,7 +590,7 @@ class ElfFoo {
  private:
   int test_field EXCLUSIVE_LOCK_FUNCTION(); // \
     // expected-warning {{'exclusive_lock_function' attribute only applies to functions}}
-  void test_method() EXCLUSIVE_LOCK_FUNCTION(); // expected-warning {{'exclusive_lock_function' attribute requires type annotated with 'capability' attribute; type here is 'ElfFoo *'}}
+  void test_method() EXCLUSIVE_LOCK_FUNCTION();
 };
 
 class EXCLUSIVE_LOCK_FUNCTION() ElfTestClass { // \
@@ -643,11 +643,11 @@ int elf_function_bad_7() EXCLUSIVE_LOCK_
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void slf_function() SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without arguments can only be applied to a method of a class}}
+void slf_function() SHARED_LOCK_FUNCTION();
 
 void slf_function_args() SHARED_LOCK_FUNCTION(mu1, mu2);
 
-int slf_testfn(int y) SHARED_LOCK_FUNCTION(); // expected-warning {{'shared_lock_function' attribute without arguments can only be applied to a method of a class}}
+int slf_testfn(int y) SHARED_LOCK_FUNCTION();
 
 int slf_testfn(int y) {
   int x SHARED_LOCK_FUNCTION() = y; // \
@@ -665,8 +665,7 @@ class SlfFoo {
  private:
   int test_field SHARED_LOCK_FUNCTION(); // \
     // expected-warning {{'shared_lock_function' attribute only applies to functions}}
-  void test_method() SHARED_LOCK_FUNCTION(); // \
-    // expected-warning {{'shared_lock_function' attribute requires type annotated with 'capability' attribute; type here is 'SlfFoo *'}}
+  void test_method() SHARED_LOCK_FUNCTION();
 };
 
 class SHARED_LOCK_FUNCTION() SlfTestClass { // \
@@ -717,16 +716,14 @@ int slf_function_bad_7() SHARED_LOCK_FUN
 // takes a mandatory boolean or integer argument specifying the retval
 // plus an optional list of locks (vars/fields)
 
-void etf_function() __attribute__((exclusive_trylock_function)); // \
-  // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}} \
+void etf_function() __attribute__((exclusive_trylock_function));  // \
+  // expected-error {{'exclusive_trylock_function' attribute takes at least 1 argument}}
 
 void etf_function_args() EXCLUSIVE_TRYLOCK_FUNCTION(1, mu2);
 
-void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
-  // expected-warning {{'exclusive_trylock_function' attribute without arguments can only be applied to a method of a class}}
+void etf_function_arg() EXCLUSIVE_TRYLOCK_FUNCTION(1);
 
-int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
-  // expected-warning {{'exclusive_trylock_function' attribute without arguments can only be applied to a method of a class}}
+int etf_testfn(int y) EXCLUSIVE_TRYLOCK_FUNCTION(1);
 
 int etf_testfn(int y) {
   int x EXCLUSIVE_TRYLOCK_FUNCTION(1) = y; // \
@@ -735,14 +732,13 @@ int etf_testfn(int y) {
 };
 
 int etf_test_var EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
-  // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}} \
+  // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
 
 class EtfFoo {
  private:
   int test_field EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
     // expected-warning {{'exclusive_trylock_function' attribute only applies to functions}}
-  void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1); // \
-    // expected-warning {{'exclusive_trylock_function' attribute requires type annotated with 'capability' attribute; type here is 'EtfFoo *'}}
+  void test_method() EXCLUSIVE_TRYLOCK_FUNCTION(1);
 };
 
 class EXCLUSIVE_TRYLOCK_FUNCTION(1) EtfTestClass { // \
@@ -763,8 +759,7 @@ int etf_function_5() EXCLUSIVE_TRYLOCK_F
 int etf_function_6() EXCLUSIVE_TRYLOCK_FUNCTION(1, muRef);
 int etf_function_7() EXCLUSIVE_TRYLOCK_FUNCTION(1, muDoubleWrapper.getWrapper()->getMu());
 int etf_functetfn_8() EXCLUSIVE_TRYLOCK_FUNCTION(1, muPointer);
-int etf_function_9() EXCLUSIVE_TRYLOCK_FUNCTION(true); // \
-  // expected-warning {{'exclusive_trylock_function' attribute without arguments can only be applied to a method of a class}}
+int etf_function_9() EXCLUSIVE_TRYLOCK_FUNCTION(true);
 
 
 // illegal attribute arguments
@@ -799,11 +794,9 @@ void stf_function() __attribute__((share
 
 void stf_function_args() SHARED_TRYLOCK_FUNCTION(1, mu2);
 
-void stf_function_arg() SHARED_TRYLOCK_FUNCTION(1); // \
-  // expected-warning {{'shared_trylock_function' attribute without arguments can only be applied to a method of a class}}
+void stf_function_arg() SHARED_TRYLOCK_FUNCTION(1);
 
-int stf_testfn(int y) SHARED_TRYLOCK_FUNCTION(1); // \
-  // expected-warning {{'shared_trylock_function' attribute without arguments can only be applied to a method of a class}}
+int stf_testfn(int y) SHARED_TRYLOCK_FUNCTION(1);
 
 int stf_testfn(int y) {
   int x SHARED_TRYLOCK_FUNCTION(1) = y; // \
@@ -822,8 +815,7 @@ class StfFoo {
  private:
   int test_field SHARED_TRYLOCK_FUNCTION(1); // \
     // expected-warning {{'shared_trylock_function' attribute only applies to functions}}
-  void test_method() SHARED_TRYLOCK_FUNCTION(1); // \
-    // expected-warning {{'shared_trylock_function' attribute requires type annotated with 'capability' attribute; type here is 'StfFoo *'}}
+  void test_method() SHARED_TRYLOCK_FUNCTION(1);
 };
 
 class SHARED_TRYLOCK_FUNCTION(1) StfTestClass { // \
@@ -841,8 +833,7 @@ int stf_function_5() SHARED_TRYLOCK_FUNC
 int stf_function_6() SHARED_TRYLOCK_FUNCTION(1, muRef);
 int stf_function_7() SHARED_TRYLOCK_FUNCTION(1, muDoubleWrapper.getWrapper()->getMu());
 int stf_function_8() SHARED_TRYLOCK_FUNCTION(1, muPointer);
-int stf_function_9() SHARED_TRYLOCK_FUNCTION(true); // \
-  // expected-warning {{'shared_trylock_function' attribute without arguments can only be applied to a method of a class}}
+int stf_function_9() SHARED_TRYLOCK_FUNCTION(true);
 
 
 // illegal attribute arguments
@@ -871,14 +862,11 @@ int stf_function_bad_6() SHARED_TRYLOCK_
 
 // takes zero or more arguments, all locks (vars/fields)
 
-void uf_function() UNLOCK_FUNCTION(); // \
-  // expected-warning {{'unlock_function' attribute without arguments can only be applied to a method of a class}}
-
+void uf_function() UNLOCK_FUNCTION();
 
 void uf_function_args() UNLOCK_FUNCTION(mu1, mu2);
 
-int uf_testfn(int y) UNLOCK_FUNCTION(); //\
-  // expected-warning {{'unlock_function' attribute without arguments can only be applied to a method of a class}}
+int uf_testfn(int y) UNLOCK_FUNCTION();
 
 int uf_testfn(int y) {
   int x UNLOCK_FUNCTION() = y; // \
@@ -893,8 +881,7 @@ class UfFoo {
  private:
   int test_field UNLOCK_FUNCTION(); // \
     // expected-warning {{'unlock_function' attribute only applies to functions}}
-  void test_method() UNLOCK_FUNCTION(); // \
-    // expected-warning {{'unlock_function' attribute requires type annotated with 'capability' attribute; type here is 'UfFoo *'}}
+  void test_method() UNLOCK_FUNCTION();
 };
 
 class NO_THREAD_SAFETY_ANALYSIS UfTestClass { // \
@@ -1537,4 +1524,4 @@ namespace CRASH_POST_R301735 {
      Mutex mu_;
    };
 }
-#endif
+#endif
\ No newline at end of file




More information about the cfe-commits mailing list