From 5c6134fd09bc5b738dafdd1c774edde13d95cb20 Mon Sep 17 00:00:00 2001 From: DeLesley Hutchins Date: Fri, 17 May 2013 23:02:59 +0000 Subject: [PATCH] Thread safety analysis: add two new attributes to the thread safety analysis: assert_exclusive_lock and assert_shared_lock. These attributes are used to mark functions that dynamically check (i.e. assert) that a lock is held. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182170 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/Attr.td | 14 +++ lib/Analysis/ThreadSafety.cpp | 49 ++++++-- lib/Sema/SemaDeclAttr.cpp | 34 ++++++ test/SemaCXX/warn-thread-safety-analysis.cpp | 111 ++++++++++++++++++- test/SemaCXX/warn-thread-safety-parsing.cpp | 16 ++- 5 files changed, 211 insertions(+), 13 deletions(-) diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td index 441a79a23b..7c59c16f14 100644 --- a/include/clang/Basic/Attr.td +++ b/include/clang/Basic/Attr.td @@ -852,6 +852,20 @@ def SharedLockFunction : InheritableAttr { let TemplateDependent = 1; } +def AssertExclusiveLock : InheritableAttr { + let Spellings = [GNU<"assert_exclusive_lock">]; + let Args = [VariadicExprArgument<"Args">]; + let LateParsed = 1; + let TemplateDependent = 1; +} + +def AssertSharedLock : InheritableAttr { + let Spellings = [GNU<"assert_shared_lock">]; + let Args = [VariadicExprArgument<"Args">]; + let LateParsed = 1; + let TemplateDependent = 1; +} + // The first argument is an integer or boolean value specifying the return value // of a successful lock acquisition. def ExclusiveTrylockFunction : InheritableAttr { diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp index 479d9a301f..c12dc64fc1 100644 --- a/lib/Analysis/ThreadSafety.cpp +++ b/lib/Analysis/ThreadSafety.cpp @@ -750,16 +750,18 @@ struct LockData { /// /// FIXME: add support for re-entrant locking and lock up/downgrading LockKind LKind; + bool Asserted; // for asserted locks bool Managed; // for ScopedLockable objects SExpr UnderlyingMutex; // for ScopedLockable objects - LockData(SourceLocation AcquireLoc, LockKind LKind, bool M = false) - : AcquireLoc(AcquireLoc), LKind(LKind), Managed(M), + LockData(SourceLocation AcquireLoc, LockKind LKind, bool M=false, + bool Asrt=false) + : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(Asrt), Managed(M), UnderlyingMutex(Decl::EmptyShell()) {} LockData(SourceLocation AcquireLoc, LockKind LKind, const SExpr &Mu) - : AcquireLoc(AcquireLoc), LKind(LKind), Managed(false), + : AcquireLoc(AcquireLoc), LKind(LKind), Asserted(false), Managed(false), UnderlyingMutex(Mu) {} @@ -1484,7 +1486,8 @@ void ThreadSafetyAnalyzer::addLock(FactSet &FSet, const SExpr &Mutex, return; if (FSet.findLock(FactMan, Mutex)) { - Handler.handleDoubleLock(Mutex.toString(), LDat.AcquireLoc); + if (!LDat.Asserted) + Handler.handleDoubleLock(Mutex.toString(), LDat.AcquireLoc); } else { FSet.addLock(FactMan, Mutex, LDat); } @@ -1909,6 +1912,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK) { /// the same signature as const method calls can be also treated as reads. /// void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { + SourceLocation Loc = Exp->getExprLoc(); const AttrVec &ArgAttrs = D->getAttrs(); MutexIDList ExclusiveLocksToAdd; MutexIDList SharedLocksToAdd; @@ -1933,6 +1937,32 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { break; } + // An assert will add a lock to the lockset, but will not generate + // a warning if it is already there, and will not generate a warning + // if it is not removed. + case attr::AssertExclusiveLock: { + AssertExclusiveLockAttr *A = cast(At); + + MutexIDList AssertLocks; + Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); + for (unsigned i=0,n=AssertLocks.size(); iaddLock(FSet, AssertLocks[i], + LockData(Loc, LK_Exclusive, false, true)); + } + break; + } + case attr::AssertSharedLock: { + AssertSharedLockAttr *A = cast(At); + + MutexIDList AssertLocks; + Analyzer->getMutexIDs(AssertLocks, A, Exp, D, VD); + for (unsigned i=0,n=AssertLocks.size(); iaddLock(FSet, AssertLocks[i], + LockData(Loc, LK_Shared, false, true)); + } + break; + } + // 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: { @@ -1986,7 +2016,6 @@ void BuildLockset::handleCall(Expr *Exp, const NamedDecl *D, VarDecl *VD) { } // Add locks. - SourceLocation Loc = Exp->getExprLoc(); for (unsigned i=0,n=ExclusiveLocksToAdd.size(); iaddLock(FSet, ExclusiveLocksToAdd[i], LockData(Loc, LK_Exclusive, isScopedVar)); @@ -2165,7 +2194,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1, const SExpr &FSet2Mutex = FactMan[*I].MutID; const LockData &LDat2 = FactMan[*I].LDat; - if (const LockData *LDat1 = FSet1.findLock(FactMan, FSet2Mutex)) { + if (LockData *LDat1 = FSet1.findLock(FactMan, FSet2Mutex)) { if (LDat1->LKind != LDat2.LKind) { Handler.handleExclusiveAndShared(FSet2Mutex.toString(), LDat2.AcquireLoc, @@ -2175,6 +2204,10 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1, FSet1.addLock(FactMan, FSet2Mutex, LDat2); } } + if (LDat1->Asserted && !LDat2.Asserted) { + // The non-asserted lock is the one we want to track. + *LDat1 = LDat2; + } } else { if (LDat2.UnderlyingMutex.isValid()) { if (FSet2.findLock(FactMan, LDat2.UnderlyingMutex)) { @@ -2186,7 +2219,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1, JoinLoc, LEK1); } } - else if (!LDat2.Managed && !FSet2Mutex.isUniversal()) + else if (!LDat2.Managed && !FSet2Mutex.isUniversal() && !LDat2.Asserted) Handler.handleMutexHeldEndOfScope(FSet2Mutex.toString(), LDat2.AcquireLoc, JoinLoc, LEK1); @@ -2209,7 +2242,7 @@ void ThreadSafetyAnalyzer::intersectAndWarn(FactSet &FSet1, JoinLoc, LEK1); } } - else if (!LDat1.Managed && !FSet1Mutex.isUniversal()) + else if (!LDat1.Managed && !FSet1Mutex.isUniversal() && !LDat1.Asserted) Handler.handleMutexHeldEndOfScope(FSet1Mutex.toString(), LDat1.AcquireLoc, JoinLoc, LEK2); diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index d9b8e16061..c365bfc508 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -788,6 +788,34 @@ static void handleExclusiveLockFunctionAttr(Sema &S, Decl *D, Attr.getAttributeSpellingListIndex())); } +static void handleAssertSharedLockAttr(Sema &S, Decl *D, + const AttributeList &Attr) { + SmallVector Args; + if (!checkLockFunAttrCommon(S, D, Attr, Args)) + return; + + unsigned Size = Args.size(); + Expr **StartArg = Size == 0 ? 0 : &Args[0]; + D->addAttr(::new (S.Context) + AssertSharedLockAttr(Attr.getRange(), S.Context, StartArg, Size, + Attr.getAttributeSpellingListIndex())); +} + +static void handleAssertExclusiveLockAttr(Sema &S, Decl *D, + const AttributeList &Attr) { + SmallVector Args; + if (!checkLockFunAttrCommon(S, D, Attr, Args)) + return; + + unsigned Size = Args.size(); + Expr **StartArg = Size == 0 ? 0 : &Args[0]; + D->addAttr(::new (S.Context) + AssertExclusiveLockAttr(Attr.getRange(), S.Context, + StartArg, Size, + Attr.getAttributeSpellingListIndex())); +} + + static bool checkTryLockFunAttrCommon(Sema &S, Decl *D, const AttributeList &Attr, SmallVector &Args) { @@ -4918,6 +4946,12 @@ static void ProcessInheritableDeclAttr(Sema &S, Scope *scope, Decl *D, break; // Thread safety attributes: + case AttributeList::AT_AssertExclusiveLock: + handleAssertExclusiveLockAttr(S, D, Attr); + break; + case AttributeList::AT_AssertSharedLock: + handleAssertSharedLockAttr(S, D, Attr); + break; case AttributeList::AT_GuardedVar: handleGuardedVarAttr(S, D, Attr); break; diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp index bc4b40ead7..20b1a56354 100644 --- a/test/SemaCXX/warn-thread-safety-analysis.cpp +++ b/test/SemaCXX/warn-thread-safety-analysis.cpp @@ -11,8 +11,10 @@ #define PT_GUARDED_VAR __attribute__ ((pt_guarded_var)) #define ACQUIRED_AFTER(...) __attribute__ ((acquired_after(__VA_ARGS__))) #define ACQUIRED_BEFORE(...) __attribute__ ((acquired_before(__VA_ARGS__))) -#define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__ ((exclusive_lock_function(__VA_ARGS__))) -#define SHARED_LOCK_FUNCTION(...) __attribute__ ((shared_lock_function(__VA_ARGS__))) +#define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__ ((exclusive_lock_function(__VA_ARGS__))) +#define SHARED_LOCK_FUNCTION(...) __attribute__ ((shared_lock_function(__VA_ARGS__))) +#define ASSERT_EXCLUSIVE_LOCK(...) __attribute__ ((assert_exclusive_lock(__VA_ARGS__))) +#define ASSERT_SHARED_LOCK(...) __attribute__ ((assert_shared_lock(__VA_ARGS__))) #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__ ((exclusive_trylock_function(__VA_ARGS__))) #define SHARED_TRYLOCK_FUNCTION(...) __attribute__ ((shared_trylock_function(__VA_ARGS__))) #define UNLOCK_FUNCTION(...) __attribute__ ((unlock_function(__VA_ARGS__))) @@ -33,6 +35,9 @@ class __attribute__((lockable)) Mutex { bool TryLock() __attribute__((exclusive_trylock_function(true))); bool ReaderTryLock() __attribute__((shared_trylock_function(true))); void LockWhen(const int &cond) __attribute__((exclusive_lock_function)); + + void AssertHeld() ASSERT_EXCLUSIVE_LOCK(); + void AssertReaderHeld() ASSERT_SHARED_LOCK(); }; class __attribute__((scoped_lockable)) MutexLock { @@ -3985,3 +3990,105 @@ private: } // end namespace LockUnlockFunctionTest + +namespace AssertHeldTest { + +class Foo { +public: + int c; + int a GUARDED_BY(mu_); + Mutex mu_; + + void test1() { + mu_.AssertHeld(); + int b = a; + a = 0; + } + + void test2() { + mu_.AssertReaderHeld(); + int b = a; + a = 0; // expected-warning {{writing variable 'a' requires locking 'mu_' exclusively}} + } + + void test3() { + if (c) { + mu_.AssertHeld(); + } + else { + mu_.AssertHeld(); + } + int b = a; + a = 0; + } + + void test4() EXCLUSIVE_LOCKS_REQUIRED(mu_) { + mu_.AssertHeld(); + int b = a; + a = 0; + } + + void test5() UNLOCK_FUNCTION(mu_) { + mu_.AssertHeld(); + mu_.Unlock(); + } + + void test6() { + mu_.AssertHeld(); + mu_.Unlock(); + } // should this be a warning? + + void test7() { + if (c) { + mu_.AssertHeld(); + } + else { + mu_.Lock(); + } + int b = a; + a = 0; + mu_.Unlock(); + } + + void test8() { + if (c) { + mu_.Lock(); + } + else { + mu_.AssertHeld(); + } + int b = a; + a = 0; + mu_.Unlock(); + } + + void test9() { + if (c) { + mu_.AssertHeld(); + } + else { + mu_.Lock(); // expected-note {{mutex acquired here}} + } + } // expected-warning {{mutex 'mu_' is still locked at the end of function}} + + void test10() { + if (c) { + mu_.Lock(); // expected-note {{mutex acquired here}} + } + else { + mu_.AssertHeld(); + } + } // expected-warning {{mutex 'mu_' is still locked at the end of function}} + + void assertMu() ASSERT_EXCLUSIVE_LOCK(mu_); + + void test11() { + assertMu(); + int b = a; + a = 0; + } +}; + +} // end namespace AssertHeldTest + + diff --git a/test/SemaCXX/warn-thread-safety-parsing.cpp b/test/SemaCXX/warn-thread-safety-parsing.cpp index df9415cf86..ee6ec92752 100644 --- a/test/SemaCXX/warn-thread-safety-parsing.cpp +++ b/test/SemaCXX/warn-thread-safety-parsing.cpp @@ -8,8 +8,10 @@ #define PT_GUARDED_VAR __attribute__ ((pt_guarded_var)) #define ACQUIRED_AFTER(...) __attribute__ ((acquired_after(__VA_ARGS__))) #define ACQUIRED_BEFORE(...) __attribute__ ((acquired_before(__VA_ARGS__))) -#define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__ ((exclusive_lock_function(__VA_ARGS__))) -#define SHARED_LOCK_FUNCTION(...) __attribute__ ((shared_lock_function(__VA_ARGS__))) +#define EXCLUSIVE_LOCK_FUNCTION(...) __attribute__ ((exclusive_lock_function(__VA_ARGS__))) +#define SHARED_LOCK_FUNCTION(...) __attribute__ ((shared_lock_function(__VA_ARGS__))) +#define ASSERT_EXCLUSIVE_LOCK(...) __attribute__ ((assert_exclusive_lock(__VA_ARGS__))) +#define ASSERT_SHARED_LOCK(...) __attribute__ ((assert_shared_lock(__VA_ARGS__))) #define EXCLUSIVE_TRYLOCK_FUNCTION(...) __attribute__ ((exclusive_trylock_function(__VA_ARGS__))) #define SHARED_TRYLOCK_FUNCTION(...) __attribute__ ((shared_trylock_function(__VA_ARGS__))) #define UNLOCK_FUNCTION(...) __attribute__ ((unlock_function(__VA_ARGS__))) @@ -24,7 +26,15 @@ class LOCKABLE Mutex { public: - void Lock(); + void Lock() EXCLUSIVE_LOCK_FUNCTION(); + void ReaderLock() SHARED_LOCK_FUNCTION(); + void Unlock() UNLOCK_FUNCTION(); + + bool TryLock() EXCLUSIVE_TRYLOCK_FUNCTION(true); + bool ReaderTryLock() SHARED_TRYLOCK_FUNCTION(true); + + void AssertHeld() ASSERT_EXCLUSIVE_LOCK(); + void AssertReaderHeld() ASSERT_SHARED_LOCK(); }; class UnlockableMu{ -- 2.40.0