Author: rtrieu Date: Fri Sep 21 18:50:52 2018 New Revision: 342794 URL: http://llvm.org/viewvc/llvm-project?rev=342794&view=rev Log: Update smart pointer detection for thread safety analysis.
Objects are determined to be smart pointers if they have both a star and arrow operator. Some implementations of smart pointers have these overloaded operators in a base class, while the check only searched the derived class. This fix will also look for the operators in the base class. Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=342794&r1=342793&r2=342794&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Sep 21 18:50:52 2018 @@ -425,17 +425,36 @@ static bool isIntOrBool(Expr *Exp) { // Check to see if the type is a smart pointer of some kind. We assume // it's a smart pointer if it defines both operator-> and operator*. static bool threadSafetyCheckIsSmartPointer(Sema &S, const RecordType* RT) { - DeclContextLookupResult Res1 = RT->getDecl()->lookup( - S.Context.DeclarationNames.getCXXOperatorName(OO_Star)); - if (Res1.empty()) - return false; + auto IsOverloadedOperatorPresent = [&S](const RecordDecl *Record, + OverloadedOperatorKind Op) { + DeclContextLookupResult Result = + Record->lookup(S.Context.DeclarationNames.getCXXOperatorName(Op)); + return !Result.empty(); + }; + + const RecordDecl *Record = RT->getDecl(); + bool foundStarOperator = IsOverloadedOperatorPresent(Record, OO_Star); + bool foundArrowOperator = IsOverloadedOperatorPresent(Record, OO_Arrow); + if (foundStarOperator && foundArrowOperator) + return true; - DeclContextLookupResult Res2 = RT->getDecl()->lookup( - S.Context.DeclarationNames.getCXXOperatorName(OO_Arrow)); - if (Res2.empty()) + const CXXRecordDecl *CXXRecord = dyn_cast<CXXRecordDecl>(Record); + if (!CXXRecord) return false; - return true; + for (auto BaseSpecifier : CXXRecord->bases()) { + if (!foundStarOperator) + foundStarOperator = IsOverloadedOperatorPresent( + BaseSpecifier.getType()->getAsRecordDecl(), OO_Star); + if (!foundArrowOperator) + foundArrowOperator = IsOverloadedOperatorPresent( + BaseSpecifier.getType()->getAsRecordDecl(), OO_Arrow); + } + + if (foundStarOperator && foundArrowOperator) + return true; + + return false; } /// Check if passed in Decl is a pointer type. Modified: cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp?rev=342794&r1=342793&r2=342794&view=diff ============================================================================== --- cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp Fri Sep 21 18:50:52 2018 @@ -5481,3 +5481,98 @@ void f() { int &i = i; // expected-warning {{reference 'i' is not yet bound to a value when used within its own initialization}} } } + +namespace Derived_Smart_Pointer { +template <class T> +class SmartPtr_Derived : public SmartPtr<T> {}; + +class Foo { +public: + SmartPtr_Derived<Mutex> mu_; + int a GUARDED_BY(mu_); + int b GUARDED_BY(mu_.get()); + int c GUARDED_BY(*mu_); + + void Lock() EXCLUSIVE_LOCK_FUNCTION(mu_); + void Unlock() UNLOCK_FUNCTION(mu_); + + void test0() { + a = 1; // expected-warning {{writing variable 'a' requires holding mutex 'mu_' exclusively}} + b = 1; // expected-warning {{writing variable 'b' requires holding mutex 'mu_' exclusively}} + c = 1; // expected-warning {{writing variable 'c' requires holding mutex 'mu_' exclusively}} + } + + void test1() { + Lock(); + a = 1; + b = 1; + c = 1; + Unlock(); + } +}; + +class Bar { + SmartPtr_Derived<Foo> foo; + + void test0() { + foo->a = 1; // expected-warning {{writing variable 'a' requires holding mutex 'foo->mu_' exclusively}} + (*foo).b = 1; // expected-warning {{writing variable 'b' requires holding mutex 'foo->mu_' exclusively}} + foo.get()->c = 1; // expected-warning {{writing variable 'c' requires holding mutex 'foo->mu_' exclusively}} + } + + void test1() { + foo->Lock(); + foo->a = 1; + foo->Unlock(); + + foo->mu_->Lock(); + foo->b = 1; + foo->mu_->Unlock(); + + MutexLock lock(foo->mu_.get()); + foo->c = 1; + } +}; + +class PointerGuard { + Mutex mu1; + Mutex mu2; + SmartPtr_Derived<int> i GUARDED_BY(mu1) PT_GUARDED_BY(mu2); + + void test0() { + i.get(); // expected-warning {{reading variable 'i' requires holding mutex 'mu1'}} + *i = 2; // expected-warning {{reading variable 'i' requires holding mutex 'mu1'}} \ + // expected-warning {{reading the value pointed to by 'i' requires holding mutex 'mu2'}} + + } + + void test1() { + mu1.Lock(); + + i.get(); + *i = 2; // expected-warning {{reading the value pointed to by 'i' requires holding mutex 'mu2'}} + + mu1.Unlock(); + } + + void test2() { + mu2.Lock(); + + i.get(); // expected-warning {{reading variable 'i' requires holding mutex 'mu1'}} + *i = 2; // expected-warning {{reading variable 'i' requires holding mutex 'mu1'}} + + mu2.Unlock(); + } + + void test3() { + mu1.Lock(); + mu2.Lock(); + + i.get(); + *i = 2; + + mu2.Unlock(); + mu1.Unlock(); + } +}; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits