aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--include/clang/Basic/DiagnosticSemaKinds.td2
-rw-r--r--lib/Analysis/ThreadSafety.cpp90
-rw-r--r--lib/Sema/AnalysisBasedWarnings.cpp11
-rw-r--r--test/SemaCXX/warn-thread-safety-analysis.cpp36
4 files changed, 97 insertions, 42 deletions
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 38ba954d35..41581ae49c 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1575,7 +1575,7 @@ def warn_fun_excludes_mutex : Warning<
"cannot call function '%0' while mutex '%1' is locked">,
InGroup<ThreadSafety>, DefaultIgnore;
def warn_cannot_resolve_lock : Warning<
- "cannot resolve lock expression to a specific lockable object">,
+ "cannot resolve lock expression">,
InGroup<ThreadSafety>, DefaultIgnore;
diff --git a/lib/Analysis/ThreadSafety.cpp b/lib/Analysis/ThreadSafety.cpp
index 5f03b5832b..5b813ff2ea 100644
--- a/lib/Analysis/ThreadSafety.cpp
+++ b/lib/Analysis/ThreadSafety.cpp
@@ -164,6 +164,11 @@ class MutexID {
/// ensure that the original expression is a valid mutex expression.
void buildMutexID(Expr *Exp, Expr *Parent, int NumArgs,
const NamedDecl **FunArgDecls, Expr **FunArgs) {
+ if (!Exp) {
+ DeclSeq.clear();
+ return;
+ }
+
if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp)) {
if (FunArgDecls) {
// Substitute call arguments for references to function parameters
@@ -204,6 +209,7 @@ class MutexID {
Expr **FunArgs = 0;
SmallVector<const NamedDecl*, 8> FunArgDecls;
+ // If we are processing a raw attribute expression, with no substitutions.
if (DeclExp == 0) {
buildMutexID(MutexExp, 0, 0, 0, 0);
return;
@@ -254,6 +260,18 @@ public:
return !DeclSeq.empty();
}
+ /// Issue a warning about an invalid lock expression
+ static void warnInvalidLock(ThreadSafetyHandler &Handler, Expr* MutexExp,
+ Expr *DeclExp, const NamedDecl* D) {
+ SourceLocation Loc;
+ if (DeclExp)
+ Loc = DeclExp->getExprLoc();
+
+ // FIXME: add a note about the attribute location in MutexExp or D
+ if (Loc.isValid())
+ Handler.handleInvalidLockExp(Loc);
+ }
+
bool operator==(const MutexID &other) const {
return DeclSeq == other.DeclSeq;
}
@@ -333,6 +351,8 @@ typedef llvm::ImmutableMap<MutexID, LockData> Lockset;
/// output error messages related to missing locks.
/// FIXME: In future, we may be able to not inherit from a visitor.
class BuildLockset : public StmtVisitor<BuildLockset> {
+ friend class ThreadSafetyAnalyzer;
+
ThreadSafetyHandler &Handler;
Lockset LSet;
Lockset::Factory &LocksetFactory;
@@ -343,6 +363,8 @@ class BuildLockset : public StmtVisitor<BuildLockset> {
template <class AttrType>
void addLocksToSet(LockKind LK, AttrType *Attr, Expr *Exp, NamedDecl *D);
+ void removeLocksFromSet(UnlockFunctionAttr *Attr,
+ Expr *Exp, NamedDecl* FunDecl);
const ValueDecl *getValueDecl(Expr *Exp);
void warnIfMutexNotHeld (const NamedDecl *D, Expr *Exp, AccessKind AK,
@@ -407,7 +429,8 @@ void BuildLockset::addLock(SourceLocation LockLoc, MutexID &Mutex,
// FIXME: Don't always warn when we have support for reentrant locks.
if (locksetContains(Mutex))
Handler.handleDoubleLock(Mutex.getName(), LockLoc);
- LSet = LocksetFactory.add(LSet, Mutex, NewLock);
+ else
+ LSet = LocksetFactory.add(LSet, Mutex, NewLock);
}
/// \brief Remove a lock from the lockset, warning if the lock is not there.
@@ -417,8 +440,8 @@ void BuildLockset::removeLock(SourceLocation UnlockLoc, MutexID &Mutex) {
Lockset NewLSet = LocksetFactory.remove(LSet, Mutex);
if(NewLSet == LSet)
Handler.handleUnmatchedUnlock(Mutex.getName(), UnlockLoc);
-
- LSet = NewLSet;
+ else
+ LSet = NewLSet;
}
/// \brief This function, parameterized by an attribute type, is used to add a
@@ -432,10 +455,9 @@ void BuildLockset::addLocksToSet(LockKind LK, AttrType *Attr,
if (Attr->args_size() == 0) {
// The mutex held is the "this" object.
-
MutexID Mutex(0, Exp, FunDecl);
if (!Mutex.isValid())
- Handler.handleInvalidLockExp(Exp->getExprLoc());
+ MutexID::warnInvalidLock(Handler, 0, Exp, FunDecl);
else
addLock(ExpLocation, Mutex, LK);
return;
@@ -444,12 +466,39 @@ void BuildLockset::addLocksToSet(LockKind LK, AttrType *Attr,
for (iterator_type I=Attr->args_begin(), E=Attr->args_end(); I != E; ++I) {
MutexID Mutex(*I, Exp, FunDecl);
if (!Mutex.isValid())
- Handler.handleInvalidLockExp(Exp->getExprLoc());
+ MutexID::warnInvalidLock(Handler, *I, Exp, FunDecl);
else
addLock(ExpLocation, Mutex, LK);
}
}
+/// \brief This function removes a set of locks specified as attribute
+/// arguments from the lockset.
+void BuildLockset::removeLocksFromSet(UnlockFunctionAttr *Attr,
+ Expr *Exp, NamedDecl* FunDecl) {
+ SourceLocation ExpLocation;
+ if (Exp) ExpLocation = Exp->getExprLoc();
+
+ if (Attr->args_size() == 0) {
+ // The mutex held is the "this" object.
+ MutexID Mu(0, Exp, FunDecl);
+ if (!Mu.isValid())
+ MutexID::warnInvalidLock(Handler, 0, Exp, FunDecl);
+ else
+ removeLock(ExpLocation, Mu);
+ return;
+ }
+
+ for (UnlockFunctionAttr::args_iterator I = Attr->args_begin(),
+ E = Attr->args_end(); I != E; ++I) {
+ MutexID Mutex(*I, Exp, FunDecl);
+ if (!Mutex.isValid())
+ MutexID::warnInvalidLock(Handler, *I, Exp, FunDecl);
+ else
+ removeLock(ExpLocation, Mutex);
+ }
+}
+
/// \brief Gets the value decl pointer from DeclRefExprs or MemberExprs
const ValueDecl *BuildLockset::getValueDecl(Expr *Exp) {
if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Exp))
@@ -470,7 +519,7 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, Expr *Exp,
MutexID Mutex(MutexExp, Exp, D);
if (!Mutex.isValid())
- Handler.handleInvalidLockExp(MutexExp->getExprLoc());
+ MutexID::warnInvalidLock(Handler, MutexExp, Exp, D);
else if (!locksetContainsAtLeast(Mutex, LK))
Handler.handleMutexNotHeld(D, POK, Mutex.getName(), LK, Exp->getExprLoc());
}
@@ -533,8 +582,6 @@ void BuildLockset::checkAccess(Expr *Exp, AccessKind AK) {
/// FIXME: Do not flag an error for member variables accessed in constructors/
/// destructors
void BuildLockset::handleCall(Expr *Exp, NamedDecl *D) {
- SourceLocation ExpLocation = Exp->getExprLoc();
-
AttrVec &ArgAttrs = D->getAttrs();
for(unsigned i = 0; i < ArgAttrs.size(); ++i) {
Attr *Attr = ArgAttrs[i];
@@ -559,24 +606,7 @@ void BuildLockset::handleCall(Expr *Exp, NamedDecl *D) {
// mutexes from the lockset, and flag a warning if they are not there.
case attr::UnlockFunction: {
UnlockFunctionAttr *UFAttr = cast<UnlockFunctionAttr>(Attr);
-
- if (UFAttr->args_size() == 0) { // The lock held is the "this" object.
- MutexID Mu(0, Exp, D);
- if (!Mu.isValid())
- Handler.handleInvalidLockExp(Exp->getExprLoc());
- else
- removeLock(ExpLocation, Mu);
- break;
- }
-
- for (UnlockFunctionAttr::args_iterator I = UFAttr->args_begin(),
- E = UFAttr->args_end(); I != E; ++I) {
- MutexID Mutex(*I, Exp, D);
- if (!Mutex.isValid())
- Handler.handleInvalidLockExp(Exp->getExprLoc());
- else
- removeLock(ExpLocation, Mutex);
- }
+ removeLocksFromSet(UFAttr, Exp, D);
break;
}
@@ -605,10 +635,10 @@ void BuildLockset::handleCall(Expr *Exp, NamedDecl *D) {
E = LEAttr->args_end(); I != E; ++I) {
MutexID Mutex(*I, Exp, D);
if (!Mutex.isValid())
- Handler.handleInvalidLockExp((*I)->getExprLoc());
+ MutexID::warnInvalidLock(Handler, *I, Exp, D);
else if (locksetContains(Mutex))
Handler.handleFunExcludesLock(D->getName(), Mutex.getName(),
- ExpLocation);
+ Exp->getExprLoc());
}
break;
}
@@ -741,7 +771,7 @@ Lockset ThreadSafetyAnalyzer::addLock(Lockset &LSet, Expr *MutexExp,
LockKind LK, SourceLocation Loc) {
MutexID Mutex(MutexExp, 0, D);
if (!Mutex.isValid()) {
- Handler.handleInvalidLockExp(MutexExp->getExprLoc());
+ MutexID::warnInvalidLock(Handler, MutexExp, 0, D);
return LSet;
}
LockData NewLock(Loc, LK);
diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp
index a8ee0b8617..bd34dece6e 100644
--- a/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/lib/Sema/AnalysisBasedWarnings.cpp
@@ -648,15 +648,21 @@ struct SortDiagBySourceLocation {
class ThreadSafetyReporter : public clang::thread_safety::ThreadSafetyHandler {
Sema &S;
DiagList Warnings;
+ SourceLocation FunLocation;
// Helper functions
void warnLockMismatch(unsigned DiagID, Name LockName, SourceLocation Loc) {
+ // Gracefully handle rare cases when the analysis can't get a more
+ // precise source location.
+ if (!Loc.isValid())
+ Loc = FunLocation;
PartialDiagnostic Warning = S.PDiag(DiagID) << LockName;
Warnings.push_back(DelayedDiag(Loc, Warning));
}
public:
- ThreadSafetyReporter(Sema &S) : S(S) {}
+ ThreadSafetyReporter(Sema &S, SourceLocation FL)
+ : S(S), FunLocation(FL) {}
/// \brief Emit all buffered diagnostics in order of sourcelocation.
/// We need to output diagnostics produced while iterating through
@@ -913,7 +919,8 @@ AnalysisBasedWarnings::IssueWarnings(sema::AnalysisBasedWarnings::Policy P,
// Check for thread safety violations
if (P.enableThreadSafetyAnalysis) {
- thread_safety::ThreadSafetyReporter Reporter(S);
+ SourceLocation FL = AC.getDecl()->getLocation();
+ thread_safety::ThreadSafetyReporter Reporter(S, FL);
thread_safety::runThreadSafetyAnalysis(AC, Reporter);
Reporter.emitDiagnostics();
}
diff --git a/test/SemaCXX/warn-thread-safety-analysis.cpp b/test/SemaCXX/warn-thread-safety-analysis.cpp
index 06c013887a..14ef6d1c04 100644
--- a/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ b/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -749,23 +749,22 @@ Mutex UPmu;
// FIXME: add support for lock expressions involving arrays.
Mutex mua[5];
-int x __attribute__((guarded_by(UPmu = sls_mu))); // \
- // expected-warning{{cannot resolve lock expression to a specific lockable object}}
-int y __attribute__((guarded_by(mua[0]))); // \
- // expected-warning{{cannot resolve lock expression to a specific lockable object}}
+int x __attribute__((guarded_by(UPmu = sls_mu)));
+int y __attribute__((guarded_by(mua[0])));
void testUnparse() {
- // no errors, since the lock expressions are not resolved
- x = 5;
- y = 5;
+ x = 5; // \
+ // expected-warning{{cannot resolve lock expression}}
+ y = 5; // \
+ // expected-warning{{cannot resolve lock expression}}
}
void testUnparse2() {
mua[0].Lock(); // \
- // expected-warning{{cannot resolve lock expression to a specific lockable object}}
+ // expected-warning{{cannot resolve lock expression}}
(&(mua[0]) + 4)->Lock(); // \
- // expected-warning{{cannot resolve lock expression to a specific lockable object}}
+ // expected-warning{{cannot resolve lock expression}}
}
@@ -1476,6 +1475,7 @@ namespace substitution_test {
} // end namespace substituation_test
+
namespace constructor_destructor_tests {
Mutex fooMu;
int myVar GUARDED_BY(fooMu);
@@ -1494,3 +1494,21 @@ namespace constructor_destructor_tests {
}
}
+
+namespace invalid_lock_expression_test {
+
+class LOCKABLE MyLockable {
+public:
+ MyLockable() __attribute__((exclusive_lock_function)) { }
+ ~MyLockable() __attribute__((unlock_function)) { }
+};
+
+// create an empty lock expression
+void foo() {
+ MyLockable lock; // \
+ // expected-warning {{cannot resolve lock expression}}
+}
+
+} // end namespace invalid_lock_expression_test
+
+
generated by cgit v1.2.3-18-g5258 (git 2.32.0) at 2025-08-02 14:26:05 +0000