[clang] Reland "[clang analysis][NFCI] Preparatory work for D153131. (#67420)… (PR #67775)
via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 29 01:31:58 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
<details>
<summary>Changes</summary>
…" (#<!-- -->67523)
Discussion in https://reviews.llvm.org/D153132.
This reverts commit f70377471c990aa567584ae429e77adc9a55491b.
---
Full diff: https://github.com/llvm/llvm-project/pull/67775.diff
1 Files Affected:
- (modified) clang/lib/Analysis/ThreadSafety.cpp (+70-59)
``````````diff
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);
``````````
</details>
https://github.com/llvm/llvm-project/pull/67775
More information about the cfe-commits
mailing list