[cfe-commits] [PATCH][Review request]Support for c++0x nullptr in static analyzer
Ted Kremenek
kremenek at apple.com
Wed Apr 20 11:39:24 PDT 2011
Hi Jim,
I'm not an expert on nullptr, but couldn't we just represent it with an SVal that was a loc::ConcreteInt with a value of 0? Do we need to add a new SVal?
Ted
On Apr 19, 2011, at 9:13 PM, Jim Goodnow II wrote:
> Ok, here's a stab at implementing support for the C++ nullptr in the static analyzer. There's also a test file. Please review and give any feedback.
>
> - jim
>
> Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
> ===================================================================
> --- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h (revision 129845)
> +++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h (working copy)
> @@ -150,6 +150,10 @@
> DefinedSVal getBlockPointer(const BlockDecl *block, CanQualType locTy,
> const LocationContext *locContext);
>
> + DefinedSVal makeNullPtrVal( void ) {
> + return nonloc::NullPtrVal();
> + }
> +
> NonLoc makeCompoundVal(QualType type, llvm::ImmutableList<SVal> vals) {
> return nonloc::CompoundVal(BasicVals.getCompoundValData(type, vals));
> }
> Index: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
> ===================================================================
> --- include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (revision 129845)
> +++ include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h (working copy)
> @@ -276,7 +276,8 @@
> namespace nonloc {
>
> enum Kind { ConcreteIntKind, SymbolValKind, SymExprValKind,
> - LocAsIntegerKind, CompoundValKind, LazyCompoundValKind };
> + LocAsIntegerKind, CompoundValKind, LazyCompoundValKind,
> + NullPtrValKind };
>
> class SymbolVal : public NonLoc {
> public:
> @@ -420,6 +421,27 @@
> }
> };
>
> +// Specail SVal class for C0xx nullptr.
> +class NullPtrVal : public NonLoc {
> + friend class ento::SValBuilder;
> +
> + explicit NullPtrVal( void) :
> + NonLoc(NullPtrValKind, NULL) {
> + }
> +
> +public:
> +
> + // Implement isa<T> support.
> + static inline bool classof(const SVal* V) {
> + return V->getBaseKind() == NonLocKind &&
> + V->getSubKind() == NullPtrValKind;
> + }
> +
> + static inline bool classof(const Loc* V) {
> + return V->getSubKind() == NullPtrValKind;
> + }
> +};
> +
> } // end namespace ento::nonloc
>
> //==------------------------------------------------------------------------==//
> Index: lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (revision 129845)
> +++ lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (working copy)
> @@ -85,7 +85,7 @@
> DefinedOrUnknownSVal location = cast<DefinedOrUnknownSVal>(l);
>
> // Check for null dereferences.
> - if (!isa<Loc>(location))
> + if (!isa<Loc>(location) && !isa<nonloc::NullPtrVal>(location))
> return;
>
> const Stmt *S = C.getStmt();
> Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Core/ExprEngine.cpp (revision 129845)
> +++ lib/StaticAnalyzer/Core/ExprEngine.cpp (working copy)
> @@ -424,7 +424,6 @@
> case Stmt::CXXCatchStmtClass:
> case Stmt::CXXDependentScopeMemberExprClass:
> case Stmt::CXXForRangeStmtClass:
> - case Stmt::CXXNullPtrLiteralExprClass:
> case Stmt::CXXPseudoDestructorExprClass:
> case Stmt::CXXTemporaryObjectExprClass:
> case Stmt::CXXThrowExprClass:
> @@ -523,6 +522,7 @@
> case Stmt::ExprWithCleanupsClass:
> case Stmt::FloatingLiteralClass:
> case Stmt::SizeOfPackExprClass:
> + case Stmt::CXXNullPtrLiteralExprClass:
> Dst.Add(Pred); // No-op. Simply propagate the current state unchanged.
> break;
>
> Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (revision 129845)
> +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (working copy)
> @@ -89,6 +89,10 @@
> return UnknownVal();
> }
>
> + // Special C0xx nullptr case, just return it.
> + if (isa<nonloc::NullPtrVal>(val))
> + return val;
> +
> if (!isa<nonloc::ConcreteInt>(val))
> return UnknownVal();
>
> @@ -293,6 +297,16 @@
> return evalCastFromNonLoc(lhs, resultTy);
> }
>
> + // For nullptr, assume that only exact comparison is legal
> + // for which we know the result for all cases, otherwise,
> + // the result is unknown and should be caught by a checker.
> + if (isa<nonloc::NullPtrVal>(rhs)) {
> + if (op == BO_EQ)
> + return makeTruthVal(false, resultTy);
> + if (op == BO_NE)
> + return makeTruthVal(true, resultTy);
> + return UnknownVal();
> + }
> while (1) {
> switch (lhs.getSubKind()) {
> default:
> @@ -803,6 +817,7 @@
> // If we get here, we have no way of comparing the regions.
> return UnknownVal();
> }
> +
> }
> }
>
> @@ -831,8 +846,20 @@
> return evalBinOpLL(state, op, lhs, loc::ConcreteInt(*x), resultTy);
> }
> }
> + if (isa<nonloc::NullPtrVal>(rhs)) {
> + // We know for sure that the two fields are not the same, since lhs
> + // is a Loc.
> + if (op == BO_EQ)
> + return makeTruthVal(false, resultTy);
> + if (op == BO_NE)
> + return makeTruthVal(true, resultTy);
> + }
> }
>
> + // any other op with nullptr is bad
> + if (isa<nonloc::NullPtrVal>(rhs))
> + return UnknownVal();
> +
> // We are dealing with pointer arithmetic.
>
> // Handle pointer arithmetic on constant values.
> Index: lib/StaticAnalyzer/Core/SVals.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Core/SVals.cpp (revision 129845)
> +++ lib/StaticAnalyzer/Core/SVals.cpp (working copy)
> @@ -338,6 +338,9 @@
> << '}';
> break;
> }
> + case nonloc::NullPtrValKind:
> + os << "NullPtrVal";
> + break;
> default:
> assert (false && "Pretty-printed not implemented for this NonLoc.");
> break;
> Index: lib/StaticAnalyzer/Core/Environment.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Core/Environment.cpp (revision 129845)
> +++ lib/StaticAnalyzer/Core/Environment.cpp (working copy)
> @@ -64,6 +64,9 @@
> else
> return svalBuilder.makeIntVal(cast<IntegerLiteral>(E));
> }
> + // For special C0xx nullptr case, make a null pointer SVal.
> + case Stmt::CXXNullPtrLiteralExprClass:
> + return svalBuilder.makeNullPtrVal();
> case Stmt::ImplicitCastExprClass:
> case Stmt::CXXFunctionalCastExprClass:
> case Stmt::CStyleCastExprClass: {
> Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp (revision 129845)
> +++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp (working copy)
> @@ -196,6 +196,10 @@
> return isFeasible ? state : NULL;
> }
>
> + // Reverse assumption for special C0xx nullptr case.
> + case nonloc::NullPtrValKind:
> + return Assumption ? NULL : state;
> +
> case nonloc::LocAsIntegerKind:
> return assumeAux(state, cast<nonloc::LocAsInteger>(Cond).getLoc(),
> Assumption);
> Index: lib/StaticAnalyzer/Core/Store.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Core/Store.cpp (revision 129845)
> +++ lib/StaticAnalyzer/Core/Store.cpp (working copy)
> @@ -238,7 +238,7 @@
> }
>
> SVal StoreManager::getLValueFieldOrIvar(const Decl* D, SVal Base) {
> - if (Base.isUnknownOrUndef())
> + if (Base.isUnknownOrUndef() || isa<nonloc::NullPtrVal>(Base))
> return Base;
>
> Loc BaseL = cast<Loc>(Base);
> @@ -280,7 +280,9 @@
> // FIXME: For absolute pointer addresses, we just return that value back as
> // well, although in reality we should return the offset added to that
> // value.
> - if (Base.isUnknownOrUndef() || isa<loc::ConcreteInt>(Base))
> + // Also, for nullptr, just return the base. Checker should detect this.
> + if (Base.isUnknownOrUndef() || isa<loc::ConcreteInt>(Base) ||
> + isa<nonloc::NullPtrVal>(Base))
> return Base;
>
> const MemRegion* BaseRegion = cast<loc::MemRegionVal>(Base).getRegion();
>
> =====================
> test/Analysis/nullptr.cpp
> =====================
> // RUN: %clang_cc1 -std=c++0x -analyze -analyzer-checker=core -analyzer-inline-call -analyzer-store region -verify %s
>
> // test to see if nullptr is detected as a null pointer
> void foo1(void) {
> char *np = nullptr;
> *np = 0; // expected-warning{{Dereference of null pointer}}
> }
>
> // check if comparing nullptr to nullptr is detected properly
> void foo2(void) {
> char *np1 = nullptr;
> char *np2 = np1;
> char c;
> if (np1 == np2)
> np1 = &c;
> *np1 = 0; // no-warning
> }
>
> // invoving a nullptr in a more complex operation should be cause a warning
> void foo3(void) {
> struct foo {
> int a, f;
> };
> char *np = nullptr;
> // casting a nullptr to anything should be caught eventually
> int *ip = &(((struct foo *)np)->f);
> *ip = 0; // expected-warning{{Dereference of null pointer}}
> // should be error here too, but analysis gets stopped
> // *np = 0;
> }
>
> #ifdef TEST_THIS
> // check to determine if the correct function is picked
> // which was the whole point of the nullptr to begin with
> // FIXME: disabled for now until inline-call works again
> struct A {
> int x;
> A(int a) { x = a; }
> A(int *ip) { x = 1; }
> int getx() const { return x; }
> };
>
> void f1() {
> A x((int *)0);
> if (x.getx() == 0) {
> int *p = 0;
> *p = 3; // warning
> } else {
> int *p = 0;
> *p = 3; // no-warning
> }
> }
>
> void f2() {
> A x(nullptr);
> if (x.getx() == 1) {
> int *p = 0;
> *p = 3; // warning
> } else {
> int *p = 0;
> *p = 3; // no-warning
> }
> }
> #endif
>
>
> <nullptr.cpp><NullPtr.patch>
More information about the cfe-commits
mailing list