diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h b/clang/include/clang/StaticAnalyzer/Core/Checker.h index 31cc095c29bfe..d9a7c00254a00 100644 --- a/clang/include/clang/StaticAnalyzer/Core/Checker.h +++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h @@ -209,8 +209,8 @@ class Location { class Bind { template 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: diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h index c8e6f1265a3db..bf33ce616d954 100644 --- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h +++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h @@ -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. @@ -499,8 +498,8 @@ class CheckerManager { using CheckLocationFunc = CheckerFn; - using CheckBindFunc = - CheckerFn; + using CheckBindFunc = CheckerFn; using CheckBlockEntranceFunc = CheckerFn; diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h index fbb34340a5c67..2335588dbd27c 100644 --- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -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 diff --git a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp index 3b3def7ce3324..e64153d53bbd6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp @@ -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"; } diff --git a/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp index 837cbbce8f45f..921114a7a215f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp @@ -29,7 +29,8 @@ class BoolAssignmentChecker : public Checker { 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 @@ -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. diff --git a/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp b/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp index 350db4b1bc2bc..392c7eeea234a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp @@ -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. /// diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index 152129ea91fd4..395d724cdfd11 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -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 &Ranges, @@ -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; diff --git a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp index 7ad54c08d5c71..7eb9a1da90bb4 100644 --- a/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp @@ -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, @@ -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) { diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp index 9744d1abf7790..eeb6b720cce2a 100644 --- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp @@ -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; @@ -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(L.getAsRegion()); if (!TVR) diff --git a/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp index ace3426387568..e40b4f84d55ec 100644 --- a/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp @@ -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; @@ -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 diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp index 65ff902ef1755..1762505f58f0f 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp @@ -1136,7 +1136,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(); diff --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h index 8854e1062fc1d..dc8bad60944b4 100644 --- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h +++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h @@ -280,7 +280,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; diff --git a/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp index afad41939cdca..2bb391799d696 100644 --- a/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp @@ -26,53 +26,11 @@ class StoreToImmutableChecker : public Checker { 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(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(S); - return ConstructExp && ConstructExp->isElidable(); -} - static bool isEffectivelyConstRegion(const MemRegion *MR, CheckerContext &C) { if (isa(MR)) return true; @@ -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(); @@ -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 diff --git a/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp index e98de333f8dd3..7f8923c7c09fc 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp @@ -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; diff --git a/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp index cb73ac68edd1e..116dd93b2ecd9 100644 --- a/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp @@ -62,7 +62,8 @@ class VforkChecker : public CheckergetLocationContext(); PostStmt PS(StoreE, LC); if (!PP) @@ -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); @@ -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(), - Val, LC, /* notifyChanges = */ !atDeclInit); + state = state->bindLoc(location.castAs(), Val, LC, + /* notifyChanges = */ !AtDeclInit); const MemRegion *LocReg = nullptr; if (std::optional LocRegVal = diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index fe70558dfc45c..c0b28d2ebb212 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -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) { diff --git a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp index 12be2289c3174..ab4b8c79fcd1a 100644 --- a/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp +++ b/clang/unittests/StaticAnalyzer/ExprEngineVisitTest.cpp @@ -55,11 +55,13 @@ class MemAccessChecker : public Checker { ", Stmt = " + S->getStmtClassName()); } - 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 { emitErrorReport(C, Bug, "checkBind: Loc = " + dumpToString(Loc) + ", Val = " + dumpToString(Val) + - ", Stmt = " + S->getStmtClassName()); + ", Stmt = " + S->getStmtClassName() + + ", AtDeclInit = " + (AtDeclInit ? "true" : "false")); } private: @@ -140,7 +142,7 @@ TEST(ExprEngineVisitTest, checkLocationAndBind) { "Stmt = ImplicitCastExpr"; std::string BindMsg = "checkBind: Loc = &MyClassWrite, Val = lazyCompoundVal{0x0,MyClassRead}, " - "Stmt = CXXOperatorCallExpr"; + "Stmt = CXXOperatorCallExpr, AtDeclInit = false"; std::size_t LocPos = Diags.find(LocMsg); std::size_t BindPos = Diags.find(BindMsg); EXPECT_NE(LocPos, std::string::npos); @@ -150,4 +152,20 @@ TEST(ExprEngineVisitTest, checkLocationAndBind) { EXPECT_TRUE(LocPos > BindPos); } +TEST(ExprEngineVisitTest, checkLocationAndBindInitialization) { + std::string Diags; + EXPECT_TRUE(runCheckerOnCode(R"( + class MyClass{ + public: + int Value; + }; + void top(MyClass param) { + MyClass MyClassWrite = param; + } + )", + Diags)); + + EXPECT_TRUE(StringRef(Diags).contains("AtDeclInit = true")); +} + } // namespace diff --git a/clang/unittests/StaticAnalyzer/SValTest.cpp b/clang/unittests/StaticAnalyzer/SValTest.cpp index 58e9a8da0e99d..71f682a6d571c 100644 --- a/clang/unittests/StaticAnalyzer/SValTest.cpp +++ b/clang/unittests/StaticAnalyzer/SValTest.cpp @@ -61,7 +61,8 @@ using SVals = llvm::StringMap; /// can test whatever we gathered. class SValCollector : public Checker { 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 { // Skip instantly if we finished testing. // Also, we care only for binds happening in variable initializations. if (Tested || !isa(S))