[cfe-commits] r161214 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp test/Analysis/initializer.cpp test/Analysis/misc-ps-region-store.cpp test/Analysis/reference.cpp
Anna Zaks
ganna at apple.com
Fri Aug 3 09:54:27 PDT 2012
On Aug 2, 2012, at 2:33 PM, Jordan Rose wrote:
> Author: jrose
> Date: Thu Aug 2 16:33:42 2012
> New Revision: 161214
>
> URL: http://llvm.org/viewvc/llvm-project?rev=161214&view=rev
> Log:
> [analyzer] Add a simple check for initializing reference variables with null.
>
Jordan,
It is not clear which new warnings we start catching after this commit.
> 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 <rdar://problem/11212286>.
>
> Modified:
> cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
> cfe/trunk/test/Analysis/initializer.cpp
> cfe/trunk/test/Analysis/misc-ps-region-store.cpp
> cfe/trunk/test/Analysis/reference.cpp
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp?rev=161214&r1=161213&r2=161214&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp Thu Aug 2 16:33:42 2012
> @@ -26,13 +26,18 @@
> namespace {
> class DereferenceChecker
> : public Checker< check::Location,
> - EventDispatcher<ImplicitNullDerefEvent> > {
> + check::Bind,
> + EventDispatcher<ImplicitNullDerefEvent> > {
> mutable OwningPtr<BuiltinBug> BT_null;
> mutable OwningPtr<BuiltinBug> 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<SourceRange> &Ranges,
> @@ -76,6 +81,108 @@
> 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<SourceRange, 2> Ranges;
> +
> + // Walk through lvalue casts to get the original expression
> + // that syntactically caused the load.
> + if (const Expr *expr = dyn_cast<Expr>(S))
> + S = expr->IgnoreParenLValueCasts();
> +
> + const MemRegion *sourceR = 0;
> +
> + if (IsBind) {
> + if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(S)) {
> + if (BO->isAssignmentOp())
> + S = BO->getRHS();
> + } else if (const DeclStmt *DS = dyn_cast<DeclStmt>(S)) {
> + assert(DS->isSingleDecl() && "We process decls one by one");
> + if (const VarDecl *VD = dyn_cast<VarDecl>(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<ArraySubscriptExpr>(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<UnaryOperator>(S);
> + sourceR = AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
> + State.getPtr(), N->getLocationContext(), true);
> + break;
> + }
> + case Stmt::MemberExprClass: {
> + const MemberExpr *M = cast<MemberExpr>(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<ObjCIvarRefExpr>(S);
> + if (const DeclRefExpr *DR =
> + dyn_cast<DeclRefExpr>(IV->getBase()->IgnoreParenCasts())) {
> + if (const VarDecl *VD = dyn_cast<VarDecl>(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<SourceRange>::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 @@
> 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<SourceRange, 2> Ranges;
> -
> - // Walk through lvalue casts to get the original expression
> - // that syntactically caused the load.
> - if (const Expr *expr = dyn_cast<Expr>(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<ArraySubscriptExpr>(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<UnaryOperator>(S);
> - sourceR =
> - AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(),
> - state.getPtr(), LCtx, true);
> - break;
> - }
> - case Stmt::MemberExprClass: {
> - const MemberExpr *M = cast<MemberExpr>(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<ObjCIvarRefExpr>(S);
> - if (const DeclRefExpr *DR =
> - dyn_cast<DeclRefExpr>(IV->getBase()->IgnoreParenCasts())) {
> - if (const VarDecl *VD = dyn_cast<VarDecl>(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<SourceRange>::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)));
> - }
> +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;
>
> - C.EmitReport(report);
> + const MemRegion *MR = L.getAsRegion();
> + const TypedValueRegion *TVR = dyn_cast_or_null<TypedValueRegion>(MR);
> + if (!TVR)
> + return;
> +
> + if (!TVR->getValueType()->isReferenceType())
> + return;
> +
> + ProgramStateRef State = C.getState();
> +
> + ProgramStateRef StNonNull, StNull;
> + llvm::tie(StNonNull, StNull) = State->assume(cast<DefinedOrUnknownSVal>(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) {
>
> Modified: cfe/trunk/test/Analysis/initializer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/initializer.cpp?rev=161214&r1=161213&r2=161214&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/initializer.cpp (original)
> +++ cfe/trunk/test/Analysis/initializer.cpp Thu Aug 2 16:33:42 2012
> @@ -43,3 +43,24 @@
> 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
> +}
>
> Modified: cfe/trunk/test/Analysis/misc-ps-region-store.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.cpp?rev=161214&r1=161213&r2=161214&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/misc-ps-region-store.cpp (original)
> +++ cfe/trunk/test/Analysis/misc-ps-region-store.cpp Thu Aug 2 16:33:42 2012
> @@ -271,13 +271,27 @@
> const Rdar9212495_A& rdar9212495(const Rdar9212495_C* ptr) {
> const Rdar9212495_A& val = dynamic_cast<const Rdar9212495_A&>(*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<const Rdar9212495_A*>(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);
>
> Modified: cfe/trunk/test/Analysis/reference.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=161214&r1=161213&r2=161214&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/reference.cpp (original)
> +++ cfe/trunk/test/Analysis/reference.cpp Thu Aug 2 16:33:42 2012
> @@ -90,3 +90,29 @@
> 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;
> +}
> +
Did we not warn here before this patch? (I checked that an old version of clang was catching this...)
> +
> +// ------------------------------------
> +// 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!
> + }
> +
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list