From: Ted Kremenek Date: Tue, 23 Mar 2010 01:11:38 +0000 (+0000) Subject: Tweak null dereference diagnostics to give clearer diagnostics when X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=452b84ded735d7e7de6d099953ab959a4c9910f0;p=clang Tweak null dereference diagnostics to give clearer diagnostics when a null dereference results from a field access. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@99236 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Checker/DereferenceChecker.cpp b/lib/Checker/DereferenceChecker.cpp index 0cbc408670..dfd3b61b0a 100644 --- a/lib/Checker/DereferenceChecker.cpp +++ b/lib/Checker/DereferenceChecker.cpp @@ -29,9 +29,9 @@ public: DereferenceChecker() : BT_null(0), BT_undef(0) {} static void *getTag() { static int tag = 0; return &tag; } void VisitLocation(CheckerContext &C, const Stmt *S, SVal location); - + std::pair - getImplicitNodes() const { + getImplicitNodes() const { return std::make_pair(ImplicitNullDerefNodes.data(), ImplicitNullDerefNodes.data() + ImplicitNullDerefNodes.size()); @@ -59,7 +59,7 @@ void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S, if (ExplodedNode *N = C.GenerateSink()) { if (!BT_undef) BT_undef = new BuiltinBug("Dereference of undefined pointer value"); - + EnhancedBugReport *report = new EnhancedBugReport(*BT_undef, BT_undef->getDescription(), N); report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, @@ -68,31 +68,32 @@ void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S, } return; } - + DefinedOrUnknownSVal location = cast(l); - - // Check for null dereferences. + + // Check for null dereferences. if (!isa(location)) return; - + const GRState *state = C.getState(); const GRState *notNullState, *nullState; llvm::tie(notNullState, nullState) = state->Assume(location); - + // The explicit NULL case. if (nullState) { - if (!notNullState) { + 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. + // we call an "explicit" null dereference. if (!BT_null) BT_null = new BuiltinBug("Dereference of null pointer"); - + llvm::SmallString<100> buf; + llvm::SmallVector Ranges; switch (S->getStmtClass()) { case Stmt::UnaryOperatorClass: { @@ -101,10 +102,26 @@ void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S, if (const DeclRefExpr *DR = dyn_cast(SU)) { if (const VarDecl *VD = dyn_cast(DR->getDecl())) { llvm::raw_svector_ostream os(buf); - os << "Dereference of null pointer loaded from variable '" - << VD->getName() << '\''; + os << "Dereference of null pointer (loaded from variable '" + << VD->getName() << "')"; + Ranges.push_back(DR->getSourceRange()); } } + break; + } + case Stmt::MemberExprClass: { + const MemberExpr *M = cast(S); + if (M->isArrow()) + if (DeclRefExpr *DR = + dyn_cast(M->getBase()->IgnoreParenCasts())) { + if (const VarDecl *VD = dyn_cast(DR->getDecl())) { + llvm::raw_svector_ostream os(buf); + os << "Field access results in a dereference of a null pointer " + "(loaded from variable '" << VD->getName() << "')"; + Ranges.push_back(M->getBase()->getSourceRange()); + } + } + break; } default: break; @@ -117,19 +134,23 @@ void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S, report->addVisitorCreator(bugreporter::registerTrackNullOrUndefValue, bugreporter::GetDerefExpr(N)); - + + for (llvm::SmallVectorImpl::iterator + I = Ranges.begin(), E = Ranges.end(); I!=E; ++I) + report->addRange(*I); + C.EmitReport(report); 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. + // dereference. if (ExplodedNode *N = C.GenerateSink(nullState)) ImplicitNullDerefNodes.push_back(N); } } - + // From this point forward, we know that the location is not null. C.addTransition(notNullState); } diff --git a/test/Analysis/inline.c b/test/Analysis/inline.c index 13d4f7fba4..952de737f7 100644 --- a/test/Analysis/inline.c +++ b/test/Analysis/inline.c @@ -15,6 +15,6 @@ void f2() { } if (x == 2) { int *p = 0; - *p = 3; // expected-warning{{Dereference of null pointer loaded from variable}} + *p = 3; // expected-warning{{Dereference of null pointer (loaded from variable 'p')}} } } diff --git a/test/Analysis/null-deref-ps.c b/test/Analysis/null-deref-ps.c index 704ad339e1..5376ca0eb3 100644 --- a/test/Analysis/null-deref-ps.c +++ b/test/Analysis/null-deref-ps.c @@ -26,7 +26,7 @@ int f2(struct foo_struct* p) { if (p) p->x = 1; - return p->x++; // expected-warning{{Dereference of null pointer}} + return p->x++; // expected-warning{{Field access results in a dereference of a null pointer (loaded from variable 'p')}} } int f3(char* x) { @@ -57,7 +57,7 @@ int f4(int *p) { return 1; int *q = (int*) x; - return *q; // expected-warning{{Dereference of null pointer loaded from variable 'q'}} + return *q; // expected-warning{{Dereference of null pointer (loaded from variable 'q')}} } int f4_b() { diff --git a/test/Analysis/plist-output.m b/test/Analysis/plist-output.m index f49fef5d6d..aa866de03c 100644 --- a/test/Analysis/plist-output.m +++ b/test/Analysis/plist-output.m @@ -124,7 +124,7 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: line5 -// CHECK: col3 +// CHECK: col4 // CHECK: file0 // CHECK: // CHECK: @@ -135,12 +135,12 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: extended_message -// CHECK: Dereference of null pointer loaded from variable 'p' +// CHECK: Dereference of null pointer (loaded from variable 'p') // CHECK: message -// CHECK: Dereference of null pointer loaded from variable 'p' +// CHECK: Dereference of null pointer (loaded from variable 'p') // CHECK: // CHECK: -// CHECK: descriptionDereference of null pointer loaded from variable 'p' +// CHECK: descriptionDereference of null pointer (loaded from variable 'p') // CHECK: categoryLogic error // CHECK: typeDereference of null pointer // CHECK: location @@ -262,7 +262,7 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: line11 -// CHECK: col3 +// CHECK: col4 // CHECK: file0 // CHECK: // CHECK: @@ -273,12 +273,12 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: extended_message -// CHECK: Dereference of null pointer loaded from variable 'p' +// CHECK: Dereference of null pointer (loaded from variable 'p') // CHECK: message -// CHECK: Dereference of null pointer loaded from variable 'p' +// CHECK: Dereference of null pointer (loaded from variable 'p') // CHECK: // CHECK: -// CHECK: descriptionDereference of null pointer loaded from variable 'p' +// CHECK: descriptionDereference of null pointer (loaded from variable 'p') // CHECK: categoryLogic error // CHECK: typeDereference of null pointer // CHECK: location @@ -400,7 +400,7 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: line18 -// CHECK: col3 +// CHECK: col4 // CHECK: file0 // CHECK: // CHECK: @@ -411,12 +411,12 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: extended_message -// CHECK: Dereference of null pointer loaded from variable 'q' +// CHECK: Dereference of null pointer (loaded from variable 'q') // CHECK: message -// CHECK: Dereference of null pointer loaded from variable 'q' +// CHECK: Dereference of null pointer (loaded from variable 'q') // CHECK: // CHECK: -// CHECK: descriptionDereference of null pointer loaded from variable 'q' +// CHECK: descriptionDereference of null pointer (loaded from variable 'q') // CHECK: categoryLogic error // CHECK: typeDereference of null pointer // CHECK: location @@ -538,7 +538,7 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: line23 -// CHECK: col5 +// CHECK: col6 // CHECK: file0 // CHECK: // CHECK: @@ -549,12 +549,12 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: extended_message -// CHECK: Dereference of null pointer loaded from variable 'p' +// CHECK: Dereference of null pointer (loaded from variable 'p') // CHECK: message -// CHECK: Dereference of null pointer loaded from variable 'p' +// CHECK: Dereference of null pointer (loaded from variable 'p') // CHECK: // CHECK: -// CHECK: descriptionDereference of null pointer loaded from variable 'p' +// CHECK: descriptionDereference of null pointer (loaded from variable 'p') // CHECK: categoryLogic error // CHECK: typeDereference of null pointer // CHECK: location @@ -710,7 +710,7 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: line30 -// CHECK: col5 +// CHECK: col6 // CHECK: file0 // CHECK: // CHECK: @@ -721,12 +721,12 @@ void test_null_field(void) { // CHECK: // CHECK: // CHECK: extended_message -// CHECK: Dereference of null pointer loaded from variable 'p' +// CHECK: Dereference of null pointer (loaded from variable 'p') // CHECK: message -// CHECK: Dereference of null pointer loaded from variable 'p' +// CHECK: Dereference of null pointer (loaded from variable 'p') // CHECK: // CHECK: -// CHECK: descriptionDereference of null pointer loaded from variable 'p' +// CHECK: descriptionDereference of null pointer (loaded from variable 'p') // CHECK: categoryLogic error // CHECK: typeDereference of null pointer // CHECK: location