diff options
author | Richard Trieu <rtrieu@google.com> | 2018-09-22 01:50:52 +0000 |
---|---|---|
committer | Richard Trieu <rtrieu@google.com> | 2018-09-22 01:50:52 +0000 |
commit | 4a16751240ff7828a5369b8e5057f4a1f14b1c4a (patch) | |
tree | 05a086cb0c3d783e22016e4ec41dbb16c116730a | |
parent | 199e34dc0892f012d215560ee07e715e33222b0c (diff) |
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.
-rw-r--r-- | clang/lib/Sema/SemaDeclAttr.cpp | 35 | ||||
-rw-r--r-- | clang/test/SemaCXX/warn-thread-safety-analysis.cpp | 95 |
2 files changed, 122 insertions, 8 deletions
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index f6df0d0e963..ff432a6fba4 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -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(); + }; - DeclContextLookupResult Res2 = RT->getDecl()->lookup( - S.Context.DeclarationNames.getCXXOperatorName(OO_Arrow)); - if (Res2.empty()) + const RecordDecl *Record = RT->getDecl(); + bool foundStarOperator = IsOverloadedOperatorPresent(Record, OO_Star); + bool foundArrowOperator = IsOverloadedOperatorPresent(Record, OO_Arrow); + if (foundStarOperator && foundArrowOperator) + return true; + + 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. diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index 0be2668a48d..057fd17608f 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -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(); + } +}; +} |