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

gamesh411 · web-flow · commit 4f2ed926dbbb · 2025-08-08T19:16:47.000+02:00
Binding a value to location can happen when a new value is created or
when and existing value is updated. This modification exposes whether
the value binding happens at a declaration.
This helps simplify the hacky logic of the BindToImmutable checker.
diff --git 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 <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:
diff --git 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<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 &)>;
diff --git 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
@@ -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
@@ -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
 
@@ -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
@@ -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
@@ -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,
@@ -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
@@ -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
@@ -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<TypedValueRegion>(L.getAsRegion());
   if (!TVR)
diff --git 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
@@ -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
@@ -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
@@ -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;
@@ -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
@@ -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
@@ -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;
 };
 
@@ -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))
diff --git a/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp b/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
@@ -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(); }
@@ -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);
     }
   };
 
@@ -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); }};
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -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)
@@ -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<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 =
diff --git 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
diff --git a/clang/unittests/StaticAnalyzer/SValTest.cpp b/clang/unittests/StaticAnalyzer/SValTest.cpp

Original file line number	Diff line number	Diff line change
`@@ -183,7 +183,8 @@ class AnalysisOrderChecker`
`183`	`183`	`llvm::errs() << "NewAllocator\n";`
`184`	`184`	`}`
`185`	`185`
`186`		`- void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const {`
	`186`	`+ void checkBind(SVal Loc, SVal Val, const Stmt *S, bool AtDeclInit,`
	`187`	`+ CheckerContext &C) const {`
`187`	`188`	`if (isCallbackEnabled(C, "Bind"))`
`188`	`189`	`llvm::errs() << "Bind\n";`
`189`	`190`	`}`