diff options
| -rw-r--r-- | include/clang/Basic/DiagnosticSemaKinds.td | 35 | ||||
| -rw-r--r-- | lib/Sema/AnalysisBasedWarnings.cpp | 170 | ||||
| -rw-r--r-- | test/SemaCXX/warn-thread-safety-analysis.cpp | 110 |
3 files changed, 153 insertions, 162 deletions
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index f5d06566ca..08877a063b 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1388,47 +1388,48 @@ def err_attribute_argument_not_lockable : Error< def err_attribute_decl_not_lockable : Error< "%0 attribute can only be applied in a context annotated " "with 'lockable' attribute">; -def warn_unlock_but_no_acquire : Warning< - "unlocking '%0' that was not acquired">, +def warn_unlock_but_no_lock : Warning< + "unlocking '%0' that was not locked">, InGroup<ThreadSafety>, DefaultIgnore; def warn_double_lock : Warning< - "locking '%0' that is already acquired">, + "locking '%0' that is already locked">, InGroup<ThreadSafety>, DefaultIgnore; -def warn_locks_not_released : Warning< - "lock '%0' is not released at the end of function '%1'">, +def warn_no_unlock : Warning< + "mutex '%0' is still held at the end of function '%1'">, InGroup<ThreadSafety>, DefaultIgnore; -def warn_lock_not_released_in_scope : Warning< - "lock '%0' is not released at the end of its scope">, +// FIXME: improve the error message about locks not in scope +def warn_lock_at_end_of_scope : Warning< + "mutex '%0' is still held at the end of its scope">, InGroup<ThreadSafety>, DefaultIgnore; def warn_expecting_lock_held_on_loop : Warning< - "expecting lock '%0' to be %select{held|held exclusively}1 at start of each " - "loop">, + "expecting lock on '%0' to be %select{held|held exclusively}1 at start of " + "each loop">, InGroup<ThreadSafety>, DefaultIgnore; def warn_lock_exclusive_and_shared : Warning< - "lock '%0' is held exclusively and shared in the same scope">, + "lock '%0' is exclusive and shared in the same scope">, InGroup<ThreadSafety>, DefaultIgnore; def note_lock_exclusive_and_shared : Note< - "the other acquire of lock '%0' is here">, + "the other lock of mutex '%0' is here">, InGroup<ThreadSafety>, DefaultIgnore; def warn_variable_requires_lock : Warning< - "%select{reading|writing}2 variable '%0' requires lock '%1' to be " + "%select{reading|writing}2 variable '%0' requires lock on '%1' to be " "%select{held|held exclusively}2">, InGroup<ThreadSafety>, DefaultIgnore; def warn_variable_requires_any_lock : Warning< "accessing variable '%0' requires some lock">, InGroup<ThreadSafety>, DefaultIgnore; def warn_var_deref_requires_lock : Warning< - "%select{reading|writing}2 the value pointed to by '%0' requires lock '%1' to" - " be %select{held|held exclusively}2">, + "%select{reading|writing}2 the value pointed to by '%0' requires lock on '%1'" + " to be %select{held|held exclusively}2">, InGroup<ThreadSafety>, DefaultIgnore; def warn_var_deref_requires_any_lock : Warning< "accessing the value pointed to by '%0' requires some lock">, InGroup<ThreadSafety>, DefaultIgnore; def warn_fun_requires_lock : Warning< - "calling function '%0' requires %select{shared|exclusive}2 lock '%1'">, + "calling function '%0' requires %select{shared|exclusive}2 lock on '%1'">, InGroup<ThreadSafety>, DefaultIgnore; -def warn_fun_excludes_lock : Warning< - "cannot call function '%0' while holding lock '%1'">, +def warn_fun_excludes_mutex : Warning< + "cannot call function '%0' while holding mutex '%1'">, InGroup<ThreadSafety>, DefaultIgnore; diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index 642638bf06..afbfabf674 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -676,12 +676,12 @@ public: } }; -/// \brief A LockID object uniquely identifies a particular lock acquired, and +/// \brief A MutexID object uniquely identifies a particular mutex, and /// is built from an Expr* (i.e. calling a lock function). /// /// Thread-safety analysis works by comparing lock expressions. Within the /// body of a function, an expression such as "x->foo->bar.mu" will resolve to -/// a particular lock object at run-time. Subsequent occurrences of the same +/// a particular mutex object at run-time. Subsequent occurrences of the same /// expression (where "same" means syntactic equality) will refer to the same /// run-time object if three conditions hold: /// (1) Local variables in the expression, such as "x" have not changed. @@ -692,7 +692,7 @@ public: /// /// Clang introduces an additional wrinkle, which is that it is difficult to /// derive canonical expressions, or compare expressions directly for equality. -/// Thus, we identify a lock not by an Expr, but by the set of named +/// Thus, we identify a mutex not by an Expr, but by the set of named /// declarations that are referenced by the Expr. In other words, /// x->foo->bar.mu will be a four element vector with the Decls for /// mu, bar, and foo, and x. The vector will uniquely identify the expression @@ -704,36 +704,26 @@ public: /// For example: /// class C { Mutex Mu; void lock() EXCLUSIVE_LOCK_FUNCTION(this->Mu); }; /// void myFunc(C *X) { ... X->lock() ... } -/// The original expression for the lock acquired by myFunc is "this->Mu", but +/// The original expression for the mutex acquired by myFunc is "this->Mu", but /// "X" is substituted for "this" so we get X->Mu(); /// /// For another example: /// foo(MyList *L) EXCLUSIVE_LOCKS_REQUIRED(L->Mu) { ... } /// MyList *MyL; /// foo(MyL); // requires lock MyL->Mu to be held -/// -/// FIXME: In C++0x Mutexes are the objects that control access to shared -/// variables, while Locks are the objects that acquire and release Mutexes. We -/// may want to switch to this new terminology soon, in which case we should -/// rename this class "Mutex" and rename "LockId" to "MutexId", as well as -/// making sure that the terms Lock and Mutex throughout this code are -/// consistent with C++0x -/// -/// FIXME: We should also pick one and canonicalize all usage of lock vs acquire -/// and unlock vs release as verbs. -class LockID { +class MutexID { SmallVector<NamedDecl*, 2> DeclSeq; /// Build a Decl sequence representing the lock from the given expression. /// Recursive function that bottoms out when the final DeclRefExpr is reached. - void buildLock(Expr *Exp) { + void buildMutexID(Expr *Exp) { if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp)) { NamedDecl *ND = cast<NamedDecl>(DRE->getDecl()->getCanonicalDecl()); DeclSeq.push_back(ND); } else if (MemberExpr *ME = dyn_cast<MemberExpr>(Exp)) { NamedDecl *ND = ME->getMemberDecl(); DeclSeq.push_back(ND); - buildLock(ME->getBase()); + buildMutexID(ME->getBase()); } else if (isa<CXXThisExpr>(Exp)) { return; } else { @@ -743,32 +733,32 @@ class LockID { } public: - LockID(Expr *LExpr) { - buildLock(LExpr); + MutexID(Expr *LExpr) { + buildMutexID(LExpr); assert(!DeclSeq.empty()); } - bool operator==(const LockID &other) const { + bool operator==(const MutexID &other) const { return DeclSeq == other.DeclSeq; } - bool operator!=(const LockID &other) const { + bool operator!=(const MutexID &other) const { return !(*this == other); } // SmallVector overloads Operator< to do lexicographic ordering. Note that // we use pointer equality (and <) to compare NamedDecls. This means the order - // of LockIDs in a lockset is nondeterministic. In order to output + // of MutexIDs in a lockset is nondeterministic. In order to output // diagnostics in a deterministic ordering, we must order all diagnostics to // output by SourceLocation when iterating through this lockset. - bool operator<(const LockID &other) const { + bool operator<(const MutexID &other) const { return DeclSeq < other.DeclSeq; } - /// \brief Returns the name of the first Decl in the list for a given LockID; + /// \brief Returns the name of the first Decl in the list for a given MutexID; /// e.g. the lock expression foo.bar() has name "bar". /// The caret will point unambiguously to the lock expression, so using this - /// name in diagnostics is a way to get simple, and consistent, lock names. + /// name in diagnostics is a way to get simple, and consistent, mutex names. /// We do not want to output the entire expression text for security reasons. StringRef getName() const { return DeclSeq.front()->getName(); @@ -795,7 +785,7 @@ enum AccessKind { /// \brief This is a helper class that stores info about the most recent /// accquire of a Lock. /// -/// The main body of the analysis maps LockIDs to LockDatas. +/// The main body of the analysis maps MutexIDs to LockDatas. struct LockData { SourceLocation AcquireLoc; @@ -824,9 +814,9 @@ struct LockData { } }; -/// A Lockset maps each LockID (defined above) to information about how it has +/// A Lockset maps each MutexID (defined above) to information about how it has /// been locked. -typedef llvm::ImmutableMap<LockID, LockData> Lockset; +typedef llvm::ImmutableMap<MutexID, LockData> Lockset; /// \brief We use this class to visit different types of expressions in /// CFGBlocks, and build up the lockset. @@ -842,8 +832,8 @@ class BuildLockset : public StmtVisitor<BuildLockset> { void removeLock(SourceLocation UnlockLoc, Expr *LockExp); void addLock(SourceLocation LockLoc, Expr *LockExp, LockKind LK); const ValueDecl *getValueDecl(Expr *Exp); - void warnIfLockNotHeld (const NamedDecl *D, Expr *Exp, AccessKind AK, - LockID &Lock, unsigned DiagID); + void warnIfMutexNotHeld (const NamedDecl *D, Expr *Exp, AccessKind AK, + MutexID &Mutex, unsigned DiagID); void checkAccess(Expr *Exp, AccessKind AK); void checkDereference(Expr *Exp, AccessKind AK); @@ -852,13 +842,13 @@ class BuildLockset : public StmtVisitor<BuildLockset> { /// \brief Returns true if the lockset contains a lock, regardless of whether /// the lock is held exclusively or shared. - bool locksetContains(LockID Lock) { + bool locksetContains(MutexID Lock) { return LSet.lookup(Lock); } /// \brief Returns true if the lockset contains a lock with the passed in /// locktype. - bool locksetContains(LockID Lock, LockKind KindRequested) const { + bool locksetContains(MutexID Lock, LockKind KindRequested) const { const LockData *LockHeld = LSet.lookup(Lock); return (LockHeld && KindRequested == LockHeld->LKind); } @@ -884,24 +874,24 @@ public: void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp, LockKind LK) { // FIXME: deal with acquired before/after annotations - LockID Lock(LockExp); - LockData NewLockData(LockLoc, LK); + MutexID Mutex(LockExp); + LockData NewLock(LockLoc, LK); // FIXME: Don't always warn when we have support for reentrant locks. - if (locksetContains(Lock)) - S.Diag(LockLoc, diag::warn_double_lock) << Lock.getName(); - LSet = LocksetFactory.add(LSet, Lock, NewLockData); + if (locksetContains(Mutex)) + S.Diag(LockLoc, diag::warn_double_lock) << Mutex.getName(); + LSet = LocksetFactory.add(LSet, Mutex, NewLock); } /// \brief Remove a lock from the lockset, warning if the lock is not there. /// \param LockExp The lock expression corresponding to the lock to be removed /// \param UnlockLoc The source location of the unlock (only used in error msg) void BuildLockset::removeLock(SourceLocation UnlockLoc, Expr *LockExp) { - LockID Lock(LockExp); + MutexID Mutex(LockExp); - Lockset NewLSet = LocksetFactory.remove(LSet, Lock); + Lockset NewLSet = LocksetFactory.remove(LSet, Mutex); if(NewLSet == LSet) - S.Diag(UnlockLoc, diag::warn_unlock_but_no_acquire) << Lock.getName(); + S.Diag(UnlockLoc, diag::warn_unlock_but_no_lock) << Mutex.getName(); LSet = NewLSet; } @@ -919,19 +909,19 @@ const ValueDecl *BuildLockset::getValueDecl(Expr *Exp) { /// \brief Warn if the LSet does not contain a lock sufficient to protect access /// of at least the passed in AccessType. -void BuildLockset::warnIfLockNotHeld(const NamedDecl *D, Expr *Exp, - AccessKind AK, LockID &Lock, - unsigned DiagID) { +void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp, + AccessKind AK, MutexID &Mutex, + unsigned DiagID) { switch (AK) { case AK_Read: - if (!locksetContains(Lock)) + if (!locksetContains(Mutex)) S.Diag(Exp->getExprLoc(), DiagID) - << D->getName() << Lock.getName() << LK_Shared; + << D->getName() << Mutex.getName() << LK_Shared; break; case AK_Written : - if (!locksetContains(Lock, LK_Exclusive)) + if (!locksetContains(Mutex, LK_Exclusive)) S.Diag(Exp->getExprLoc(), DiagID) - << D->getName() << Lock.getName() << LK_Exclusive; + << D->getName() << Mutex.getName() << LK_Exclusive; break; } } @@ -962,15 +952,15 @@ void BuildLockset::checkDereference(Expr *Exp, AccessKind AK) { if (ArgAttrs[i]->getKind() != attr::PtGuardedBy) continue; const PtGuardedByAttr *PGBAttr = cast<PtGuardedByAttr>(ArgAttrs[i]); - LockID Lock(PGBAttr->getArg()); - warnIfLockNotHeld(D, Exp, AK, Lock, diag::warn_var_deref_requires_lock); + MutexID Mutex(PGBAttr->getArg()); + warnIfMutexNotHeld(D, Exp, AK, Mutex, diag::warn_var_deref_requires_lock); } } /// \brief Checks guarded_by and guarded_var attributes. /// Whenever we identify an access (read or write) of a DeclRefExpr or /// MemberExpr, we need to check whether there are any guarded_by or -/// guarded_var attributes, and make sure we hold the appropriate locks. +/// guarded_var attributes, and make sure we hold the appropriate mutexes. void BuildLockset::checkAccess(Expr *Exp, AccessKind AK) { const ValueDecl *D = getValueDecl(Exp); if(!D || !D->hasAttrs()) @@ -985,13 +975,13 @@ void BuildLockset::checkAccess(Expr *Exp, AccessKind AK) { if (ArgAttrs[i]->getKind() != attr::GuardedBy) continue; const GuardedByAttr *GBAttr = cast<GuardedByAttr>(ArgAttrs[i]); - LockID Lock(GBAttr->getArg()); - warnIfLockNotHeld(D, Exp, AK, Lock, diag::warn_variable_requires_lock); + MutexID Mutex(GBAttr->getArg()); + warnIfMutexNotHeld(D, Exp, AK, Mutex, diag::warn_variable_requires_lock); } } /// \brief For unary operations which read and write a variable, we need to -/// check whether we hold any required locks. Reads are checked in +/// check whether we hold any required mutexes. Reads are checked in /// VisitCastExpr. void BuildLockset::VisitUnaryOperator(UnaryOperator *UO) { switch (UO->getOpcode()) { @@ -1010,7 +1000,7 @@ void BuildLockset::VisitUnaryOperator(UnaryOperator *UO) { } /// For binary operations which assign to a variable (writes), we need to check -/// whether we hold any required locks. +/// whether we hold any required mutexes. /// FIXME: Deal with non-primitive types. void BuildLockset::VisitBinaryOperator(BinaryOperator *BO) { if (!BO->isAssignmentOp()) @@ -1021,7 +1011,7 @@ void BuildLockset::VisitBinaryOperator(BinaryOperator *BO) { } /// Whenever we do an LValue to Rvalue cast, we are reading a variable and -/// need to ensure we hold any required locks. +/// need to ensure we hold any required mutexes. /// FIXME: Deal with non-primitive types. void BuildLockset::VisitCastExpr(CastExpr *CE) { if (CE->getCastKind() != CK_LValueToRValue) @@ -1042,7 +1032,7 @@ void BuildLockset::addLocksToSet(LockKind LK, Attr *Attr, AttrType *SpecificAttr = cast<AttrType>(Attr); if (SpecificAttr->args_size() == 0) { - // The lock held is the "this" object. + // The mutex held is the "this" object. addLock(ExpLocation, Parent, LK); return; } @@ -1076,19 +1066,19 @@ void BuildLockset::VisitCXXMemberCallExpr(CXXMemberCallExpr *Exp) { Attr *Attr = ArgAttrs[i]; switch (Attr->getKind()) { // When we encounter an exclusive lock function, we need to add the lock - // to our lockset, marked as exclusive. + // to our lockset with kind exclusive. case attr::ExclusiveLockFunction: addLocksToSet<ExclusiveLockFunctionAttr>(LK_Exclusive, Attr, Exp); break; // When we encounter a shared lock function, we need to add the lock - // to our lockset, marked as not exclusive + // to our lockset with kind shared. case attr::SharedLockFunction: addLocksToSet<SharedLockFunctionAttr>(LK_Shared, Attr, Exp); break; - // When we encounter an unlock function, we need to remove unlocked locks - // from the lockset, and flag a warning if they are not there. + // When we encounter an unlock function, we need to remove unlocked + // mutexes from the lockset, and flag a warning if they are not there. case attr::UnlockFunction: { UnlockFunctionAttr *UFAttr = cast<UnlockFunctionAttr>(Attr); @@ -1112,8 +1102,8 @@ void BuildLockset::VisitCXXMemberCallExpr(CXXMemberCallExpr *Exp) { for (ExclusiveLocksRequiredAttr::args_iterator I = ELRAttr->args_begin(), E = ELRAttr->args_end(); I != E; ++I) { - LockID Lock(*I); - warnIfLockNotHeld(D, Exp, AK_Written, Lock, + MutexID Mutex(*I); + warnIfMutexNotHeld(D, Exp, AK_Written, Mutex, diag::warn_fun_requires_lock); } break; @@ -1127,8 +1117,8 @@ void BuildLockset::VisitCXXMemberCallExpr(CXXMemberCallExpr *Exp) { for (SharedLocksRequiredAttr::args_iterator I = SLRAttr->args_begin(), E = SLRAttr->args_end(); I != E; ++I) { - LockID Lock(*I); - warnIfLockNotHeld(D, Exp, AK_Read, Lock, + MutexID Mutex(*I); + warnIfMutexNotHeld(D, Exp, AK_Read, Mutex, diag::warn_fun_requires_lock); } break; @@ -1138,10 +1128,10 @@ void BuildLockset::VisitCXXMemberCallExpr(CXXMemberCallExpr *Exp) { LocksExcludedAttr *LEAttr = cast<LocksExcludedAttr>(Attr); for (LocksExcludedAttr::args_iterator I = LEAttr->args_begin(), E = LEAttr->args_end(); I != E; ++I) { - LockID Lock(*I); - if (locksetContains(Lock)) - S.Diag(ExpLocation, diag::warn_fun_excludes_lock) - << D->getName() << Lock.getName(); + MutexID Mutex(*I); + if (locksetContains(Mutex)) + S.Diag(ExpLocation, diag::warn_fun_excludes_mutex) + << D->getName() << Mutex.getName(); } break; } @@ -1191,22 +1181,22 @@ static Lockset warnIfNotInFirstSetOrNotSameKind(Sema &S, const Lockset LSet1, Lockset Intersection, Lockset::Factory &Fact) { for (Lockset::iterator I = LSet2.begin(), E = LSet2.end(); I != E; ++I) { - const LockID &LSet2Lock = I.getKey(); + const MutexID &LSet2Mutex = I.getKey(); const LockData &LSet2LockData = I.getData(); - if (const LockData *LD = LSet1.lookup(LSet2Lock)) { + if (const LockData *LD = LSet1.lookup(LSet2Mutex)) { if (LD->LKind != LSet2LockData.LKind) { PartialDiagnostic Warning = - S.PDiag(diag::warn_lock_exclusive_and_shared) << LSet2Lock.getName(); + S.PDiag(diag::warn_lock_exclusive_and_shared) << LSet2Mutex.getName(); PartialDiagnostic Note = - S.PDiag(diag::note_lock_exclusive_and_shared) << LSet2Lock.getName(); + S.PDiag(diag::note_lock_exclusive_and_shared) << LSet2Mutex.getName(); Warnings.push_back(DelayedDiag(LSet2LockData.AcquireLoc, Warning)); Warnings.push_back(DelayedDiag(LD->AcquireLoc, Note)); if (LD->LKind != LK_Exclusive) - Intersection = Fact.add(Intersection, LSet2Lock, LSet2LockData); + Intersection = Fact.add(Intersection, LSet2Mutex, LSet2LockData); } } else { PartialDiagnostic Warning = - S.PDiag(diag::warn_lock_not_released_in_scope) << LSet2Lock.getName(); + S.PDiag(diag::warn_lock_at_end_of_scope) << LSet2Mutex.getName(); Warnings.push_back(DelayedDiag(LSet2LockData.AcquireLoc, Warning)); } } @@ -1233,12 +1223,12 @@ static Lockset intersectAndWarn(Sema &S, const Lockset LSet1, for (Lockset::iterator I = LSet1.begin(), E = LSet1.end(); I != E; ++I) { if (!LSet2.contains(I.getKey())) { - const LockID &MissingLock = I.getKey(); - const LockData &MissingLockData = I.getData(); + const MutexID &Mutex = I.getKey(); + const LockData &MissingLock = I.getData(); PartialDiagnostic Warning = - S.PDiag(diag::warn_lock_not_released_in_scope) << MissingLock.getName(); - Warnings.push_back(DelayedDiag(MissingLockData.AcquireLoc, Warning)); - Intersection = Fact.remove(Intersection, MissingLock); + S.PDiag(diag::warn_lock_at_end_of_scope) << Mutex.getName(); + Warnings.push_back(DelayedDiag(MissingLock.AcquireLoc, Warning)); + Intersection = Fact.remove(Intersection, Mutex); } } @@ -1268,8 +1258,8 @@ static SourceLocation getFirstStmtLocation(CFGBlock *Block) { /// we need to verify that the lockset before taking the backedge is the /// same as the lockset before entering the loop. /// -/// \param LoopEntrySet Locks held before starting the loop -/// \param LoopReentrySet Locks held in the last CFG block of the loop +/// \param LoopEntrySet Locks before starting the loop +/// \param LoopReentrySet Locks in the last CFG block of the loop static void warnBackEdgeUnequalLocksets(Sema &S, const Lockset LoopReentrySet, const Lockset LoopEntrySet, SourceLocation FirstLocInLoop, @@ -1281,11 +1271,11 @@ static void warnBackEdgeUnequalLocksets(Sema &S, const Lockset LoopReentrySet, for (Lockset::iterator I = LoopEntrySet.begin(), E = LoopEntrySet.end(); I != E; ++I) { if (!LoopReentrySet.contains(I.getKey())) { - const LockID &MissingLock = I.getKey(); + const MutexID &Mutex = I.getKey(); // We report this error at the location of the first statement in a loop PartialDiagnostic Warning = S.PDiag(diag::warn_expecting_lock_held_on_loop) - << MissingLock.getName() << LK_Shared; + << Mutex.getName() << LK_Shared; Warnings.push_back(DelayedDiag(FirstLocInLoop, Warning)); } } @@ -1299,7 +1289,7 @@ static void warnBackEdgeUnequalLocksets(Sema &S, const Lockset LoopReentrySet, /// \brief Check a function's CFG for thread-safety violations. /// -/// We traverse the blocks in the CFG, compute the set of locks that are held +/// We traverse the blocks in the CFG, compute the set of mutexes that are held /// at the end of each block, and issue warnings for thread safety violations. /// Each block in the CFG is traversed exactly once. static void checkThreadSafety(Sema &S, AnalysisContext &AC) { @@ -1339,7 +1329,7 @@ static void checkThreadSafety(Sema &S, AnalysisContext &AC) { // FIXME: By keeping the intersection, we may output more errors in future // for a lock which is not in the intersection, but was in the union. We // may want to also keep the union in future. As an example, let's say - // the intersection contains Lock L, and the union contains L and M. + // the intersection contains Mutex L, and the union contains L and M. // Later we unlock M. At this point, we would output an error because we // never locked M; although the real error is probably that we forgot to // lock M on all code paths. Conversely, let's say that later we lock M. @@ -1404,8 +1394,8 @@ static void checkThreadSafety(Sema &S, AnalysisContext &AC) { DiagList Warnings; for (Lockset::iterator I=FinalLockset.begin(), E=FinalLockset.end(); I != E; ++I) { - const LockID &MissingLock = I.getKey(); - const LockData &MissingLockData = I.getData(); + const MutexID &Mutex = I.getKey(); + const LockData &MissingLock = I.getData(); std::string FunName = "<unknown>"; if (const NamedDecl *ContextDecl = dyn_cast<NamedDecl>(AC.getDecl())) { @@ -1413,9 +1403,9 @@ static void checkThreadSafety(Sema &S, AnalysisContext &AC) { } PartialDiagnostic Warning = - S.PDiag(diag::warn_locks_not_released) - << MissingLock.getName() << FunName; - Warnings.push_back(DelayedDiag(MissingLockData.AcquireLoc, Warning)); + S.PDiag(diag::warn_no_unlock) + << Mutex.getName() << FunName; + Warnings.push_back(DelayedDiag(MissingLock.AcquireLoc, Warning)); } EmitDiagnostics(S, Warnings); } diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index a834ee34c7..cb74504a2c 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -123,33 +123,33 @@ void sls_fun_good_7() { void sls_fun_bad_1() { sls_mu.Unlock(); // \ - // expected-warning{{unlocking 'sls_mu' that was not acquired}} + // expected-warning{{unlocking 'sls_mu' that was not locked}} } void sls_fun_bad_2() { sls_mu.Lock(); sls_mu.Lock(); // \ - // expected-warning{{locking 'sls_mu' that is already acquired}} + // expected-warning{{locking 'sls_mu' that is already locked}} sls_mu.Unlock(); } void sls_fun_bad_3() { sls_mu.Lock(); // \ - // expected-warning{{lock 'sls_mu' is not released at the end of function 'sls_fun_bad_3'}} + // expected-warning{{mutex 'sls_mu' is still held at the end of function 'sls_fun_bad_3'}} } void sls_fun_bad_4() { if (getBool()) sls_mu.Lock(); // \ - // expected-warning{{lock 'sls_mu' is not released at the end of its scope}} + // expected-warning{{mutex 'sls_mu' is still held at the end of its scope}} else sls_mu2.Lock(); // \ - // expected-warning{{lock 'sls_mu2' is not released at the end of its scope}} + // expected-warning{{mutex 'sls_mu2' is still held at the end of its scope}} } void sls_fun_bad_5() { sls_mu.Lock(); // \ - // expected-warning{{lock 'sls_mu' is not released at the end of its scope}} + // expected-warning{{mutex 'sls_mu' is still held at the end of its scope}} if (getBool()) sls_mu.Unlock(); } @@ -157,7 +157,7 @@ void sls_fun_bad_5() { void sls_fun_bad_6() { if (getBool()) { sls_mu.Lock(); // \ - // expected-warning{{lock 'sls_mu' is not released at the end of its scope}} + // expected-warning{{mutex 'sls_mu' is still held at the end of its scope}} } else { if (getBool()) { getBool(); // EMPTY @@ -166,13 +166,13 @@ void sls_fun_bad_6() { } } sls_mu.Unlock(); // \ - // expected-warning{{unlocking 'sls_mu' that was not acquired}} + // expected-warning{{unlocking 'sls_mu' that was not locked}} } void sls_fun_bad_7() { sls_mu.Lock(); while (getBool()) { // \ - // expected-warning{{expecting lock 'sls_mu' to be held at start of each loop}} + // expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}} sls_mu.Unlock(); if (getBool()) { if (getBool()) { @@ -180,7 +180,7 @@ void sls_fun_bad_7() { } } sls_mu.Lock(); // \ - // expected-warning{{lock 'sls_mu' is not released at the end of its scope}} + // expected-warning{{mutex 'sls_mu' is still held at the end of its scope}} } sls_mu.Unlock(); } @@ -189,23 +189,23 @@ void sls_fun_bad_8() { sls_mu.Lock(); do { sls_mu.Unlock(); // \ - // expected-warning{{expecting lock 'sls_mu' to be held at start of each loop}} + // expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}} } while (getBool()); } void sls_fun_bad_9() { do { sls_mu.Lock(); // \ - // expected-warning{{lock 'sls_mu' is not released at the end of its scope}} + // expected-warning{{mutex 'sls_mu' is still held at the end of its scope}} } while (getBool()); sls_mu.Unlock(); } void sls_fun_bad_10() { sls_mu.Lock(); // \ - // expected-warning{{lock 'sls_mu' is not released at the end of function 'sls_fun_bad_10'}} + // expected-warning{{mutex 'sls_mu' is still held at the end of function 'sls_fun_bad_10'}} while(getBool()) { // \ - // expected-warning{{expecting lock 'sls_mu' to be held at start of each loop}} + // expected-warning{{expecting lock on 'sls_mu' to be held at start of each loop}} sls_mu.Unlock(); } } @@ -213,10 +213,10 @@ void sls_fun_bad_10() { void sls_fun_bad_11() { while (getBool()) { sls_mu.Lock(); // \ - // expected-warning{{lock 'sls_mu' is not released at the end of its scope}} + // expected-warning{{mutex 'sls_mu' is still held at the end of its scope}} } sls_mu.Unlock(); // \ - // expected-warning{{unlocking 'sls_mu' that was not acquired}} + // expected-warning{{unlocking 'sls_mu' that was not locked}} } //-----------------------------------------// @@ -240,19 +240,19 @@ void aa_fun_1() { void aa_fun_bad_1() { glock.globalUnlock(); // \ - // expected-warning{{unlocking 'aa_mu' that was not acquired}} + // expected-warning{{unlocking 'aa_mu' that was not locked}} } void aa_fun_bad_2() { glock.globalLock(); glock.globalLock(); // \ - // expected-warning{{locking 'aa_mu' that is already acquired}} + // expected-warning{{locking 'aa_mu' that is already locked}} glock.globalUnlock(); } void aa_fun_bad_3() { glock.globalLock(); // \ - // expected-warning{{lock 'aa_mu' is not released at the end of function 'aa_fun_bad_3'}} + // expected-warning{{mutex 'aa_mu' is still held at the end of function 'aa_fun_bad_3'}} } //--------------------------------------------------// @@ -265,19 +265,19 @@ Mutex wmu; class WeirdMethods { WeirdMethods() { wmu.Lock(); // \ - // expected-warning {{lock 'wmu' is not released at the end of function 'WeirdMethods'}} + // expected-warning {{mutex 'wmu' is still held at the end of function 'WeirdMethods'}} } ~WeirdMethods() { wmu.Lock(); // \ - // expected-warning {{lock 'wmu' is not released at the end of function '~WeirdMethods'}} + // expected-warning {{mutex 'wmu' is still held at the end of function '~WeirdMethods'}} } void operator++() { wmu.Lock(); // \ - // expected-warning {{lock 'wmu' is not released at the end of function 'operator++'}} + // expected-warning {{mutex 'wmu' is still held at the end of function 'operator++'}} } operator int*() { wmu.Lock(); // \ - // expected-warning {{lock 'wmu' is not released at the end of function 'operator int *'}} + // expected-warning {{mutex 'wmu' is still held at the end of function 'operator int *'}} return 0; } }; @@ -296,13 +296,13 @@ class PGBFoo { __attribute__((pt_guarded_by(sls_mu))); void testFoo() { pgb_field = &x; // \ - // expected-warning {{writing variable 'pgb_field' requires lock 'sls_mu2' to be held exclusively}} - *pgb_field = x; // expected-warning {{reading variable 'pgb_field' requires lock 'sls_mu2' to be held}} \ - // expected-warning {{writing the value pointed to by 'pgb_field' requires lock 'sls_mu' to be held exclusively}} - x = *pgb_field; // expected-warning {{reading variable 'pgb_field' requires lock 'sls_mu2' to be held}} \ - // expected-warning {{reading the value pointed to by 'pgb_field' requires lock 'sls_mu' to be held}} - (*pgb_field)++; // expected-warning {{reading variable 'pgb_field' requires lock 'sls_mu2' to be held}} \ - // expected-warning {{writing the value pointed to by 'pgb_field' requires lock 'sls_mu' to be held exclusively}} + // expected-warning {{writing variable 'pgb_field' requires lock on 'sls_mu2' to be held exclusively}} + *pgb_field = x; // expected-warning {{reading variable 'pgb_field' requires lock on 'sls_mu2' to be held}} \ + // expected-warning {{writing the value pointed to by 'pgb_field' requires lock on 'sls_mu' to be held exclusively}} + x = *pgb_field; // expected-warning {{reading variable 'pgb_field' requires lock on 'sls_mu2' to be held}} \ + // expected-warning {{reading the value pointed to by 'pgb_field' requires lock on 'sls_mu' to be held}} + (*pgb_field)++; // expected-warning {{reading variable 'pgb_field' requires lock on 'sls_mu2' to be held}} \ + // expected-warning {{writing the value pointed to by 'pgb_field' requires lock on 'sls_mu' to be held exclusively}} } }; @@ -312,7 +312,7 @@ class GBFoo { void testFoo() { gb_field = 0; // \ - // expected-warning {{writing variable 'gb_field' requires lock 'sls_mu' to be held exclusively}} + // expected-warning {{writing variable 'gb_field' requires lock on 'sls_mu' to be held exclusively}} } void testNoAnal() __attribute__((no_thread_safety_analysis)) { @@ -355,12 +355,12 @@ void gb_bad_1() { void gb_bad_2() { sls_guardby_var = 1; // \ - // expected-warning {{writing variable 'sls_guardby_var' requires lock 'sls_mu' to be held exclusively}} + // expected-warning {{writing variable 'sls_guardby_var' requires lock on 'sls_mu' to be held exclusively}} } void gb_bad_3() { int x = sls_guardby_var; // \ - // expected-warning {{reading variable 'sls_guardby_var' requires lock 'sls_mu' to be held}} + // expected-warning {{reading variable 'sls_guardby_var' requires lock on 'sls_mu' to be held}} } void gb_bad_4() { @@ -375,18 +375,18 @@ void gb_bad_5() { void gb_bad_6() { *pgb_var = 1; // \ - // expected-warning {{writing the value pointed to by 'pgb_var' requires lock 'sls_mu' to be held exclusively}} + // expected-warning {{writing the value pointed to by 'pgb_var' requires lock on 'sls_mu' to be held exclusively}} } void gb_bad_7() { int x = *pgb_var; // \ - // expected-warning {{reading the value pointed to by 'pgb_var' requires lock 'sls_mu' to be held}} + // expected-warning {{reading the value pointed to by 'pgb_var' requires lock on 'sls_mu' to be held}} } void gb_bad_8() { GBFoo G; G.gb_field = 0; // \ - // expected-warning {{writing variable 'gb_field' requires lock 'sls_mu'}} + // expected-warning {{writing variable 'gb_field' requires lock on 'sls_mu'}} } void gb_bad_9() { @@ -413,11 +413,11 @@ public: void test() { a = 0; // \ - // expected-warning{{writing variable 'a' requires lock 'mu' to be held exclusively}} + // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}} b = a; // \ - // expected-warning {{reading variable 'a' requires lock 'mu' to be held}} + // expected-warning {{reading variable 'a' requires lock on 'mu' to be held}} c = 0; // \ - // expected-warning {{writing variable 'c' requires lock 'mu' to be held exclusively}} + // expected-warning {{writing variable 'c' requires lock on 'mu' to be held exclusively}} } int c __attribute__((guarded_by(mu))); @@ -443,7 +443,7 @@ void shared_fun_1() { do { sls_mu.Unlock(); sls_mu.Lock(); // \ - // expected-warning {{lock 'sls_mu' is held exclusively and shared in the same scope}} + // expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}} } while (getBool()); sls_mu.Unlock(); } @@ -469,10 +469,10 @@ void shared_fun_4() { void shared_fun_8() { if (getBool()) sls_mu.Lock(); // \ - // expected-warning {{lock 'sls_mu' is held exclusively and shared in the same scope}} + // expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}} else sls_mu.ReaderLock(); // \ - // expected-note {{the other acquire of lock 'sls_mu' is here}} + // expected-note {{the other lock of mutex 'sls_mu' is here}} sls_mu.Unlock(); } @@ -481,7 +481,7 @@ void shared_bad_0() { do { sls_mu.Unlock(); sls_mu.ReaderLock(); // \ - // expected-warning {{lock 'sls_mu' is held exclusively and shared in the same scope}} + // expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}} } while (getBool()); sls_mu.Unlock(); } @@ -489,10 +489,10 @@ void shared_bad_0() { void shared_bad_1() { if (getBool()) sls_mu.Lock(); // \ - // expected-warning {{lock 'sls_mu' is held exclusively and shared in the same scope}} + // expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}} else sls_mu.ReaderLock(); // \ - // expected-note {{the other acquire of lock 'sls_mu' is here}} + // expected-note {{the other lock of mutex 'sls_mu' is here}} *pgb_var = 1; sls_mu.Unlock(); } @@ -500,10 +500,10 @@ void shared_bad_1() { void shared_bad_2() { if (getBool()) sls_mu.ReaderLock(); // \ - // expected-warning {{lock 'sls_mu' is held exclusively and shared in the same scope}} + // expected-warning {{lock 'sls_mu' is exclusive and shared in the same scope}} else sls_mu.Lock(); // \ - // expected-note {{the other acquire of lock 'sls_mu' is here}} + // expected-note {{the other lock of mutex 'sls_mu' is here}} *pgb_var = 1; sls_mu.Unlock(); } @@ -582,48 +582,48 @@ void es_fun_8() { void es_bad_0() { Bar.aa_elr_fun(); // \ - // expected-warning {{calling function 'aa_elr_fun' requires exclusive lock 'aa_mu'}} + // expected-warning {{calling function 'aa_elr_fun' requires exclusive lock on 'aa_mu'}} } void es_bad_1() { aa_mu.ReaderLock(); Bar.aa_elr_fun(); // \ - // expected-warning {{calling function 'aa_elr_fun' requires exclusive lock 'aa_mu'}} + // expected-warning {{calling function 'aa_elr_fun' requires exclusive lock on 'aa_mu'}} aa_mu.Unlock(); } void es_bad_2() { Bar.aa_elr_fun_s(); // \ - // expected-warning {{calling function 'aa_elr_fun_s' requires shared lock 'aa_mu'}} + // expected-warning {{calling function 'aa_elr_fun_s' requires shared lock on 'aa_mu'}} } void es_bad_3() { MyLRFoo.test(); // \ - // expected-warning {{calling function 'test' requires exclusive lock 'sls_mu'}} + // expected-warning {{calling function 'test' requires exclusive lock on 'sls_mu'}} } void es_bad_4() { MyLRFoo.testShared(); // \ - // expected-warning {{calling function 'testShared' requires shared lock 'sls_mu2'}} + // expected-warning {{calling function 'testShared' requires shared lock on 'sls_mu2'}} } void es_bad_5() { sls_mu.ReaderLock(); MyLRFoo.test(); // \ - // expected-warning {{calling function 'test' requires exclusive lock 'sls_mu'}} + // expected-warning {{calling function 'test' requires exclusive lock on 'sls_mu'}} sls_mu.Unlock(); } void es_bad_6() { sls_mu.Lock(); Bar.le_fun(); // \ - // expected-warning {{cannot call function 'le_fun' while holding lock 'sls_mu'}} + // expected-warning {{cannot call function 'le_fun' while holding mutex 'sls_mu'}} |