Skip to content

[analyzer][NFCi] Pass if bind is to a Decl or not to checkBind #152137

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.

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 7 commits into from
Aug 8, 2025
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 2 additions & 2 deletions clang/include/clang/StaticAnalyzer/Core/Checker.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ class Location {
class Bind {
template <typename CHECKER>
static void _checkBind(void *checker, SVal location, SVal val, const Stmt *S,
CheckerContext &C) {
((const CHECKER *)checker)->checkBind(location, val, S, C);
bool AtDeclInit, CheckerContext &C) {
((const CHECKER *)checker)->checkBind(location, val, S, AtDeclInit, C);
}

public:
Expand Down
11 changes: 5 additions & 6 deletions clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,9 @@ class CheckerManager {
ExprEngine &Eng);

/// Run checkers for binding of a value to a location.
void runCheckersForBind(ExplodedNodeSet &Dst,
const ExplodedNodeSet &Src,
SVal location, SVal val,
const Stmt *S, ExprEngine &Eng,
void runCheckersForBind(ExplodedNodeSet &Dst, const ExplodedNodeSet &Src,
SVal location, SVal val, const Stmt *S,
bool AtDeclInit, ExprEngine &Eng,
const ProgramPoint &PP);

/// Run checkers after taking a control flow edge.
Expand Down Expand Up @@ -499,8 +498,8 @@ class CheckerManager {
using CheckLocationFunc = CheckerFn<void(SVal location, bool isLoad,
const Stmt *S, CheckerContext &)>;

using CheckBindFunc =
CheckerFn<void(SVal location, SVal val, const Stmt *S, CheckerContext &)>;
using CheckBindFunc = CheckerFn<void(SVal location, SVal val, const Stmt *S,
bool AtDeclInit, CheckerContext &)>;

using CheckBlockEntranceFunc =
CheckerFn<void(const BlockEntrance &, CheckerContext &)>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ class ExprEngine {
/// evalBind - Handle the semantics of binding a value to a specific location.
/// This method is used by evalStore, VisitDeclStmt, and others.
void evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE, ExplodedNode *Pred,
SVal location, SVal Val, bool atDeclInit = false,
SVal location, SVal Val, bool AtDeclInit = false,
const ProgramPoint *PP = nullptr);

ProgramStateRef
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ class AnalysisOrderChecker
llvm::errs() << "NewAllocator\n";
}

void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const {
void checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const {
if (isCallbackEnabled(C, "Bind"))
llvm::errs() << "Bind\n";
}
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ class BoolAssignmentChecker : public Checker<check::Bind> {
bool IsTainted = false) const;

public:
void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const;
void checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;
};
} // end anonymous namespace

Expand All @@ -55,6 +56,7 @@ static bool isBooleanType(QualType Ty) {
}

void BoolAssignmentChecker::checkBind(SVal Loc, SVal Val, const Stmt *S,
bool AtDeclInit,
CheckerContext &C) const {

// We are only interested in stores into Booleans.
Expand Down
5 changes: 4 additions & 1 deletion clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,12 @@ class CheckerDocumentation
/// \param Loc The value of the location (pointer).
/// \param Val The value which will be stored at the location Loc.
/// \param S The bind is performed while processing the statement S.
/// \param AtDeclInit Whether the bind is performed during declaration
/// initialization.
///
/// check::Bind
void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &) const {}
void checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit,
CheckerContext &) const {}

/// Called after a CFG edge is taken within a function.
///
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ class DereferenceChecker
public:
void checkLocation(SVal location, bool isLoad, const Stmt* S,
CheckerContext &C) const;
void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
void checkBind(SVal L, SVal V, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;

static void AddDerefSource(raw_ostream &os,
SmallVectorImpl<SourceRange> &Ranges,
Expand Down Expand Up @@ -309,7 +310,7 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S,
}

void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
CheckerContext &C) const {
bool AtDeclInit, CheckerContext &C) const {
// If we're binding to a reference, check if the value is known to be null.
if (V.isUndef())
return;
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ class IteratorModeling
IteratorModeling() = default;

void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const;
void checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;
void checkPostStmt(const UnaryOperator *UO, CheckerContext &C) const;
void checkPostStmt(const BinaryOperator *BO, CheckerContext &C) const;
void checkPostStmt(const MaterializeTemporaryExpr *MTE,
Expand Down Expand Up @@ -234,7 +235,7 @@ void IteratorModeling::checkPostCall(const CallEvent &Call,
}

void IteratorModeling::checkBind(SVal Loc, SVal Val, const Stmt *S,
CheckerContext &C) const {
bool AtDeclInit, CheckerContext &C) const {
auto State = C.getState();
const auto *Pos = getIteratorPosition(State, Val);
if (Pos) {
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ class NullabilityChecker
// libraries.
bool NoDiagnoseCallsToSystemHeaders = false;

void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
void checkBind(SVal L, SVal V, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;
void checkPostStmt(const ExplicitCastExpr *CE, CheckerContext &C) const;
void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
void checkPostObjCMessage(const ObjCMethodCall &M, CheckerContext &C) const;
Expand Down Expand Up @@ -1250,7 +1251,7 @@ static bool isARCNilInitializedLocal(CheckerContext &C, const Stmt *S) {
/// Propagate the nullability information through binds and warn when nullable
/// pointer or null symbol is assigned to a pointer with a nonnull type.
void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
CheckerContext &C) const {
bool AtDeclInit, CheckerContext &C) const {
const TypedValueRegion *TVR =
dyn_cast_or_null<TypedValueRegion>(L.getAsRegion());
if (!TVR)
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ class ObjCSelfInitChecker : public Checker< check::PostObjCMessage,
void checkPreStmt(const ReturnStmt *S, CheckerContext &C) const;
void checkLocation(SVal location, bool isLoad, const Stmt *S,
CheckerContext &C) const;
void checkBind(SVal loc, SVal val, const Stmt *S, CheckerContext &C) const;
void checkBind(SVal loc, SVal val, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;

void checkPreCall(const CallEvent &CE, CheckerContext &C) const;
void checkPostCall(const CallEvent &CE, CheckerContext &C) const;
Expand Down Expand Up @@ -311,9 +312,8 @@ void ObjCSelfInitChecker::checkLocation(SVal location, bool isLoad,
C);
}


void ObjCSelfInitChecker::checkBind(SVal loc, SVal val, const Stmt *S,
CheckerContext &C) const {
bool AtDeclInit, CheckerContext &C) const {
// Allow assignment of anything to self. Self is a local variable in the
// initializer, so it is legal to assign anything to it, like results of
// static functions/method calls. After self is assigned something we cannot
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,7 @@ ExplodedNode * RetainCountChecker::checkReturnWithRetEffect(const ReturnStmt *S,
//===----------------------------------------------------------------------===//

void RetainCountChecker::checkBind(SVal loc, SVal val, const Stmt *S,
CheckerContext &C) const {
bool AtDeclInit, CheckerContext &C) const {
ProgramStateRef state = C.getState();
const MemRegion *MR = loc.getAsRegion();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ class RetainCountChecker
void printState(raw_ostream &Out, ProgramStateRef State,
const char *NL, const char *Sep) const override;

void checkBind(SVal loc, SVal val, const Stmt *S, CheckerContext &C) const;
void checkBind(SVal loc, SVal val, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;
void checkPostStmt(const BlockExpr *BE, CheckerContext &C) const;
void checkPostStmt(const CastExpr *CE, CheckerContext &C) const;

Expand Down
51 changes: 4 additions & 47 deletions clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,53 +26,11 @@ class StoreToImmutableChecker : public Checker<check::Bind> {
const BugType BT{this, "Write to immutable memory", "CERT Environment (ENV)"};

public:
void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const;
void checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;
};
} // end anonymous namespace

static bool isInitializationContext(const Stmt *S, CheckerContext &C) {
// Check if this is a DeclStmt (variable declaration)
if (isa<DeclStmt>(S))
return true;

// This part is specific for initialization of const lambdas pre-C++17.
// Lets look at the AST of the statement:
// ```
// const auto lambda = [](){};
// ```
//
// The relevant part of the AST for this case prior to C++17 is:
// ...
// `-DeclStmt
// `-VarDecl
// `-ExprWithCleanups
// `-CXXConstructExpr
// ...
// In C++17 and later, the AST is different:
// ...
// `-DeclStmt
// `-VarDecl
// `-ImplicitCastExpr
// `-LambdaExpr
// |-CXXRecordDecl
// `-CXXConstructExpr
// ...
// And even beside this, the statement `S` that is given to the checkBind
// callback is the VarDecl in C++17 and later, and the CXXConstructExpr in
// C++14 and before. So in order to support the C++14 we need the following
// ugly hack to detect whether this construction is used to initialize a
// variable.
//
// FIXME: This should be eliminated by improving the API of checkBind to
// ensure that it consistently passes the `VarDecl` (instead of the
// `CXXConstructExpr`) when the constructor call denotes the initialization
// of a variable with a lambda, or maybe less preferably, try the more
// invasive approach of passing the information forward to the checkers
// whether the current bind is an initialization or an assignment.
const auto *ConstructExp = dyn_cast<CXXConstructExpr>(S);
return ConstructExp && ConstructExp->isElidable();
}

static bool isEffectivelyConstRegion(const MemRegion *MR, CheckerContext &C) {
if (isa<GlobalImmutableSpaceRegion>(MR))
return true;
Expand Down Expand Up @@ -128,6 +86,7 @@ getInnermostEnclosingConstDeclRegion(const MemRegion *MR, CheckerContext &C) {
}

void StoreToImmutableChecker::checkBind(SVal Loc, SVal Val, const Stmt *S,
bool AtDeclInit,
CheckerContext &C) const {
// We are only interested in stores to memory regions
const MemRegion *MR = Loc.getAsRegion();
Expand All @@ -136,9 +95,7 @@ void StoreToImmutableChecker::checkBind(SVal Loc, SVal Val, const Stmt *S,

// Skip variable declarations and initializations - we only want to catch
// actual writes
// FIXME: If the API of checkBind would allow to distinguish between
// initialization and assignment, we could use that instead.
if (isInitializationContext(S, C))
if (AtDeclInit)
return;

// Check if the region is in the global immutable space
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ class UndefinedAssignmentChecker
const BugType BT{this, "Assigned value is uninitialized"};

public:
void checkBind(SVal location, SVal val, const Stmt *S,
void checkBind(SVal location, SVal val, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;
};
}

void UndefinedAssignmentChecker::checkBind(SVal location, SVal val,
const Stmt *StoreE,
const Stmt *StoreE, bool AtDeclInit,
CheckerContext &C) const {
if (!val.isUndef())
return;
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ class VforkChecker : public Checker<check::PreCall, check::PostCall,

void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
void checkBind(SVal L, SVal V, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const;
void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
};

Expand Down Expand Up @@ -188,7 +189,7 @@ void VforkChecker::checkPreCall(const CallEvent &Call,
}

// Prohibit writes in child process (except for vfork's lhs).
void VforkChecker::checkBind(SVal L, SVal V, const Stmt *S,
void VforkChecker::checkBind(SVal L, SVal V, const Stmt *S, bool AtDeclInit,
CheckerContext &C) const {
ProgramStateRef State = C.getState();
if (!isChildProcess(State))
Expand Down
16 changes: 9 additions & 7 deletions clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,11 +376,13 @@ namespace {
const Stmt *S;
ExprEngine &Eng;
const ProgramPoint &PP;
bool AtDeclInit;

CheckBindContext(const CheckersTy &checkers,
SVal loc, SVal val, const Stmt *s, ExprEngine &eng,
CheckBindContext(const CheckersTy &checkers, SVal loc, SVal val,
const Stmt *s, bool AtDeclInit, ExprEngine &eng,
const ProgramPoint &pp)
: Checkers(checkers), Loc(loc), Val(val), S(s), Eng(eng), PP(pp) {}
: Checkers(checkers), Loc(loc), Val(val), S(s), Eng(eng), PP(pp),
AtDeclInit(AtDeclInit) {}

CheckersTy::const_iterator checkers_begin() { return Checkers.begin(); }
CheckersTy::const_iterator checkers_end() { return Checkers.end(); }
Expand All @@ -391,7 +393,7 @@ namespace {
const ProgramPoint &L = PP.withTag(checkFn.Checker);
CheckerContext C(Bldr, Eng, Pred, L);

checkFn(Loc, Val, S, C);
checkFn(Loc, Val, S, AtDeclInit, C);
}
};

Expand All @@ -408,10 +410,10 @@ namespace {
/// Run checkers for binding of a value to a location.
void CheckerManager::runCheckersForBind(ExplodedNodeSet &Dst,
const ExplodedNodeSet &Src,
SVal location, SVal val,
const Stmt *S, ExprEngine &Eng,
SVal location, SVal val, const Stmt *S,
bool AtDeclInit, ExprEngine &Eng,
const ProgramPoint &PP) {
CheckBindContext C(BindCheckers, location, val, S, Eng, PP);
CheckBindContext C(BindCheckers, location, val, S, AtDeclInit, Eng, PP);
llvm::TimeTraceScope TimeScope{
"CheckerManager::runCheckersForBind",
[&val]() { return getTimeTraceBindMetadata(val); }};
Expand Down
11 changes: 5 additions & 6 deletions clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3714,9 +3714,8 @@ ExprEngine::notifyCheckersOfPointerEscape(ProgramStateRef State,
/// evalBind - Handle the semantics of binding a value to a specific location.
/// This method is used by evalStore and (soon) VisitDeclStmt, and others.
void ExprEngine::evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE,
ExplodedNode *Pred,
SVal location, SVal Val,
bool atDeclInit, const ProgramPoint *PP) {
ExplodedNode *Pred, SVal location, SVal Val,
bool AtDeclInit, const ProgramPoint *PP) {
const LocationContext *LC = Pred->getLocationContext();
PostStmt PS(StoreE, LC);
if (!PP)
Expand All @@ -3725,7 +3724,7 @@ void ExprEngine::evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE,
// Do a previsit of the bind.
ExplodedNodeSet CheckedSet;
getCheckerManager().runCheckersForBind(CheckedSet, Pred, location, Val,
StoreE, *this, *PP);
StoreE, AtDeclInit, *this, *PP);

StmtNodeBuilder Bldr(CheckedSet, Dst, *currBldrCtx);

Expand All @@ -3748,8 +3747,8 @@ void ExprEngine::evalBind(ExplodedNodeSet &Dst, const Stmt *StoreE,
// When binding the value, pass on the hint that this is a initialization.
// For initializations, we do not need to inform clients of region
// changes.
state = state->bindLoc(location.castAs<Loc>(),
Val, LC, /* notifyChanges = */ !atDeclInit);
state = state->bindLoc(location.castAs<Loc>(), Val, LC,
/* notifyChanges = */ !AtDeclInit);

const MemRegion *LocReg = nullptr;
if (std::optional<loc::MemRegionVal> LocRegVal =
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V,
/*isLoad=*/true);
for (ExplodedNode *N : Tmp)
evalBind(Dst, CallExpr, N, ThisVal, V, true);
evalBind(Dst, CallExpr, N, ThisVal, V, !AlwaysReturnsLValue);

PostStmt PS(CallExpr, LCtx);
for (ExplodedNode *N : Dst) {
Expand Down
Loading