From: DeLesley Hutchins Date: Thu, 14 Aug 2014 19:17:06 +0000 (+0000) Subject: Thread Safety Analysis: fix to improve handling of references to guarded X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b95f6d560e0e9483325435b8d5410153a82d52bd;p=clang Thread Safety Analysis: fix to improve handling of references to guarded data members and range based for loops. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@215671 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index 2108bbb3c5..c49e6e7049 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -1215,7 +1215,7 @@ class BuildLockset : public StmtVisitor { // helper functions void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK, Expr *MutexExp, ProtectedOperationKind POK, - StringRef DiagKind); + StringRef DiagKind, SourceLocation Loc); void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp, StringRef DiagKind); @@ -1247,7 +1247,7 @@ public: void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK, Expr *MutexExp, ProtectedOperationKind POK, - StringRef DiagKind) { + StringRef DiagKind, SourceLocation Loc) { LockKind LK = getLockKindFromAccessKind(AK); CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp); @@ -1263,7 +1263,7 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp); if (LDat) { Analyzer->Handler.handleFunExcludesLock( - DiagKind, D->getNameAsString(), (!Cp).toString(), Exp->getExprLoc()); + DiagKind, D->getNameAsString(), (!Cp).toString(), Loc); return; } @@ -1276,7 +1276,7 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, LDat = FSet.findLock(Analyzer->FactMan, Cp); if (!LDat) { Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(), - LK_Shared, Exp->getExprLoc()); + LK_Shared, Loc); } return; } @@ -1290,30 +1290,25 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, // Warn that there's no precise match. std::string PartMatchStr = LDat->toString(); StringRef PartMatchName(PartMatchStr); - Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, - Cp.toString(), - LK, Exp->getExprLoc(), - &PartMatchName); + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(), + LK, Loc, &PartMatchName); } else { // Warn that there's no match at all. - Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, - Cp.toString(), - LK, Exp->getExprLoc()); + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(), + LK, Loc); } NoError = false; } // Make sure the mutex we found is the right kind. if (NoError && LDat && !LDat->isAtLeast(LK)) { - Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, - Cp.toString(), - LK, Exp->getExprLoc()); + Analyzer->Handler.handleMutexNotHeld(DiagKind, D, POK, Cp.toString(), + LK, Loc); } } /// \brief Warn if the LSet contains the given lock. void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, - Expr *MutexExp, - StringRef DiagKind) { + Expr *MutexExp, StringRef DiagKind) { CapabilityExpr Cp = Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp); if (Cp.isInvalid()) { warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, DiagKind); @@ -1337,6 +1332,23 @@ void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK) { Exp = Exp->IgnoreParenCasts(); + SourceLocation Loc = Exp->getExprLoc(); + + if (Analyzer->Handler.issueBetaWarnings()) { + // Local variables of reference type cannot be re-assigned; + // map them to their initializer. + while (auto *DRE = dyn_cast(Exp)) { + auto *VD = dyn_cast(DRE->getDecl()->getCanonicalDecl()); + if (VD && VD->isLocalVarDecl() && VD->getType()->isReferenceType()) { + if (auto *E = VD->getInit()) { + Exp = E; + continue; + } + } + break; + } + } + if (const UnaryOperator *UO = dyn_cast(Exp)) { // For dereferences if (UO->getOpcode() == clang::UO_Deref) @@ -1362,14 +1374,15 @@ void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK) { if (D->hasAttr() && FSet.isEmpty(Analyzer->FactMan)) { Analyzer->Handler.handleNoMutexHeld("mutex", D, POK_VarAccess, AK, - Exp->getExprLoc()); + Loc); } for (const auto *I : D->specific_attrs()) warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK_VarAccess, - ClassifyDiagnostic(I)); + ClassifyDiagnostic(I), Loc); } + /// \brief Checks pt_guarded_by and pt_guarded_var attributes. void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) { while (true) { @@ -1400,7 +1413,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) { for (auto const *I : D->specific_attrs()) warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK_VarDereference, - ClassifyDiagnostic(I)); + ClassifyDiagnostic(I), Exp->getExprLoc()); } /// \brief Process a function call, method call, constructor call, @@ -1480,7 +1493,8 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { RequiresCapabilityAttr *A = cast(At); for (auto *Arg : A->args()) warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg, - POK_FunctionCall, ClassifyDiagnostic(A)); + POK_FunctionCall, ClassifyDiagnostic(A), + Exp->getExprLoc()); break; } diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index 61e92db2d5..b9f999d079 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -98,6 +98,7 @@ public: }; +// For testing operator overloading template class MyMap { public: @@ -105,6 +106,29 @@ public: }; +// For testing handling of containers. +template +class MyContainer { +public: + MyContainer(); + + typedef T* iterator; + typedef const T* const_iterator; + + T* begin(); + T* end(); + + const T* cbegin(); + const T* cend(); + + T& operator[](int i); + const T& operator[](int i) const; + +private: + T* ptr_; +}; + + Mutex sls_mu; @@ -4645,3 +4669,62 @@ class Foo { } // end namespace AssertSharedExclusive +namespace RangeBasedForAndReferences { + +class Foo { + struct MyStruct { + int a; + }; + + Mutex mu; + int a GUARDED_BY(mu); + MyContainer cntr GUARDED_BY(mu); + MyStruct s GUARDED_BY(mu); + int arr[10] GUARDED_BY(mu); + + void nonref_test() { + int b = a; // expected-warning {{reading variable 'a' requires holding mutex 'mu'}} + b = 0; // no warning + } + + void auto_test() { + auto b = a; // expected-warning {{reading variable 'a' requires holding mutex 'mu'}} + b = 0; // no warning + auto &c = a; // no warning + c = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}} + } + + void ref_test() { + int &b = a; + int &c = b; + int &d = c; + b = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}} + c = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}} + d = 0; // expected-warning {{writing variable 'a' requires holding mutex 'mu' exclusively}} + + MyStruct &rs = s; + rs.a = 0; // expected-warning {{writing variable 's' requires holding mutex 'mu' exclusively}} + + int (&rarr)[10] = arr; + rarr[2] = 0; // expected-warning {{writing variable 'arr' requires holding mutex 'mu' exclusively}} + } + + void ptr_test() { + int *b = &a; + *b = 0; // no expected warning yet + } + + void for_test() { + int total = 0; + for (int i : cntr) { // expected-warning2 {{reading variable 'cntr' requires holding mutex 'mu'}} + total += i; + } + } +}; + + +} // end namespace RangeBasedForAndReferences + + + +