From a5796f87229b4aeebca71fa6ee1790ae7a5a0382 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Tue, 9 Apr 2013 02:30:33 +0000 Subject: [PATCH] [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() plus BuiltinType::isInteger(). isIntegerType() is hereby banned in the analyzer - lib/StaticAnalysis and include/clang/StaticAnalysis. :-) Fixes real assertion failures. PR15703 / git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179081 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/PathSensitive/BasicValueFactory.h | 2 +- .../Core/PathSensitive/ProgramState.h | 3 ++- .../Core/PathSensitive/SValBuilder.h | 3 ++- .../Checkers/BasicObjCFoundationChecks.cpp | 2 +- .../Checkers/CheckSecuritySyntaxOnly.cpp | 6 ++--- .../Checkers/MallocSizeofChecker.cpp | 2 +- .../Core/BugReporterVisitors.cpp | 2 +- lib/StaticAnalyzer/Core/ExprEngine.cpp | 10 ++++--- lib/StaticAnalyzer/Core/ExprEngineC.cpp | 11 +++++--- lib/StaticAnalyzer/Core/ProgramState.cpp | 2 +- lib/StaticAnalyzer/Core/RegionStore.cpp | 2 +- lib/StaticAnalyzer/Core/SValBuilder.cpp | 10 +++---- .../Core/SimpleConstraintManager.cpp | 2 +- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp | 4 +-- lib/StaticAnalyzer/Core/SymbolManager.cpp | 4 +-- test/Analysis/enum.cpp | 26 +++++++++++++++++++ 16 files changed, 62 insertions(+), 29 deletions(-) create mode 100644 test/Analysis/enum.cpp diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h b/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h index 1135b51144..2c799c0db4 100644 --- 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 6ea7211090..3d5e8f1f93 100644 --- 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(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 f7e49a3c75..4a4b37a786 100644 --- 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 ed503d1cf9..e0c3962cb6 100644 --- 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 7ef13ab538..63080ea230 100644 --- 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 ce7d4ccf7a..d29f34fb03 100644 --- 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 4ffdc6afa4..9b5f6b2a8c 100644 --- 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 ab4dbd7525..c79cf96c4a 100644 --- 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(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(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(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 3a3c9713dd..7a53fccf2e 100644 --- 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() && - RHS->getType()->isIntegerType() && RightV.isUnknown()) { + RHS->getType()->isIntegralOrEnumerationType() && + RightV.isUnknown()) { RightV = svalBuilder.conjureSymbolVal(RHS, LCtx, RHS->getType(), Count); } if (RightV.getAs() && - 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()->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 bff2242925..4b857b0ac2 100644 --- 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(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 0f4a6824a2..51fe56ea86 100644 --- 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 c72e780801..19a4353ffc 100644 --- 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(), castTy); // Check for casts from integers to pointers. - if (Loc::isLocType(castTy) && originalTy->isIntegerType()) { + if (Loc::isLocType(castTy) && originalTy->isIntegralOrEnumerationType()) { if (Optional LV = val.getAs()) { 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 9b759df48f..34de3453ca 100644 --- 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 5cc8926a44..61f9275ce8 100644 --- 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().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()) diff --git a/lib/StaticAnalyzer/Core/SymbolManager.cpp b/lib/StaticAnalyzer/Core/SymbolManager.cpp index de2f5bc7b3..7c75b6c3d2 100644 --- 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 index 0000000000..571fa7ba22 --- /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(i); + int j = static_cast(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}} + } +} -- 2.40.0