[analyzer] Replace isIntegerType() with isIntegerOrEnumerationType().

Previously, the analyzer used isIntegerType() everywhere, which uses the C
definition of "integer". The C++ predicate with the same behavior is
isIntegerOrUnscopedEnumerationType().

However, the analyzer is /really/ using this to ask if it's some sort of
"integrally representable" type, i.e. it should include C++11 scoped
enumerations as well. hasIntegerRepresentation() sounds like the right
predicate, but that includes vectors, which the analyzer represents by its
elements.

This commit audits all uses of isIntegerType() and replaces them with the
general isIntegerOrEnumerationType(), except in some specific cases where
it makes sense to exclude scoped enumerations, or any enumerations. These
cases now use isIntegerOrUnscopedEnumerationType() and getAs<BuiltinType>()
plus BuiltinType::isInteger().

isIntegerType() is hereby banned in the analyzer - lib/StaticAnalysis and
include/clang/StaticAnalysis. :-)

Fixes real assertion failures. PR15703 / <rdar://problem/12350701>

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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h b/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
index 1135b511441d5ffcf25f3a84eb1ed4cc7a49ed92..2c799c0db44e6a2b1100053f15494d6d05929f73 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -93,7 +93,7 @@ public:
 
   /// Returns the type of the APSInt used to store values of the given QualType.
   APSIntType getAPSIntType(QualType T) const {
-    assert(T->isIntegerType() || Loc::isLocType(T));
+    assert(T->isIntegralOrEnumerationType() || Loc::isLocType(T));
     return APSIntType(Ctx.getTypeSize(T),
                       !T->isSignedIntegerOrEnumerationType());
   }

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
index 6ea7211090bac458b0f9c9b11cf2bcf682ea4da7..3d5e8f1f93100c9c5abcec4cdfa4323f4136cdf5 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -711,7 +711,8 @@ ProgramState::getSValAsScalarOrLoc(const Stmt *S,
                                    const LocationContext *LCtx) const {
   if (const Expr *Ex = dyn_cast<Expr>(S)) {
     QualType T = Ex->getType();
-    if (Ex->isGLValue() || Loc::isLocType(T) || T->isIntegerType())
+    if (Ex->isGLValue() || Loc::isLocType(T) ||
+        T->isIntegralOrEnumerationType())
       return getSVal(S, LCtx);
   }
 

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index f7e49a3c7539b5131b16c8911a8104d6a3ed8457..4a4b37a78612ed9175f28a4ac389208ac321f926 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -78,7 +78,8 @@ public:
     // FIXME: Remove the second disjunct when we support symbolic
     // truncation/extension.
     return (Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2) ||
-            (Ty1->isIntegerType() && Ty2->isIntegerType()));
+            (Ty1->isIntegralOrEnumerationType() &&
+             Ty2->isIntegralOrEnumerationType()));
   }
 
   SVal evalCast(SVal val, QualType castTy, QualType originalType);

