Skip to content

Reland "[clang analysis][NFCI] Preparatory work for D153131. (#67420)… #67775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Reland "[clang analysis][NFCI] Preparatory work for D153131. (#67420)… #67775

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 70 additions & 59 deletions clang/lib/Analysis/ThreadSafety.cpp
View file
Open in desktop
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