[clang] 0342d2f - [NFC][analyzer] Refactor Environment to map Expr to SVal instead of Stmt to SVal (#193295)

via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 28 05:17:41 PDT 2026


Author: tigbr
Date: 2026-04-28T12:17:36Z
New Revision: 0342d2f10d0bcf84108f879aaf55a7414653a102

URL: https://github.com/llvm/llvm-project/commit/0342d2f10d0bcf84108f879aaf55a7414653a102
DIFF: https://github.com/llvm/llvm-project/commit/0342d2f10d0bcf84108f879aaf55a7414653a102.diff

LOG: [NFC][analyzer] Refactor Environment to map Expr to SVal instead of Stmt to SVal (#193295)

Previously the `Environment` mapped `{Stmt *, LocationContext}` pairs to
symbolic values; but semantically it represents the values produced by
the evaluation of _expressions_, so there was no good reason to use
non-`Expr` statements in this mapping.

This commit replaces `Stmt` with `Expr` (its subclass) in this mapping
to accurately represent the actually relevant type.

This change is also propagated to methods, variables etc. that handle
the `Environment`.

There was a special case in `Environment::getSVal` that allowed looking
up a `ReturnStmt` in the `Environment` (and translated this to a lookup
of the "return value" sub-expression of the `ReturnStmt`). This commit
eliminates this and modifies the callers to explicitly look up the
sub-expression of the `ReturnStmt`.

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Checkers/Taint.h
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
    clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
    clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
    clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
    clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
    clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
    clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
    clang/lib/StaticAnalyzer/Checkers/Taint.cpp
    clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
    clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
    clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    clang/lib/StaticAnalyzer/Core/Environment.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Taint.h b/clang/include/clang/StaticAnalyzer/Checkers/Taint.h
index 3ec8dbfb09ee3..1df3fa277a95d 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Taint.h
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Taint.h
@@ -26,8 +26,9 @@ using TaintTagType = unsigned;
 
 static constexpr TaintTagType TaintTagGeneric = 0;
 
-/// Create a new state in which the value of the statement is marked as tainted.
-[[nodiscard]] ProgramStateRef addTaint(ProgramStateRef State, const Stmt *S,
+/// Create a new state in which the value of the expression is marked as
+/// tainted.
+[[nodiscard]] ProgramStateRef addTaint(ProgramStateRef State, const Expr *E,
                                        const LocationContext *LCtx,
                                        TaintTagType Kind = TaintTagGeneric);
 
@@ -61,8 +62,8 @@ addPartialTaint(ProgramStateRef State, SymbolRef ParentSym,
                 const SubRegion *SubRegion,
                 TaintTagType Kind = TaintTagGeneric);
 
-/// Check if the statement has a tainted value in the given state.
-bool isTainted(ProgramStateRef State, const Stmt *S,
+/// Check if the expression has a tainted value in the given state.
+bool isTainted(ProgramStateRef State, const Expr *E,
                const LocationContext *LCtx,
                TaintTagType Kind = TaintTagGeneric);
 
@@ -79,8 +80,8 @@ bool isTainted(ProgramStateRef State, SymbolRef Sym,
 bool isTainted(ProgramStateRef State, const MemRegion *Reg,
                TaintTagType Kind = TaintTagGeneric);
 
-/// Returns the tainted Symbols for a given Statement and state.
-std::vector<SymbolRef> getTaintedSymbols(ProgramStateRef State, const Stmt *S,
+/// Returns the tainted Symbols for a given expression and state.
+std::vector<SymbolRef> getTaintedSymbols(ProgramStateRef State, const Expr *E,
                                          const LocationContext *LCtx,
                                          TaintTagType Kind = TaintTagGeneric);
 
@@ -99,7 +100,7 @@ std::vector<SymbolRef> getTaintedSymbols(ProgramStateRef State,
                                          TaintTagType Kind = TaintTagGeneric);
 
 std::vector<SymbolRef> getTaintedSymbolsImpl(ProgramStateRef State,
-                                             const Stmt *S,
+                                             const Expr *E,
                                              const LocationContext *LCtx,
                                              TaintTagType Kind,
                                              bool returnFirstOnly);

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 0b9202bcf274e..30fedd91a942c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -199,8 +199,8 @@ class CallEvent {
   virtual void cloneTo(void *Dest) const = 0;
 
   /// Get the value of arbitrary expressions at this point in the path.
-  SVal getSVal(const Stmt *S) const {
-    return getState()->getSVal(S, getLocationContext());
+  SVal getSVal(const Expr *E) const {
+    return getState()->getSVal(E, getLocationContext());
   }
 
   using ValueList = SmallVectorImpl<SVal>;

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
index 13e8d8c6d0ab0..d3b18f7ec36dc 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -168,9 +168,7 @@ class CheckerContext {
   }
 
   /// Get the value of arbitrary expressions at this point in the path.
-  SVal getSVal(const Stmt *S) const {
-    return Pred->getSVal(S);
-  }
+  SVal getSVal(const Expr *E) const { return Pred->getSVal(E); }
 
   ConstCFGElementRef getCFGElementRef() const { return Eng.getCFGElementRef(); }
 

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
index 56453df79819e..2e980110c2ff0 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
@@ -32,23 +32,18 @@ class SymbolReaper;
 /// This allows the environment to manage context-sensitive bindings,
 /// which is essentially for modeling recursive function analysis, among
 /// other things.
-/// FIXME: Use 'Expr' instead of 'Stmt' because associating a result with a
-/// non-expression statement does not make sense. Currently the environment
-/// only containts 'Expr's; and there is only one easy-to-eliminate hack in
-/// 'processCallExit' and 'Environment::getSVal' that constructs and handles
-/// 'EnvironmentEntry' instances with a 'ReturnStmt' as the 'first' part.
-class EnvironmentEntry : public std::pair<const Stmt *,
-                                          const StackFrameContext *> {
+class EnvironmentEntry
+    : public std::pair<const Expr *, const StackFrameContext *> {
 public:
-  EnvironmentEntry(const Stmt *s, const LocationContext *L);
+  EnvironmentEntry(const Expr *E, const LocationContext *L);
 
-  const Stmt *getStmt() const { return first; }
+  const Expr *getExpr() const { return first; }
   const LocationContext *getLocationContext() const { return second; }
 
   /// Profile an EnvironmentEntry for inclusion in a FoldingSet.
   static void Profile(llvm::FoldingSetNodeID &ID,
                       const EnvironmentEntry &E) {
-    ID.AddPointer(E.getStmt());
+    ID.AddPointer(E.getExpr());
     ID.AddPointer(E.getLocationContext());
   }
 

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
index 2988b1bbad68f..e86afffd8d5d7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
@@ -169,8 +169,8 @@ class ExplodedNode : public llvm::FoldingSetNode {
   }
 
   /// Get the value of an arbitrary expression at this node.
-  SVal getSVal(const Stmt *S) const {
-    return getState()->getSVal(S, getLocationContext());
+  SVal getSVal(const Expr *E) const {
+    return getState()->getSVal(E, getLocationContext());
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID,

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
index 942f5544c2b2e..95d86c722b6a0 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -377,10 +377,10 @@ class ProgramState : public llvm::FoldingSetNode {
   /// Get the lvalue for an array index.
   SVal getLValue(QualType ElementType, SVal Idx, SVal Base) const;
 
-  /// Returns the SVal bound to the statement 'S' in the state's environment.
-  SVal getSVal(const Stmt *S, const LocationContext *LCtx) const;
+  /// Returns the SVal bound to the expression \p E in the state's environment.
+  SVal getSVal(const Expr *E, const LocationContext *LCtx) const;
 
-  SVal getSValAsScalarOrLoc(const Stmt *Ex, const LocationContext *LCtx) const;
+  SVal getSValAsScalarOrLoc(const Expr *E, const LocationContext *LCtx) const;
 
   /// Return the value bound to the specified location.
   /// Returns UnknownVal() if none found.
@@ -792,22 +792,17 @@ inline SVal ProgramState::getLValue(QualType ElementType, SVal Idx, SVal Base) c
   return UnknownVal();
 }
 
-inline SVal ProgramState::getSVal(const Stmt *Ex,
-                                  const LocationContext *LCtx) const{
-  return Env.getSVal(EnvironmentEntry(Ex, LCtx),
-                     *getStateManager().svalBuilder);
+inline SVal ProgramState::getSVal(const Expr *E,
+                                  const LocationContext *LCtx) const {
+  return Env.getSVal(EnvironmentEntry(E, LCtx), *getStateManager().svalBuilder);
 }
 
 inline SVal
-ProgramState::getSValAsScalarOrLoc(const Stmt *S,
+ProgramState::getSValAsScalarOrLoc(const Expr *E,
                                    const LocationContext *LCtx) const {
-  if (const Expr *Ex = dyn_cast<Expr>(S)) {
-    QualType T = Ex->getType();
-    if (Ex->isGLValue() || Loc::isLocType(T) ||
-        T->isIntegralOrEnumerationType())
-      return getSVal(S, LCtx);
-  }
-
+  QualType T = E->getType();
+  if (E->isGLValue() || Loc::isLocType(T) || T->isIntegralOrEnumerationType())
+    return getSVal(E, LCtx);
   return UnknownVal();
 }
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index f226f80aa441f..cd7a4b0edad8c 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -885,8 +885,8 @@ static ProgramStateRef checkElementNonNil(CheckerContext &C,
     const VarDecl *ElemDecl = cast<VarDecl>(DS->getSingleDecl());
     assert(ElemDecl->getInit() == nullptr);
     ElementLoc = State->getLValue(ElemDecl, LCtx);
-  } else {
-    ElementLoc = State->getSVal(Element, LCtx).getAs<Loc>();
+  } else if (const auto *E = dyn_cast<Expr>(Element)) {
+    ElementLoc = State->getSVal(E, LCtx).getAs<Loc>();
   }
 
   if (!ElementLoc)

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
index bb4446df87dbb..2ac9ae0e4b806 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
@@ -270,7 +270,7 @@ void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const {
   if (!State->get<ReleasedParameter>())
     return;
 
-  SVal V = C.getSVal(RS);
+  SVal V = RS->getRetValue() ? C.getSVal(RS->getRetValue()) : UndefinedVal();
   if (mayBeSuccess(V, C))
     return;
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 2746083431dce..06a9415b79f3f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -661,7 +661,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
   if (State->get<InvariantViolated>())
     return;
 
-  auto RetSVal = C.getSVal(S).getAs<DefinedOrUnknownSVal>();
+  auto RetSVal = C.getSVal(RetExpr).getAs<DefinedOrUnknownSVal>();
   if (!RetSVal)
     return;
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 1762505f58f0f..9f9d64b43a7f7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -229,7 +229,8 @@ void RetainCountChecker::processObjCLiterals(CheckerContext &C,
   ProgramStateRef state = C.getState();
   const ExplodedNode *pred = C.getPredecessor();
   for (const Stmt *Child : Ex->children()) {
-    SVal V = pred->getSVal(Child);
+    const auto *ChildAsExpr = dyn_cast<Expr>(Child);
+    SVal V = ChildAsExpr ? pred->getSVal(ChildAsExpr) : UnknownVal();
     if (SymbolRef sym = V.getAsSymbol())
       if (const RefVal* T = getRefBinding(state, sym)) {
         RefVal::Kind hasErr = (RefVal::Kind) 0;

diff  --git a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
index cad2c72438d4a..eb56fb190e762 100644
--- a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
@@ -660,8 +660,7 @@ static AllocationInfo GetAllocationSite(ProgramStateManager &StateMgr,
       if (auto CEP = N->getLocation().getAs<CallEnter>()) {
         const Stmt *CE = CEP->getCallExpr();
         if (const auto *ME = dyn_cast_or_null<ObjCMessageExpr>(CE)) {
-          const Stmt *RecExpr = ME->getInstanceReceiver();
-          if (RecExpr) {
+          if (const Expr *RecExpr = ME->getInstanceReceiver()) {
             SVal RecV = St->getSVal(RecExpr, NContext);
             if (ME->getMethodFamily() == OMF_init && RecV.getAsSymbol() == Sym)
               InitMethodContext = CEP->getCalleeContext();

diff  --git a/clang/lib/StaticAnalyzer/Checkers/Taint.cpp b/clang/lib/StaticAnalyzer/Checkers/Taint.cpp
index e55d064253b84..f10aecd22d17b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/Taint.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/Taint.cpp
@@ -43,10 +43,10 @@ void taint::dumpTaint(ProgramStateRef State) {
   printTaint(State, llvm::errs());
 }
 
-ProgramStateRef taint::addTaint(ProgramStateRef State, const Stmt *S,
+ProgramStateRef taint::addTaint(ProgramStateRef State, const Expr *E,
                                 const LocationContext *LCtx,
                                 TaintTagType Kind) {
-  return addTaint(State, State->getSVal(S, LCtx), Kind);
+  return addTaint(State, State->getSVal(E, LCtx), Kind);
 }
 
 ProgramStateRef taint::addTaint(ProgramStateRef State, SVal V,
@@ -145,9 +145,9 @@ ProgramStateRef taint::addPartialTaint(ProgramStateRef State,
   return NewState;
 }
 
-bool taint::isTainted(ProgramStateRef State, const Stmt *S,
+bool taint::isTainted(ProgramStateRef State, const Expr *E,
                       const LocationContext *LCtx, TaintTagType Kind) {
-  return !getTaintedSymbolsImpl(State, S, LCtx, Kind, /*ReturnFirstOnly=*/true)
+  return !getTaintedSymbolsImpl(State, E, LCtx, Kind, /*ReturnFirstOnly=*/true)
               .empty();
 }
 
@@ -168,10 +168,11 @@ bool taint::isTainted(ProgramStateRef State, SymbolRef Sym, TaintTagType Kind) {
 }
 
 std::vector<SymbolRef> taint::getTaintedSymbols(ProgramStateRef State,
-                                                const Stmt *S,
+                                                const Expr *E,
                                                 const LocationContext *LCtx,
                                                 TaintTagType Kind) {
-  return getTaintedSymbolsImpl(State, S, LCtx, Kind, /*ReturnFirstOnly=*/false);
+  return getTaintedSymbolsImpl(State, E, LCtx, Kind,
+                               /*ReturnFirstOnly=*/false);
 }
 
 std::vector<SymbolRef> taint::getTaintedSymbols(ProgramStateRef State, SVal V,
@@ -192,11 +193,11 @@ std::vector<SymbolRef> taint::getTaintedSymbols(ProgramStateRef State,
 }
 
 std::vector<SymbolRef> taint::getTaintedSymbolsImpl(ProgramStateRef State,
-                                                    const Stmt *S,
+                                                    const Expr *E,
                                                     const LocationContext *LCtx,
                                                     TaintTagType Kind,
                                                     bool returnFirstOnly) {
-  SVal val = State->getSVal(S, LCtx);
+  SVal val = State->getSVal(E, LCtx);
   return getTaintedSymbolsImpl(State, val, Kind, returnFirstOnly);
 }
 

diff  --git a/clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
index 77cec7deffb84..f69e373e0944d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/TestAfterDivZeroChecker.cpp
@@ -242,7 +242,7 @@ void TestAfterDivZeroChecker::checkBranchCondition(const Stmt *Condition,
     if (hasDivZeroMap(Val, C))
       reportBug(Val, C);
     else {
-      SVal Val = C.getSVal(Condition);
+      SVal Val = C.getSVal(IE);
 
       if (hasDivZeroMap(Val, C))
         reportBug(Val, C);

diff  --git a/clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
index aa478b69aade1..3e8377c3ae189 100644
--- a/clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
@@ -64,7 +64,9 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition,
   // ObjCForCollection is a loop, but has no actual condition.
   if (isa<ObjCForCollectionStmt>(Condition))
     return;
-  if (!Ctx.getSVal(Condition).isUndef())
+
+  const auto *Ex = cast<Expr>(Condition);
+  if (!Ctx.getSVal(Ex).isUndef())
     return;
 
   // Generate a sink node, which implicitly marks both outgoing branches as
@@ -87,7 +89,6 @@ void UndefBranchChecker::checkBranchCondition(const Stmt *Condition,
   // since all the BlockEdge did was act as an error sink since the value
   // had to already be undefined.
   assert(!N->pred_empty());
-  const Expr *Ex = cast<Expr>(Condition);
   ExplodedNode *PrevN = *N->pred_begin();
   ProgramPoint P = PrevN->getLocation();
   ProgramStateRef St = N->getState();

diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 7df5fab0843ac..2942eae367767 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -949,7 +949,10 @@ class ReturnVisitor : public TrackingBugReporterVisitor {
     // Okay, we're at the right return statement, but do we have the return
     // value available?
     ProgramStateRef State = N->getState();
-    SVal V = State->getSVal(Ret, CalleeSFC);
+    const Expr *RV = Ret->getRetValue();
+    if (!RV)
+      return nullptr;
+    SVal V = State->getSVal(RV, CalleeSFC);
     if (V.isUnknownOrUndef())
       return nullptr;
 
@@ -1737,12 +1740,12 @@ PathDiagnosticPieceRef StoreSiteFinder::VisitNode(const ExplodedNode *Succ,
 
     if (DS) {
       SI.StoreKind = StoreInfo::Initialization;
-    } else if (isa<BlockExpr>(S)) {
+    } else if (const auto *BExpr = dyn_cast<BlockExpr>(S)) {
       SI.StoreKind = StoreInfo::BlockCapture;
       if (VR) {
         // See if we can get the BlockVarRegion.
         ProgramStateRef State = StoreSite->getState();
-        SVal V = StoreSite->getSVal(S);
+        SVal V = StoreSite->getSVal(BExpr);
         if (const auto *BDR =
                 dyn_cast_or_null<BlockDataRegion>(V.getAsRegion())) {
           if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) {

diff  --git a/clang/lib/StaticAnalyzer/Core/Environment.cpp b/clang/lib/StaticAnalyzer/Core/Environment.cpp
index bfe973afc3210..da0f058ec2ddb 100644
--- a/clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -63,17 +63,9 @@ static const Expr *ignoreTransparentExprs(const Expr *E) {
   return ignoreTransparentExprs(E);
 }
 
-static const Stmt *ignoreTransparentExprs(const Stmt *S) {
-  if (const auto *E = dyn_cast<Expr>(S))
-    return ignoreTransparentExprs(E);
-  return S;
-}
-
-EnvironmentEntry::EnvironmentEntry(const Stmt *S, const LocationContext *L)
-    : std::pair<const Stmt *,
-                const StackFrameContext *>(ignoreTransparentExprs(S),
-                                           L ? L->getStackFrame()
-                                             : nullptr) {}
+EnvironmentEntry::EnvironmentEntry(const Expr *E, const LocationContext *L)
+    : std::pair<const Expr *, const StackFrameContext *>(
+          ignoreTransparentExprs(E), L ? L->getStackFrame() : nullptr) {}
 
 SVal Environment::lookupExpr(const EnvironmentEntry &E) const {
   const SVal* X = ExprBindings.lookup(E);
@@ -86,16 +78,10 @@ SVal Environment::lookupExpr(const EnvironmentEntry &E) const {
 
 SVal Environment::getSVal(const EnvironmentEntry &Entry,
                           SValBuilder& svalBuilder) const {
-  const Stmt *S = Entry.getStmt();
-  assert(!isa<ObjCForCollectionStmt>(S) &&
-         "Use ExprEngine::hasMoreIteration()!");
-  assert((isa<Expr, ReturnStmt>(S)) &&
-         "Environment can only argue about Exprs, since only they express "
-         "a value! Any non-expression statement stored in Environment is a "
-         "result of a hack!");
+  const Expr *Ex = Entry.getExpr();
   const LocationContext *LCtx = Entry.getLocationContext();
 
-  switch (S->getStmtClass()) {
+  switch (Ex->getStmtClass()) {
   case Stmt::CXXBindTemporaryExprClass:
   case Stmt::ExprWithCleanupsClass:
   case Stmt::GenericSelectionExprClass:
@@ -118,23 +104,11 @@ SVal Environment::getSVal(const EnvironmentEntry &Entry,
   case Stmt::SizeOfPackExprClass:
   case Stmt::PredefinedExprClass:
     // Known constants; defer to SValBuilder.
-    return *svalBuilder.getConstantVal(cast<Expr>(S));
-
-  case Stmt::ReturnStmtClass: {
-    // FIXME: Move this logic to ExprEngine::processCallExit (the only location
-    // passes a ReturnStmt to this method) and then there will be no need to
-    // accept non-expression statements in getSVal (in fact, it will be
-    // possible to change the first member of EnvironmentEntry from const Stmt*
-    // to const Expr*).
-    const auto *RS = cast<ReturnStmt>(S);
-    if (const Expr *RE = RS->getRetValue())
-      return getSVal(EnvironmentEntry(RE, LCtx), svalBuilder);
-    return UndefinedVal();
-  }
+    return *svalBuilder.getConstantVal(Ex);
 
-  // Handle all other Stmt* using a lookup.
+  // Handle all other Expr* using a lookup.
   default:
-    return lookupExpr(EnvironmentEntry(S, LCtx));
+    return lookupExpr(EnvironmentEntry(Ex, LCtx));
   }
 }
 
@@ -200,11 +174,7 @@ EnvironmentManager::removeDeadBindings(Environment Env,
     const EnvironmentEntry &BlkExpr = I.getKey();
     SVal X = I.getData();
 
-    const Expr *E = dyn_cast<Expr>(BlkExpr.getStmt());
-    if (!E)
-      continue;
-
-    if (SymReaper.isLive(E, BlkExpr.getLocationContext())) {
+    if (SymReaper.isLive(BlkExpr.getExpr(), BlkExpr.getLocationContext())) {
       // Copy the binding to the new map.
       EBMapRef = EBMapRef.add(BlkExpr, X);
 
@@ -265,9 +235,9 @@ void Environment::printJson(raw_ostream &Out, const ASTContext &Ctx,
         Out << '[' << NL;
       }
 
-      const Stmt *S = I->first.getStmt();
-      (void)S;
-      assert(S != nullptr && "Expected non-null Stmt");
+      const Expr *Ex = I->first.getExpr();
+      (void)Ex;
+      assert(Ex != nullptr && "Expected non-null Expr");
 
       LastI = I;
     }
@@ -277,11 +247,11 @@ void Environment::printJson(raw_ostream &Out, const ASTContext &Ctx,
       if (I->first.getLocationContext() != LC)
         continue;
 
-      const Stmt *S = I->first.getStmt();
+      const Expr *Ex = I->first.getExpr();
       Indent(Out, InnerSpace, IsDot)
-          << "{ \"stmt_id\": " << S->getID(Ctx) << ", \"kind\": \""
-          << S->getStmtClassName() << "\", \"pretty\": ";
-      S->printJson(Out, nullptr, PP, /*AddQuotes=*/true);
+          << "{ \"stmt_id\": " << Ex->getID(Ctx) << ", \"kind\": \""
+          << Ex->getStmtClassName() << "\", \"pretty\": ";
+      Ex->printJson(Out, nullptr, PP, /*AddQuotes=*/true);
 
       Out << ", \"value\": ";
       I->second.printJson(Out, /*AddQuotes=*/true);

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index b69b45acb7989..7cfb3bb907f7a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1411,7 +1411,7 @@ void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
   ProgramStateRef State = Pred->getState();
   const LocationContext *LCtx = Pred->getLocationContext();
   const CXXDeleteExpr *DE = Dtor.getDeleteExpr();
-  const Stmt *Arg = DE->getArgument();
+  const Expr *Arg = DE->getArgument();
   QualType DTy = DE->getDestroyedType();
   SVal ArgVal = State->getSVal(Arg, LCtx);
 
@@ -2061,7 +2061,9 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
                                       ->getType()->isRecordType()))
           for (auto Child : Ex->children()) {
             assert(Child);
-            SVal Val = State->getSVal(Child, LCtx);
+            const auto *ChildExpr = dyn_cast<Expr>(Child);
+            SVal Val =
+                ChildExpr ? State->getSVal(ChildExpr, LCtx) : UnknownVal();
             State = escapeValues(State, Val, PSK_EscapeOther);
           }
 
@@ -2780,9 +2782,9 @@ bool ExprEngine::hasMoreIteration(ProgramStateRef State,
 /// Returns a (HasMoreIteration, HasNoMoreIteration) pair, or std::nullopt when
 /// the acquisition of the loop condition value failed.
 static std::optional<std::pair<ProgramStateRef, ProgramStateRef>>
-assumeCondition(const Stmt *Condition, ExplodedNode *N) {
+assumeCondition(const Stmt *ConditionStmt, ExplodedNode *N) {
   ProgramStateRef State = N->getState();
-  if (const auto *ObjCFor = dyn_cast<ObjCForCollectionStmt>(Condition)) {
+  if (const auto *ObjCFor = dyn_cast<ObjCForCollectionStmt>(ConditionStmt)) {
     bool HasMoreIteraton =
         ExprEngine::hasMoreIteration(State, ObjCFor, N->getLocationContext());
     // Checkers have already ran on branch conditions, so the current
@@ -2795,18 +2797,22 @@ assumeCondition(const Stmt *Condition, ExplodedNode *N) {
     else
       return std::pair<ProgramStateRef, ProgramStateRef>{nullptr, State};
   }
-  SVal X = State->getSVal(Condition, N->getLocationContext());
+
+  const auto *ConditionExpr = dyn_cast<Expr>(ConditionStmt);
+  assert(ConditionExpr && "The condition must be an Expr from here!");
+
+  SVal X = State->getSVal(ConditionExpr, N->getLocationContext());
 
   if (X.isUnknownOrUndef()) {
     // Give it a chance to recover from unknown.
-    if (const auto *Ex = dyn_cast<Expr>(Condition)) {
+    if (const auto *Ex = dyn_cast<Expr>(ConditionExpr)) {
       if (Ex->getType()->isIntegralOrEnumerationType()) {
         // Try to recover some path-sensitivity.  Right now casts of symbolic
         // integers that promote their values are currently not tracked well.
-        // If 'Condition' is such an expression, try and recover the
+        // If 'ConditionExpr' is such an expression, try and recover the
         // underlying value and use that instead.
         SVal recovered =
-            RecoverCastedSymbol(State, Condition, N->getLocationContext(),
+            RecoverCastedSymbol(State, ConditionExpr, N->getLocationContext(),
                                 N->getState()->getStateManager().getContext());
 
         if (!recovered.isUnknown()) {

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 3dc3c9d05f24e..d3a0c20fd8671 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -297,11 +297,10 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
   if (CE) {
     if (const ReturnStmt *RS = dyn_cast_or_null<ReturnStmt>(LastSt)) {
       const LocationContext *LCtx = CEBNode->getLocationContext();
-      // FIXME: This tries to look up the return statement in the environment,
-      // which is special cased to look up the subexpression RS->getRetValue()
-      // in environment. Instead of relying on this hack, pass
-      // RS->getRetValue() to getSVal() after checking it for nullness.
-      SVal V = State->getSVal(RS, LCtx);
+
+      SVal V = UndefinedVal();
+      if (RS->getRetValue())
+        V = State->getSVal(RS->getRetValue(), LCtx);
 
       // Ensure that the return type matches the type of the returned Expr.
       if (wasDifferentDeclUsedForInlining(Call, CalleeCtx)) {

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
index 07703bdf38239..7d9702ce98396 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -111,18 +111,19 @@ void ExprEngine::VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S,
   //    result in state splitting.
 
   const Stmt *elem = S->getElement();
-  const Stmt *collection = S->getCollection();
+  const Expr *collection = S->getCollection();
   const ConstCFGElementRef &elemRef = getCFGElementRef();
   ProgramStateRef state = Pred->getState();
+
   SVal collectionV = state->getSVal(collection, Pred->getLocationContext());
 
-  SVal elementV;
+  SVal elementV = UnknownVal();
   if (const auto *DS = dyn_cast<DeclStmt>(elem)) {
     const VarDecl *elemD = cast<VarDecl>(DS->getSingleDecl());
     assert(elemD->getInit() == nullptr);
     elementV = state->getLValue(elemD, Pred->getLocationContext());
-  } else {
-    elementV = state->getSVal(elem, Pred->getLocationContext());
+  } else if (const auto *Ex = dyn_cast<Expr>(elem)) {
+    elementV = state->getSVal(Ex, Pred->getLocationContext());
   }
 
   bool isContainerNull = state->isNull(collectionV).isConstrainedTrue();


        


More information about the cfe-commits mailing list