[analyzer] Suppress nullability warning for defensive super initializer idiom.

A common idiom in Objective-C initializers is for a defensive nil-check on the
result of a call to a super initializer:
  if (self = [super init]) {
     ...
  }
  return self;

To avoid warning on this idiom, the nullability checker now suppress diagnostics
for returns of nil on syntactic 'return self' even in initializers with non-null
return types.

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

diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index b532deb55336d50885ca996567aa41687b935459..079a71e017453009ee1a87959a682aa1738472a8 100644 (file)
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -470,6 +470,22 @@ static const Expr *lookThroughImplicitCasts(const Expr *E) {
   return E;
 }
 
+/// Returns true when the return statement is a syntactic 'return self' in
+/// Objective-C.
+static bool isReturnSelf(const ReturnStmt *RS, CheckerContext &C) {
+  const ImplicitParamDecl *SelfDecl =
+    C.getCurrentAnalysisDeclContext()->getSelfDecl();
+  if (!SelfDecl)
+    return false;
+
+  const Expr *ReturnExpr = lookThroughImplicitCasts(RS->getRetValue());
+  auto *RefExpr = dyn_cast<DeclRefExpr>(ReturnExpr);
+  if (!RefExpr)
+    return false;
+
+  return RefExpr->getDecl() == SelfDecl;
+}
+
 /// This method check when nullable pointer or null value is returned from a
 /// function that has nonnull return type.
 ///
@@ -494,16 +510,28 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
   if (!RetSVal)
     return;
 
+  bool IsReturnSelfInObjCInit = false;
+
   QualType RequiredRetType;
   AnalysisDeclContext *DeclCtxt =
       C.getLocationContext()->getAnalysisDeclContext();
   const Decl *D = DeclCtxt->getDecl();
-  if (auto *MD = dyn_cast<ObjCMethodDecl>(D))
+  if (auto *MD = dyn_cast<ObjCMethodDecl>(D)) {
     RequiredRetType = MD->getReturnType();
-  else if (auto *FD = dyn_cast<FunctionDecl>(D))
+    // Suppress diagnostics for returns of nil that are syntactic returns of
+    // self in ObjC initializers. This avoids warning under the common idiom of
+    // a defensive check of the result of a call to super:
+    //   if (self = [super init]) {
+    //     ...
+    //   }
+    //   return self; // no-warning
+    IsReturnSelfInObjCInit = (MD->getMethodFamily() == OMF_init) &&
+                              isReturnSelf(S, C);
+  } else if (auto *FD = dyn_cast<FunctionDecl>(D)) {
     RequiredRetType = FD->getReturnType();
-  else
+  } else {
     return;
+  }
 
   NullConstraint Nullness = getNullConstraint(*RetSVal, State);
 
@@ -520,7 +548,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
   if (Filter.CheckNullReturnedFromNonnull &&
       Nullness == NullConstraint::IsNull &&
       RetExprTypeLevelNullability != Nullability::Nonnull &&
-      RequiredNullability == Nullability::Nonnull) {
+      RequiredNullability == Nullability::Nonnull &&
+      !IsReturnSelfInObjCInit) {
     static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull");
     ExplodedNode *N = C.generateErrorNode(State, &Tag);
     if (!N)

diff --git a/test/Analysis/nullability.mm b/test/Analysis/nullability.mm
index aa909fbb8c420071767e8d275fd446b1772aa68a..b119e63ffe2b33feb35ed65ad3be848d3f30ed37 100644 (file)
--- a/test/Analysis/nullability.mm
+++ b/test/Analysis/nullability.mm
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fobjc-arc -analyze -analyzer-checker=core,nullability -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,nullability -verify %s
 
 #define nil 0
 #define BOOL int
@@ -279,10 +280,21 @@ Dummy *_Nonnull testDefensiveInlineChecks(Dummy * p) {
   return p;
 }
 
-@interface SomeClass : NSObject
+
+@interface SomeClass : NSObject {
+  int instanceVar;
+}
 @end
 
 @implementation SomeClass (MethodReturn)
+- (id)initWithSomething:(int)i {
+  if (self = [super init]) {
+    instanceVar = i;
+  }
+
+  return self;
+}
+
 - (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}}
@@ -301,3 +313,41 @@ Dummy *_Nonnull testDefensiveInlineChecks(Dummy * p) {
     return p; // no-warning
 }
 @end
+
+@interface ClassWithInitializers : NSObject
+@end
+
+@implementation ClassWithInitializers
+- (instancetype _Nonnull)initWithNonnullReturnAndSelfCheckingIdiom {
+  // This defensive check is a common-enough idiom that we filter don't want
+  // to issue a diagnostic for it,
+  if (self = [super init]) {
+  }
+
+  return self; // no-warning
+}
+
+- (instancetype _Nonnull)initWithNonnullReturnAndNilReturnViaLocal {
+  self = [super init];
+  // This leaks, but we're not checking for that here.
+
+  ClassWithInitializers *other = nil;
+  // Still warn when when not returning via self.
+  return other; // expected-warning {{Null is returned from a function that is expected to return a non-null value}}
+}
+@end
+
+@interface SubClassWithInitializers : ClassWithInitializers
+@end
+
+@implementation SubClassWithInitializers
+// Note: Because this is overridding
+// -[ClassWithInitializers initWithNonnullReturnAndSelfCheckingIdiom],
+// the return type of this method becomes implicitly id _Nonnull.
+- (id)initWithNonnullReturnAndSelfCheckingIdiom {
+  if (self = [super initWithNonnullReturnAndSelfCheckingIdiom]) {
+  }
+
+  return self; // no-warning
+}
+@end