From 9b05ffd15d976bbd74b90933c3d98bd91b338d28 Mon Sep 17 00:00:00 2001
From: Devin Coughlin <dcoughlin@apple.com>
Date: Tue, 12 Apr 2016 19:29:52 +0000
Subject: [PATCH] [analyzer] Nullability: Suppress return diagnostics in
 inlined functions.
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

The nullability checker can sometimes miss detecting nullability precondition
violations in inlined functions because the binding for the parameter
that violated the precondition becomes dead before the return:

int * _Nonnull callee(int * _Nonnull p2) {
  if (!p2)
    // p2 becomes dead here, so binding removed.
    return 0; // warning here because value stored in p2 is symbolic.
  else
   return p2;
}

int *caller(int * _Nonnull p1) {
  return callee(p1);
}

The fix, which is quite blunt, is to not warn about null returns in inlined
methods/functions. This won’t lose much coverage for ObjC because the analyzer
always analyzes each ObjC method at the top level in addition to inlined. It
*will* lose coverage for C — but there aren’t that many codebases with C
nullability annotations.

rdar://problem/25615050

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@266109 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../Checkers/NullabilityChecker.cpp           |  3 +-
 test/Analysis/nullability.mm                  | 35 +++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 35620d33ac..d8a224ea2f 100644
--- a/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -562,7 +562,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
   if (Filter.CheckNullReturnedFromNonnull &&
       NullReturnedFromNonNull &&
       RetExprTypeLevelNullability != Nullability::Nonnull &&
-      !InSuppressedMethodFamily) {
+      !InSuppressedMethodFamily &&
+      C.getLocationContext()->inTopFrame()) {
     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 0a3ae7a196..1e01cdf4ca 100644
--- a/test/Analysis/nullability.mm
+++ b/test/Analysis/nullability.mm
@@ -238,6 +238,41 @@ void testPreconditionViolationInInlinedFunction(Dummy *p) {
   doNotWarnWhenPreconditionIsViolated(p);
 }
 
+@interface TestInlinedPreconditionViolationClass : NSObject
+@end
+
+@implementation TestInlinedPreconditionViolationClass
+-(Dummy * _Nonnull) calleeWithParam:(Dummy * _Nonnull) p2 {
+  Dummy *x = 0;
+  if (!p2) // p2 binding becomes dead at this point.
+    return x; // no-warning
+  else
+   return p2;
+}
+
+-(Dummy *)callerWithParam:(Dummy * _Nonnull) p1 {
+  return [self calleeWithParam:p1];
+}
+
+@end
+
+int * _Nonnull InlinedPreconditionViolationInFunctionCallee(int * _Nonnull p2) {
+  int *x = 0;
+  if (!p2) // p2 binding becomes dead at this point.
+    return x; // no-warning
+  else
+   return p2;
+}
+
+int * _Nonnull InlinedReturnNullOverSuppressionCallee(int * _Nonnull p2) {
+  int *result = 0;
+  return result; // no-warning; but this is an over suppression
+}
+
+int *InlinedReturnNullOverSuppressionCaller(int * _Nonnull p1) {
+  return InlinedReturnNullOverSuppressionCallee(p1);
+}
+
 void inlinedNullable(Dummy *_Nullable p) {
   if (p) return;
 }
-- 
2.50.1