Reland "[clang analysis][NFCI] Preparatory work for D153131. (#67420)… #67775
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
)" (llvm#67523) Discussion in https://reviews.llvm.org/D153132. This reverts commit f703774.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Changes…" (#67523) Discussion in https://reviews.llvm.org/D153132. This reverts commit f703774. Full diff: https://github.com/llvm/llvm-project/pull/67775.diff 1 Files Affected:
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 3e6ceb7d54c427a..58dd7113665b132 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1055,6 +1055,19 @@ class ThreadSafetyAnalyzer {
}
void runAnalysis(AnalysisDeclContext &AC);
+
+ void warnIfMutexNotHeld(const FactSet &FSet, const NamedDecl *D,
+ const Expr *Exp, AccessKind AK, Expr *MutexExp,
+ ProtectedOperationKind POK, til::LiteralPtr *Self,
+ SourceLocation Loc);
+ void warnIfMutexHeld(const FactSet &FSet, const NamedDecl *D, const Expr *Exp,
+ Expr *MutexExp, til::LiteralPtr *Self,
+ SourceLocation Loc);
+
+ void checkAccess(const FactSet &FSet, const Expr *Exp, AccessKind AK,
+ ProtectedOperationKind POK);
+ void checkPtAccess(const FactSet &FSet, const Expr *Exp, AccessKind AK,
+ ProtectedOperationKind POK);
};
} // namespace
@@ -1534,16 +1547,15 @@ class BuildLockset : public ConstStmtVisitor<BuildLockset> {
unsigned CtxIndex;
// helper functions
- void warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp, AccessKind AK,
- Expr *MutexExp, ProtectedOperationKind POK,
- til::LiteralPtr *Self, SourceLocation Loc);
- void warnIfMutexHeld(const NamedDecl *D, const Expr *Exp, Expr *MutexExp,
- til::LiteralPtr *Self, SourceLocation Loc);
void checkAccess(const Expr *Exp, AccessKind AK,
- ProtectedOperationKind POK = POK_VarAccess);
+ ProtectedOperationKind POK = POK_VarAccess) {
+ Analyzer->checkAccess(FSet, Exp, AK, POK);
+ }
void checkPtAccess(const Expr *Exp, AccessKind AK,
- ProtectedOperationKind POK = POK_VarAccess);
+ ProtectedOperationKind POK = POK_VarAccess) {
+ Analyzer->checkPtAccess(FSet, Exp, AK, POK);
+ }
void handleCall(const Expr *Exp, const NamedDecl *D,
til::LiteralPtr *Self = nullptr,
@@ -1571,17 +1583,14 @@ class BuildLockset : public ConstStmtVisitor<BuildLockset> {
/// Warn if the LSet does not contain a lock sufficient to protect access
/// of at least the passed in AccessKind.
-void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
- AccessKind AK, Expr *MutexExp,
- ProtectedOperationKind POK,
- til::LiteralPtr *Self,
- SourceLocation Loc) {
+void ThreadSafetyAnalyzer::warnIfMutexNotHeld(
+ const FactSet &FSet, const NamedDecl *D, const Expr *Exp, AccessKind AK,
+ Expr *MutexExp, ProtectedOperationKind POK, til::LiteralPtr *Self,
+ SourceLocation Loc) {
LockKind LK = getLockKindFromAccessKind(AK);
-
- CapabilityExpr Cp =
- Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp, Self);
+ CapabilityExpr Cp = SxBuilder.translateAttrExpr(MutexExp, D, Exp, Self);
if (Cp.isInvalid()) {
- warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind());
+ warnInvalidLock(Handler, MutexExp, D, Exp, Cp.getKind());
return;
} else if (Cp.shouldIgnore()) {
return;
@@ -1589,68 +1598,67 @@ void BuildLockset::warnIfMutexNotHeld(const NamedDecl *D, const Expr *Exp,
if (Cp.negative()) {
// Negative capabilities act like locks excluded
- const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, !Cp);
+ const FactEntry *LDat = FSet.findLock(FactMan, !Cp);
if (LDat) {
- Analyzer->Handler.handleFunExcludesLock(
- Cp.getKind(), D->getNameAsString(), (!Cp).toString(), Loc);
+ Handler.handleFunExcludesLock(Cp.getKind(), D->getNameAsString(),
+ (!Cp).toString(), Loc);
return;
}
// If this does not refer to a negative capability in the same class,
// then stop here.
- if (!Analyzer->inCurrentScope(Cp))
+ if (!inCurrentScope(Cp))
return;
// Otherwise the negative requirement must be propagated to the caller.
- LDat = FSet.findLock(Analyzer->FactMan, Cp);
+ LDat = FSet.findLock(FactMan, Cp);
if (!LDat) {
- Analyzer->Handler.handleNegativeNotHeld(D, Cp.toString(), Loc);
+ Handler.handleNegativeNotHeld(D, Cp.toString(), Loc);
}
return;
}
- const FactEntry *LDat = FSet.findLockUniv(Analyzer->FactMan, Cp);
+ const FactEntry *LDat = FSet.findLockUniv(FactMan, Cp);
bool NoError = true;
if (!LDat) {
// No exact match found. Look for a partial match.
- LDat = FSet.findPartialMatch(Analyzer->FactMan, Cp);
+ LDat = FSet.findPartialMatch(FactMan, Cp);
if (LDat) {
// Warn that there's no precise match.
std::string PartMatchStr = LDat->toString();
StringRef PartMatchName(PartMatchStr);
- Analyzer->Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(),
- LK, Loc, &PartMatchName);
+ Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(), LK, Loc,
+ &PartMatchName);
} else {
// Warn that there's no match at all.
- Analyzer->Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(),
- LK, Loc);
+ Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(), LK, Loc);
}
NoError = false;
}
// Make sure the mutex we found is the right kind.
if (NoError && LDat && !LDat->isAtLeast(LK)) {
- Analyzer->Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(),
- LK, Loc);
+ Handler.handleMutexNotHeld(Cp.getKind(), D, POK, Cp.toString(), LK, Loc);
}
}
/// Warn if the LSet contains the given lock.
-void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp,
- Expr *MutexExp, til::LiteralPtr *Self,
- SourceLocation Loc) {
- CapabilityExpr Cp =
- Analyzer->SxBuilder.translateAttrExpr(MutexExp, D, Exp, Self);
+void ThreadSafetyAnalyzer::warnIfMutexHeld(const FactSet &FSet,
+ const NamedDecl *D, const Expr *Exp,
+ Expr *MutexExp,
+ til::LiteralPtr *Self,
+ SourceLocation Loc) {
+ CapabilityExpr Cp = SxBuilder.translateAttrExpr(MutexExp, D, Exp, Self);
if (Cp.isInvalid()) {
- warnInvalidLock(Analyzer->Handler, MutexExp, D, Exp, Cp.getKind());
+ warnInvalidLock(Handler, MutexExp, D, Exp, Cp.getKind());
return;
} else if (Cp.shouldIgnore()) {
return;
}
- const FactEntry *LDat = FSet.findLock(Analyzer->FactMan, Cp);
+ const FactEntry *LDat = FSet.findLock(FactMan, Cp);
if (LDat) {
- Analyzer->Handler.handleFunExcludesLock(Cp.getKind(), D->getNameAsString(),
- Cp.toString(), Loc);
+ Handler.handleFunExcludesLock(Cp.getKind(), D->getNameAsString(),
+ Cp.toString(), Loc);
}
}
@@ -1659,8 +1667,9 @@ void BuildLockset::warnIfMutexHeld(const NamedDecl *D, const Expr *Exp,
/// marked with guarded_by, we must ensure the appropriate mutexes are held.
/// Similarly, we check if the access is to an expression that dereferences
/// a pointer marked with pt_guarded_by.
-void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK,
- ProtectedOperationKind POK) {
+void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp,
+ AccessKind AK,
+ ProtectedOperationKind POK) {
Exp = Exp->IgnoreImplicit()->IgnoreParenCasts();
SourceLocation Loc = Exp->getExprLoc();
@@ -1684,49 +1693,50 @@ void BuildLockset::checkAccess(const Expr *Exp, AccessKind AK,
if (const auto *UO = dyn_cast<UnaryOperator>(Exp)) {
// For dereferences
if (UO->getOpcode() == UO_Deref)
- checkPtAccess(UO->getSubExpr(), AK, POK);
+ checkPtAccess(FSet, UO->getSubExpr(), AK, POK);
return;
}
if (const auto *BO = dyn_cast<BinaryOperator>(Exp)) {
switch (BO->getOpcode()) {
case BO_PtrMemD: // .*
- return checkAccess(BO->getLHS(), AK, POK);
+ return checkAccess(FSet, BO->getLHS(), AK, POK);
case BO_PtrMemI: // ->*
- return checkPtAccess(BO->getLHS(), AK, POK);
+ return checkPtAccess(FSet, BO->getLHS(), AK, POK);
default:
return;
}
}
if (const auto *AE = dyn_cast<ArraySubscriptExpr>(Exp)) {
- checkPtAccess(AE->getLHS(), AK, POK);
+ checkPtAccess(FSet, AE->getLHS(), AK, POK);
return;
}
if (const auto *ME = dyn_cast<MemberExpr>(Exp)) {
if (ME->isArrow())
- checkPtAccess(ME->getBase(), AK, POK);
+ checkPtAccess(FSet, ME->getBase(), AK, POK);
else
- checkAccess(ME->getBase(), AK, POK);
+ checkAccess(FSet, ME->getBase(), AK, POK);
}
const ValueDecl *D = getValueDecl(Exp);
if (!D || !D->hasAttrs())
return;
- if (D->hasAttr<GuardedVarAttr>() && FSet.isEmpty(Analyzer->FactMan)) {
- Analyzer->Handler.handleNoMutexHeld(D, POK, AK, Loc);
+ if (D->hasAttr<GuardedVarAttr>() && FSet.isEmpty(FactMan)) {
+ Handler.handleNoMutexHeld(D, POK, AK, Loc);
}
for (const auto *I : D->specific_attrs<GuardedByAttr>())
- warnIfMutexNotHeld(D, Exp, AK, I->getArg(), POK, nullptr, Loc);
+ warnIfMutexNotHeld(FSet, D, Exp, AK, I->getArg(), POK, nullptr, Loc);
}
/// Checks pt_guarded_by and pt_guarded_var attributes.
/// POK is the same operationKind that was passed to checkAccess.
-void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK,
- ProtectedOperationKind POK) {
+void ThreadSafetyAnalyzer::checkPtAccess(const FactSet &FSet, const Expr *Exp,
+ AccessKind AK,
+ ProtectedOperationKind POK) {
while (true) {
if (const auto *PE = dyn_cast<ParenExpr>(Exp)) {
Exp = PE->getSubExpr();
@@ -1736,7 +1746,7 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK,
if (CE->getCastKind() == CK_ArrayToPointerDecay) {
// If it's an actual array, and not a pointer, then it's elements
// are protected by GUARDED_BY, not PT_GUARDED_BY;
- checkAccess(CE->getSubExpr(), AK, POK);
+ checkAccess(FSet, CE->getSubExpr(), AK, POK);
return;
}
Exp = CE->getSubExpr();
@@ -1753,11 +1763,11 @@ void BuildLockset::checkPtAccess(const Expr *Exp, AccessKind AK,
if (!D || !D->hasAttrs())
return;
- if (D->hasAttr<PtGuardedVarAttr>() && FSet.isEmpty(Analyzer->FactMan))
- Analyzer->Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc());
+ if (D->hasAttr<PtGuardedVarAttr>() && FSet.isEmpty(FactMan))
+ Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc());
for (auto const *I : D->specific_attrs<PtGuardedByAttr>())
- warnIfMutexNotHeld(D, Exp, AK, I->getArg(), PtPOK, nullptr,
+ warnIfMutexNotHeld(FSet, D, Exp, AK, I->getArg(), PtPOK, nullptr,
Exp->getExprLoc());
}
@@ -1869,8 +1879,9 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
case attr::RequiresCapability: {
const auto *A = cast<RequiresCapabilityAttr>(At);
for (auto *Arg : A->args()) {
- warnIfMutexNotHeld(D, Exp, A->isShared() ? AK_Read : AK_Written, Arg,
- POK_FunctionCall, Self, Loc);
+ Analyzer->warnIfMutexNotHeld(FSet, D, Exp,
+ A->isShared() ? AK_Read : AK_Written,
+ Arg, POK_FunctionCall, Self, Loc);
// use for adopting a lock
if (!Scp.shouldIgnore())
Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, Self);
@@ -1881,7 +1892,7 @@ void BuildLockset::handleCall(const Expr *Exp, const NamedDecl *D,
case attr::LocksExcluded: {
const auto *A = cast<LocksExcludedAttr>(At);
for (auto *Arg : A->args()) {
- warnIfMutexHeld(D, Exp, Arg, Self, Loc);
+ Analyzer->warnIfMutexHeld(FSet, D, Exp, Arg, Self, Loc);
// use for deferring a lock
if (!Scp.shouldIgnore())
Analyzer->getMutexIDs(ScopedReqsAndExcludes, A, Exp, D, Self);
|
legrosbuffle
added a commit
to legrosbuffle/llvm-project
that referenced
this pull request
Sep 29, 2023
)… (llvm#67775) …" (llvm#67523) Discussion in https://reviews.llvm.org/D153132. This reverts commit f703774.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…" (#67523)
Discussion in https://reviews.llvm.org/D153132.
This reverts commit f703774.