[analyzer] introduce getSVal(Stmt *) helper on ExplodedNode, make sur…

…e the helper is used consistently

In most cases using
`N->getState()->getSVal(E, N->getLocationContext())`
is ugly, verbose, and also opens up more surface area for bugs if an
inconsistent location context is used.

This patch introduces a helper on an exploded node, and ensures
consistent usage of either `ExplodedNode::getSVal` or
`CheckContext::getSVal` across the codebase.
As a result, a large number of redundant lines is removed.

Differential Revision: https://reviews.llvm.org/D42155

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@322753 91177308-0d34-0410-b5e6-96231b3b80d8

examples/analyzer-plugin/MainCallChecker.cpp

@@ -16,10 +16,8 @@ class MainCallChecker : public Checker < check::PreStmt<CallExpr> > {
 } // end anonymous namespace
 
 void MainCallChecker::checkPreStmt(const CallExpr *CE, CheckerContext &C) const {
-  const ProgramStateRef state = C.getState();
-  const LocationContext *LC = C.getLocationContext();
   const Expr *Callee = CE->getCallee();
-  const FunctionDecl *FD = state->getSVal(Callee, LC).getAsFunctionDecl();
+  const FunctionDecl *FD = C.getSVal(Callee).getAsFunctionDecl();
 
   if (!FD)
     return;

include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h

@@ -193,7 +193,7 @@ class CheckerContext {
 
   /// \brief Get the value of arbitrary expressions at this point in the path.
   SVal getSVal(const Stmt *S) const {
-    return getState()->getSVal(S, getLocationContext());
+    return Pred->getSVal(S);
   }
 
   /// \brief Returns true if the value of \p E is greater than or equal to \p

include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h

@@ -156,6 +156,11 @@ class ExplodedNode : public llvm::FoldingSetNode {
     return Location.getAs<T>();
   }
 
+  /// Get the value of an arbitrary expression at this node.
+  SVal getSVal(const Stmt *S) const {
+    return getState()->getSVal(S, getLocationContext());
+  }
+
   static void Profile(llvm::FoldingSetNodeID &ID,
                       const ProgramPoint &Loc,
                       const ProgramStateRef &state,

lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp

@@ -436,8 +436,7 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE,
     return;
 
   // Get the value of the "theType" argument.
-  const LocationContext *LCtx = C.getLocationContext();
-  SVal TheTypeVal = state->getSVal(CE->getArg(1), LCtx);
+  SVal TheTypeVal = C.getSVal(CE->getArg(1));
 
   // FIXME: We really should allow ranges of valid theType values, and
   //   bifurcate the state appropriately.
@@ -457,7 +456,7 @@ void CFNumberChecker::checkPreStmt(const CallExpr *CE,
   // Look at the value of the integer being passed by reference.  Essentially
   // we want to catch cases where the value passed in is not equal to the
   // size of the type being created.
-  SVal TheValueExpr = state->getSVal(CE->getArg(2), LCtx);
+  SVal TheValueExpr = C.getSVal(CE->getArg(2));
 
   // FIXME: Eventually we should handle arbitrary locations.  We can do this
   //  by having an enhanced memory model that does low-level typing.
@@ -571,7 +570,7 @@ void CFRetainReleaseChecker::checkPreStmt(const CallExpr *CE,
 
   // Get the argument's value.
   const Expr *Arg = CE->getArg(0);
-  SVal ArgVal = state->getSVal(Arg, C.getLocationContext());
+  SVal ArgVal = C.getSVal(Arg);
   Optional<DefinedSVal> DefArgVal = ArgVal.getAs<DefinedSVal>();
   if (!DefArgVal)
     return;
@@ -977,8 +976,7 @@ assumeCollectionNonEmpty(CheckerContext &C, ProgramStateRef State,
   if (!State)
     return nullptr;
 
-  SymbolRef CollectionS =
-    State->getSVal(FCS->getCollection(), C.getLocationContext()).getAsSymbol();
+  SymbolRef CollectionS = C.getSVal(FCS->getCollection()).getAsSymbol();
   return assumeCollectionNonEmpty(C, State, CollectionS, Assumption);
 }
 
@@ -1206,7 +1204,7 @@ ProgramStateRef
 ObjCNonNilReturnValueChecker::assumeExprIsNonNull(const Expr *NonNullExpr,
                                                   ProgramStateRef State,
                                                   CheckerContext &C) const {
-  SVal Val = State->getSVal(NonNullExpr, C.getLocationContext());
+  SVal Val = C.getSVal(NonNullExpr);
   if (Optional<DefinedOrUnknownSVal> DV = Val.getAs<DefinedOrUnknownSVal>())
     return State->assume(*DV, true);
   return State;

lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp

@@ -43,7 +43,7 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE,
 
   case Builtin::BI__builtin_assume: {
     assert (CE->arg_begin() != CE->arg_end());
-    SVal ArgSVal = state->getSVal(CE->getArg(0), LCtx);
+    SVal ArgSVal = C.getSVal(CE->getArg(0));
     if (ArgSVal.isUndef())
       return true; // Return true to model purity.
 
@@ -68,7 +68,7 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE,
     // __builtin_addressof is going from a reference to a pointer, but those
     // are represented the same way in the analyzer.
     assert (CE->arg_begin() != CE->arg_end());
-    SVal X = state->getSVal(*(CE->arg_begin()), LCtx);
+    SVal X = C.getSVal(*(CE->arg_begin()));
     C.addTransition(state->BindExpr(CE, LCtx, X));
     return true;
   }
@@ -83,8 +83,7 @@ bool BuiltinFunctionChecker::evalCall(const CallExpr *CE,
     // Set the extent of the region in bytes. This enables us to use the
     // SVal of the argument directly. If we save the extent in bits, we
     // cannot represent values like symbol*8.
-    DefinedOrUnknownSVal Size =
-        state->getSVal(*(CE->arg_begin()), LCtx).castAs<DefinedOrUnknownSVal>();
+    auto Size = C.getSVal(*(CE->arg_begin())).castAs<DefinedOrUnknownSVal>();
 
     SValBuilder& svalBuilder = C.getSValBuilder();
     DefinedOrUnknownSVal Extent = R->getExtent(svalBuilder);

lib/StaticAnalyzer/Checkers/CStringChecker.cpp

@@ -366,7 +366,7 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C,
   QualType PtrTy = Ctx.getPointerType(Ctx.CharTy);
 
   // Check that the first buffer is non-null.
-  SVal BufVal = state->getSVal(FirstBuf, LCtx);
+  SVal BufVal = C.getSVal(FirstBuf);
   state = checkNonNull(C, state, FirstBuf, BufVal);
   if (!state)
     return nullptr;
@@ -378,7 +378,7 @@ ProgramStateRef CStringChecker::CheckBufferAccess(CheckerContext &C,
   // Get the access length and make sure it is known.
   // FIXME: This assumes the caller has already checked that the access length
   // is positive. And that it's unsigned.
-  SVal LengthVal = state->getSVal(Size, LCtx);
+  SVal LengthVal = C.getSVal(Size);
   Optional<NonLoc> Length = LengthVal.getAs<NonLoc>();
   if (!Length)
     return state;
@@ -2149,7 +2149,7 @@ void CStringChecker::checkPreStmt(const DeclStmt *DS, CheckerContext &C) const {
     if (!MR)
       continue;
 
-    SVal StrVal = state->getSVal(Init, C.getLocationContext());
+    SVal StrVal = C.getSVal(Init);
     assert(StrVal.isValid() && "Initializer string is unknown or undefined");
     DefinedOrUnknownSVal strLength =
         getCStringLength(C, state, Init, StrVal).castAs<DefinedOrUnknownSVal>();

lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp

@@ -101,7 +101,7 @@ void CastSizeChecker::checkPreStmt(const CastExpr *CE,CheckerContext &C) const {
     return;
 
   ProgramStateRef state = C.getState();
-  const MemRegion *R = state->getSVal(E, C.getLocationContext()).getAsRegion();
+  const MemRegion *R = C.getSVal(E).getAsRegion();
   if (!R)
     return;
 

lib/StaticAnalyzer/Checkers/ChrootChecker.cpp

@@ -105,7 +105,7 @@ void ChrootChecker::Chdir(CheckerContext &C, const CallExpr *CE) const {
 
   // After chdir("/"), enter the jail, set the enum value JAIL_ENTERED.
   const Expr *ArgExpr = CE->getArg(0);
-  SVal ArgVal = state->getSVal(ArgExpr, C.getLocationContext());
+  SVal ArgVal = C.getSVal(ArgExpr);
 
   if (const MemRegion *R = ArgVal.getAsRegion()) {
     R = R->StripCasts();

lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp

@@ -110,8 +110,6 @@ DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::VisitNode(
   if (Satisfied)
     return nullptr;
 
-  ProgramStateRef State = N->getState();
-  const LocationContext *LC = N->getLocationContext();
   const Stmt *S = PathDiagnosticLocation::getStmt(N);
   if (!S)
     return nullptr;
@@ -128,7 +126,7 @@ DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::VisitNode(
   }
 
   // Region associated with the current cast expression.
-  const MemRegion *M = State->getSVal(CastE, LC).getAsRegion();
+  const MemRegion *M = N->getSVal(CastE).getAsRegion();
   if (!M)
     return nullptr;
 

lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp

@@ -57,7 +57,7 @@ void DivZeroChecker::checkPreStmt(const BinaryOperator *B,
   if (!B->getRHS()->getType()->isScalarType())
     return;
 
-  SVal Denom = C.getState()->getSVal(B->getRHS(), C.getLocationContext());
+  SVal Denom = C.getSVal(B->getRHS());
   Optional<DefinedSVal> DV = Denom.getAs<DefinedSVal>();
 
   // Divide-by-undefined handled in the generic checking for uses of

lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp

@@ -562,7 +562,7 @@ void DynamicTypePropagation::checkPostStmt(const CastExpr *CE,
       DestObjectPtrType->isUnspecialized())
     return;
 
-  SymbolRef Sym = State->getSVal(CE, C.getLocationContext()).getAsSymbol();
+  SymbolRef Sym = C.getSVal(CE).getAsSymbol();
   if (!Sym)
     return;
 

lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp

@@ -44,8 +44,7 @@ void FixedAddressChecker::checkPreStmt(const BinaryOperator *B,
   if (!T->isPointerType())
     return;
 
-  ProgramStateRef state = C.getState();
-  SVal RV = state->getSVal(B->getRHS(), C.getLocationContext());
+  SVal RV = C.getSVal(B->getRHS());
 
   if (!RV.isConstant() || RV.isZeroConstant())
     return;

lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

@@ -468,7 +468,7 @@ bool GenericTaintChecker::checkPre(const CallExpr *CE, CheckerContext &C) const{
 Optional<SVal> GenericTaintChecker::getPointedToSVal(CheckerContext &C,
                                                      const Expr *Arg) {
   ProgramStateRef State = C.getState();
-  SVal AddrVal = State->getSVal(Arg->IgnoreParens(), C.getLocationContext());
+  SVal AddrVal = C.getSVal(Arg->IgnoreParens());
   if (AddrVal.isUnknownOrUndef())
     return None;
 
@@ -621,7 +621,7 @@ ProgramStateRef GenericTaintChecker::postRetTaint(const CallExpr *CE,
 
 bool GenericTaintChecker::isStdin(const Expr *E, CheckerContext &C) {
   ProgramStateRef State = C.getState();
-  SVal Val = State->getSVal(E, C.getLocationContext());
+  SVal Val = C.getSVal(E);
 
   // stdin is a pointer, so it would be a region.
   const MemRegion *MemReg = Val.getAsRegion();

lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

@@ -355,12 +355,11 @@ void IteratorChecker::checkPostStmt(const MaterializeTemporaryExpr *MTE,
                                     CheckerContext &C) const {
   /* Transfer iterator state to temporary objects */
   auto State = C.getState();
-  const auto *LCtx = C.getLocationContext();
   const auto *Pos =
-      getIteratorPosition(State, State->getSVal(MTE->GetTemporaryExpr(), LCtx));
+      getIteratorPosition(State, C.getSVal(MTE->GetTemporaryExpr()));
   if (!Pos)
     return;
-  State = setIteratorPosition(State, State->getSVal(MTE, LCtx), *Pos);
+  State = setIteratorPosition(State, C.getSVal(MTE), *Pos);
   C.addTransition(State);
 }
 

lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp

@@ -1017,8 +1017,7 @@ NonLocalizedStringBRVisitor::VisitNode(const ExplodedNode *Succ,
   if (!LiteralExpr)
     return nullptr;
 
-  ProgramStateRef State = Succ->getState();
-  SVal LiteralSVal = State->getSVal(LiteralExpr, Succ->getLocationContext());
+  SVal LiteralSVal = Succ->getSVal(LiteralExpr);
   if (LiteralSVal.getAsRegion() != NonLocalizedString)
     return nullptr;
 

lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp

@@ -202,7 +202,7 @@ static bool isBadDeallocationArgument(const MemRegion *Arg) {
 static SymbolRef getAsPointeeSymbol(const Expr *Expr,
                                     CheckerContext &C) {
   ProgramStateRef State = C.getState();
-  SVal ArgV = State->getSVal(Expr, C.getLocationContext());
+  SVal ArgV = C.getSVal(Expr);
 
   if (Optional<loc::MemRegionVal> X = ArgV.getAs<loc::MemRegionVal>()) {
     StoreManager& SM = C.getStoreManager();
@@ -297,7 +297,7 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
 
   // Check the argument to the deallocator.
   const Expr *ArgExpr = CE->getArg(paramIdx);
-  SVal ArgSVal = State->getSVal(ArgExpr, C.getLocationContext());
+  SVal ArgSVal = C.getSVal(ArgExpr);
 
   // Undef is reported by another checker.
   if (ArgSVal.isUndef())
@@ -426,8 +426,7 @@ void MacOSKeychainAPIChecker::checkPostStmt(const CallExpr *CE,
     // allocated value symbol, since our diagnostics depend on the value
     // returned by the call. Ex: Data should only be freed if noErr was
     // returned during allocation.)
-    SymbolRef RetStatusSymbol =
-      State->getSVal(CE, C.getLocationContext()).getAsSymbol();
+    SymbolRef RetStatusSymbol = C.getSVal(CE).getAsSymbol();
     C.getSymbolManager().addSymbolDependency(V, RetStatusSymbol);
 
     // Track the allocated value in the checker state.

lib/StaticAnalyzer/Checkers/MallocChecker.cpp

@@ -758,7 +758,7 @@ llvm::Optional<ProgramStateRef> MallocChecker::performKernelMalloc(
     return None;
 
   const Expr *FlagsEx = CE->getArg(CE->getNumArgs() - 1);
-  const SVal V = State->getSVal(FlagsEx, C.getLocationContext());
+  const SVal V = C.getSVal(FlagsEx);
   if (!V.getAs<NonLoc>()) {
     // The case where 'V' can be a location can only be due to a bad header,
     // so in this case bail out.
@@ -972,8 +972,7 @@ ProgramStateRef MallocChecker::ProcessZeroAllocation(CheckerContext &C,
 
   assert(Arg);
 
-  Optional<DefinedSVal> DefArgVal =
-      State->getSVal(Arg, C.getLocationContext()).getAs<DefinedSVal>();
+  Optional<DefinedSVal> DefArgVal = C.getSVal(Arg).getAs<DefinedSVal>();
 
   if (!DefArgVal)
     return State;
@@ -988,7 +987,7 @@ ProgramStateRef MallocChecker::ProcessZeroAllocation(CheckerContext &C,
       State->assume(SvalBuilder.evalEQ(State, *DefArgVal, Zero));
 
   if (TrueState && !FalseState) {
-    SVal retVal = State->getSVal(E, C.getLocationContext());
+    SVal retVal = C.getSVal(E);
     SymbolRef Sym = retVal.getAsLocSymbol();
     if (!Sym)
       return State;
@@ -1092,7 +1091,7 @@ ProgramStateRef MallocChecker::addExtentSize(CheckerContext &C,
   const SubRegion *Region;
   if (NE->isArray()) {
     const Expr *SizeExpr = NE->getArraySize();
-    ElementCount = State->getSVal(SizeExpr, C.getLocationContext());
+    ElementCount = C.getSVal(SizeExpr);
     // Store the extent size for the (symbolic)region
     // containing the elements.
     Region = (State->getSVal(NE, LCtx))
@@ -1212,8 +1211,7 @@ ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
   if (!State)
     return nullptr;
 
-  return MallocMemAux(C, CE, State->getSVal(SizeEx, C.getLocationContext()),
-                      Init, State, Family);
+  return MallocMemAux(C, CE, C.getSVal(SizeEx), Init, State, Family);
 }
 
 ProgramStateRef MallocChecker::MallocMemAux(CheckerContext &C,
@@ -1268,7 +1266,7 @@ ProgramStateRef MallocChecker::MallocUpdateRefState(CheckerContext &C,
     return nullptr;
 
   // Get the return value.
-  SVal retVal = State->getSVal(E, C.getLocationContext());
+  SVal retVal = C.getSVal(E);
 
   // We expect the malloc functions to return a pointer.
   if (!retVal.getAs<Loc>())
@@ -1457,7 +1455,7 @@ ProgramStateRef MallocChecker::FreeMemAux(CheckerContext &C,
   if (!State)
     return nullptr;
 
-  SVal ArgVal = State->getSVal(ArgExpr, C.getLocationContext());
+  SVal ArgVal = C.getSVal(ArgExpr);
   if (!ArgVal.getAs<DefinedOrUnknownSVal>())
     return nullptr;
   DefinedOrUnknownSVal location = ArgVal.castAs<DefinedOrUnknownSVal>();
@@ -2084,8 +2082,7 @@ ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C,
     return nullptr;
 
   const Expr *arg0Expr = CE->getArg(0);
-  const LocationContext *LCtx = C.getLocationContext();
-  SVal Arg0Val = State->getSVal(arg0Expr, LCtx);
+  SVal Arg0Val = C.getSVal(arg0Expr);
   if (!Arg0Val.getAs<DefinedOrUnknownSVal>())
     return nullptr;
   DefinedOrUnknownSVal arg0Val = Arg0Val.castAs<DefinedOrUnknownSVal>();
@@ -2099,7 +2096,7 @@ ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C,
   const Expr *Arg1 = CE->getArg(1);
 
   // Get the value of the size argument.
-  SVal TotalSize = State->getSVal(Arg1, LCtx);
+  SVal TotalSize = C.getSVal(Arg1);
   if (SuffixWithN)
     TotalSize = evalMulForBufferSize(C, Arg1, CE->getArg(2));
   if (!TotalSize.getAs<DefinedOrUnknownSVal>())
@@ -2133,7 +2130,7 @@ ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C,
   // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, size).
   assert(!PrtIsNull);
   SymbolRef FromPtr = arg0Val.getAsSymbol();
-  SVal RetVal = State->getSVal(CE, LCtx);
+  SVal RetVal = C.getSVal(CE);
   SymbolRef ToPtr = RetVal.getAsSymbol();
   if (!FromPtr || !ToPtr)
     return nullptr;
@@ -2406,7 +2403,7 @@ void MallocChecker::checkPreStmt(const ReturnStmt *S, CheckerContext &C) const {
 
   // Check if we are returning a symbol.
   ProgramStateRef State = C.getState();
-  SVal RetVal = State->getSVal(E, C.getLocationContext());
+  SVal RetVal = C.getSVal(E);
   SymbolRef Sym = RetVal.getAsSymbol();
   if (!Sym)
     // If we are returning a field of the allocated struct or an array element,
@@ -2436,8 +2433,7 @@ void MallocChecker::checkPostStmt(const BlockExpr *BE,
 
   ProgramStateRef state = C.getState();
   const BlockDataRegion *R =
-    cast<BlockDataRegion>(state->getSVal(BE,
-                                         C.getLocationContext()).getAsRegion());
+    cast<BlockDataRegion>(C.getSVal(BE).getAsRegion());
 
   BlockDataRegion::referenced_vars_iterator I = R->referenced_vars_begin(),
                                             E = R->referenced_vars_end();