The release_capability, release_shared_capability and release_generic_capability functions are now functionally distinct for capability analysis. The unlock_function attribute maps directly to release_generic_capability.

llvm-svn: 204469
This commit is contained in:
Aaron Ballman 2014-03-21 14:48:48 +00:00
parent 2f4529f864
commit df115d9bf3
6 changed files with 72 additions and 30 deletions

View File

@ -39,7 +39,8 @@ enum ProtectedOperationKind {
/// mutex.
enum LockKind {
LK_Shared, ///< Shared/reader lock of a mutex.
LK_Exclusive ///< Exclusive/writer lock of a mutex.
LK_Exclusive, ///< Exclusive/writer lock of a mutex.
LK_Generic ///< Can be either Shared or Exclusive
};
/// This enum distinguishes between different ways to access (read or write) a
@ -82,6 +83,18 @@ public:
/// \param Loc -- The SourceLocation of the Unlock
virtual void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) {}
/// Warn about an unlock function call that attempts to unlock a lock with
/// the incorrect lock kind. For instance, a shared lock being unlocked
/// exclusively, or vice versa.
/// \param LockName -- A StringRef name for the lock expression, to be printed
/// in the error message.
/// \param Expected -- the kind of lock expected.
/// \param Received -- the kind of lock received.
/// \param Loc -- The SourceLocation of the Unlock.
virtual void handleIncorrectUnlockKind(Name LockName, LockKind Expected,
LockKind Received,
SourceLocation Loc) {}
/// Warn about lock function calls for locks which are already held.
/// \param LockName -- A StringRef name for the lock expression, to be printed
/// in the error message.

View File

@ -1388,7 +1388,8 @@ def ReleaseCapability : InheritableAttr {
CXX11<"clang", "release_shared_capability">]>,
Accessor<"isGeneric",
[GNU<"release_generic_capability">,
CXX11<"clang", "release_generic_capability">]>];
CXX11<"clang", "release_generic_capability">,
GNU<"unlock_function">]>];
let Documentation = [ReleaseCapabilityDocs];
}

View File

@ -2201,6 +2201,10 @@ def err_attribute_argument_out_of_range : Error<
def warn_unlock_but_no_lock : Warning<
"unlocking '%0' that was not locked">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
def warn_unlock_kind_mismatch : Warning<
"unlocking '%0' using %select{shared|exclusive}1 access, expected "
"%select{shared|exclusive}2 access">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;
def warn_double_lock : Warning<
"locking '%0' that is already locked">,
InGroup<ThreadSafetyAnalysis>, DefaultIgnore;

View File

