[analyzer] Replace adjacent assumeInBound calls to assumeInBoundDual

[martong]

Summary

This is to minimize superfluous assume calls.

[martong]

• Set parent revision (in the commit message b/c the phab ui 'edit related revisions' is not operational)

[steakhal]

LGTM; Although, I missed some doc comments here and there. This is a really important API, it deserves a few words.

[martong]

• Rebase to parent

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

public:
   /// Assumes both "true" and "false" for \p cond, and returns both
   /// corresponding states (respectively).
   ///
   /// This is more efficient than calling assume() twice. Note that one (but not
   /// both) of the returned states may be NULL.
   LLVM_NODISCARD std::pair<ProgramStateRef, ProgramStateRef>
   assume(DefinedOrUnknownSVal cond) const;
 
+  LLVM_NODISCARD std::pair<ProgramStateRef, ProgramStateRef>
+  assumeInBoundDual(DefinedOrUnknownSVal idx, DefinedOrUnknownSVal upperBound,
+                    QualType IndexType = QualType()) const;
+
   LLVM_NODISCARD ProgramStateRef
   assumeInBound(DefinedOrUnknownSVal idx, DefinedOrUnknownSVal upperBound,
                 bool assumption, QualType IndexType = QualType()) const;
 
   /// Assumes that the value of \p Val is bounded with [\p From; \p To]
   /// (if \p assumption is "true") or it is fully out of this range
   /// (if \p assumption is "false").
   ///

clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp

if (Idx.isZeroConstant())
     return;
 
   ProgramStateRef state = C.getState();
 
   // Get the size of the array.
   DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
       state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType());
 
