From: Devin Coughlin <dcoughlin@apple.com>
Date: Fri, 19 Aug 2016 01:05:31 +0000 (+0000)
Subject: [analyzer] Weaken assertion in trackNullOrUndefValue()
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1319614c1b7fd72d64c7561b89f3b9872a4f7a69;p=clang

[analyzer] Weaken assertion in trackNullOrUndefValue()

We should ignore paren casts when making sure that the semantic expression
in a PseudoObjectExpr for an ObjC getter is a message send.

This has no other intended functionality change.

Adding a test for this exposed an interesting issue in another test case
that only manifests under ARC. trackNullOrUndefValue() is not properly
suppressing for nil values that are the result of nil propagation from a nil
receiver when the nil is returned from a function. I've added a FIXME for that
missing suppression.

rdar://problem/27290568

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@279181 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 3b72244a52..a38d1d9bdb 100644
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -916,7 +916,7 @@ static const Expr *peelOffOuterExpr(const Expr *Ex,
     if (PropRef && PropRef->isMessagingGetter()) {
       const Expr *GetterMessageSend =
           POE->getSemanticExpr(POE->getNumSemanticExprs() - 1);
-      assert(isa<ObjCMessageExpr>(GetterMessageSend));
+      assert(isa<ObjCMessageExpr>(GetterMessageSend->IgnoreParenCasts()));
       return peelOffOuterExpr(GetterMessageSend, N);
     }
   }
diff --git a/test/Analysis/inlining/false-positive-suppression.m b/test/Analysis/inlining/false-positive-suppression.m
index 1a5ff662c1..685e29e231 100644
--- a/test/Analysis/inlining/false-positive-suppression.m
+++ b/test/Analysis/inlining/false-positive-suppression.m
@@ -1,8 +1,11 @@
 // 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 -fobjc-arc -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
+#define ARC __has_feature(objc_arc)
+
+#if defined(SUPPRESSED) && !ARC
 // expected-no-diagnostics
 #endif
 
@@ -24,8 +27,9 @@ void testNilReceiverHelperA(int *x) {
 
 void testNilReceiverHelperB(int *x) {
   *x = 1;
-#ifndef SUPPRESSED
-  // expected-warning@-2 {{Dereference of null pointer}}
+// FIXME: Suppression for this case isn't working under ARC. It should.
+#if !defined(SUPPRESSED) || (defined(SUPPRESSED) && ARC)
+  // expected-warning@-3 {{Dereference of null pointer}}
 #endif
 }
 
@@ -40,13 +44,17 @@ void testNilReceiver(int coin) {
 // FALSE NEGATIVES (over-suppression)
 
 __attribute__((objc_root_class))
-@interface SomeClass
+@interface SomeClass {
+  int ivar;
+}
 -(int *)methodReturningNull;
 
 @property(readonly) int *propertyReturningNull;
 
 @property(readonly) int *synthesizedProperty;
 
+@property(readonly) SomeClass *propertyReturningNil;
+
 @end
 
 @interface SubOfSomeClass : SomeClass
@@ -64,6 +72,10 @@ __attribute__((objc_root_class))
   return 0;
 }
 
+-(SomeClass *)propertyReturningNil {
+  return 0;
+}
+
 +(int *)classPropertyReturningNull {
   return 0;
 }
@@ -103,6 +115,16 @@ void testClassPropertyReturningNull() {
 #endif
 }
 
+@implementation SomeClass (ForTestOfPropertyReturningNil)
+void testPropertyReturningNil(SomeClass *sc) {
+  SomeClass *result = sc.propertyReturningNil;
+  result->ivar = 1;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Access to instance variable 'ivar' results in a dereference of a null pointer (loaded from variable 'result')}}
+#endif
+}
+@end
+
 void testSynthesizedPropertyReturningNull(SomeClass *sc) {
   if (sc.synthesizedProperty)
     return;