From dcd6224911e234ab3657b7d0b79a2add1ae4fdd8 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Fri, 3 May 2013 05:47:24 +0000 Subject: [PATCH] [analyzer] Fix trackNullOrUndef when tracking args that have nil receivers. There were actually two bugs here: - if we decided to look for an interesting lvalue or call expression, we wouldn't go find its node if we also knew we were at a (different) call. - if we looked through one message send with a nil receiver, we thought we were still looking at an argument to the original call. Put together, this kept us from being able to track the right values, which means sub-par diagnostics and worse false-positive suppression. Noticed by inspection. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@180996 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Core/BugReporterVisitors.cpp | 4 +- .../inlining/false-positive-suppression.m | 38 ++ test/Analysis/inlining/path-notes.m | 342 ++++++++++++++++++ 3 files changed, 382 insertions(+), 2 deletions(-) create mode 100644 test/Analysis/inlining/false-positive-suppression.m diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 7224c667c9..7b970a09dc 100644 --- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -884,7 +884,7 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, Inner = Ex; } - if (IsArg) { + if (IsArg && !Inner) { assert(N->getLocation().getAs() && "Tracking arg but not at call"); } else { // Walk through nodes until we get one that matches the statement exactly. @@ -913,7 +913,7 @@ bool bugreporter::trackNullOrUndefValue(const ExplodedNode *N, // At this point in the path, the receiver should be live since we are at the // message send expr. If it is nil, start tracking it. if (const Expr *Receiver = NilReceiverBRVisitor::getNilReceiver(S, N)) - trackNullOrUndefValue(N, Receiver, report, IsArg, EnableNullFPSuppression); + trackNullOrUndefValue(N, Receiver, report, false, EnableNullFPSuppression); // See if the expression we're interested refers to a variable. diff --git a/test/Analysis/inlining/false-positive-suppression.m b/test/Analysis/inlining/false-positive-suppression.m new file mode 100644 index 0000000000..53ec138367 --- /dev/null +++ b/test/Analysis/inlining/false-positive-suppression.m @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-config suppress-null-return-paths=false -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core -verify -DSUPPRESSED=1 %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-config avoid-suppressing-null-argument-paths=true -DSUPPRESSED=1 -DNULL_ARGS=1 -verify %s + +#ifdef SUPPRESSED +// expected-no-diagnostics +#endif + +@interface PointerWrapper +- (int *)getPtr; +- (id)getObject; +@end + +id getNil() { + return 0; +} + +void testNilReceiverHelperA(int *x) { + *x = 1; +#ifndef SUPPRESSED + // expected-warning@-2 {{Dereference of null pointer}} +#endif +} + +void testNilReceiverHelperB(int *x) { + *x = 1; +#ifndef SUPPRESSED + // expected-warning@-2 {{Dereference of null pointer}} +#endif +} + +void testNilReceiver(int coin) { + id x = getNil(); + if (coin) + testNilReceiverHelperA([x getPtr]); + else + testNilReceiverHelperB([[x getObject] getPtr]); +} diff --git a/test/Analysis/inlining/path-notes.m b/test/Analysis/inlining/path-notes.m index d660973ce1..74f088a382 100644 --- a/test/Analysis/inlining/path-notes.m +++ b/test/Analysis/inlining/path-notes.m @@ -65,6 +65,31 @@ int testDispatchSyncInliningNoPruning(int coin) { } +@interface PointerWrapper +- (int *)getPtr; +@end + +id getNil() { + return 0; +} + +void testNilReceiverHelper(int *x) { + *x = 1; // expected-warning {{Dereference of null pointer}} + // expected-note@-1 {{Dereference of null pointer (loaded from variable 'x')}} +} + +void testNilReceiver(id *x) { + if (*x) { + // expected-note@-1 {{Taking false branch}} + return; + } + testNilReceiverHelper([*x getPtr]); + // expected-note@-1 {{'getPtr' not called because the receiver is nil}} + // expected-note@-2 {{Passing null pointer value via 1st parameter 'x'}} + // expected-note@-3 {{Calling 'testNilReceiverHelper'}} +} + + // CHECK: diagnostics // CHECK-NEXT: // CHECK-NEXT: @@ -1012,4 +1037,321 @@ int testDispatchSyncInliningNoPruning(int coin) { // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: path +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line82 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line82 +// CHECK-NEXT: col4 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col23 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col23 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col26 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col26 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col26 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col26 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col27 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: 'getPtr' not called because the receiver is nil +// CHECK-NEXT: message +// CHECK-NEXT: 'getPtr' not called because the receiver is nil +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col26 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col26 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col25 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col25 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col25 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col25 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col35 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Passing null pointer value via 1st parameter 'x' +// CHECK-NEXT: message +// CHECK-NEXT: Passing null pointer value via 1st parameter 'x' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line86 +// CHECK-NEXT: col36 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Calling 'testNilReceiverHelper' +// CHECK-NEXT: message +// CHECK-NEXT: Calling 'testNilReceiverHelper' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line76 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Entered call from 'testNilReceiver' +// CHECK-NEXT: message +// CHECK-NEXT: Entered call from 'testNilReceiver' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line76 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line76 +// CHECK-NEXT: col4 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col6 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col6 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col6 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col4 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col4 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Dereference of null pointer (loaded from variable 'x') +// CHECK-NEXT: message +// CHECK-NEXT: Dereference of null pointer (loaded from variable 'x') +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: descriptionDereference of null pointer (loaded from variable 'x') +// CHECK-NEXT: categoryLogic error +// CHECK-NEXT: typeDereference of null pointer +// CHECK-NEXT: issue_context_kindfunction +// CHECK-NEXT: issue_contexttestNilReceiverHelper +// CHECK-NEXT: issue_hash1 +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line77 +// CHECK-NEXT: col6 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: // CHECK-NEXT: -- 2.40.0