aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--include/clang/Analysis/Analyses/ThreadSafety.h1
-rw-r--r--include/clang/Basic/DiagnosticSemaKinds.td3
-rw-r--r--lib/Analysis/ThreadSafety.cpp58
-rw-r--r--lib/Sema/AnalysisBasedWarnings.cpp4
-rw-r--r--test/SemaCXX/warn-thread-safety-analysis.cpp74
5 files changed, 119 insertions, 21 deletions
diff --git a/include/clang/Analysis/Analyses/ThreadSafety.h b/include/clang/Analysis/Analyses/ThreadSafety.h
index 2f19973d8c..b006b36f93 100644
--- a/include/clang/Analysis/Analyses/ThreadSafety.h
+++ b/include/clang/Analysis/Analyses/ThreadSafety.h
@@ -47,6 +47,7 @@ public:
typedef llvm::StringRef Name;
ThreadSafetyHandler() {}
virtual ~ThreadSafetyHandler() {}
+ virtual void handleInvalidLockExp(SourceLocation Loc) {}
virtual void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) {}
virtual void handleDoubleLock(Name LockName, SourceLocation Loc) {}
virtual void handleMutexHeldEndOfScope(Name LockName, SourceLocation Loc){}
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 0dd2099e5d..568e0a3b6b 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1424,6 +1424,9 @@ def warn_fun_requires_lock : Warning<
def warn_fun_excludes_mutex : Warning<
"cannot call function '%0' while holding mutex '%1'">,
InGroup<ThreadSafety>, DefaultIgnore;
+def warn_cannot_resolve_lock : Warning<
+ "cannot resolve lock expression to a specific lockable object">,
+ InGroup<ThreadSafety>, DefaultIgnore;
def warn_impcast_vector_scalar : Warning<
diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index b0d1d30afc..cefedfdf88 100644
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -37,6 +37,15 @@
using namespace clang;
using namespace thread_safety;
+// Helper functions
+static Expr *getParent(Expr *Exp) {
+ if (MemberExpr *ME = dyn_cast<MemberExpr>(Exp))
+ return ME->getBase();
+ if (CXXMemberCallExpr *CE = dyn_cast<CXXMemberCallExpr>(Exp))
+ return CE->getImplicitObjectArgument();
+ return 0;
+}
+
namespace {
/// \brief Implements a set of CFGBlocks using a BitVector.
///
@@ -146,28 +155,32 @@ public:
/// foo(MyL); // requires lock MyL->Mu to be held
class MutexID {
SmallVector<NamedDecl*, 2> DeclSeq;
+ ThreadSafetyHandler &Handler;
/// Build a Decl sequence representing the lock from the given expression.
/// Recursive function that bottoms out when the final DeclRefExpr is reached.
- void buildMutexID(Expr *Exp) {
+ void buildMutexID(Expr *Exp, Expr *Parent) {
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);
- buildMutexID(ME->getBase());
+ buildMutexID(ME->getBase(), Parent);
} else if (isa<CXXThisExpr>(Exp)) {
- return;
- } else {
- // FIXME: add diagnostic
- llvm::report_fatal_error("Expected lock expression!");
- }
+ if (!Parent)
+ return;
+ buildMutexID(Parent, 0);
+ } else if (CastExpr *CE = dyn_cast<CastExpr>(Exp))
+ buildMutexID(CE->getSubExpr(), Parent);
+ else
+ Handler.handleInvalidLockExp(Exp->getExprLoc());
}
public:
- MutexID(Expr *LExpr) {
- buildMutexID(LExpr);
+ MutexID(ThreadSafetyHandler &Handler, Expr *LExpr, Expr *ParentExpr)
+ : Handler(Handler) {
+ buildMutexID(LExpr, ParentExpr);
assert(!DeclSeq.empty());
}
@@ -252,8 +265,9 @@ class BuildLockset : public StmtVisitor<BuildLockset> {
Lockset::Factory &LocksetFactory;
// Helper functions
- void removeLock(SourceLocation UnlockLoc, Expr *LockExp);
- void addLock(SourceLocation LockLoc, Expr *LockExp, LockKind LK);
+ void removeLock(SourceLocation UnlockLoc, Expr *LockExp, Expr *Parent);
+ void addLock(SourceLocation LockLoc, Expr *LockExp, Expr *Parent,
+ LockKind LK);
const ValueDecl *getValueDecl(Expr *Exp);
void warnIfMutexNotHeld (const NamedDecl *D, Expr *Exp, AccessKind AK,
Expr *MutexExp, ProtectedOperationKind POK);
@@ -307,10 +321,10 @@ public:
/// \brief Add a new lock to the lockset, warning if the lock is already there.
/// \param LockLoc The source location of the acquire
/// \param LockExp The lock expression corresponding to the lock to be added
-void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp,
+void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp, Expr *Parent,
LockKind LK) {
// FIXME: deal with acquired before/after annotations
- MutexID Mutex(LockExp);
+ MutexID Mutex(Handler, LockExp, Parent);
LockData NewLock(LockLoc, LK);
// FIXME: Don't always warn when we have support for reentrant locks.
@@ -322,8 +336,9 @@ void BuildLockset::addLock(SourceLocation LockLoc, Expr *LockExp,
/// \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) {
- MutexID Mutex(LockExp);
+void BuildLockset::removeLock(SourceLocation UnlockLoc, Expr *LockExp,
+ Expr *Parent) {
+ MutexID Mutex(Handler, LockExp, Parent);
Lockset NewLSet = LocksetFactory.remove(LSet, Mutex);
if(NewLSet == LSet)
@@ -349,7 +364,8 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp,
AccessKind AK, Expr *MutexExp,
ProtectedOperationKind POK) {
LockKind LK = getLockKindFromAccessKind(AK);
- MutexID Mutex(MutexExp);
+ Expr *Parent = getParent(Exp);
+ MutexID Mutex(Handler, MutexExp, Parent);
if (!locksetContainsAtLeast(Mutex, LK))
Handler.handleMutexNotHeld(D, POK, Mutex.getName(), LK, Exp->getExprLoc());
}
@@ -451,13 +467,13 @@ void BuildLockset::addLocksToSet(LockKind LK, Attr *Attr,
if (SpecificAttr->args_size() == 0) {
// The mutex held is the "this" object.
- addLock(ExpLocation, Parent, LK);
+ addLock(ExpLocation, Parent, Parent, LK);
return;
}
for (iterator_type I = SpecificAttr->args_begin(),
E = SpecificAttr->args_end(); I != E; ++I)
- addLock(ExpLocation, *I, LK);
+ addLock(ExpLocation, *I, Parent, LK);
}
/// \brief When visiting CXXMemberCallExprs we need to examine the attributes on
@@ -501,13 +517,13 @@ void BuildLockset::VisitCXXMemberCallExpr(CXXMemberCallExpr *Exp) {
UnlockFunctionAttr *UFAttr = cast<UnlockFunctionAttr>(Attr);
if (UFAttr->args_size() == 0) { // The lock held is the "this" object.
- removeLock(ExpLocation, Parent);
+ removeLock(ExpLocation, Parent, Parent);
break;
}
for (UnlockFunctionAttr::args_iterator I = UFAttr->args_begin(),
E = UFAttr->args_end(); I != E; ++I)
- removeLock(ExpLocation, *I);
+ removeLock(ExpLocation, *I, Parent);
break;
}
@@ -540,7 +556,7 @@ 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) {
- MutexID Mutex(*I);
+ MutexID Mutex(Handler, *I, Parent);
if (locksetContains(Mutex))
Handler.handleFunExcludesLock(D->getName(), Mutex.getName(),
ExpLocation);
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index 405ba9e724..823cbf74f5 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -648,6 +648,10 @@ class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
S.Diag(I->first, I->second);
}
+ void handleInvalidLockExp(SourceLocation Loc) {
+ PartialDiagnostic Warning = S.PDiag(diag::warn_cannot_resolve_lock) << Loc;
+ Warnings.push_back(DelayedDiag(Loc, Warning));
+ }
void handleUnmatchedUnlock(Name LockName, SourceLocation Loc) {
warnLockMismatch(diag::warn_unlock_but_no_lock, LockName, Loc);
}
diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp
index 9cecea7890..85de91853e 100644
--- a/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -425,6 +425,80 @@ public:
Mutex mu;
};
+class LateBar {
+ public:
+ int a_ __attribute__((guarded_by(mu1_)));
+ int b_;
+ int *q __attribute__((pt_guarded_by(mu)));
+ Mutex mu1_;
+ Mutex mu;
+ LateFoo Foo;
+ LateFoo Foo2;
+ LateFoo *FooPointer;
+};
+
+LateBar b1, *b3;
+
+void late_0() {
+ LateFoo FooA;
+ LateFoo FooB;
+ FooA.mu.Lock();
+ FooA.a = 5;
+ FooA.mu.Unlock();
+}
+
+void late_1() {
+ LateBar BarA;
+ BarA.FooPointer->mu.Lock();
+ BarA.FooPointer->a = 2;
+ BarA.FooPointer->mu.Unlock();
+}
+
+void late_bad_0() {
+ LateFoo fooA;
+ LateFoo fooB;
+ fooA.mu.Lock();
+ fooB.a = 5; // \
+ // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}}
+ fooA.mu.Unlock();
+}
+
+void late_bad_1() {
+ Mutex mu;
+ mu.Lock();
+ b1.mu1_.Lock();
+ int res = b1.a_ + b3->b_;
+ b3->b_ = *b1.q; // \
+ // expected-warning{{reading the value pointed to by 'q' requires lock on 'mu' to be held}}
+ b1.mu1_.Unlock();
+ b1.b_ = res;
+ mu.Unlock();
+}
+
+void late_bad_2() {
+ LateBar BarA;
+ BarA.FooPointer->mu.Lock();
+ BarA.Foo.a = 2; // \
+ // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}}
+ BarA.FooPointer->mu.Unlock();
+}
+
+void late_bad_3() {
+ LateBar BarA;
+ BarA.Foo.mu.Lock();
+ BarA.FooPointer->a = 2; // \
+ // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}}
+ BarA.Foo.mu.Unlock();
+}
+
+void late_bad_4() {
+ LateBar BarA;
+ BarA.Foo.mu.Lock();
+ BarA.Foo2.a = 2; // \
+ // expected-warning{{writing variable 'a' requires lock on 'mu' to be held exclusively}}
+ BarA.Foo.mu.Unlock();
+}
+
//-----------------------------------------------//
// Extra warnings for shared vs. exclusive locks
// ----------------------------------------------//
generated by cgit v1.2.3-18-g5258 (git 2.32.0) at 2025-08-04 15:03:02 +0000