r333612 - [analyzer] Trust _Nonnull annotations, and trust analyzer knowledge about receiver nullability

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Wed May 30 17:28:13 2018
New Revision: 333612

URL: http://llvm.org/viewvc/llvm-project?rev=333612&view=rev
Log:
[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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
    cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h
    cfe/trunk/test/Analysis/trustnonnullchecker_test.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp?rev=333612&r1=333611&r2=333612&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp Wed May 30 17:28:13 2018
@@ -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);
 

Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h?rev=333612&r1=333611&r2=333612&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h Wed May 30 17:28:13 2018
@@ -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 getPossiblyNullStrin
 NSString* _Nonnull  getString();
 
 @protocol MyProtocol
-- (_Nonnull NSString *) getString;
+- (NSString * _Nonnull) getString;
 @end
 
 NS_ASSUME_NONNULL_END

Modified: cfe/trunk/test/Analysis/trustnonnullchecker_test.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/trustnonnullchecker_test.m?rev=333612&r1=333611&r2=333612&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/trustnonnullchecker_test.m (original)
+++ cfe/trunk/test/Analysis/trustnonnullchecker_test.m Wed May 30 17:28:13 2018
@@ -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<
   if (out) {};
   return out; // expected-warning{{}}
 }
+