diff --git a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index ed503d1cf910b9c5db7e96a87275f61c18fa65ee..e0c3962cb668bcbc692b0a36d549a8a5742a7e2c 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -388,7 +388,7 @@ void CFNumberCreateChecker::checkPreStmt(const CallExpr *CE,
   // FIXME: If the pointee isn't an integer type, should we flag a warning?
   //  People can do weird stuff with pointers.
 
-  if (!T->isIntegerType())
+  if (!T->isIntegralOrEnumerationType())
     return;
 
   uint64_t SourceSize = Ctx.getTypeSize(T);

diff --git a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
index 7ef13ab5386594d7d355b47593d50572739ce19e..63080ea230f87d092b8fab5b4cf420c5539b9f22 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ b/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -345,7 +345,7 @@ void WalkAST::checkCall_getpw(const CallExpr *CE, const FunctionDecl *FD) {
     return;
 
   // Verify the first argument type is integer.
-  if (!FPT->getArgType(0)->isIntegerType())
+  if (!FPT->getArgType(0)->isIntegralOrUnscopedEnumerationType())
     return;
 
   // Verify the second argument type is char*.
@@ -602,7 +602,7 @@ void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) {
     if (!PT)
       return;
 
-    if (! PT->getPointeeType()->isIntegerType())
+    if (! PT->getPointeeType()->isIntegralOrUnscopedEnumerationType())
       return;
   }
   else if (FTP->getNumArgs() != 0)
@@ -725,7 +725,7 @@ void WalkAST::checkUncheckedReturnValue(CallExpr *CE) {
 
   // The arguments must be integers.
   for (unsigned i = 0; i < FTP->getNumArgs(); i++)
-    if (! FTP->getArgType(i)->isIntegerType())
+    if (! FTP->getArgType(i)->isIntegralOrUnscopedEnumerationType())
       return;
 
   // Issue a warning.

diff --git a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
index ce7d4ccf7a0a80e4e384ae99ac106c7177e0dbe8..d29f34fb03e2d5bbe5f8df5c992298ab22bb0f3d 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
@@ -188,7 +188,7 @@ public:
 
       for (CallExpr::const_arg_iterator ai = i->AllocCall->arg_begin(),
            ae = i->AllocCall->arg_end(); ai != ae; ++ai) {
-        if (!(*ai)->getType()->isIntegerType())
+        if (!(*ai)->getType()->isIntegralOrUnscopedEnumerationType())
           continue;
 
         SizeofFinder SFinder;

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 4ffdc6afa4a437f858588612e6bfaa55dbada5c1..9b5f6b2a8c7266965352cdc8ddde2d8892d11b5c 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1351,7 +1351,7 @@ ConditionBRVisitor::VisitConditionVariable(StringRef LhsString,
     Out << (tookTrue ? "not nil" : "nil");
   else if (Ty->isBooleanType())
     Out << (tookTrue ? "true" : "false");
-  else if (Ty->isIntegerType())
+  else if (Ty->isIntegralOrEnumerationType())
     Out << (tookTrue ? "non-zero" : "zero");
   else
     return 0;

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index ab4dbd752519ab1285886a6b6bf8dd2d4856a0da..c79cf96c4a891a9e2dbd7d1b0106746a12787d40 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -106,7 +106,8 @@ ProgramStateRef ExprEngine::getInitialState(const LocationContext *InitLoc) {
 
       const ParmVarDecl *PD = FD->getParamDecl(0);
       QualType T = PD->getType();
-      if (!T->isIntegerType())
+      const BuiltinType *BT = dyn_cast<BuiltinType>(T);
+      if (!BT || !BT->isInteger())
         break;
 
       const MemRegion *R = state->getRegion(PD, InitLoc);
@@ -1235,7 +1236,7 @@ static SVal RecoverCastedSymbol(ProgramStateManager& StateMgr,
   while (const CastExpr *CE = dyn_cast<CastExpr>(Ex)) {
     QualType T = CE->getType();
 
-    if (!T->isIntegerType())
+    if (!T->isIntegralOrEnumerationType())
       return UnknownVal();
 
     uint64_t newBits = Ctx.getTypeSize(T);
@@ -1250,7 +1251,8 @@ static SVal RecoverCastedSymbol(ProgramStateManager& StateMgr,
   // We reached a non-cast.  Is it a symbolic value?
   QualType T = Ex->getType();
 
-  if (!bitsInit || !T->isIntegerType() || Ctx.getTypeSize(T) > bits)
+  if (!bitsInit || !T->isIntegralOrEnumerationType() ||
+      Ctx.getTypeSize(T) > bits)
     return UnknownVal();
 
   return state->getSVal(Ex, LCtx);
@@ -1342,7 +1344,7 @@ void ExprEngine::processBranch(const Stmt *Condition, const Stmt *Term,
     if (X.isUnknownOrUndef()) {
       // Give it a chance to recover from unknown.
       if (const Expr *Ex = dyn_cast<Expr>(Condition)) {
-        if (Ex->getType()->isIntegerType()) {
+        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

diff --git a/lib/StaticAnalyzer/Core/ExprEngineC.cpp b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
index 3a3c9713dd20ce9039f590c70b2151a9571485fe..7a53fccf2edd17fcedb69ae1816d1d31af663078 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -68,12 +68,14 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
         // SymSymExpr.
         unsigned Count = currBldrCtx->blockCount();
         if (LeftV.getAs<Loc>() &&
-            RHS->getType()->isIntegerType() && RightV.isUnknown()) {
+            RHS->getType()->isIntegralOrEnumerationType() &&
+            RightV.isUnknown()) {
           RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(),
                                                 Count);
         }
         if (RightV.getAs<Loc>() &&
-            LHS->getType()->isIntegerType() && LeftV.isUnknown()) {
+            LHS->getType()->isIntegralOrEnumerationType() &&
+            LeftV.isUnknown()) {
           LeftV = svalBuilder.conjureSymbolVal(LHS, LCtx, LHS->getType(),
                                                Count);
         }
@@ -662,8 +664,9 @@ VisitOffsetOfExpr(const OffsetOfExpr *OOE,
   APSInt IV;
   if (OOE->EvaluateAsInt(IV, getContext())) {
     assert(IV.getBitWidth() == getContext().getTypeSize(OOE->getType()));
-    assert(OOE->getType()->isIntegerType());
-    assert(IV.isSigned() == OOE->getType()->isSignedIntegerOrEnumerationType());
+    assert(OOE->getType()->isBuiltinType());
+    assert(OOE->getType()->getAs<BuiltinType>()->isInteger());
+    assert(IV.isSigned() == OOE->getType()->isSignedIntegerType());
     SVal X = svalBuilder.makeIntVal(IV);
     B.generateNode(OOE, Pred,
                    Pred->getState()->BindExpr(OOE, Pred->getLocationContext(),

diff --git a/lib/StaticAnalyzer/Core/ProgramState.cpp b/lib/StaticAnalyzer/Core/ProgramState.cpp
index bff2242925e5ccc7731dc53aa91d87c6247c6f29..4b857b0ac249ba50e043981dac92a89e15e73735 100644 (file)
--- a/lib/StaticAnalyzer/Core/ProgramState.cpp
+++ b/lib/StaticAnalyzer/Core/ProgramState.cpp
@@ -270,7 +270,7 @@ SVal ProgramState::getSValAsScalarOrLoc(const MemRegion *R) const {
 
   if (const TypedValueRegion *TR = dyn_cast<TypedValueRegion>(R)) {
     QualType T = TR->getValueType();
-    if (Loc::isLocType(T) || T->isIntegerType())
+    if (Loc::isLocType(T) || T->isIntegralOrEnumerationType())
       return getSVal(R);
   }
 

diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index 0f4a6824a24a7a3f8066ece9efb3289faaf43655..51fe56ea86b8fecdbacfe279770e5a5604274416 100644 (file)
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -1907,7 +1907,7 @@ RegionStoreManager::setImplicitDefaultValue(RegionBindingsConstRef B,
 
   if (Loc::isLocType(T))
     V = svalBuilder.makeNull();
-  else if (T->isIntegerType())
+  else if (T->isIntegralOrEnumerationType())
     V = svalBuilder.makeZeroVal(T);
   else if (T->isStructureOrClassType() || T->isArrayType()) {
     // Set the default value to a zero constant when it is a structure

diff --git a/lib/StaticAnalyzer/Core/SValBuilder.cpp b/lib/StaticAnalyzer/Core/SValBuilder.cpp
index c72e7808010ea7e0126d01793f7693391c78c8a1..19a4353ffc0f466ec5d5ffcc8122f6e708fb24d4 100644 (file)
--- a/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -33,7 +33,7 @@ DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) {
   if (Loc::isLocType(type))
     return makeNull();
 
-  if (type->isIntegerType())
+  if (type->isIntegralOrEnumerationType())
     return makeIntVal(0, type);
 
   // FIXME: Handle floats.
@@ -327,11 +327,11 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) {
       return val;
   
   // Check for casts from pointers to integers.
-  if (castTy->isIntegerType() && Loc::isLocType(originalTy))
+  if (castTy->isIntegralOrEnumerationType() && Loc::isLocType(originalTy))
     return evalCastFromLoc(val.castAs<Loc>(), castTy);
 
   // Check for casts from integers to pointers.
-  if (Loc::isLocType(castTy) && originalTy->isIntegerType()) {
+  if (Loc::isLocType(castTy) && originalTy->isIntegralOrEnumerationType()) {
     if (Optional<nonloc::LocAsInteger> LV = val.getAs<nonloc::LocAsInteger>()) {
       if (const MemRegion *R = LV->getLoc().getAsRegion()) {
         StoreManager &storeMgr = StateMgr.getStoreManager();
@@ -361,7 +361,7 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) {
 
     // Are we casting from an array to an integer?  If so, cast the decayed
     // pointer value to an integer.
-    assert(castTy->isIntegerType());
+    assert(castTy->isIntegralOrEnumerationType());
 
     // FIXME: Keep these here for now in case we decide soon that we
     // need the original decayed type.
@@ -373,7 +373,7 @@ SVal SValBuilder::evalCast(SVal val, QualType castTy, QualType originalTy) {
   // Check for casts from a region to a specific type.
   if (const MemRegion *R = val.getAsRegion()) {
     // Handle other casts of locations to integers.
-    if (castTy->isIntegerType())
+    if (castTy->isIntegralOrEnumerationType())
       return evalCastFromLoc(loc::MemRegionVal(R), castTy);
 
     // FIXME: We should handle the case where we strip off view layers to get

diff --git a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
index 9b759df48f2847f736794e02b6180758caf2767b..34de3453ca3311b1e2fcc9a5abdb776b80171c0f 100644 (file)
--- a/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -137,7 +137,7 @@ SimpleConstraintManager::assumeAuxForSymbol(ProgramStateRef State,
   QualType T = Sym->getType();
 
   // None of the constraint solvers currently support non-integer types.
-  if (!T->isIntegerType())
+  if (!T->isIntegralOrEnumerationType())
     return State;
 
   const llvm::APSInt &zero = BVF.getValue(0, T);

diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 5cc8926a4449836f7910e0d847c80415b9de838c..61f9275ce886c419cf417c2d589b201643ae4e32 100644 (file)
--- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -109,7 +109,7 @@ SVal SimpleSValBuilder::evalCastFromNonLoc(NonLoc val, QualType castTy) {
 
   // Only handle casts from integers to integers - if val is an integer constant
   // being cast to a non integer type, produce unknown.
-  if (!isLocType && !castTy->isIntegerType())
+  if (!isLocType && !castTy->isIntegralOrEnumerationType())
     return UnknownVal();
 
   llvm::APSInt i = val.castAs<nonloc::ConcreteInt>().getValue();
@@ -137,7 +137,7 @@ SVal SimpleSValBuilder::evalCastFromLoc(Loc val, QualType castTy) {
   if (castTy->isUnionType())
     return UnknownVal();
 
-  if (castTy->isIntegerType()) {
+  if (castTy->isIntegralOrEnumerationType()) {
     unsigned BitWidth = Context.getTypeSize(castTy);
 
     if (!val.getAs<loc::ConcreteInt>())

diff --git a/lib/StaticAnalyzer/Core/SymbolManager.cpp b/lib/StaticAnalyzer/Core/SymbolManager.cpp
index de2f5bc7b37320ed4400138178b15cf095995f67..7c75b6c3d2fd957fa900744186b7cbd544cebbd7 100644 (file)
--- a/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ b/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -340,8 +340,8 @@ bool SymbolManager::canSymbolicate(QualType T) {
   if (Loc::isLocType(T))
     return true;
 
-  if (T->isIntegerType())
-    return T->isScalarType();
+  if (T->isIntegralOrEnumerationType())
+    return true;
 
   if (T->isRecordType() && !T->isUnionType())
     return true;

diff --git a/test/Analysis/enum.cpp b/test/Analysis/enum.cpp
new file mode 100644 (file)
index 0000000..571fa7b
--- /dev/null
+++ b/test/Analysis/enum.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=debug.ExprInspection %s
+
+void clang_analyzer_eval(bool);
+
+enum class Foo {
+  Zero
+};
+
+bool pr15703(int x) {
+  return Foo::Zero == (Foo)x; // don't crash
+}
+
+void testCasting(int i) {
+  Foo f = static_cast<Foo>(i);
+  int j = static_cast<int>(f);
+  if (i == 0)
+  {
+    clang_analyzer_eval(f == Foo::Zero); // expected-warning{{TRUE}}
+    clang_analyzer_eval(j == 0); // expected-warning{{TRUE}}
+  }
+  else
+  {
+    clang_analyzer_eval(f == Foo::Zero); // expected-warning{{FALSE}}
+    clang_analyzer_eval(j == 0); // expected-warning{{FALSE}}
+  }
+}