-  ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
-  ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
+  ProgramStateRef StInBound, StOutBound;
+  std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, ElementCount);
   if (StOutBound && !StInBound) {
     ExplodedNode *N = C.generateErrorNode(StOutBound);
     if (!N)
       return;
 
     if (!BT)
       BT.reset(new BuiltinBug(
           this, "Out-of-bound array access",

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

ProgramStateRef CStringChecker::CheckLocation(CheckerContext &C,
   // Get the size of the array.
   const auto *superReg = cast<SubRegion>(ER->getSuperRegion());
   DefinedOrUnknownSVal Size =
       getDynamicExtent(state, superReg, C.getSValBuilder());
 
   // Get the index of the accessed element.
   DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>();
 
-  ProgramStateRef StInBound = state->assumeInBound(Idx, Size, true);
-  ProgramStateRef StOutBound = state->assumeInBound(Idx, Size, false);
+  ProgramStateRef StInBound, StOutBound;
+  std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, Size);
   if (StOutBound && !StInBound) {
     // These checks are either enabled by the CString out-of-bounds checker
     // explicitly or implicitly by the Malloc checker.
     // In the latter case we only do modeling but do not emit warning.
     if (!Filter.CheckCStringOutOfBounds)
       return nullptr;
 
     // Emit a bug report.
#define REGISTER_CHECKER(name)                                                 \
   }                                                                            \
                                                                                \
   bool ento::shouldRegister##name(const CheckerManager &mgr) { return true; }
 
 REGISTER_CHECKER(CStringNullArg)
 REGISTER_CHECKER(CStringOutOfBounds)
 REGISTER_CHECKER(CStringBufferOverlap)
 REGISTER_CHECKER(CStringNotNullTerm)
-REGISTER_CHECKER(CStringUninitializedRead)
+REGISTER_CHECKER(CStringUninitializedRead)
 

clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp

if (Name.equals("CFArrayGetValueAtIndex")) {
     const Expr *IdxExpr = CE->getArg(1);
     SVal IdxVal = C.getSVal(IdxExpr);
     if (IdxVal.isUnknownOrUndef())
       return;
     DefinedSVal Idx = IdxVal.castAs<DefinedSVal>();
 
     // Now, check if 'Idx in [0, Size-1]'.
     const QualType T = IdxExpr->getType();
-    ProgramStateRef StInBound = State->assumeInBound(Idx, *Size, true, T);
-    ProgramStateRef StOutBound = State->assumeInBound(Idx, *Size, false, T);
+    ProgramStateRef StInBound, StOutBound;
+    std::tie(StInBound, StOutBound) = State->assumeInBoundDual(Idx, *Size, T);
     if (StOutBound && !StInBound) {
       ExplodedNode *N = C.generateErrorNode(StOutBound);
       if (!N)
         return;
       initBugType();
       auto R = std::make_unique<PathSensitiveBugReport>(
           *BT, "Index is out of bounds", N);
       R->addRange(IdxExpr->getSourceRange());

clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp

void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt *RS,
   DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
       state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType());
 
   // We assume that the location after the last element in the array is used as
   // end() iterator. Reporting on these would return too many false positives.
   if (Idx == ElementCount)
     return;
 
-  ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
-  ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
+  ProgramStateRef StInBound, StOutBound;
+  std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, ElementCount);
   if (StOutBound && !StInBound) {
     ExplodedNode *N = C.generateErrorNode(StOutBound);
 
     if (!N)
       return;
 
     // FIXME: This bug correspond to CWE-466.  Eventually we should have bug
     // types explicitly reference such exploit categories (when applicable).

clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp

static bool isArrayIndexOutOfBounds(CheckerContext &C, const Expr *Ex) {
   const MemRegion *MR = Loc.castAs<loc::MemRegionVal>().getRegion();
   const ElementRegion *ER = dyn_cast<ElementRegion>(MR);
   if (!ER)
     return false;
 
   DefinedOrUnknownSVal Idx = ER->getIndex().castAs<DefinedOrUnknownSVal>();
   DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
       state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType());
-  ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
-  ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
+  ProgramStateRef StInBound, StOutBound;
+  std::tie(StInBound, StOutBound) = state->assumeInBoundDual(Idx, ElementCount);
   return StOutBound && !StInBound;
 }
 
 static bool isShiftOverflow(const BinaryOperator *B, CheckerContext &C) {
   return C.isGreaterOrEqual(
       B->getRHS(), C.getASTContext().getIntWidth(B->getLHS()->getType()));
 }
 

clang/lib/StaticAnalyzer/Core/ProgramState.cpp

ProgramStateRef ProgramState::BindExpr(const Stmt *S,
   if (NewEnv == Env)
     return this;
 
   ProgramState NewSt = *this;
   NewSt.Env = NewEnv;
   return getStateManager().getPersistentState(NewSt);
 }
 
-ProgramStateRef ProgramState::assumeInBound(DefinedOrUnknownSVal Idx,
-                                      DefinedOrUnknownSVal UpperBound,
-                                      bool Assumption,
-                                      QualType indexTy) const {
+LLVM_NODISCARD std::pair<ProgramStateRef, ProgramStateRef>
+ProgramState::assumeInBoundDual(DefinedOrUnknownSVal Idx,
   if (Idx.isUnknown() || UpperBound.isUnknown())
-    return this;
+    return {this, this};
 
   // Build an expression for 0 <= Idx < UpperBound.
   // This is the same as Idx + MIN < UpperBound + MIN, if overflow is allowed.
   // FIXME: This should probably be part of SValBuilder.
   ProgramStateManager &SM = getStateManager();
   SValBuilder &svalBuilder = SM.getSValBuilder();
   ASTContext &Ctx = svalBuilder.getContext();
 
   // Get the offset: the minimum value of the array index type.
   BasicValueFactory &BVF = svalBuilder.getBasicValueFactory();
   if (indexTy.isNull())
     indexTy = svalBuilder.getArrayIndexType();
   nonloc::ConcreteInt Min(BVF.getMinValue(indexTy));
 
   // Adjust the index.
   SVal newIdx = svalBuilder.evalBinOpNN(this, BO_Add,
                                         Idx.castAs<NonLoc>(), Min, indexTy);
   if (newIdx.isUnknownOrUndef())
-    return this;
+    return {this, this};
 
   // Adjust the upper bound.
   SVal newBound =
     svalBuilder.evalBinOpNN(this, BO_Add, UpperBound.castAs<NonLoc>(),
                             Min, indexTy);
 
   if (newBound.isUnknownOrUndef())
-    return this;
+    return {this, this};
 
   // Build the actual comparison.
   SVal inBound = svalBuilder.evalBinOpNN(this, BO_LT, newIdx.castAs<NonLoc>(),
                                          newBound.castAs<NonLoc>(), Ctx.IntTy);
   if (inBound.isUnknownOrUndef())
-    return this;
+    return {this, this};
 
   // Finally, let the constraint manager take care of it.
   ConstraintManager &CM = SM.getConstraintManager();
-  return CM.assume(this, inBound.castAs<DefinedSVal>(), Assumption);
+  return CM.assumeDual(this, inBound.castAs<DefinedSVal>());
+}
+
+ProgramStateRef ProgramState::assumeInBound(DefinedOrUnknownSVal Idx,
+                                            DefinedOrUnknownSVal UpperBound,
+                                            bool Assumption,
+                                            QualType indexTy) const {
+  std::pair<ProgramStateRef, ProgramStateRef> R =
+      assumeInBoundDual(Idx, UpperBound, indexTy);
+  return Assumption ? R.first : R.second;
 }
 
 ConditionTruthVal ProgramState::isNonNull(SVal V) const {
   ConditionTruthVal IsNull = isNull(V);
   if (IsNull.isUnderconstrained())
     return IsNull;
   return ConditionTruthVal(!IsNull.getValue());
 }