Skip to content

Commit

Permalink
Reland "[clang analysis][NFCI] Preparatory work for D153131. (#67420)… (
Browse files Browse the repository at this point in the history 
#67775)

…" (#67523)

Discussion in https://reviews.llvm.org/D153132.

This reverts commit f703774.
  • Loading branch information
@legrosbuffle
legrosbuffle committed Sep 29, 2023
1 parent b454b04 commit a0ea5a4
Showing 1 changed file with 70 additions and 59 deletions.
129 changes: 70 additions & 59 deletions clang/lib/Analysis/ThreadSafety.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1571,86 +1583,82 @@ 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;
}

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);
}
}

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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());
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit a0ea5a4

Please sign in to comment.