r180996 - [analyzer] Fix trackNullOrUndef when tracking args that have nil receivers.
Jordan Rose
jordan_rose at apple.com
Thu May 2 22:47:24 PDT 2013
Author: jrose
Date: Fri May 3 00:47:24 2013
New Revision: 180996
URL: http://llvm.org/viewvc/llvm-project?rev=180996&view=rev
Log:
[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.
Added:
cfe/trunk/test/Analysis/inlining/false-positive-suppression.m
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/inlining/path-notes.m
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=180996&r1=180995&r2=180996&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri May 3 00:47:24 2013
@@ -884,7 +884,7 @@ bool bugreporter::trackNullOrUndefValue(
Inner = Ex;
}
- if (IsArg) {
+ if (IsArg && !Inner) {
assert(N->getLocation().getAs<CallEnter>() && "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(
// 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.
Added: cfe/trunk/test/Analysis/inlining/false-positive-suppression.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/false-positive-suppression.m?rev=180996&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/inlining/false-positive-suppression.m (added)
+++ cfe/trunk/test/Analysis/inlining/false-positive-suppression.m Fri May 3 00:47:24 2013
@@ -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
+
+ at interface PointerWrapper
+- (int *)getPtr;
+- (id)getObject;
+ at end
+
+id getNil() {
+ return 0;
+}
+
+void testNilReceiverHelperA(int *x) {
+ *x = 1;
+#ifndef SUPPRESSED
+ // expected-warning at -2 {{Dereference of null pointer}}
+#endif
+}
+
+void testNilReceiverHelperB(int *x) {
+ *x = 1;
+#ifndef SUPPRESSED
+ // expected-warning at -2 {{Dereference of null pointer}}
+#endif
+}
+
+void testNilReceiver(int coin) {
+ id x = getNil();
+ if (coin)
+ testNilReceiverHelperA([x getPtr]);
+ else
+ testNilReceiverHelperB([[x getObject] getPtr]);
+}
Modified: cfe/trunk/test/Analysis/inlining/path-notes.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/path-notes.m?rev=180996&r1=180995&r2=180996&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/path-notes.m (original)
+++ cfe/trunk/test/Analysis/inlining/path-notes.m Fri May 3 00:47:24 2013
@@ -65,6 +65,31 @@ int testDispatchSyncInliningNoPruning(in
}
+ at interface PointerWrapper
+- (int *)getPtr;
+ at end
+
+id getNil() {
+ return 0;
+}
+
+void testNilReceiverHelper(int *x) {
+ *x = 1; // expected-warning {{Dereference of null pointer}}
+ // expected-note at -1 {{Dereference of null pointer (loaded from variable 'x')}}
+}
+
+void testNilReceiver(id *x) {
+ if (*x) {
+ // expected-note at -1 {{Taking false branch}}
+ return;
+ }
+ testNilReceiverHelper([*x getPtr]);
+ // expected-note at -1 {{'getPtr' not called because the receiver is nil}}
+ // expected-note at -2 {{Passing null pointer value via 1st parameter 'x'}}
+ // expected-note at -3 {{Calling 'testNilReceiverHelper'}}
+}
+
+
// CHECK: <key>diagnostics</key>
// CHECK-NEXT: <array>
// CHECK-NEXT: <dict>
@@ -1012,4 +1037,321 @@ int testDispatchSyncInliningNoPruning(in
// CHECK-NEXT: <key>file</key><integer>0</integer>
// CHECK-NEXT: </dict>
// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>path</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>82</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>82</integer>
+// CHECK-NEXT: <key>col</key><integer>4</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>23</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>23</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>26</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>26</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>event</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>26</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <key>ranges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>26</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>27</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>depth</key><integer>0</integer>
+// CHECK-NEXT: <key>extended_message</key>
+// CHECK-NEXT: <string>'getPtr' not called because the receiver is nil</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>'getPtr' not called because the receiver is nil</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>26</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>26</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>25</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>25</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>event</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>25</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <key>ranges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>25</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>35</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>depth</key><integer>0</integer>
+// CHECK-NEXT: <key>extended_message</key>
+// CHECK-NEXT: <string>Passing null pointer value via 1st parameter 'x'</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Passing null pointer value via 1st parameter 'x'</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>event</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <key>ranges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>86</integer>
+// CHECK-NEXT: <key>col</key><integer>36</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>depth</key><integer>0</integer>
+// CHECK-NEXT: <key>extended_message</key>
+// CHECK-NEXT: <string>Calling 'testNilReceiverHelper'</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Calling 'testNilReceiverHelper'</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>event</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>76</integer>
+// CHECK-NEXT: <key>col</key><integer>1</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <key>depth</key><integer>1</integer>
+// CHECK-NEXT: <key>extended_message</key>
+// CHECK-NEXT: <string>Entered call from 'testNilReceiver'</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Entered call from 'testNilReceiver'</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>76</integer>
+// CHECK-NEXT: <key>col</key><integer>1</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>76</integer>
+// CHECK-NEXT: <key>col</key><integer>4</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>3</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>6</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>6</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>event</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>6</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <key>ranges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>4</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>4</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>depth</key><integer>1</integer>
+// CHECK-NEXT: <key>extended_message</key>
+// CHECK-NEXT: <string>Dereference of null pointer (loaded from variable 'x')</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Dereference of null pointer (loaded from variable 'x')</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>description</key><string>Dereference of null pointer (loaded from variable 'x')</string>
+// CHECK-NEXT: <key>category</key><string>Logic error</string>
+// CHECK-NEXT: <key>type</key><string>Dereference of null pointer</string>
+// CHECK-NEXT: <key>issue_context_kind</key><string>function</string>
+// CHECK-NEXT: <key>issue_context</key><string>testNilReceiverHelper</string>
+// CHECK-NEXT: <key>issue_hash</key><string>1</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>77</integer>
+// CHECK-NEXT: <key>col</key><integer>6</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </dict>
// CHECK-NEXT: </array>
More information about the cfe-commits
mailing list