From 9f3b9d54ccbbf212591602f389ebde7923627490 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Thu, 2 Aug 2012 21:33:42 +0000 Subject: [PATCH] [analyzer] Add a simple check for initializing reference variables with null. There's still more work to be done here; this doesn't catch reference parameters or return values. But it's a step in the right direction. Part of . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161214 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/DereferenceChecker.cpp | 246 +++++++++++------- test/Analysis/initializer.cpp | 21 ++ test/Analysis/misc-ps-region-store.cpp | 16 +- test/Analysis/reference.cpp | 26 ++ 4 files changed, 214 insertions(+), 95 deletions(-) diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp index 9b56c9fe13..a0022541f9 100644 --- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp @@ -26,13 +26,18 @@ using namespace ento; namespace { class DereferenceChecker : public Checker< check::Location, - EventDispatcher > { + check::Bind, + EventDispatcher > { mutable OwningPtr BT_null; mutable OwningPtr BT_undef; + void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext &C, + bool IsBind = false) const; + public: void checkLocation(SVal location, bool isLoad, const Stmt* S, CheckerContext &C) const; + void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const; static const MemRegion *AddDerefSource(raw_ostream &os, SmallVectorImpl &Ranges, @@ -76,6 +81,108 @@ DereferenceChecker::AddDerefSource(raw_ostream &os, return sourceR; } +void DereferenceChecker::reportBug(ProgramStateRef State, const Stmt *S, + CheckerContext &C, bool IsBind) const { + // Generate an error node. + ExplodedNode *N = C.generateSink(State); + if (!N) + return; + + // We know that 'location' cannot be non-null. This is what + // we call an "explicit" null dereference. + if (!BT_null) + BT_null.reset(new BuiltinBug("Dereference of null pointer")); + + SmallString<100> buf; + SmallVector Ranges; + + // Walk through lvalue casts to get the original expression + // that syntactically caused the load. + if (const Expr *expr = dyn_cast(S)) + S = expr->IgnoreParenLValueCasts(); + + const MemRegion *sourceR = 0; + + if (IsBind) { + if (const BinaryOperator *BO = dyn_cast(S)) { + if (BO->isAssignmentOp()) + S = BO->getRHS(); + } else if (const DeclStmt *DS = dyn_cast(S)) { + assert(DS->isSingleDecl() && "We process decls one by one"); + if (const VarDecl *VD = dyn_cast(DS->getSingleDecl())) + if (const Expr *Init = VD->getAnyInitializer()) + S = Init; + } + } + + switch (S->getStmtClass()) { + case Stmt::ArraySubscriptExprClass: { + llvm::raw_svector_ostream os(buf); + os << "Array access"; + const ArraySubscriptExpr *AE = cast(S); + sourceR = AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(), + State.getPtr(), N->getLocationContext()); + os << " results in a null pointer dereference"; + break; + } + case Stmt::UnaryOperatorClass: { + llvm::raw_svector_ostream os(buf); + os << "Dereference of null pointer"; + const UnaryOperator *U = cast(S); + sourceR = AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(), + State.getPtr(), N->getLocationContext(), true); + break; + } + case Stmt::MemberExprClass: { + const MemberExpr *M = cast(S); + if (M->isArrow()) { + llvm::raw_svector_ostream os(buf); + os << "Access to field '" << M->getMemberNameInfo() + << "' results in a dereference of a null pointer"; + sourceR = AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(), + State.getPtr(), N->getLocationContext(), true); + } + break; + } + case Stmt::ObjCIvarRefExprClass: { + const ObjCIvarRefExpr *IV = cast(S); + if (const DeclRefExpr *DR = + dyn_cast(IV->getBase()->IgnoreParenCasts())) { + if (const VarDecl *VD = dyn_cast(DR->getDecl())) { + llvm::raw_svector_ostream os(buf); + os << "Instance variable access (via '" << VD->getName() + << "') results in a null pointer dereference"; + } + } + Ranges.push_back(IV->getSourceRange()); + break; + } + default: + break; + } + + BugReport *report = + new BugReport(*BT_null, + buf.empty() ? BT_null->getDescription() : buf.str(), + N); + + report->addVisitor( + bugreporter::getTrackNullOrUndefValueVisitor(N, + bugreporter::GetDerefExpr(N), + report)); + + for (SmallVectorImpl::iterator + I = Ranges.begin(), E = Ranges.end(); I!=E; ++I) + report->addRange(*I); + + if (sourceR) { + report->markInteresting(sourceR); + report->markInteresting(State->getRawSVal(loc::MemRegionVal(sourceR))); + } + + C.EmitReport(report); +} + void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, CheckerContext &C) const { // Check for dereference of an undefined value. @@ -101,115 +208,66 @@ void DereferenceChecker::checkLocation(SVal l, bool isLoad, const Stmt* S, return; ProgramStateRef state = C.getState(); - const LocationContext *LCtx = C.getLocationContext(); + ProgramStateRef notNullState, nullState; llvm::tie(notNullState, nullState) = state->assume(location); // The explicit NULL case. if (nullState) { if (!notNullState) { - // Generate an error node. - ExplodedNode *N = C.generateSink(nullState); - if (!N) - return; - - // We know that 'location' cannot be non-null. This is what - // we call an "explicit" null dereference. - if (!BT_null) - BT_null.reset(new BuiltinBug("Dereference of null pointer")); - - SmallString<100> buf; - SmallVector Ranges; - - // Walk through lvalue casts to get the original expression - // that syntactically caused the load. - if (const Expr *expr = dyn_cast(S)) - S = expr->IgnoreParenLValueCasts(); - - const MemRegion *sourceR = 0; - - switch (S->getStmtClass()) { - case Stmt::ArraySubscriptExprClass: { - llvm::raw_svector_ostream os(buf); - os << "Array access"; - const ArraySubscriptExpr *AE = cast(S); - sourceR = - AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts(), - state.getPtr(), LCtx); - os << " results in a null pointer dereference"; - break; - } - case Stmt::UnaryOperatorClass: { - llvm::raw_svector_ostream os(buf); - os << "Dereference of null pointer"; - const UnaryOperator *U = cast(S); - sourceR = - AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(), - state.getPtr(), LCtx, true); - break; - } - case Stmt::MemberExprClass: { - const MemberExpr *M = cast(S); - if (M->isArrow()) { - llvm::raw_svector_ostream os(buf); - os << "Access to field '" << M->getMemberNameInfo() - << "' results in a dereference of a null pointer"; - sourceR = - AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(), - state.getPtr(), LCtx, true); - } - break; - } - case Stmt::ObjCIvarRefExprClass: { - const ObjCIvarRefExpr *IV = cast(S); - if (const DeclRefExpr *DR = - dyn_cast(IV->getBase()->IgnoreParenCasts())) { - if (const VarDecl *VD = dyn_cast(DR->getDecl())) { - llvm::raw_svector_ostream os(buf); - os << "Instance variable access (via '" << VD->getName() - << "') results in a null pointer dereference"; - } - } - Ranges.push_back(IV->getSourceRange()); - break; - } - default: - break; - } + reportBug(nullState, S, C); + return; + } - BugReport *report = - new BugReport(*BT_null, - buf.empty() ? BT_null->getDescription():buf.str(), - N); + // Otherwise, we have the case where the location could either be + // null or not-null. Record the error node as an "implicit" null + // dereference. + if (ExplodedNode *N = C.generateSink(nullState)) { + ImplicitNullDerefEvent event = { l, isLoad, N, &C.getBugReporter() }; + dispatchEvent(event); + } + } - report->addVisitor(bugreporter::getTrackNullOrUndefValueVisitor(N, - bugreporter::GetDerefExpr(N), report)); + // From this point forward, we know that the location is not null. + C.addTransition(notNullState); +} - for (SmallVectorImpl::iterator - I = Ranges.begin(), E = Ranges.end(); I!=E; ++I) - report->addRange(*I); +void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S, + CheckerContext &C) const { + // If we're binding to a reference, check if the value is potentially null. + if (V.isUndef()) + return; - if (sourceR) { - report->markInteresting(sourceR); - report->markInteresting(state->getRawSVal(loc::MemRegionVal(sourceR))); - } + const MemRegion *MR = L.getAsRegion(); + const TypedValueRegion *TVR = dyn_cast_or_null(MR); + if (!TVR) + return; - C.EmitReport(report); + if (!TVR->getValueType()->isReferenceType()) + return; + + ProgramStateRef State = C.getState(); + + ProgramStateRef StNonNull, StNull; + llvm::tie(StNonNull, StNull) = State->assume(cast(V)); + + if (StNull) { + if (!StNonNull) { + reportBug(StNull, S, C, /*isBind=*/true); return; } - else { - // Otherwise, we have the case where the location could either be - // null or not-null. Record the error node as an "implicit" null - // dereference. - if (ExplodedNode *N = C.generateSink(nullState)) { - ImplicitNullDerefEvent event = { l, isLoad, N, &C.getBugReporter() }; - dispatchEvent(event); - } + + // At this point the value could be either null or non-null. + // Record this as an "implicit" null dereference. + if (ExplodedNode *N = C.generateSink(StNull)) { + ImplicitNullDerefEvent event = { V, /*isLoad=*/true, N, + &C.getBugReporter() }; + dispatchEvent(event); } } - // From this point forward, we know that the location is not null. - C.addTransition(notNullState); + // From here on out, assume the value is non-null. + C.addTransition(StNonNull); } void ento::registerDereferenceChecker(CheckerManager &mgr) { diff --git a/test/Analysis/initializer.cpp b/test/Analysis/initializer.cpp index 3f008ffbbf..f7a6fd7bea 100644 --- a/test/Analysis/initializer.cpp +++ b/test/Analysis/initializer.cpp @@ -43,3 +43,24 @@ void testIndirectMember() { IndirectMember obj(3); clang_analyzer_eval(obj.getX() == 3); // expected-warning{{TRUE}} } + + +// ------------------------------------ +// False negatives +// ------------------------------------ + +struct RefWrapper { + RefWrapper(int *p) : x(*p) {} + RefWrapper(int &r) : x(r) {} + int &x; +}; + +void testReferenceMember() { + int *p = 0; + RefWrapper X(p); // should warn in the constructor +} + +void testReferenceMember2() { + int *p = 0; + RefWrapper X(*p); // should warn here +} diff --git a/test/Analysis/misc-ps-region-store.cpp b/test/Analysis/misc-ps-region-store.cpp index 381aa03316..fcffe07536 100644 --- a/test/Analysis/misc-ps-region-store.cpp +++ b/test/Analysis/misc-ps-region-store.cpp @@ -271,13 +271,27 @@ class Rdar9212495_A : public Rdar9212495_B {}; const Rdar9212495_A& rdar9212495(const Rdar9212495_C* ptr) { const Rdar9212495_A& val = dynamic_cast(*ptr); + // This is not valid C++; dynamic_cast with a reference type will throw an + // exception if the pointer does not match the expected type. if (&val == 0) { - val.bar(); // FIXME: This should eventually be a null dereference. + val.bar(); // no warning (unreachable) + int *p = 0; + *p = 0xDEAD; // no warning (unreachable) } return val; } +const Rdar9212495_A* rdar9212495_ptr(const Rdar9212495_C* ptr) { + const Rdar9212495_A* val = dynamic_cast(ptr); + + if (val == 0) { + val->bar(); // expected-warning{{Called C++ object pointer is null}} + } + + return val; +} + // Test constructors invalidating arguments. Previously this raised // an uninitialized value warning. extern "C" void __attribute__((noreturn)) PR9645_exit(int i); diff --git a/test/Analysis/reference.cpp b/test/Analysis/reference.cpp index a12e0bd41c..c9bfadced7 100644 --- a/test/Analysis/reference.cpp +++ b/test/Analysis/reference.cpp @@ -90,3 +90,29 @@ namespace PR13440 { clang_analyzer_eval(s2.x[0] == 42); // expected-warning{{TRUE}} } } + +void testRef() { + int *x = 0; + int &y = *x; // expected-warning{{Dereference of null pointer}} + y = 5; +} + + +// ------------------------------------ +// False negatives +// ------------------------------------ + +namespace rdar11212286 { + class B{}; + + B test() { + B *x = 0; + return *x; // should warn here! + } + + B &testRef() { + B *x = 0; + return *x; // should warn here! + } + +} -- 2.40.0