[cfe-commits] [PATCH][Review request]Support for c++0x nullptr in static analyzer
Jim Goodnow II
Jim at TheGoodnows.net
Wed Apr 20 15:52:55 PDT 2011
Hi Ted,
Yes, we do. The real purpose of the nullptr is the last part of the test
program where it allows you to distinguish between two overloaded
functions, i.e.:
void func( int );
void func( int *);
If you call it with the standard NULL, it will call the "int" version,
whereas if you call it with nullptr, it will call the "int *" version.
It doesn't add that much and I was going to refactor the code a bit if
it was approved to just have a single function isNullPtr() that would
check for ConcreteInt 0 or NullPtrVal.
- jim
On 4/20/2011 11:39 AM, Ted Kremenek wrote:
> 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