[analyzer] Nullability: Look through implicit casts when suppressing warnings on...

In r256567 I changed the nullability checker to suppress warnings about returning a null
value from a function/method with a non-null return type when the type of the returned
expression is itself nonnull. This enables the programmer to silence nullability warnings
by casting to _Nonnull:

  return (SomeObject * _Nonnull)nil;

Unfortunately, under ObjC automated reference counting, Sema adds implicit casts to
_Nonnull to return expressions of nullable or unspecified types in functions with
non-null function/method return types. With r256567, these casts cause all nullability
warnings for returns of reference-counted types to be suppressed under ARC, leading to
false negatives.

This commit updates the nullability checker to look through implicit casts before
determining the type of the returned expression. It also updates the tests to turn on
ARC for the nullability_nullonly.mm testfile and adds a new testfile to test when ARC
is turned off.

rdar://problem/24200117

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

diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 01c7287c97f454e1fad2c28447f01d32b72c0593..b532deb55336d50885ca996567aa41687b935459 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -456,6 +456,20 @@ void NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const {
   }
 }
 
+/// Find the outermost subexpression of E that is not an implicit cast.
+/// This looks through the implicit casts to _Nonnull that ARC adds to
+/// return expressions of ObjC types when the return type of the function or
+/// method is non-null but the express is not.
+static const Expr *lookThroughImplicitCasts(const Expr *E) {
+  assert(E);
+
+  while (auto *ICE = dyn_cast<ImplicitCastExpr>(E)) {
+    E = ICE->getSubExpr();
+  }
+
+  return E;
+}
+
 /// This method check when nullable pointer or null value is returned from a
 /// function that has nonnull return type.
 ///
@@ -501,7 +515,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
   // function with a _Nonnull return type:
   //    return (NSString * _Nonnull)0;
   Nullability RetExprTypeLevelNullability =
-        getNullabilityAnnotation(RetExpr->getType());
+        getNullabilityAnnotation(lookThroughImplicitCasts(RetExpr)->getType());
 
   if (Filter.CheckNullReturnedFromNonnull &&
       Nullness == NullConstraint::IsNull &&

diff --git a/test/Analysis/nullability-no-arc.mm b/test/Analysis/nullability-no-arc.mm
new file mode 100644 (file)
index 0000000..c0e693e
--- /dev/null
+++ b/test/Analysis/nullability-no-arc.mm
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,nullability -verify %s
+
+#define nil 0
+
+@protocol NSObject
++ (id)alloc;
+- (id)init;
+@end
+
+__attribute__((objc_root_class))
+@interface
+NSObject<NSObject>
+@end
+
+@interface TestObject : NSObject
+@end
+
+TestObject * _Nonnull returnsNilObjCInstanceIndirectly() {
+  TestObject *local = 0;
+  return local; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+}
+
+TestObject * _Nonnull returnsNilObjCInstanceIndirectlyWithSupressingCast() {
+  TestObject *local = 0;
+  return (TestObject * _Nonnull)local; // no-warning
+}
+
+TestObject * _Nonnull returnsNilObjCInstanceDirectly() {
+  // The first warning is from Sema. The second is from the static analyzer.
+  return nil; // expected-warning {{null returned from function that requires a non-null return value}}
+              // expected-warning@-1 {{Null is returned from a function that is expected to return a non-null value}}
+}
+
+TestObject * _Nonnull returnsNilObjCInstanceDirectlyWithSuppressingCast() {
+  return (TestObject * _Nonnull)nil; // no-warning
+}
+
+void testObjCNonARCNoInitialization(TestObject * _Nonnull p) {
+  TestObject * _Nonnull implicitlyZeroInitialized; // no-warning
+  implicitlyZeroInitialized = p;
+}
+
+void testObjCNonARCExplicitZeroInitialization() {
+  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{Null is assigned to a pointer which is expected to have non-null value}}
+}

