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>
>
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
