From f489d2b86c21246bf045083be3f1a25b7391f83d Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins Date: Wed, 5 Dec 2012 01:20:45 +0000 Subject: [PATCH] Thread-safety analysis: check locks on method calls, operator=, and copy constructors. llvm-svn: 169350 --- clang/lib/Analysis/ThreadSafety.cpp | 44 +++++++ .../SemaCXX/warn-thread-safety-analysis.cpp | 111 +++++++++++++++++- 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index bc3f85c7b5f3..fdfd599ba52e 100644 --- a/clang/lib/Analysis/ThreadSafety.cpp +++ b/clang/lib/Analysis/ThreadSafety.cpp @@ -2056,6 +2056,43 @@ void BuildLockset::VisitCastExpr(CastExpr *CE) { void BuildLockset::VisitCallExpr(CallExpr *Exp) { + if (Analyzer->Handler.issueBetaWarnings()) { + if (CXXMemberCallExpr *CE = dyn_cast(Exp)) { + MemberExpr *ME = dyn_cast(CE->getCallee()); + // ME can be null when calling a method pointer + CXXMethodDecl *MD = CE->getMethodDecl(); + + if (ME && MD) { + if (ME->isArrow()) { + if (MD->isConst()) { + checkPtAccess(CE->getImplicitObjectArgument(), AK_Read); + } else { // FIXME -- should be AK_Written + checkPtAccess(CE->getImplicitObjectArgument(), AK_Read); + } + } else { + if (MD->isConst()) + checkAccess(CE->getImplicitObjectArgument(), AK_Read); + else // FIXME -- should be AK_Written + checkAccess(CE->getImplicitObjectArgument(), AK_Read); + } + } + } else if (CXXOperatorCallExpr *OE = dyn_cast(Exp)) { + switch (OE->getOperator()) { + case OO_Equal: { + const Expr *Target = OE->getArg(0); + const Expr *Source = OE->getArg(1); + checkAccess(Target, AK_Written); + checkAccess(Source, AK_Read); + break; + } + default: { + const Expr *Source = OE->getArg(0); + checkAccess(Source, AK_Read); + break; + } + } + } + } NamedDecl *D = dyn_cast_or_null(Exp->getCalleeDecl()); if(!D || !D->hasAttrs()) return; @@ -2063,6 +2100,13 @@ void BuildLockset::VisitCallExpr(CallExpr *Exp) { } void BuildLockset::VisitCXXConstructExpr(CXXConstructExpr *Exp) { + if (Analyzer->Handler.issueBetaWarnings()) { + const CXXConstructorDecl *D = Exp->getConstructor(); + if (D && D->isCopyConstructor()) { + const Expr* Source = Exp->getArg(0); + checkAccess(Source, AK_Read); + } + } // FIXME -- only handles constructors in DeclStmt below. } diff --git a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp index bd555ac56c36..c8552f93bf25 100644 --- a/clang/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/clang/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wthread-safety -Wthread-safety-beta %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety -std=c++11 -Wc++98-compat %s // FIXME: should also run %clang_cc1 -fsyntax-only -verify -Wthread-safety %s @@ -3712,3 +3712,112 @@ void Foo::test() { } // end namespace MultipleAttributeTest +namespace GuardedNonPrimitiveTypeTest { + + +class Data { +public: + Data(int i) : dat(i) { } + + int getValue() const { return dat; } + void setValue(int i) { dat = i; } + + int operator[](int i) const { return dat; } + int& operator[](int i) { return dat; } + + void operator()() { } + +private: + int dat; +}; + + +class DataCell { +public: + DataCell(const Data& d) : dat(d) { } + +private: + Data dat; +}; + + +void showDataCell(const DataCell& dc); + + +class Foo { +public: + // method call tests + void test() { + data_.setValue(0); // FIXME -- should be writing \ + // expected-warning {{reading variable 'data_' requires locking 'mu_'}} + int a = data_.getValue(); // \ + // expected-warning {{reading variable 'data_' requires locking 'mu_'}} + + datap1_->setValue(0); // FIXME -- should be writing \ + // expected-warning {{reading variable 'datap1_' requires locking 'mu_'}} + a = datap1_->getValue(); // \ + // expected-warning {{reading variable 'datap1_' requires locking 'mu_'}} + + datap2_->setValue(0); // FIXME -- should be writing \ + // expected-warning {{reading the value pointed to by 'datap2_' requires locking 'mu_'}} + a = datap2_->getValue(); // \ + // expected-warning {{reading the value pointed to by 'datap2_' requires locking 'mu_'}} + + (*datap2_).setValue(0); // FIXME -- should be writing \ + // expected-warning {{reading the value pointed to by 'datap2_' requires locking 'mu_'}} + a = (*datap2_).getValue(); // \ + // expected-warning {{reading the value pointed to by 'datap2_' requires locking 'mu_'}} + + mu_.Lock(); + data_.setValue(1); + datap1_->setValue(1); + datap2_->setValue(1); + mu_.Unlock(); + + mu_.ReaderLock(); + a = data_.getValue(); + datap1_->setValue(0); // reads datap1_, writes *datap1_ + a = datap1_->getValue(); + a = datap2_->getValue(); + mu_.Unlock(); + } + + // operator tests + void test2() { + data_ = Data(1); // expected-warning {{writing variable 'data_' requires locking 'mu_' exclusively}} + *datap1_ = data_; // expected-warning {{reading variable 'datap1_' requires locking 'mu_'}} \ + // expected-warning {{reading variable 'data_' requires locking 'mu_'}} + *datap2_ = data_; // expected-warning {{writing the value pointed to by 'datap2_' requires locking 'mu_' exclusively}} \ + // expected-warning {{reading variable 'data_' requires locking 'mu_'}} + data_ = *datap1_; // expected-warning {{writing variable 'data_' requires locking 'mu_' exclusively}} \ + // expected-warning {{reading variable 'datap1_' requires locking 'mu_'}} + data_ = *datap2_; // expected-warning {{writing variable 'data_' requires locking 'mu_' exclusively}} \ + // expected-warning {{reading the value pointed to by 'datap2_' requires locking 'mu_'}} + + data_[0] = 0; // expected-warning {{reading variable 'data_' requires locking 'mu_'}} + (*datap2_)[0] = 0; // expected-warning {{reading the value pointed to by 'datap2_' requires locking 'mu_'}} + + data_(); // expected-warning {{reading variable 'data_' requires locking 'mu_'}} + } + + // const operator tests + void test3() const { + Data mydat(data_); // expected-warning {{reading variable 'data_' requires locking 'mu_'}} + + //FIXME + //showDataCell(data_); // xpected-warning {{reading variable 'data_' requires locking 'mu_'}} + //showDataCell(*datap2_); // xpected-warning {{reading the value pointed to by 'datap2_' requires locking 'mu_'}} + + int a = data_[0]; // expected-warning {{reading variable 'data_' requires locking 'mu_'}} + } + +private: + Mutex mu_; + Data data_ GUARDED_BY(mu_); + Data* datap1_ GUARDED_BY(mu_); + Data* datap2_ PT_GUARDED_BY(mu_); +}; + + +} // end namespace GuardedNonPrimitiveTypeTest +