@ -1430,7 +1430,7 @@ public:
void addLock(FactSet &FSet, const SExpr &Mutex, const LockData &LDat);
void removeLock(FactSet &FSet, const SExpr &Mutex,
SourceLocation UnlockLoc, bool FullyRemove=false);
SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind);
template <typename AttrType>
void getMutexIDs(MutexIDList &Mtxs, AttrType *Attr, Expr *Exp,
@ -1486,10 +1486,9 @@ void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const SExpr &Mutex,
/// \brief Remove a lock from the lockset, warning if the lock is not there.
/// \param Mutex The lock expression corresponding to the lock to be removed
/// \param UnlockLoc The source location of the unlock (only used in error msg)
void ThreadSafetyAnalyzer::removeLock(FactSet &FSet,
const SExpr &Mutex,
void ThreadSafetyAnalyzer::removeLock(FactSet &FSet, const SExpr &Mutex,
SourceLocation UnlockLoc,
bool FullyRemove) {
bool FullyRemove, LockKind ReceivedKind) {
if (Mutex.shouldIgnore())
return;
@ -1499,6 +1498,14 @@ void ThreadSafetyAnalyzer::removeLock(FactSet &FSet,
return;
}
// Generic lock removal doesn't care about lock kind mismatches, but
// otherwise diagnose when the lock kinds are mismatched.
if (ReceivedKind != LK_Generic && LDat->LKind != ReceivedKind) {
Handler.handleIncorrectUnlockKind(Mutex.toString(), LDat->LKind,
ReceivedKind, UnlockLoc);
return;
}
if (LDat->UnderlyingMutex.isValid()) {
// This is scoped lockable object, which manages the real mutex.
if (FullyRemove) {
@ -1926,9 +1933,8 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) {
void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
SourceLocation Loc = Exp->getExprLoc();
const AttrVec &ArgAttrs = D->getAttrs();
MutexIDList ExclusiveLocksToAdd;
MutexIDList SharedLocksToAdd;
MutexIDList LocksToRemove;
MutexIDList ExclusiveLocksToAdd, SharedLocksToAdd;
MutexIDList ExclusiveLocksToRemove, SharedLocksToRemove, GenericLocksToRemove;
for(unsigned i = 0; i < ArgAttrs.size(); ++i) {
Attr *At = const_cast<Attr*>(ArgAttrs[i]);
@ -1973,7 +1979,12 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
// mutexes from the lockset, and flag a warning if they are not there.
case attr::ReleaseCapability: {
auto *A = cast<ReleaseCapabilityAttr>(At);
Analyzer->getMutexIDs(LocksToRemove, A, Exp, D, VD);
if (A->isGeneric())
Analyzer->getMutexIDs(GenericLocksToRemove, A, Exp, D, VD);
else if (A->isShared())
Analyzer->getMutexIDs(SharedLocksToRemove, A, Exp, D, VD);
else
Analyzer->getMutexIDs(ExclusiveLocksToRemove, A, Exp, D, VD);
break;
}
@ -2014,14 +2025,10 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
}
// Add locks.
for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
Analyzer->addLock(FSet, ExclusiveLocksToAdd[i],
LockData(Loc, LK_Exclusive, isScopedVar));
}
for (unsigned i=0,n=SharedLocksToAdd.size(); i<n; ++i) {
Analyzer->addLock(FSet, SharedLocksToAdd[i],
LockData(Loc, LK_Shared, isScopedVar));
}
for (const auto &M : ExclusiveLocksToAdd)
Analyzer->addLock(FSet, M, LockData(Loc, LK_Exclusive, isScopedVar));
for (const auto &M : SharedLocksToAdd)
Analyzer->addLock(FSet, M, LockData(Loc, LK_Shared, isScopedVar));
// Add the managing object as a dummy mutex, mapped to the underlying mutex.
// FIXME -- this doesn't work if we acquire multiple locks.
@ -2030,22 +2037,21 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) {
DeclRefExpr DRE(VD, false, VD->getType(), VK_LValue, VD->getLocation());
SExpr SMutex(&DRE, 0, 0);
for (unsigned i=0,n=ExclusiveLocksToAdd.size(); i<n; ++i) {
Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive,
ExclusiveLocksToAdd[i]));
}
for (unsigned i=0,n=SharedLocksToAdd.size(); i<n; ++i) {
Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Shared,
SharedLocksToAdd[i]));
}
for (const auto &M : ExclusiveLocksToAdd)
Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Exclusive, M));
for (const auto &M : SharedLocksToAdd)
Analyzer->addLock(FSet, SMutex, LockData(MLoc, LK_Shared, M));
}
// Remove locks.
// FIXME -- should only fully remove if the attribute refers to 'this'.
bool Dtor = isa<CXXDestructorDecl>(D);
for (unsigned i=0,n=LocksToRemove.size(); i<n; ++i) {
Analyzer->removeLock(FSet, LocksToRemove[i], Loc, Dtor);
}
for (const auto &M : ExclusiveLocksToRemove)
Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Exclusive);
for (const auto &M : SharedLocksToRemove)
Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Shared);
for (const auto &M : GenericLocksToRemove)
Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic);
}

View File

@ -1452,7 +1452,15 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) override {
warnLockMismatch(diag::warn_unlock_but_no_lock, LockName, Loc);
}
void handleIncorrectUnlockKind(Name LockName, LockKind Expected,
LockKind Received,
SourceLocation Loc) override {
if (Loc.isInvalid())
Loc = FunLocation;
PartialDiagnosticAt Warning(Loc, S.PDiag(diag::warn_unlock_kind_mismatch)
<< LockName << Received << Expected);
Warnings.push_back(DelayedDiag(Warning, OptionalNotes()));
}
void handleDoubleLock(Name LockName, SourceLocation Loc) override {
warnLockMismatch(diag::warn_double_lock, LockName, Loc);
}

View File

@ -35,6 +35,8 @@ struct Foo {
void mutex_exclusive_lock(struct Mutex *mu) EXCLUSIVE_LOCK_FUNCTION(mu);
void mutex_shared_lock(struct Mutex *mu) SHARED_LOCK_FUNCTION(mu);
void mutex_unlock(struct Mutex *mu) UNLOCK_FUNCTION(mu);
void mutex_shared_unlock(struct Mutex *mu) __attribute__((release_shared_capability(mu)));
void mutex_exclusive_unlock(struct Mutex *mu) __attribute__((release_capability(mu)));
// Define global variables.
struct Mutex mu1;
@ -114,5 +116,13 @@ int main() {
(void)(*d_ == 1);
mutex_unlock(foo_.mu_);
mutex_exclusive_lock(&mu1);
mutex_shared_unlock(&mu1); // expected-warning {{unlocking 'mu1' using shared access, expected exclusive access}}
mutex_exclusive_unlock(&mu1);
mutex_shared_lock(&mu1);
mutex_exclusive_unlock(&mu1); // expected-warning {{unlocking 'mu1' using exclusive access, expected shared access}}
mutex_shared_unlock(&mu1);
return 0;
}