diff --git a/test/Analysis/nullability.mm b/test/Analysis/nullability.mm
index 2c29d0088e18f7abbf36e5bfcb0acd7d056887b9..aa909fbb8c420071767e8d275fd446b1772aa68a 100644 (file)
--- a/test/Analysis/nullability.mm
+++ b/test/Analysis/nullability.mm
@@ -120,12 +120,12 @@ void testArgumentTracking(Dummy *_Nonnull nonnull, Dummy *_Nullable nullable) {
 
 Dummy *_Nonnull testNullableReturn(Dummy *_Nullable a) {
   Dummy *p = a;
-  return p; // expected-warning {{}}
+  return p; // expected-warning {{Nullable pointer is returned from a function that is expected to return a non-null value}}
 }
 
 Dummy *_Nonnull testNullReturn() {
   Dummy *p = 0;
-  return p; // expected-warning {{}}
+  return p; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
 }
 
 void testObjCMessageResultNullability() {
@@ -279,11 +279,25 @@ Dummy *_Nonnull testDefensiveInlineChecks(Dummy * p) {
   return p;
 }
 
-void testObjCARCImplicitZeroInitialization() {
-  TestObject * _Nonnull implicitlyZeroInitialized; // no-warning
-  implicitlyZeroInitialized = getNonnullTestObject();
+@interface SomeClass : NSObject
+@end
+
+@implementation SomeClass (MethodReturn)
+- (TestObject * _Nonnull)testReturnsNullableInNonnullIndirectly {
+  TestObject *local = getNullableTestObject();
+  return local; // expected-warning {{Nullable pointer is returned from a function that is expected to return a non-null value}}
+}
+
+- (TestObject * _Nonnull)testReturnsCastSuppressedNullableInNonnullIndirectly {
+  TestObject *local = getNullableTestObject();
+  return (TestObject * _Nonnull)local; // no-warning
 }
 
-void testObjCARCExplicitZeroInitialization() {
-  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{Null is assigned to a pointer which is expected to have non-null value}}
+- (TestObject * _Nonnull)testReturnsNullableInNonnullWhenPreconditionViolated:(TestObject * _Nonnull) p {
+  TestObject *local = getNullableTestObject();
+  if (!p) // Pre-condition violated here.
+    return local; // no-warning
+  else
+    return p; // no-warning
 }
+@end

diff --git a/test/Analysis/nullability_nullonly.mm b/test/Analysis/nullability_nullonly.mm
index 6479d67bda61ccf4a2f7d1cb424e343cbeccce58..c9129a8067cf17ce1093b30078ba4e1ba09d0a1b 100644 (file)
--- a/test/Analysis/nullability_nullonly.mm
+++ b/test/Analysis/nullability_nullonly.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull -verify %s
+// RUN: %clang_cc1 -analyze -fobjc-arc -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull -verify %s
 
 #define nil 0
 #define BOOL int
@@ -102,6 +102,40 @@ Dummy *_Nonnull testDefensiveInlineChecks(Dummy * p) {
   return p;
 }
 
+@interface TestObject : NSObject
+@end
+
+TestObject *_Nonnull getNonnullTestObject();
+
+void testObjCARCImplicitZeroInitialization() {
+  TestObject * _Nonnull implicitlyZeroInitialized; // no-warning
+  implicitlyZeroInitialized = getNonnullTestObject();
+}
+
+void testObjCARCExplicitZeroInitialization() {
+  TestObject * _Nonnull explicitlyZeroInitialized = nil; // expected-warning {{Null is assigned to a pointer which is expected to have non-null value}}
+}
+
+// Under ARC, returned expressions of ObjC objects types are are implicitly
+// cast to _Nonnull when the functions return type is _Nonnull, so make
+// sure this doesn't implicit cast doesn't suppress a legitimate warning.
+TestObject * _Nonnull returnsNilObjCInstanceIndirectly() {
+  TestObject *local = 0;
+  return local; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+}
+
+TestObject * _Nonnull returnsNilObjCInstanceIndirectlyWithSupressingCast() {
+  TestObject *local = 0;
+  return (TestObject * _Nonnull)local; // no-warning
+}
+
+TestObject * _Nonnull returnsNilObjCInstanceDirectly() {
+  return nil; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+}
+
+TestObject * _Nonnull returnsNilObjCInstanceDirectlyWithSuppressingCast() {
+  return (TestObject * _Nonnull)nil; // no-warning
+}
 
 @interface SomeClass : NSObject
 @end