From c1d322ac64b3ca72e442b2471da33357b7255350 Mon Sep 17 00:00:00 2001
From: George Karpenkov <ekarpenkov@apple.com>
Date: Thu, 31 May 2018 00:28:13 +0000
Subject: [PATCH] [analyzer] Trust _Nonnull annotations, and trust analyzer
 knowledge about receiver nullability

Previously, the checker was using the nullability of the expression,
which is nonnull IFF both receiver and method are annotated as _Nonnull.
However, the receiver could be known to the analyzer to be nonnull
without being explicitly marked as _Nonnull.

rdar://40635584

Differential Revision: https://reviews.llvm.org/D47510

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@333612 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../Checkers/TrustNonnullChecker.cpp          | 48 +++++++++++++++++--
 .../system-header-simulator-for-nullability.h |  7 +--
 test/Analysis/trustnonnullchecker_test.m      | 30 +++++++++++-
 3 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp b/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
index 3b299441d1..f3d6801422 100644
--- a/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
@@ -25,18 +25,56 @@ using namespace ento;
 namespace {
 
 class TrustNonnullChecker : public Checker<check::PostCall> {
+private:
+  /// \returns Whether we trust the result of the method call to be
+  /// a non-null pointer.
+  bool isNonNullPtr(const CallEvent &Call, CheckerContext &C) const {
+    QualType ExprRetType = Call.getResultType();
+    if (!ExprRetType->isAnyPointerType())
+      return false;
+
+    if (getNullabilityAnnotation(ExprRetType) == Nullability::Nonnull)
+      return true;
+
+    // The logic for ObjC instance method calls is more complicated,
+    // as the return value is nil when the receiver is nil.
+    if (!isa<ObjCMethodCall>(&Call))
+      return false;
+
+    const auto *MCall = cast<ObjCMethodCall>(&Call);
+    const ObjCMethodDecl *MD = MCall->getDecl();
+
+    // Distrust protocols.
+    if (isa<ObjCProtocolDecl>(MD->getDeclContext()))
+      return false;
+
+    QualType DeclRetType = MD->getReturnType();
+    if (getNullabilityAnnotation(DeclRetType) != Nullability::Nonnull)
+      return false;
+
+    // For class messages it is sufficient for the declaration to be
+    // annotated _Nonnull.
+    if (!MCall->isInstanceMessage())
+      return true;
+
+    // Alternatively, the analyzer could know that the receiver is not null.
+    SVal Receiver = MCall->getReceiverSVal();
+    ConditionTruthVal TV = C.getState()->isNonNull(Receiver);
+    if (TV.isConstrainedTrue())
+      return true;
+
+    return false;
+  }
+
 public:
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const {
     // Only trust annotations for system headers for non-protocols.
     if (!Call.isInSystemHeader())
       return;
 
-    QualType RetType = Call.getResultType();
-    if (!RetType->isAnyPointerType())
-      return;
-
     ProgramStateRef State = C.getState();
-    if (getNullabilityAnnotation(RetType) == Nullability::Nonnull)
+
+    if (isNonNullPtr(Call, C))
       if (auto L = Call.getReturnValue().getAs<Loc>())
         State = State->assume(*L, /*Assumption=*/true);
 
diff --git a/test/Analysis/Inputs/system-header-simulator-for-nullability.h b/test/Analysis/Inputs/system-header-simulator-for-nullability.h
index 5c28269222..f99c7ee341 100644
--- a/test/Analysis/Inputs/system-header-simulator-for-nullability.h
+++ b/test/Analysis/Inputs/system-header-simulator-for-nullability.h
@@ -32,8 +32,9 @@ NSObject<NSObject>
 @interface NSString : NSObject<NSCopying>
 - (BOOL)isEqualToString : (NSString *)aString;
 - (NSString *)stringByAppendingString:(NSString *)aString;
-+ (_Nonnull NSString *) generateString;
-+ (_Nullable NSString *) generatePossiblyNullString;
++ (NSString * _Nonnull) generateString;
++ (NSString *) generateImplicitlyNonnullString;
++ (NSString * _Nullable) generatePossiblyNullString;
 @end
 
 void NSSystemFunctionTakingNonnull(NSString *s);
@@ -46,7 +47,7 @@ NSString* _Nullable getPossiblyNullString();
 NSString* _Nonnull  getString();
 
 @protocol MyProtocol
-- (_Nonnull NSString *) getString;
+- (NSString * _Nonnull) getString;
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/test/Analysis/trustnonnullchecker_test.m b/test/Analysis/trustnonnullchecker_test.m
index 83fc7826df..67b6bd27c9 100644
--- a/test/Analysis/trustnonnullchecker_test.m
+++ b/test/Analysis/trustnonnullchecker_test.m
@@ -2,14 +2,39 @@
 
 #include "Inputs/system-header-simulator-for-nullability.h"
 
-NSString* getUnknownString();
-
 NSString* _Nonnull trust_nonnull_framework_annotation() {
   NSString* out = [NSString generateString];
   if (out) {}
   return out; // no-warning
 }
 
+NSString* _Nonnull trust_instancemsg_annotation(NSString* _Nonnull param) {
+  NSString* out = [param stringByAppendingString:@"string"];
+  if (out) {}
+  return out; // no-warning
+}
+
+NSString* _Nonnull distrust_instancemsg_noannotation(NSString* param) {
+  if (param) {}
+  NSString* out = [param stringByAppendingString:@"string"];
+  if (out) {}
+  return out; // expected-warning{{}}
+}
+
+NSString* _Nonnull trust_analyzer_knowledge(NSString* param) {
+  if (!param)
+    return @"";
+  NSString* out = [param stringByAppendingString:@"string"];
+  if (out) {}
+  return out; // no-warning
+}
+
+NSString* _Nonnull trust_assume_nonnull_macro() {
+  NSString* out = [NSString generateImplicitlyNonnullString];
+  if (out) {}
+  return out; // no-warning
+}
+
 NSString* _Nonnull distrust_without_annotation() {
   NSString* out = [NSString generatePossiblyNullString];
   if (out) {}
@@ -41,3 +66,4 @@ NSString * _Nonnull distrustProtocol(id<MyProtocol> o) {
   if (out) {};
   return out; // expected-warning{{}}
 }
+
-- 
2.40.0