diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp index 3e6ceb7d54c42..58dd7113665b1 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 { 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 { /// 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(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(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(Exp)) { - checkPtAccess(AE->getLHS(), AK, POK); + checkPtAccess(FSet, AE->getLHS(), AK, POK); return; } if (const auto *ME = dyn_cast(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() && FSet.isEmpty(Analyzer->FactMan)) { - Analyzer->Handler.handleNoMutexHeld(D, POK, AK, Loc); + if (D->hasAttr() && FSet.isEmpty(FactMan)) { + Handler.handleNoMutexHeld(D, POK, AK, Loc); } for (const auto *I : D->specific_attrs()) - 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(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() && FSet.isEmpty(Analyzer->FactMan)) - Analyzer->Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc()); + if (D->hasAttr() && FSet.isEmpty(FactMan)) + Handler.handleNoMutexHeld(D, PtPOK, AK, Exp->getExprLoc()); for (auto const *I : D->specific_attrs()) - 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(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(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);